Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollback should not cause missing validatorset #250

Merged
merged 13 commits into from
Dec 5, 2024
Merged

Conversation

yzang2019
Copy link
Contributor

Describe your changes and provide context

Problem:
Currently there's a bug in rollback logic, where after rollback, the node could sometimes run into this error:

Dec 04 15:54:59 ip-172-31-26-246 seid[234375]: panic: failed to load validator set at height 118546578: couldn't find validators at height 118546577 (height 118546578 was originally requested): %!w(<nil>)
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]: goroutine 967 [running]:
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]: github.com/tendermint/tendermint/internal/state.buildLastCommitInfo(0xc0683e21c0, {0x3824a58?, 0xc01f8f2ff0?}, 0x11?)
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]:         /root/go/pkg/mod/github.com/sei-protocol/[email protected]/internal/state/execution.go:500 +0x44e
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]: github.com/tendermint/tendermint/internal/state.(*BlockExecutor).ApplyBlock(_, {_, _}, {{{0xb, 0x0}, {0xc019faff98, 0x11}}, {0xc05de3f4a0, 0x9}, 0x1, ...}, ...)
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]:         /root/go/pkg/mod/github.com/sei-protocol/[email protected]/internal/state/execution.go:257 +0x4e5
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]: github.com/tendermint/tendermint/internal/blocksync.(*Reactor).poolRoutine(0xc05c9cd080, {0x3811f68?, 0xc000053770}, 0x0, 0xc05de5e600?)
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]:         /root/go/pkg/mod/github.com/sei-protocol/[email protected]/internal/blocksync/reactor.go:705 +0x16f8
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]: created by github.com/tendermint/tendermint/internal/blocksync.(*Reactor).OnStart in goroutine 1
Dec 04 15:54:59 ip-172-31-26-246 seid[234375]:         /root/go/pkg/mod/github.com/sei-protocol/[email protected]/internal/blocksync/reactor.go:181 +0x5fd

Root cause:
ValidatorSet data are not stored on every height, it was stored only when things get changed, or every interval (100k). So the root cause is that the rollback could sometimes fill LastHeightValidatorsChanged with the wrong value. The correct logic should be to use the previous block's value of LastHeightValidatorsChanged, however in some cases it will simply just set it to be rollbackHeight + 1, which could lead to not able to find the correct validator set data.

Solution:
In order to fix it, the correct way is when rollback, we need to query previous block's LastHeightValidatorsChanged and assign that as the rolled back state.

Testing performed to validate your change

Tested in archive node

@yzang2019 yzang2019 enabled auto-merge (squash) December 5, 2024 16:27
@yzang2019 yzang2019 merged commit 57e0107 into main Dec 5, 2024
22 checks passed
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 64.28571% with 10 lines in your changes missing coverage. Please review.

Project coverage is 58.17%. Comparing base (e9a22c9) to head (57799c2).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/state/rollback.go 68.18% 4 Missing and 3 partials ⚠️
internal/state/store.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #250      +/-   ##
==========================================
+ Coverage   58.15%   58.17%   +0.02%     
==========================================
  Files         249      249              
  Lines       34549    34555       +6     
==========================================
+ Hits        20091    20102      +11     
+ Misses      12860    12853       -7     
- Partials     1598     1600       +2     
Files with missing lines Coverage Δ
cmd/tendermint/commands/rollback.go 23.68% <ø> (-1.96%) ⬇️
internal/state/store.go 56.25% <50.00%> (-0.22%) ⬇️
internal/state/rollback.go 71.79% <68.18%> (-2.12%) ⬇️

... and 13 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants