From 57e010706fd30835de9397c288ba6b79ff87373f Mon Sep 17 00:00:00 2001 From: Yiming Zang <50607998+yzang2019@users.noreply.github.com> Date: Thu, 5 Dec 2024 08:28:47 -0800 Subject: [PATCH] Rollback should not cause missing validatorset (#250) * Add logs * Fix * Add logs * Fix * Fix * Log * log * log * Log * Rollback should not cause missing validatorset issue * Fix lint * Fix comment * Fix unit test --- cmd/tendermint/commands/rollback.go | 1 - internal/state/rollback.go | 46 +++++++++++++++-------------- internal/state/rollback_test.go | 2 +- internal/state/store.go | 9 +++++- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/cmd/tendermint/commands/rollback.go b/cmd/tendermint/commands/rollback.go index 9f7a073c4..f4c9bd30d 100644 --- a/cmd/tendermint/commands/rollback.go +++ b/cmd/tendermint/commands/rollback.go @@ -48,7 +48,6 @@ application. func RollbackState(config *config.Config, removeBlock bool) (int64, []byte, error) { // use the parsed config to load the block and state store blockStore, stateStore, err := loadStateAndBlockStore(config) - fmt.Printf("Current blockStore height=%d hash=%X\n", blockStore.Height(), blockStore.LoadSeenCommit().Hash()) if err != nil { return -1, nil, err } diff --git a/internal/state/rollback.go b/internal/state/rollback.go index c4c89c54c..feb16d507 100644 --- a/internal/state/rollback.go +++ b/internal/state/rollback.go @@ -28,9 +28,7 @@ func resetPrivValidatorConfig(privValidatorConfig config.PrivValidatorConfig) er // recent previous state (height n - 1). // Note that this function does not affect application state. func Rollback(bs BlockStore, ss Store, removeBlock bool, privValidatorConfig *config.PrivValidatorConfig) (int64, []byte, error) { - // Only the latest state is stored latestState, err := ss.Load() - fmt.Printf("Initial tendermint state height=%d, appHash=%X, lastResultHash=%X\n", latestState.LastBlockHeight, latestState.AppHash, latestState.LastResultsHash) if err != nil { return -1, nil, err } @@ -38,25 +36,26 @@ func Rollback(bs BlockStore, ss Store, removeBlock bool, privValidatorConfig *co return -1, nil, errors.New("no state found") } - height := bs.Height() + latestBlockHeight := bs.Height() + latestStateHeight := latestState.LastBlockHeight + fmt.Printf("Current blockStore height=%d tendermint state height=%d appHash=%X lastResultHash=%X\n", latestBlockHeight, latestStateHeight, latestState.AppHash, latestState.LastResultsHash) + // NOTE: persistence of state and blocks don't happen atomically. Therefore it is possible that // when the user stopped the node the state wasn't updated but the blockstore was. Discard the // pending block before continuing. - if height == latestState.LastBlockHeight+1 { - fmt.Printf("Invalid state in the latest block height=%d, removing it first \n", height) - if removeBlock { - if err := bs.DeleteLatestBlock(); err != nil { - return -1, nil, fmt.Errorf("failed to remove final block from blockstore: %w", err) - } + if latestBlockHeight == latestStateHeight+1 { + fmt.Printf("Invalid state in the latest block height=%d, removing it first \n", latestBlockHeight) + if err := bs.DeleteLatestBlock(); err != nil { + return -1, nil, fmt.Errorf("failed to remove final block from blockstore: %w", err) } return latestState.LastBlockHeight, latestState.AppHash, nil } // If the state store isn't one below nor equal to the blockstore height than this violates the // invariant - if height != latestState.LastBlockHeight { + if latestBlockHeight != latestState.LastBlockHeight { return -1, nil, fmt.Errorf("statestore height (%d) is not one below or equal to blockstore height (%d)", - latestState.LastBlockHeight, height) + latestState.LastBlockHeight, latestBlockHeight) } // state store height is equal to blockstore height. We're good to proceed with rolling back state @@ -72,20 +71,23 @@ func Rollback(bs BlockStore, ss Store, removeBlock bool, privValidatorConfig *co return -1, nil, fmt.Errorf("block at height %d not found", latestState.LastBlockHeight) } - previousLastValidatorSet, err := ss.LoadValidators(rollbackHeight) - if err != nil { - return -1, nil, err - } - previousParams, err := ss.LoadConsensusParams(rollbackHeight + 1) if err != nil { return -1, nil, err } valChangeHeight := latestState.LastHeightValidatorsChanged - // this can only happen if the validator set changed since the last block if valChangeHeight > rollbackHeight { - valChangeHeight = rollbackHeight + 1 + valInfo, err := ss.(dbStore).LoadValidatorsInfo(rollbackHeight) + if err != nil { + return -1, nil, err + } + valChangeHeight = valInfo.LastHeightChanged + } + + previousLastValidatorSet, err := ss.LoadValidators(rollbackHeight) + if err != nil { + return -1, nil, err } paramsChangeHeight := latestState.LastHeightConsensusParamsChanged @@ -94,7 +96,7 @@ func Rollback(bs BlockStore, ss Store, removeBlock bool, privValidatorConfig *co paramsChangeHeight = rollbackHeight + 1 } - lastBlockHeight := rollbackBlock.Header.Height + rolledBackHeight := rollbackBlock.Header.Height rolledBackAppHash := latestBlock.Header.AppHash rolledBackLastResultHash := latestBlock.Header.LastResultsHash @@ -114,7 +116,7 @@ func Rollback(bs BlockStore, ss Store, removeBlock bool, privValidatorConfig *co ChainID: latestState.ChainID, InitialHeight: latestState.InitialHeight, - LastBlockHeight: lastBlockHeight, + LastBlockHeight: rolledBackHeight, LastBlockID: rollbackBlock.BlockID, LastBlockTime: rollbackBlock.Header.Time, @@ -150,6 +152,6 @@ func Rollback(bs BlockStore, ss Store, removeBlock bool, privValidatorConfig *co } } - fmt.Printf("Saved tendermint state height=%d, appHash=%X, lastResultHash=%X\n", lastBlockHeight, rolledBackState.AppHash, rolledBackState.LastResultsHash) - return lastBlockHeight, rolledBackState.AppHash, nil + fmt.Printf("Saved tendermint state height=%d, appHash=%X, lastResultHash=%X\n", rolledBackHeight, rolledBackState.AppHash, rolledBackState.LastResultsHash) + return rolledBackHeight, rolledBackState.AppHash, nil } diff --git a/internal/state/rollback_test.go b/internal/state/rollback_test.go index 1af87ef9b..7c2ba54b8 100644 --- a/internal/state/rollback_test.go +++ b/internal/state/rollback_test.go @@ -149,7 +149,7 @@ func setupStateStore(t *testing.T, height int64) state.Store { LastValidators: valSet, Validators: valSet.CopyIncrementProposerPriority(1), NextValidators: valSet.CopyIncrementProposerPriority(2), - LastHeightValidatorsChanged: height + 1, + LastHeightValidatorsChanged: height, ConsensusParams: *params, LastHeightConsensusParamsChanged: height + 1, } diff --git a/internal/state/store.go b/internal/state/store.go index c234968c4..ecee78518 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -507,7 +507,6 @@ func (store dbStore) SaveValidatorSets(lowerHeight, upperHeight int64, vals *typ // LoadValidators loads the ValidatorSet for a given height. // Returns ErrNoValSetForHeight if the validator set can't be found for this height. func (store dbStore) LoadValidators(height int64) (*types.ValidatorSet, error) { - valInfo, err := loadValidatorsInfo(store.db, height) if err != nil { return nil, ErrNoValSetForHeight{Height: height, Err: err} @@ -556,6 +555,14 @@ func lastStoredHeightFor(height, lastHeightChanged int64) int64 { return tmmath.MaxInt64(checkpointHeight, lastHeightChanged) } +func (store dbStore) LoadValidatorsInfo(height int64) (*tmstate.ValidatorsInfo, error) { + valInfo, err := loadValidatorsInfo(store.db, height) + if err != nil { + return nil, ErrNoValSetForHeight{Height: height, Err: err} + } + return valInfo, nil +} + // CONTRACT: Returned ValidatorsInfo can be mutated. func loadValidatorsInfo(db dbm.DB, height int64) (*tmstate.ValidatorsInfo, error) { buf, err := db.Get(validatorsKey(height))