From a067eef9ed6b97b230c3af2e5a0fc8418c4932ee Mon Sep 17 00:00:00 2001 From: yihuang Date: Tue, 30 Aug 2022 13:50:00 +0800 Subject: [PATCH 1/4] 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) --- baseapp/baseapp.go | 5 +-- server/mock/store.go | 4 ++ server/rollback.go | 11 +++-- server/types/app.go | 4 ++ server/util.go | 2 +- store/iavl/store.go | 6 +++ store/iavl/tree.go | 5 +++ store/rootmulti/rollback_test.go | 69 ++++++++++++++++++++++++++++++++ store/rootmulti/store.go | 35 ++++++++-------- store/types/store.go | 3 ++ 10 files changed, 116 insertions(+), 28 deletions(-) create mode 100644 store/rootmulti/rollback_test.go diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 6e95e65ab2ff..28ec7e3c2733 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -276,11 +276,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 } diff --git a/server/mock/store.go b/server/mock/store.go index ebc4caaa421f..8eaa002457ba 100644 --- a/server/mock/store.go +++ b/server/mock/store.go @@ -144,6 +144,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 e99088dd73cc..c3dba335466b 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/types/app.go b/server/types/app.go index 467f627c605f..69cc0476f046 100644 --- a/server/types/app.go +++ b/server/types/app.go @@ -15,6 +15,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/server/api" "github.com/cosmos/cosmos-sdk/server/config" + sdk "github.com/cosmos/cosmos-sdk/types" ) // ServerStartTime defines the time duration that the server need to stay running after startup @@ -51,6 +52,9 @@ type ( // RegisterTendermintService registers the gRPC Query service for tendermint queries. RegisterTendermintService(clientCtx client.Context) + + // Return the multistore instance + CommitMultiStore() sdk.CommitMultiStore } // AppCreator is a function that allows us to lazily initialize an diff --git a/server/util.go b/server/util.go index 9f90209c31be..564a26a3fde0 100644 --- a/server/util.go +++ b/server/util.go @@ -290,7 +290,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 e8b12c47b40b..d134d1fa7b47 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -234,6 +234,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 210663a21509..81350fc34a8b 100644 --- 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 @@ -99,3 +100,7 @@ func (it *immutableTree) GetImmutable(version int64) (*iavl.ImmutableTree, error func (it *immutableTree) AvailableVersions() []int { return []int{} } + +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..5bb94acbb74f --- /dev/null +++ b/store/rootmulti/rollback_test.go @@ -0,0 +1,69 @@ +package rootmulti_test + +import ( + "fmt" + "testing" + + "github.com/cosmos/cosmos-sdk/simapp" + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + "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, + AppOpts: simtestutil.NewAppOptionsWithFlagHome(simapp.DefaultNodeHome), + } + 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, simtestutil.NewAppOptionsWithFlagHome(simapp.DefaultNodeHome)) + 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 d8f02059bac5..0da37a788a7d 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -924,29 +924,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") - } - current := getLatestVersion(rs.db) - if target >= current { - return current - } - for ; current > target; current-- { - rs.pruningManager.HandleHeight(current) - } - if err := rs.pruneStores(); err != nil { - panic(err) +func (rs *Store) RollbackToVersion(target int64) error { + if target <= 0 { + return fmt.Errorf("invalid rollback height target: %d", target) } - // 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 + rs.flushMetadata(rs.db, target, rs.buildCommitInfo(target)) + + return rs.LoadLatestVersion() } func (rs *Store) flushMetadata(db dbm.DB, version int64, cInfo *types.CommitInfo) { diff --git a/store/types/store.go b/store/types/store.go index 37f583900909..0ef43576d426 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -188,6 +188,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------------------------------- From 00ad2a3a746ae31038afdce261524088000fbfa7 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 30 Aug 2022 13:56:44 +0800 Subject: [PATCH 2/4] fix unit test --- store/rootmulti/rollback_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/store/rootmulti/rollback_test.go b/store/rootmulti/rollback_test.go index 5bb94acbb74f..36fb13e636fb 100644 --- a/store/rootmulti/rollback_test.go +++ b/store/rootmulti/rollback_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/cosmos/cosmos-sdk/simapp" - simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" @@ -16,9 +15,13 @@ import ( func TestRollback(t *testing.T) { db := dbm.NewMemDB() options := simapp.SetupOptions{ - Logger: log.NewNopLogger(), - DB: db, - AppOpts: simtestutil.NewAppOptionsWithFlagHome(simapp.DefaultNodeHome), + 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() @@ -46,7 +49,7 @@ func TestRollback(t *testing.T) { require.Equal(t, target, app.LastBlockHeight()) // recreate app to have clean check state - app = simapp.NewSimApp(options.Logger, options.DB, nil, true, simtestutil.NewAppOptionsWithFlagHome(simapp.DefaultNodeHome)) + 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"))) From 2ec56aa83350069e6b5340a285f207a9f6011954 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 30 Aug 2022 14:52:54 +0800 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6461297889d9..7d233a55137e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (export) [#13029](https://github.com/cosmos/cosmos-sdk/pull/13029) Fix exporting the blockParams regression. * [#13046](https://github.com/cosmos/cosmos-sdk/pull/13046) Fix missing return statement in BaseApp.Query. +* (cli) [#13089](https://github.com/cosmos/cosmos-sdk/pull/13089) Fix rollback command don't actually delete multistore versions. ## [v0.46.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.1) - 2022-08-24 From b3249479162056ed7e097b7936d7c719a9b4c190 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 30 Aug 2022 22:27:42 +0800 Subject: [PATCH 4/4] api breaking changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d233a55137e..f99d7529f9cd 100644 --- a/CHANGELOG.md +++ b/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. + ### Features * (x/authz) [#13047](https://github.com/cosmos/cosmos-sdk/pull/13047) Add a GetAuthorization function to the keeper. @@ -49,7 +53,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (export) [#13029](https://github.com/cosmos/cosmos-sdk/pull/13029) Fix exporting the blockParams regression. * [#13046](https://github.com/cosmos/cosmos-sdk/pull/13046) Fix missing return statement in BaseApp.Query. -* (cli) [#13089](https://github.com/cosmos/cosmos-sdk/pull/13089) Fix rollback command don't actually delete multistore versions. ## [v0.46.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.1) - 2022-08-24