Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,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
}

Expand Down
4 changes: 4 additions & 0 deletions server/mock/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ func (ms multiStore) Restore(
panic("not implemented")
}

func (ms multiStore) RollbackToVersion(version int64) error {
panic("not implemented")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we are adding a function which panics?

Copy link
Collaborator Author

@yihuang yihuang May 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the method is added to the interface CommitMultiStore. so we need this to make the unit tests compile.

}

var _ sdk.KVStore = kvStore{}

type kvStore struct {
Expand Down
11 changes: 7 additions & 4 deletions server/rollback.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -30,14 +30,17 @@ application.
if err != nil {
return err
}
app := appCreator(ctx.Logger, db, nil, ctx.Viper)
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
4 changes: 4 additions & 0 deletions server/types/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion server/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func AddCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator type
tendermintCmd,
ExportCmd(appExport, defaultNodeHome),
version.NewVersionCommand(),
NewRollbackCmd(defaultNodeHome),
NewRollbackCmd(appCreator, defaultNodeHome),
)
}

Expand Down
6 changes: 6 additions & 0 deletions store/iavl/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions store/iavl/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
69 changes: 69 additions & 0 deletions store/rootmulti/rollback_test.go
Original file line number Diff line number Diff line change
@@ -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")))
}
35 changes: 16 additions & 19 deletions store/rootmulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -933,29 +933,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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Isn't there other metadata we need to flush too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there other metadata we need to flush too?

is there?

if target <= 0 {
yihuang marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down
3 changes: 3 additions & 0 deletions store/types/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-------------------------------
Expand Down