From 5dac560a6390031368fc9f5433db42468732b602 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 --- CHANGELOG.md | 4 +++ server/types/app.go | 14 ++-------- store/iavl/store.go | 6 ++++ store/iavl/tree.go | 5 ++++ store/rootmulti/rollback_test.go | 48 ++++++++------------------------ store/rootmulti/store.go | 22 +++++++++++++++ store/types/store.go | 10 ------- 7 files changed, 50 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f683a9697040..1b69263fd761 100644 --- 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 index e182b01116e5..35ba119b9ed8 100644 --- 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 index 87f06ead65bd..9efbfd430e05 100644 --- 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 index 1056b1e03149..5a138d3ab586 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 @@ -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 index c2e199606e24..36fb13e636fb 100644 --- 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 index 6af51d9edf69..75b8571eb4a1 100644 --- 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 index 8b38c99290e3..62db149271cd 100644 --- 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-------------------------------