fix: rollback command don't actually delete multistore versions (back…

…port cosmos#11361) (cosmos#13089) * fix: rollback command don't actually delete multistore versions (cosmos#11361) * rollback command don't actually delete multistore versions Closes: cosmos#11333 - add unit tests - use LoadVersionForOverwriting - update tendermint dependency to 0.35.x release branch Co-authored-by: Aleksandr Bezobchuk <[email protected]> flushMetadata after rollback Update server/rollback.go Co-authored-by: Aleksandr Bezobchuk <[email protected]> fix build gofumpt * fix unit test (cherry picked from commit 51d2de5) * fix unit test * changelog * api breaking changelog Co-authored-by: yihuang <[email protected]>
agoric-labs · Sep 28, 2024 · 5dac560 · 5dac560
1 parent 9e36bec
commit 5dac560
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -38,6 +38,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.
+
 ### Features
 
 * (x/authz) [#13047](https://github.com/cosmos/cosmos-sdk/pull/13047) Add a GetAuthorization function to the keeper.

diff --git a/server/types/app.go b/server/types/app.go
@@ -49,22 +49,12 @@ type (
 		RegisterTxService(client.Context)
 
 		// RegisterTendermintService registers the gRPC Query service for tendermint queries.
-		RegisterTendermintService(client.Context)
+		RegisterTendermintService(clientCtx client.Context)
 
-		// CommitMultiStore Returns the multistore instance
+		// Return the multistore instance
 		CommitMultiStore() sdk.CommitMultiStore
 	}
 
-	// ApplicationQueryService defines an extension of the Application interface
-	// that facilitates gRPC query Services.
-	//
-	// NOTE: This interfaces exists only in the v0.45.x line to ensure the existing
-	// Application interface does not introduce API breaking changes.
-	ApplicationQueryService interface {
-		// RegisterNodeService registers the node gRPC Query service.
-		RegisterNodeService(client.Context)
-	}
-
 	// AppCreator is a function that allows us to lazily initialize an
 	// application using various configurations.
 	AppCreator[T Application] func(log.Logger, corestore.KVStoreWithBatch, io.Writer, AppOptions) T

diff --git a/store/iavl/store.go b/store/iavl/store.go
@@ -252,6 +252,12 @@ func (st *Store) LoadVersionForOverwriting(targetVersion int64) (int64, error) {
 	return st.tree.LoadVersionForOverwriting(targetVersion)
 }
 
+// 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)

diff --git a/store/iavl/tree.go b/store/iavl/tree.go
@@ -34,6 +34,7 @@ type (
 		SetInitialVersion(version uint64)
 		Iterator(start, end []byte, ascending bool) (types.Iterator, error)
 		AvailableVersions() []int
+		LoadVersionForOverwriting(targetVersion int64) (int64, error)
 	}
 
 	// immutableTree is a simple wrapper around a reference to an iavl.ImmutableTree
@@ -95,3 +96,7 @@ func (it *immutableTree) GetImmutable(version int64) (*iavl.ImmutableTree, error
 func (it *immutableTree) LoadVersionForOverwriting(targetVersion int64) (int64, error) {
 	panic("cannot call 'LoadVersionForOverwriting' on an immutable IAVL tree")
 }
+
+func (it *immutableTree) LoadVersionForOverwriting(targetVersion int64) (int64, error) {
+	panic("cannot call 'LoadVersionForOverwriting' on an immutable IAVL tree")
+}
diff --git a/store/rootmulti/rollback_test.go b/store/rootmulti/rollback_test.go
@@ -1,7 +1,6 @@
 package rootmulti_test
 
 import (
-	"encoding/json"
 	"fmt"
 	"testing"
 
@@ -13,42 +12,18 @@ import (
 	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) {
-	t.Skip()
 	db := dbm.NewMemDB()
-	app := SetupWithDB(false, db)
+	options := simapp.SetupOptions{
+		Logger:             log.NewNopLogger(),
+		DB:                 db,
+		InvCheckPeriod:     0,
+		EncConfig:          simapp.MakeTestEncodingConfig(),
+		HomePath:           simapp.DefaultNodeHome,
+		SkipUpgradeHeights: map[int64]bool{},
+		AppOpts:            simapp.EmptyAppOptions{},
+	}
+	app := simapp.NewSimappWithCustomOptions(t, false, options)
 	app.Commit()
 	ver0 := app.LastBlockHeight()
 	// commit 10 blocks
@@ -74,8 +49,7 @@ func TestRollback(t *testing.T) {
 	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{})
+	app = simapp.NewSimApp(options.Logger, options.DB, nil, true, map[int64]bool{}, simapp.DefaultNodeHome, options.InvCheckPeriod, options.EncConfig, options.AppOpts)
 	store = app.NewContext(true, tmproto.Header{}).KVStore(app.GetKey("bank"))
 	require.Equal(t, []byte("value5"), store.Get([]byte("key")))
 

diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
@@ -1014,6 +1014,28 @@ func (rs *Store) RollbackToVersion(target int64) error {
 		}
 	}
 
+	rs.flushMetadata(rs.db, target, rs.buildCommitInfo(target))
+
+	return rs.LoadLatestVersion()
+}
+
+func (rs *Store) flushMetadata(db dbm.DB, version int64, cInfo *types.CommitInfo) {
+	rs.logger.Debug("flushing metadata", "height", version)
+	batch := db.NewBatch()
+	defer batch.Close()
+
+	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
+			}
+		}
+	}
+
 	flushMetadata(rs.db, target, rs.buildCommitInfo(target), []int64{})
 
 	return rs.LoadLatestVersion()

diff --git a/store/types/store.go b/store/types/store.go
@@ -214,18 +214,8 @@ type CommitMultiStore interface {
 	// SetIAVLCacheSize sets the cache size of the IAVL tree.
 	SetIAVLCacheSize(size int)
 
-	// SetIAVLDisableFastNode enables/disables fastnode feature on iavl.
-	SetIAVLDisableFastNode(disable bool)
-
 	// RollbackToVersion rollback the db to specific version(height).
 	RollbackToVersion(version int64) error
-
-	// ListeningEnabled returns if listening is enabled for the KVStore belonging the provided StoreKey
-	ListeningEnabled(key StoreKey) bool
-
-	// AddListeners adds WriteListeners for the KVStore belonging to the provided StoreKey
-	// It appends the listeners to a current set, if one already exists
-	AddListeners(key StoreKey, listeners []WriteListener)
 }
 
 //---------subsp-------------------------------