fix: rollback command don't actually delete multistore versions (backport #11361)

CHANGELOG.md

@@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### API Breaking Changes
+
+- (cli) [#13089](https://github.com/cosmos/cosmos-sdk/pull/13089) Fix rollback command don't actually delete multistore versions, added method `RollbackToVersion` to interface `CommitMultiStore` and added method `CommitMultiStore` to `Application` interface.
+
 ## v0.45.8 - 2022-08-25
 
 ### Improvements

baseapp/baseapp.go

@@ -273,11 +273,8 @@ func DefaultStoreLoader(ms sdk.CommitMultiStore) error {
 
 // CommitMultiStore returns the root multi-store.
 // App constructor can use this to access the `cms`.
-// UNSAFE: only safe to use during app initialization.
+// UNSAFE: must not be used during the abci life cycle.
 func (app *BaseApp) CommitMultiStore() sdk.CommitMultiStore {
-	if app.sealed {
-		panic("cannot call CommitMultiStore() after baseapp is sealed")
-	}
 	return app.cms
 }
 

server/mock/store.go

@@ -135,6 +135,10 @@ func (ms multiStore) Restore(
 	panic("not implemented")
 }
 
+func (ms multiStore) RollbackToVersion(version int64) error {
+	panic("not implemented")
+}
+
 var _ sdk.KVStore = kvStore{}
 
 type kvStore struct {

server/rollback.go

@@ -4,13 +4,13 @@ import (
 	"fmt"
 
 	"github.com/cosmos/cosmos-sdk/client/flags"
-	"github.com/cosmos/cosmos-sdk/store/rootmulti"
+	"github.com/cosmos/cosmos-sdk/server/types"
 	"github.com/spf13/cobra"
 	tmcmd "github.com/tendermint/tendermint/cmd/tendermint/commands"
 )
 
 // NewRollbackCmd creates a command to rollback tendermint and multistore state by one height.
-func NewRollbackCmd(defaultNodeHome string) *cobra.Command {
+func NewRollbackCmd(appCreator types.AppCreator, defaultNodeHome string) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "rollback",
 		Short: "rollback cosmos-sdk and tendermint state by one height",
@@ -30,14 +30,17 @@ application.
 			if err != nil {
 				return err
 			}
+			app := appCreator(ctx.Logger, db, nil, ctx.Viper)
 			// rollback tendermint state
 			height, hash, err := tmcmd.RollbackState(ctx.Config)
 			if err != nil {
 				return fmt.Errorf("failed to rollback tendermint state: %w", err)
 			}
 			// rollback the multistore
-			cms := rootmulti.NewStore(db, ctx.Logger)
-			cms.RollbackToVersion(height)
+
+			if err := app.CommitMultiStore().RollbackToVersion(height); err != nil {
+				return fmt.Errorf("failed to rollback to version: %w", err)
+			}
 
 			fmt.Printf("Rolled back state to height %d and hash %X", height, hash)
 			return nil

server/util.go

@@ -283,7 +283,7 @@ func AddCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator type
 		tendermintCmd,
 		ExportCmd(appExport, defaultNodeHome),
 		version.NewVersionCommand(),
-		NewRollbackCmd(defaultNodeHome),
+		NewRollbackCmd(appCreator, defaultNodeHome),
 	)
 }
 

store/iavl/store.go

@@ -233,6 +233,12 @@ func (st *Store) DeleteVersions(versions ...int64) error {
 	return st.tree.DeleteVersions(versions...)
 }
 
+// LoadVersionForOverwriting attempts to load a tree at a previously committed
+// version, or the latest version below it. Any versions greater than targetVersion will be deleted.
+func (st *Store) LoadVersionForOverwriting(targetVersion int64) (int64, error) {
+	return st.tree.LoadVersionForOverwriting(targetVersion)
+}
+
 // Implements types.KVStore.
 func (st *Store) Iterator(start, end []byte) types.Iterator {
 	iterator, err := st.tree.Iterator(start, end, true)

store/iavl/tree.go

@@ -33,6 +33,7 @@ type (
 		GetImmutable(version int64) (*iavl.ImmutableTree, error)
 		SetInitialVersion(version uint64)
 		Iterator(start, end []byte, ascending bool) (types.Iterator, error)
+		LoadVersionForOverwriting(targetVersion int64) (int64, error)
 	}
 
 	// immutableTree is a simple wrapper around a reference to an iavl.ImmutableTree
@@ -94,3 +95,7 @@ func (it *immutableTree) GetImmutable(version int64) (*iavl.ImmutableTree, error
 
 	return it.ImmutableTree, nil
 }
+
+func (it *immutableTree) LoadVersionForOverwriting(targetVersion int64) (int64, error) {
+	panic("cannot call 'LoadVersionForOverwriting' on an immutable IAVL tree")
+}

store/rootmulti/rollback_test.go

@@ -0,0 +1,97 @@
+package rootmulti_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	"github.com/cosmos/cosmos-sdk/simapp"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/libs/log"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+	dbm "github.com/tendermint/tm-db"
+)
+
+func setup(withGenesis bool, invCheckPeriod uint, db dbm.DB) (*simapp.SimApp, simapp.GenesisState) {
+	encCdc := simapp.MakeTestEncodingConfig()
+	app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, simapp.DefaultNodeHome, invCheckPeriod, encCdc, simapp.EmptyAppOptions{})
+	if withGenesis {
+		return app, simapp.NewDefaultGenesisState(encCdc.Marshaler)
+	}
+	return app, simapp.GenesisState{}
+}
+
+// Setup initializes a new SimApp. A Nop logger is set in SimApp.
+func SetupWithDB(isCheckTx bool, db dbm.DB) *simapp.SimApp {
+	app, genesisState := setup(!isCheckTx, 5, db)
+	if !isCheckTx {
+		// init chain must be called to stop deliverState from being nil
+		stateBytes, err := json.MarshalIndent(genesisState, "", " ")
+		if err != nil {
+			panic(err)
+		}
+
+		// Initialize the chain
+		app.InitChain(
+			abci.RequestInitChain{
+				Validators:      []abci.ValidatorUpdate{},
+				ConsensusParams: simapp.DefaultConsensusParams,
+				AppStateBytes:   stateBytes,
+			},
+		)
+	}
+
+	return app
+}
+
+func TestRollback(t *testing.T) {
+	db := dbm.NewMemDB()
+	app := SetupWithDB(false, db)
+	app.Commit()
+	ver0 := app.LastBlockHeight()
+	// commit 10 blocks
+	for i := int64(1); i <= 10; i++ {
+		header := tmproto.Header{
+			Height:  ver0 + i,
+			AppHash: app.LastCommitID().Hash,
+		}
+		app.BeginBlock(abci.RequestBeginBlock{Header: header})
+		ctx := app.NewContext(false, header)
+		store := ctx.KVStore(app.GetKey("bank"))
+		store.Set([]byte("key"), []byte(fmt.Sprintf("value%d", i)))
+		app.Commit()
+	}
+
+	require.Equal(t, ver0+10, app.LastBlockHeight())
+	store := app.NewContext(true, tmproto.Header{}).KVStore(app.GetKey("bank"))
+	require.Equal(t, []byte("value10"), store.Get([]byte("key")))
+
+	// rollback 5 blocks
+	target := ver0 + 5
+	require.NoError(t, app.CommitMultiStore().RollbackToVersion(target))
+	require.Equal(t, target, app.LastBlockHeight())
+
+	// recreate app to have clean check state
+	encCdc := simapp.MakeTestEncodingConfig()
+	app = simapp.NewSimApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, simapp.DefaultNodeHome, 5, encCdc, simapp.EmptyAppOptions{})
+	store = app.NewContext(true, tmproto.Header{}).KVStore(app.GetKey("bank"))
+	require.Equal(t, []byte("value5"), store.Get([]byte("key")))
+
+	// commit another 5 blocks with different values
+	for i := int64(6); i <= 10; i++ {
+		header := tmproto.Header{
+			Height:  ver0 + i,
+			AppHash: app.LastCommitID().Hash,
+		}
+		app.BeginBlock(abci.RequestBeginBlock{Header: header})
+		ctx := app.NewContext(false, header)
+		store := ctx.KVStore(app.GetKey("bank"))
+		store.Set([]byte("key"), []byte(fmt.Sprintf("VALUE%d", i)))
+		app.Commit()
+	}
+
+	require.Equal(t, ver0+10, app.LastBlockHeight())
+	store = app.NewContext(true, tmproto.Header{}).KVStore(app.GetKey("bank"))
+	require.Equal(t, []byte("VALUE10"), store.Get([]byte("key")))
+}

store/rootmulti/store.go

@@ -903,27 +903,26 @@ func (rs *Store) buildCommitInfo(version int64) *types.CommitInfo {
 }
 
 // RollbackToVersion delete the versions after `target` and update the latest version.
-func (rs *Store) RollbackToVersion(target int64) int64 {
-	if target < 0 {
-		panic("Negative rollback target")
+func (rs *Store) RollbackToVersion(target int64) error {
+	if target <= 0 {
+		return fmt.Errorf("invalid rollback height target: %d", target)
 	}
-	current := GetLatestVersion(rs.db)
-	if target >= current {
-		return current
-	}
-	for ; current > target; current-- {
-		rs.pruneHeights = append(rs.pruneHeights, current)
-	}
-	rs.PruneStores(true, nil)
 
-	// update latest height
-	bz, err := gogotypes.StdInt64Marshal(current)
-	if err != nil {
-		panic(err)
+	for key, store := range rs.stores {
+		if store.GetStoreType() == types.StoreTypeIAVL {
+			// If the store is wrapped with an inter-block cache, we must first unwrap
+			// it to get the underlying IAVL store.
+			store = rs.GetCommitKVStore(key)
+			_, err := store.(*iavl.Store).LoadVersionForOverwriting(target)
+			if err != nil {
+				return err
+			}
+		}
 	}
 
-	rs.db.Set([]byte(latestVersionKey), bz)
-	return current
+	flushMetadata(rs.db, target, rs.buildCommitInfo(target), []int64{})
+
+	return rs.LoadLatestVersion()
 }
 
 type storeParams struct {

store/types/store.go

@@ -190,6 +190,9 @@ type CommitMultiStore interface {
 
 	// SetIAVLCacheSize sets the cache size of the IAVL tree.
 	SetIAVLCacheSize(size int)
+
+	// RollbackToVersion rollback the db to specific version(height).
+	RollbackToVersion(version int64) error
 }
 
 //---------subsp-------------------------------