From 83edae27299dfbfd4695205461ad354d0f4064fb Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Sat, 19 Feb 2022 09:18:26 -0800 Subject: [PATCH] fix app hash in state rollback (#7837) (#7882) When testing rollback feature in the Cosmos SDK, we found that the app hash in Tendermint after rollback was the value after the latest block, rather than before it. Co-authored-by: Callum Waters (cherry picked from commit 8a238fdcb44461a4e029c8e7a4790c2d470d282b) Co-authored-by: yihuang --- CHANGELOG_PENDING.md | 1 + internal/state/rollback.go | 9 +++++++-- internal/state/rollback_test.go | 15 +++++++++++++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 0e8458078..e74746d2c 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -28,3 +28,4 @@ Special thanks to external contributors on this release: - [light] \#7640 Light Client: fix absence proof verification (@ashcherbakov) - [light] \#7641 Light Client: fix querying against the latest height (@ashcherbakov) +- [cli] [#7837](https://github.com/tendermint/tendermint/pull/7837) fix app hash in state rollback. (@yihuang) diff --git a/internal/state/rollback.go b/internal/state/rollback.go index ea0eff4de..38f042559 100644 --- a/internal/state/rollback.go +++ b/internal/state/rollback.go @@ -41,6 +41,11 @@ func Rollback(bs BlockStore, ss Store) (int64, []byte, error) { if rollbackBlock == nil { return -1, nil, fmt.Errorf("block at height %d not found", rollbackHeight) } + // we also need to retrieve the latest block because the app hash and last results hash is only agreed upon in the following block + latestBlock := bs.LoadBlockMeta(invalidState.LastBlockHeight) + if latestBlock == nil { + return -1, nil, fmt.Errorf("block at height %d not found", invalidState.LastBlockHeight) + } previousLastValidatorSet, err := ss.LoadValidators(rollbackHeight) if err != nil { @@ -89,8 +94,8 @@ func Rollback(bs BlockStore, ss Store) (int64, []byte, error) { ConsensusParams: previousParams, LastHeightConsensusParamsChanged: paramsChangeHeight, - LastResultsHash: rollbackBlock.Header.LastResultsHash, - AppHash: rollbackBlock.Header.AppHash, + LastResultsHash: latestBlock.Header.LastResultsHash, + AppHash: latestBlock.Header.AppHash, } // persist the new state. This overrides the invalid one. NOTE: this will also diff --git a/internal/state/rollback_test.go b/internal/state/rollback_test.go index fb5ca9796..ad7f12afc 100644 --- a/internal/state/rollback_test.go +++ b/internal/state/rollback_test.go @@ -46,12 +46,22 @@ func TestRollback(t *testing.T) { BlockID: initialState.LastBlockID, Header: types.Header{ Height: initialState.LastBlockHeight, - AppHash: initialState.AppHash, + AppHash: factory.RandomHash(), LastBlockID: factory.MakeBlockID(), LastResultsHash: initialState.LastResultsHash, }, } - blockStore.On("LoadBlockMeta", initialState.LastBlockHeight).Return(block) + nextBlock := &types.BlockMeta{ + BlockID: initialState.LastBlockID, + Header: types.Header{ + Height: nextState.LastBlockHeight, + AppHash: initialState.AppHash, + LastBlockID: block.BlockID, + LastResultsHash: nextState.LastResultsHash, + }, + } + blockStore.On("LoadBlockMeta", height).Return(block) + blockStore.On("LoadBlockMeta", nextHeight).Return(nextBlock) blockStore.On("Height").Return(nextHeight) // rollback the state @@ -81,6 +91,7 @@ func TestRollbackNoBlocks(t *testing.T) { stateStore := setupStateStore(t, height) blockStore := &mocks.BlockStore{} blockStore.On("Height").Return(height) + blockStore.On("LoadBlockMeta", height).Return(nil) blockStore.On("LoadBlockMeta", height-1).Return(nil) _, _, err := state.Rollback(blockStore, stateStore)