forked from cometbft/cometbft
-
Notifications
You must be signed in to change notification settings - Fork 1
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
cli: add --hard flag to rollback command to remove block as well #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…kport #9261) (#9465) * cli: add --hard flag to rollback command to remove block as well (#9261) Co-authored-by: Levi Aul <[email protected]> (cherry picked from commit e84d43e) * Fix lint Signed-off-by: Thane Thomson <[email protected]> Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Thane Thomson <[email protected]>
pirtleshell
requested review from
DracoLi,
nddeluca,
rhuairahrighairidh,
galxy25,
drklee3 and
evgeniy-scherbina
November 22, 2023 17:28
This was referenced Nov 22, 2023
drklee3
requested changes
Nov 27, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some errors with a mock and rollback_test that need to be updated
# github.com/tendermint/tendermint/rpc/core [github.com/tendermint/tendermint/rpc/core.test]
rpc/core/blocks_test.go:89:19: cannot use mockBlockStore{…} (value of type mockBlockStore) as "github.com/tendermint/tendermint/state".BlockStore value in assignment: mockBlockStore does not implement "github.com/tendermint/tendermint/state".BlockStore (missing method DeleteLatestBlock)
FAIL github.com/tendermint/tendermint/rpc/core [build failed]
ok github.com/tendermint/tendermint/rpc/core/types 0.012s
ok github.com/tendermint/tendermint/rpc/grpc 0.360s
ok github.com/tendermint/tendermint/rpc/jsonrpc 8.033s
ok github.com/tendermint/tendermint/rpc/jsonrpc/client 9.005s
ok github.com/tendermint/tendermint/rpc/jsonrpc/server 0.138s
? github.com/tendermint/tendermint/rpc/jsonrpc/test [no test files]
ok github.com/tendermint/tendermint/rpc/jsonrpc/types 0.008s
? github.com/tendermint/tendermint/rpc/test [no test files]
? github.com/tendermint/tendermint/scripts/json2wal [no test files]
? github.com/tendermint/tendermint/scripts/wal2json [no test files]
# github.com/tendermint/tendermint/state_test [github.com/tendermint/tendermint/state.test]
state/rollback_test.go:96:17: params.Version.App undefined (type "github.com/tendermint/tendermint/proto/tendermint/types".VersionParams has no field or method App)
state/rollback_test.go:101:24: undefined: tmversion
state/rollback_test.go:111:31: params.Hash undefined (type *"github.com/tendermint/tendermint/proto/tendermint/types".ConsensusParams has no field or method Hash)
state/rollback_test.go:119:18: assignment mismatch: 2 variables but block.MakePartSet returns 1 value
state/rollback_test.go:124:12: undefined: tmstate
state/rollback_test.go:143:24: undefined: tmversion
state/rollback_test.go:153:31: params.Hash undefined (type *"github.com/tendermint/tendermint/proto/tendermint/types".ConsensusParams has no field or method Hash)
state/rollback_test.go:161:22: assignment mismatch: 2 variables but nextBlock.MakePartSet returns 1 value
state/rollback_test.go:178:17: params.Version.App undefined (type "github.com/tendermint/tendermint/proto/tendermint/types".VersionParams has no field or method App)
state/rollback_test.go:181:12: undefined: tmstate
state/rollback_test.go:181:12: too many errors
FAIL github.com/tendermint/tendermint/state [build failed]
drklee3
approved these changes
Dec 4, 2023
nddeluca
pushed a commit
that referenced
this pull request
Jan 5, 2024
* cli: add --hard flag to rollback command to remove block as well (backport #9261) (#9465) * cli: add --hard flag to rollback command to remove block as well (#9261) Co-authored-by: Levi Aul <[email protected]> (cherry picked from commit e84d43e) * Fix lint Signed-off-by: Thane Thomson <[email protected]> Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Thane Thomson <[email protected]> * fix build failures for TestRollbackHard * implement complete interface for mock block store --------- Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Thane Thomson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherrypick / backport of cometbft@bd9ed68
Before this commit,
RollbackState
(used only by therollback
CLI command of cosmos-sdk) rolled back the tendermint state by one version, but not the block state.This means that the command can only be used to rollback one block, because if the most recent tendermint store version (block number) matches the most recent block store version, the rollback operation is a no-op. (code.
The commit updates the
RollbackState
method to have aremoveBlock
argument that is used to also trigger the removal of the block store version, allowing for rolling back the state of multiple blocks.Once merged, it will require a patch to cosmos-sdk to handle the new function signature of
RollbackState
in the rollback CLI command.tl;dr:
RollbackState
can now be used to rollback a sequence of blocks, not just the most recent block. This is not consensus breaking as the method is only used in the CLI.