Fix Restart application issue

CHANGELOG.md

@@ -44,11 +44,23 @@ balances or a single balance by denom when the `denom` query parameter is presen
 
 ### API Breaking Changes
 
+* (types) [\#5579](https://github.com/cosmos/cosmos-sdk/pull/5579) The `keepRecent` field has been removed from the `PruningOptions` type.
+The `PruningOptions` type now only includes fields `KeepEvery` and `SnapshotEvery`, where `KeepEvery`
+determines which committed heights are flushed to disk and `SnapshotEvery` determines which of these
+heights are kept after pruning. The `IsValid` method should be called whenever using these options. Methods
+`SnapshotVersion` and `FlushVersion` accept a version arugment and determine if the version should be
+flushed to disk or kept as a snapshot. Note, `KeepRecent` is automatically inferred from the options
+and provided directly the IAVL store.
 * (modules) [\#5555](https://github.com/cosmos/cosmos-sdk/pull/5555) Move x/auth/client/utils/ types and functions to x/auth/client/.
 * (modules) [\#5572](https://github.com/cosmos/cosmos-sdk/pull/5572) Move account balance logic and APIs from `x/auth` to `x/bank`.
 
 ### Bug Fixes
 
+* (types) [\#5579](https://github.com/cosmos/cosmos-sdk/pull/5579) The IAVL `Store#Commit` method has been refactored to
+delete a flushed version if it is not a snapshot version. The root multi-store now keeps track of `commitInfo` instead
+of `types.CommitID`. During `Commit` of the root multi-store, `lastCommitInfo` is updated from the saved state
+and is only flushed to disk if it is a snapshot version. During `Query` of the root multi-store, if the request height
+is the latest height, we'll use the store's `lastCommitInfo`. Otherwise, we fetch `commitInfo` from disk.
 * (x/bank) [\#5531](https://github.com/cosmos/cosmos-sdk/issues/5531) Added missing amount event to MsgMultiSend, emitted for each output.
 * (client) [\#5618](https://github.com/cosmos/cosmos-sdk/pull/5618) Fix crash on the client when the verifier is not set. 
 

baseapp/baseapp_test.go

@@ -83,7 +83,7 @@ func TestMountStores(t *testing.T) {
 // Test that LoadLatestVersion actually does.
 func TestLoadVersion(t *testing.T) {
 	logger := defaultLogger()
-	pruningOpt := SetPruning(store.PruneSyncable)
+	pruningOpt := SetPruning(store.PruneNothing)
 	db := dbm.NewMemDB()
 	name := t.Name()
 	app := NewBaseApp(name, logger, db, nil, pruningOpt)
@@ -293,7 +293,7 @@ func TestAppVersionSetterGetter(t *testing.T) {
 
 func TestLoadVersionInvalid(t *testing.T) {
 	logger := log.NewNopLogger()
-	pruningOpt := SetPruning(store.PruneSyncable)
+	pruningOpt := SetPruning(store.PruneNothing)
 	db := dbm.NewMemDB()
 	name := t.Name()
 	app := NewBaseApp(name, logger, db, nil, pruningOpt)
@@ -326,6 +326,88 @@ func TestLoadVersionInvalid(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestLoadVersionPruning(t *testing.T) {
+	logger := log.NewNopLogger()
+	pruningOptions := store.PruningOptions{
+		KeepEvery:     2,
+		SnapshotEvery: 6,
+	}
+	pruningOpt := SetPruning(pruningOptions)
+	db := dbm.NewMemDB()
+	name := t.Name()
+	app := NewBaseApp(name, logger, db, nil, pruningOpt)
+
+	// make a cap key and mount the store
+	capKey := sdk.NewKVStoreKey(MainStoreKey)
+	app.MountStores(capKey)
+	err := app.LoadLatestVersion(capKey) // needed to make stores non-nil
+	require.Nil(t, err)
+
+	emptyCommitID := sdk.CommitID{}
+
+	// fresh store has zero/empty last commit
+	lastHeight := app.LastBlockHeight()
+	lastID := app.LastCommitID()
+	require.Equal(t, int64(0), lastHeight)
+	require.Equal(t, emptyCommitID, lastID)
+
+	// execute a block
+	header := abci.Header{Height: 1}
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
+	res := app.Commit()
+
+	// execute a block, collect commit ID
+	header = abci.Header{Height: 2}
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
+	res = app.Commit()
+	commitID2 := sdk.CommitID{Version: 2, Hash: res.Data}
+
+	// execute a block
+	header = abci.Header{Height: 3}
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
+	res = app.Commit()
+	commitID3 := sdk.CommitID{Version: 3, Hash: res.Data}
+
+	// reload with LoadLatestVersion, check it loads last flushed version
+	app = NewBaseApp(name, logger, db, nil, pruningOpt)
+	app.MountStores(capKey)
+	err = app.LoadLatestVersion(capKey)
+	require.Nil(t, err)
+	testLoadVersionHelper(t, app, int64(2), commitID2)
+
+	// re-execute block 3 and check it is same CommitID
+	header = abci.Header{Height: 3}
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
+	res = app.Commit()
+	recommitID3 := sdk.CommitID{Version: 3, Hash: res.Data}
+	require.Equal(t, commitID3, recommitID3, "Commits of identical blocks not equal after reload")
+
+	// execute a block, collect commit ID
+	header = abci.Header{Height: 4}
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
+	res = app.Commit()
+	commitID4 := sdk.CommitID{Version: 4, Hash: res.Data}
+
+	// execute a block
+	header = abci.Header{Height: 5}
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
+	res = app.Commit()
+
+	// reload with LoadLatestVersion, check it loads last flushed version
+	app = NewBaseApp(name, logger, db, nil, pruningOpt)
+	app.MountStores(capKey)
+	err = app.LoadLatestVersion(capKey)
+	require.Nil(t, err)
+	testLoadVersionHelper(t, app, int64(4), commitID4)
+
+	// reload with LoadVersion of previous flushed version
+	// and check it fails since previous flush should be pruned
+	app = NewBaseApp(name, logger, db, nil, pruningOpt)
+	app.MountStores(capKey)
+	err = app.LoadVersion(2, capKey)
+	require.NotNil(t, err)
+}
+
 func testLoadVersionHelper(t *testing.T, app *BaseApp, expectedHeight int64, expectedID sdk.CommitID) {
 	lastHeight := app.LastBlockHeight()
 	lastID := app.LastCommitID()

server/start.go

@@ -43,7 +43,7 @@ default, the application will run with Tendermint in process.
 
 Pruning options can be provided via the '--pruning' flag. The options are as follows:
 
-syncable: only those states not needed for state syncing will be deleted (keeps last 100 + every 10000th)
+syncable: only those states not needed for state syncing will be deleted (flushes every 100th to disk and keeps every 10000th)
 nothing: all historic states will be saved, nothing will be deleted (i.e. archiving node)
 everything: all saved states will be deleted, storing only the current state
 

store/cache/cache_test.go

@@ -20,7 +20,7 @@ func TestGetOrSetStoreCache(t *testing.T) {
 	sKey := types.NewKVStoreKey("test")
 	tree, err := iavl.NewMutableTree(db, 100)
 	require.NoError(t, err)
-	store := iavlstore.UnsafeNewStore(tree)
+	store := iavlstore.UnsafeNewStore(tree, types.PruneNothing)
 	store2 := mngr.GetStoreCache(sKey, store)
 
 	require.NotNil(t, store2)
@@ -34,7 +34,7 @@ func TestUnwrap(t *testing.T) {
 	sKey := types.NewKVStoreKey("test")
 	tree, err := iavl.NewMutableTree(db, 100)
 	require.NoError(t, err)
-	store := iavlstore.UnsafeNewStore(tree)
+	store := iavlstore.UnsafeNewStore(tree, types.PruneNothing)
 	_ = mngr.GetStoreCache(sKey, store)
 
 	require.Equal(t, store, mngr.Unwrap(sKey))
@@ -48,7 +48,7 @@ func TestStoreCache(t *testing.T) {
 	sKey := types.NewKVStoreKey("test")
 	tree, err := iavl.NewMutableTree(db, 100)
 	require.NoError(t, err)
-	store := iavlstore.UnsafeNewStore(tree)
+	store := iavlstore.UnsafeNewStore(tree, types.PruneNothing)
 	kvStore := mngr.GetStoreCache(sKey, store)
 
 	for i := uint(0); i < cache.DefaultCommitKVStoreCacheSize*2; i++ {

store/iavl/store.go

@@ -1,9 +1,11 @@
 package iavl
 
 import (
+	"fmt"
 	"io"
 	"sync"
 
+	"github.com/pkg/errors"
 	"github.com/tendermint/iavl"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto/merkle"
@@ -29,18 +31,38 @@ var (
 
 // Store Implements types.KVStore and CommitKVStore.
 type Store struct {
-	tree Tree
+	tree    Tree
+	pruning types.PruningOptions
 }
 
-// LoadStore returns an IAVL Store as a CommitKVStore. Internally it will load the
+// LoadStore returns an IAVL Store as a CommitKVStore. Internally, it will load the
 // store's version (id) from the provided DB. An error is returned if the version
 // fails to load.
 func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyLoading bool) (types.CommitKVStore, error) {
+	if !pruning.IsValid() {
+		return nil, fmt.Errorf("pruning options are invalid: %v", pruning)
+	}
+
+	var keepRecent int64
+
+	// Determine the value of keepRecent based on the following:
+	//
+	// If KeepEvery = 1, keepRecent should be 0 since there is no need to keep
+	// latest version in a in-memory cache.
+	//
+	// If KeepEvery > 1, keepRecent should be 1 so that state changes in between
+	// flushed states can be saved in the in-memory latest tree.
+	if pruning.KeepEvery == 1 {
+		keepRecent = 0
+	} else {
+		keepRecent = 1
+	}
+
 	tree, err := iavl.NewMutableTreeWithOpts(
 		db,
 		dbm.NewMemDB(),
 		defaultIAVLCacheSize,
-		iavl.PruningOptions(pruning.KeepEvery(), pruning.KeepRecent()),
+		iavl.PruningOptions(pruning.KeepEvery, keepRecent),
 	)
 	if err != nil {
 		return nil, err
@@ -56,15 +78,23 @@ func LoadStore(db dbm.DB, id types.CommitID, pruning types.PruningOptions, lazyL
 		return nil, err
 	}
 
-	return &Store{tree: tree}, nil
+	return &Store{
+		tree:    tree,
+		pruning: pruning,
+	}, nil
 }
 
 // UnsafeNewStore returns a reference to a new IAVL Store with a given mutable
-// IAVL tree reference.
+// IAVL tree reference. It should only be used for testing purposes.
 //
 // CONTRACT: The IAVL tree should be fully loaded.
-func UnsafeNewStore(tree *iavl.MutableTree) *Store {
-	return &Store{tree: tree}
+// CONTRACT: PruningOptions passed in as argument must be the same as pruning options
+// passed into iavl.MutableTree
+func UnsafeNewStore(tree *iavl.MutableTree, po types.PruningOptions) *Store {
+	return &Store{
+		tree:    tree,
+		pruning: po,
+	}
 }
 
 // GetImmutable returns a reference to a new store backed by an immutable IAVL
@@ -82,18 +112,36 @@ func (st *Store) GetImmutable(version int64) (*Store, error) {
 		return nil, err
 	}
 
-	return &Store{tree: &immutableTree{iTree}}, nil
+	return &Store{
+		tree:    &immutableTree{iTree},
+		pruning: st.pruning,
+	}, nil
 }
 
-// Implements Committer.
+// Commit commits the current store state and returns a CommitID with the new
+// version and hash.
 func (st *Store) Commit() types.CommitID {
-	// Save a new version.
 	hash, version, err := st.tree.SaveVersion()
 	if err != nil {
 		// TODO: Do we want to extend Commit to allow returning errors?
 		panic(err)
 	}
 
+	// If the version we saved got flushed to disk, check if previous flushed
+	// version should be deleted.
+	if st.pruning.FlushVersion(version) {
+		previous := version - st.pruning.KeepEvery
+
+		// Previous flushed version should only be pruned if the previous version is
+		// not a snapshot version OR if snapshotting is disabled (SnapshotEvery == 0).
+		if previous != 0 && !st.pruning.SnapshotVersion(previous) {
+			err := st.tree.DeleteVersion(previous)
+			if errCause := errors.Cause(err); errCause != nil && errCause != iavl.ErrVersionDoesNotExist {
+				panic(err)
+			}
+		}
+	}
+
 	return types.CommitID{
 		Version: version,
 		Hash:    hash,