From 6121dbf737ca6967f7089cf793624a1c8d38dfbc Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 31 Aug 2022 14:21:02 +0200 Subject: [PATCH] fix: rollback command don't actually delete multistore versions (backport #11361) (#13089) * fix: rollback command don't actually delete multistore versions (#11361) * rollback command don't actually delete multistore versions Closes: #11333 - add unit tests - use LoadVersionForOverwriting - update tendermint dependency to 0.35.x release branch Co-authored-by: Aleksandr Bezobchuk flushMetadata after rollback Update server/rollback.go Co-authored-by: Aleksandr Bezobchuk fix build gofumpt * fix unit test (cherry picked from commit 51d2de582d036ece4af10267e889f6fdd97f8acf) * fix unit test * changelog * api breaking changelog Co-authored-by: yihuang --- baseapp/baseapp.go | 7 ++++ server/mock/store.go | 4 ++ server/rollback.go | 11 +++-- server/util.go | 2 +- store/iavl/store.go | 6 +++ store/iavl/tree.go | 5 +++ store/rootmulti/rollback_test.go | 72 ++++++++++++++++++++++++++++++++ store/rootmulti/store.go | 32 +++++++------- store/types/store.go | 3 ++ 9 files changed, 121 insertions(+), 21 deletions(-) create mode 100644 store/rootmulti/rollback_test.go diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b36d06702990..792fb8118ad8 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -273,6 +273,13 @@ func DefaultStoreLoader(ms sdk.CommitMultiStore) error { return ms.LoadLatestVersion() } +// CommitMultiStore returns the root multi-store. +// App constructor can use this to access the `cms`. +// UNSAFE: must not be used during the abci life cycle. +func (app *BaseApp) CommitMultiStore() sdk.CommitMultiStore { + return app.cms +} + // LoadVersion loads the BaseApp application version. It will panic if called // more than once on a running baseapp. func (app *BaseApp) LoadVersion(version int64) error { diff --git a/server/mock/store.go b/server/mock/store.go index c8c233bb7c37..322540e06bb6 100644 --- a/server/mock/store.go +++ b/server/mock/store.go @@ -132,6 +132,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 { diff --git a/server/rollback.go b/server/rollback.go index 7378a1ff83bb..f39c40cac4ff 100644 --- a/server/rollback.go +++ b/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 diff --git a/server/util.go b/server/util.go index 14c1daed1581..9a84b111bac5 100644 --- a/server/util.go +++ b/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), ) } diff --git a/store/iavl/store.go b/store/iavl/store.go index bae00d79aa62..23b9f5733fe0 100644 --- a/store/iavl/store.go +++ b/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) diff --git a/store/iavl/tree.go b/store/iavl/tree.go index d1228c6822ee..04861e840639 100644 --- a/store/iavl/tree.go +++ b/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") +} diff --git a/store/rootmulti/rollback_test.go b/store/rootmulti/rollback_test.go new file mode 100644 index 000000000000..36fb13e636fb --- /dev/null +++ b/store/rootmulti/rollback_test.go @@ -0,0 +1,72 @@ +package rootmulti_test + +import ( + "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 TestRollback(t *testing.T) { + db := dbm.NewMemDB() + 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 + 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 + 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"))) + + // 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"))) +} diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 0e789909e506..2eae898af610 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -964,27 +964,27 @@ 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") - } - current := getLatestVersion(rs.db) - if target >= current { - return current - } - for ; current > target; current-- { - rs.pruneHeights = append(rs.pruneHeights, current) +func (rs *Store) RollbackToVersion(target int64) error { + if target <= 0 { + return fmt.Errorf("invalid rollback height target: %d", target) } rs.pruneStores() - // 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.lastCommitInfo, rs.pruneHeights) + + return rs.LoadLatestVersion() } type storeParams struct { diff --git a/store/types/store.go b/store/types/store.go index 1bad58ea2eba..15b336ea39cf 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -191,6 +191,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-------------------------------