Merge c6e7524a8e into e4ae922c33

CHANGELOG_PENDING.md

@@ -73,6 +73,7 @@ Special thanks to external contributors on this release:
 - [rpc] \#7612 paginate mempool /unconfirmed_txs rpc endpoint (@spacech1mp)
 - [light] [\#7536](https://github.com/tendermint/tendermint/pull/7536) rpc /status call returns info about the light client (@jmalicevic)
 - [types] \#7765 Replace EvidenceData with EvidenceList to avoid unnecessary nesting of evidence fields within a block. (@jmalicevic)
+- [cli] [\#8101](https://github.com/tendermint/tendermint/pull/8101) rollback command rollback block store if it has one more block than block state.
 
 ### BUG FIXES
 

cmd/tendermint/commands/rollback_test.go

@@ -46,6 +46,13 @@ func TestRollbackIntegration(t *testing.T) {
 		height, _, err = commands.RollbackState(cfg)
 		require.NoError(t, err, "%d", height)
 	})
+	t.Run("Rollback again", func(t *testing.T) {
+		// should be able to rollback again.
+		require.NoError(t, app.Rollback())
+		height2, _, err := commands.RollbackState(cfg)
+		require.NoError(t, err, "%d", height2)
+		require.Equal(t, height-1, height2)
+	})
 	t.Run("Restart", func(t *testing.T) {
 		require.True(t, height > 0, "%d", height)
 
@@ -64,12 +71,13 @@ func TestRollbackIntegration(t *testing.T) {
 		for {
 			select {
 			case <-ctx.Done():
-				t.Fatalf("failed to make progress after 20 seconds. Min height: %d", height)
+				return
 			case <-ticker.C:
 				status, err := client.Status(ctx)
 				require.NoError(t, err)
 
-				if status.SyncInfo.LatestBlockHeight > height {
+				if status.SyncInfo.LatestBlockHeight > height+2 {
+					t.Fatalf("chain is expect to halt, because the validator won't recreate the old blocks %d", status.SyncInfo.LatestBlockHeight)
 					return
 				}
 			}

internal/consensus/replay_test.go

@@ -1226,6 +1226,10 @@ func (bs *mockBlockStore) PruneBlocks(height int64) (uint64, error) {
 	return pruned, nil
 }
 
+func (bs *mockBlockStore) Rollback() error {
+	return nil
+}
+
 //---------------------------------------
 // Test handshake/init chain
 

internal/state/mocks/block_store.go

@@ -208,3 +208,8 @@ func (_m *BlockStore) Size() int64 {
 
 	return r0
 }
+
+func (_m *BlockStore) Rollback() error {
+	_m.Called()
+	return nil
+}

internal/state/rollback.go

@@ -23,9 +23,12 @@ func Rollback(bs BlockStore, ss Store) (int64, []byte, error) {
 
 	// 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. In this situation
-	// we don't need to rollback any state and can just return early
+	// we need to rollback the block store too.
 	if height == invalidState.LastBlockHeight+1 {
-		return invalidState.LastBlockHeight, invalidState.AppHash, nil
+		if err := bs.Rollback(); err != nil {
+			return -1, nil, err
+		}
+		height--
 	}
 
 	// If the state store isn't one below nor equal to the blockstore height than this violates the

internal/state/services.go

@@ -36,6 +36,8 @@ type BlockStore interface {
 
 	LoadBlockCommit(height int64) *types.Commit
 	LoadSeenCommit() *types.Commit
+
+	Rollback() error
 }
 
 //-----------------------------------------------------------------------------

internal/store/store.go

@@ -313,28 +313,6 @@ func (bs *BlockStore) PruneBlocks(height int64) (uint64, error) {
 		return 0, fmt.Errorf("height must be equal to or less than the latest height %d", bs.Height())
 	}
 
-	// when removing the block meta, use the hash to remove the hash key at the same time
-	removeBlockHash := func(key, value []byte, batch dbm.Batch) error {
-		// unmarshal block meta
-		var pbbm = new(tmproto.BlockMeta)
-		err := proto.Unmarshal(value, pbbm)
-		if err != nil {
-			return fmt.Errorf("unmarshal to tmproto.BlockMeta: %w", err)
-		}
-
-		blockMeta, err := types.BlockMetaFromProto(pbbm)
-		if err != nil {
-			return fmt.Errorf("error from proto blockMeta: %w", err)
-		}
-
-		// delete the hash key corresponding to the block meta's hash
-		if err := batch.Delete(blockHashKey(blockMeta.BlockID.Hash)); err != nil {
-			return fmt.Errorf("failed to delete hash key: %X: %w", blockHashKey(blockMeta.BlockID.Hash), err)
-		}
-
-		return nil
-	}
-
 	// remove block meta first as this is used to indicate whether the block exists.
 	// For this reason, we also use ony block meta as a measure of the amount of blocks pruned
 	pruned, err := bs.pruneRange(blockMetaKey(0), blockMetaKey(height), removeBlockHash)
@@ -353,6 +331,28 @@ func (bs *BlockStore) PruneBlocks(height int64) (uint64, error) {
 	return pruned, nil
 }
 
+// when removing the block meta, use the hash to remove the hash key at the same time
+func removeBlockHash(key, value []byte, batch dbm.Batch) error {
+	// unmarshal block meta
+	var pbbm = new(tmproto.BlockMeta)
+	err := proto.Unmarshal(value, pbbm)
+	if err != nil {
+		return fmt.Errorf("unmarshal to tmproto.BlockMeta: %w", err)
+	}
+
+	blockMeta, err := types.BlockMetaFromProto(pbbm)
+	if err != nil {
+		return fmt.Errorf("error from proto blockMeta: %w", err)
+	}
+
+	// delete the hash key corresponding to the block meta's hash
+	if err := batch.Delete(blockHashKey(blockMeta.BlockID.Hash)); err != nil {
+		return fmt.Errorf("failed to delete hash key: %X: %w", blockHashKey(blockMeta.BlockID.Hash), err)
+	}
+
+	return nil
+}
+
 // pruneRange is a generic function for deleting a range of values based on the lowest
 // height up to but excluding retainHeight. For each key/value pair, an optional hook can be
 // executed before the deletion itself is made. pruneRange will use batch delete to delete
@@ -576,6 +576,33 @@ func (bs *BlockStore) Close() error {
 	return bs.db.Close()
 }
 
+// Rollback rollbacks the latest block from BlockStore.
+func (bs *BlockStore) Rollback() error {
+	height := bs.Height()
+	if height <= 0 {
+		return fmt.Errorf("can't rollback height %d", height)
+	}
+
+	pruned, err := bs.pruneRange(blockMetaKey(height), blockMetaKey(height+1), removeBlockHash)
+	if err != nil {
+		return err
+	}
+
+	if pruned != 1 {
+		return fmt.Errorf("the number of rollbacked blocks don't match %d", pruned)
+	}
+
+	if _, err := bs.pruneRange(blockPartKey(height, 0), blockPartKey(height+1, 0), nil); err != nil {
+		return err
+	}
+
+	if _, err := bs.pruneRange(blockCommitKey(height), blockCommitKey(height+1), nil); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 //---------------------------------- KEY ENCODING -----------------------------------------
 
 // key prefixes

test/e2e/app/state.go

@@ -12,8 +12,7 @@ import (
 	"sync"
 )
 
-const stateFileName = "app_state.json"
-const prevStateFileName = "prev_app_state.json"
+const stateFileName = "app_state_%d.json"
 
 // State is the application state.
 type State struct {
@@ -23,9 +22,9 @@ type State struct {
 	Hash   []byte
 
 	// private fields aren't marshaled to disk.
-	currentFile string
-	// app saves current and previous state for rollback functionality
-	previousFile    string
+	// app saves current and historical states for rollback functionality
+	stateFileTemplate string
+
 	persistInterval uint64
 	initialHeight   uint64
 }
@@ -33,10 +32,9 @@ type State struct {
 // NewState creates a new state.
 func NewState(dir string, persistInterval uint64) (*State, error) {
 	state := &State{
-		Values:          make(map[string]string),
-		currentFile:     filepath.Join(dir, stateFileName),
-		previousFile:    filepath.Join(dir, prevStateFileName),
-		persistInterval: persistInterval,
+		Values:            make(map[string]string),
+		stateFileTemplate: filepath.Join(dir, stateFileName),
+		persistInterval:   persistInterval,
 	}
 	state.Hash = hashItems(state.Values)
 	err := state.load()
@@ -48,25 +46,37 @@ func NewState(dir string, persistInterval uint64) (*State, error) {
 	return state, nil
 }
 
+func (s *State) currentFile() string {
+	return s.stateFileAtVersion(s.Height)
+}
+
+func (s *State) previousFile() string {
+	return s.stateFileAtVersion(s.Height - 1)
+}
+
+func (s *State) stateFileAtVersion(i uint64) string {
+	return fmt.Sprintf(s.stateFileTemplate, i)
+}
+
 // load loads state from disk. It does not take out a lock, since it is called
 // during construction.
 func (s *State) load() error {
-	bz, err := os.ReadFile(s.currentFile)
+	bz, err := os.ReadFile(s.currentFile())
 	if err != nil {
 		// if the current state doesn't exist then we try recover from the previous state
 		if errors.Is(err, os.ErrNotExist) {
-			bz, err = os.ReadFile(s.previousFile)
+			bz, err = os.ReadFile(s.previousFile())
 			if err != nil {
 				return fmt.Errorf("failed to read both current and previous state (%q): %w",
-					s.previousFile, err)
+					s.previousFile(), err)
 			}
 		} else {
-			return fmt.Errorf("failed to read state from %q: %w", s.currentFile, err)
+			return fmt.Errorf("failed to read state from %q: %w", s.currentFile(), err)
 		}
 	}
 	err = json.Unmarshal(bz, s)
 	if err != nil {
-		return fmt.Errorf("invalid state data in %q: %w", s.currentFile, err)
+		return fmt.Errorf("invalid state data in %q: %w", s.currentFile(), err)
 	}
 	return nil
 }
@@ -80,19 +90,13 @@ func (s *State) save() error {
 	}
 	// We write the state to a separate file and move it to the destination, to
 	// make it atomic.
-	newFile := fmt.Sprintf("%v.new", s.currentFile)
+	newFile := fmt.Sprintf("%v.new", s.currentFile())
 	err = os.WriteFile(newFile, bz, 0644)
 	if err != nil {
-		return fmt.Errorf("failed to write state to %q: %w", s.currentFile, err)
-	}
-	// We take the current state and move it to the previous state, replacing it
-	if _, err := os.Stat(s.currentFile); err == nil {
-		if err := os.Rename(s.currentFile, s.previousFile); err != nil {
-			return fmt.Errorf("failed to replace previous state: %w", err)
-		}
+		return fmt.Errorf("failed to write state to %q: %w", s.currentFile(), err)
 	}
 	// Finally, we take the new state and replace the current state.
-	return os.Rename(newFile, s.currentFile)
+	return os.Rename(newFile, s.currentFile())
 }
 
 // Export exports key/value pairs as JSON, used for state sync snapshots.
@@ -159,13 +163,13 @@ func (s *State) Commit() (uint64, []byte, error) {
 }
 
 func (s *State) Rollback() error {
-	bz, err := os.ReadFile(s.previousFile)
+	bz, err := os.ReadFile(s.previousFile())
 	if err != nil {
-		return fmt.Errorf("failed to read state from %q: %w", s.previousFile, err)
+		return fmt.Errorf("failed to read state from %q: %w", s.previousFile(), err)
 	}
 	err = json.Unmarshal(bz, s)
 	if err != nil {
-		return fmt.Errorf("invalid state data in %q: %w", s.previousFile, err)
+		return fmt.Errorf("invalid state data in %q: %w", s.previousFile(), err)
 	}
 	return nil
 }