diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index da095d6807..daf3c22ace 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -54,6 +54,7 @@ type Store struct { lazyLoading bool pruneHeights []int64 initialVersion int64 + removalMap map[types.StoreKey]bool traceWriter io.Writer traceContext types.TraceContext @@ -81,6 +82,7 @@ func NewStore(db dbm.DB) *Store { keysByName: make(map[string]types.StoreKey), pruneHeights: make([]int64, 0), listeners: make(map[types.StoreKey][]types.WriteListener), + removalMap: make(map[types.StoreKey]bool), } } @@ -210,9 +212,10 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { if err := deleteKVStore(store.(types.KVStore)); err != nil { return errors.Wrapf(err, "failed to delete store %s", key.Name()) } + rs.removalMap[key] = true } else if oldName := upgrades.RenamedFrom(key.Name()); oldName != "" { // handle renames specially - // make an unregistered key to satify loadCommitStore params + // make an unregistered key to satisfy loadCommitStore params oldKey := types.NewKVStoreKey(oldName) oldParams := storeParams oldParams.key = oldKey @@ -227,6 +230,11 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { if err := moveKVStoreData(oldStore.(types.KVStore), store.(types.KVStore)); err != nil { return errors.Wrapf(err, "failed to move store %s -> %s", oldName, key.Name()) } + + // add the old key so its deletion is committed + newStores[oldKey] = oldStore + // this will ensure it's not perpetually stored in commitInfo + rs.removalMap[oldKey] = true } } @@ -361,8 +369,19 @@ func (rs *Store) Commit() types.CommitID { previousHeight = rs.lastCommitInfo.GetVersion() version = previousHeight + 1 } + rs.lastCommitInfo = commitStores(version, rs.stores, rs.removalMap) - rs.lastCommitInfo = commitStores(version, rs.stores) + // remove remnants of removed stores + for sk := range rs.removalMap { + if _, ok := rs.stores[sk]; ok { + delete(rs.stores, sk) + delete(rs.storesParams, sk) + delete(rs.keysByName, sk.Name()) + } + } + + // reset the removalMap + rs.removalMap = make(map[types.StoreKey]bool) // Determine if pruneHeight height needs to be added to the list of heights to // be pruned, where pruneHeight = (commitHeight - 1) - KeepRecent. @@ -944,7 +963,7 @@ func getLatestVersion(db dbm.DB) int64 { } // Commits each store and returns a new commitInfo. -func commitStores(version int64, storeMap map[types.StoreKey]types.CommitKVStore) *types.CommitInfo { +func commitStores(version int64, storeMap map[types.StoreKey]types.CommitKVStore, removalMap map[types.StoreKey]bool) *types.CommitInfo { storeInfos := make([]types.StoreInfo, 0, len(storeMap)) for key, store := range storeMap { @@ -954,10 +973,12 @@ func commitStores(version int64, storeMap map[types.StoreKey]types.CommitKVStore continue } - si := types.StoreInfo{} - si.Name = key.Name() - si.CommitId = commitID - storeInfos = append(storeInfos, si) + if !removalMap[key] { + si := types.StoreInfo{} + si.Name = key.Name() + si.CommitId = commitID + storeInfos = append(storeInfos, si) + } } return &types.CommitInfo{ diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index cf7653b076..05817d6a84 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -300,8 +300,8 @@ func TestMultistoreLoadWithUpgrade(t *testing.T) { ci, err = getCommitInfo(db, 2) require.NoError(t, err) require.Equal(t, int64(2), ci.Version) - require.Equal(t, 4, len(ci.StoreInfos), ci.StoreInfos) - checkContains(t, ci.StoreInfos, []string{"store1", "restore2", "store3", "store4"}) + require.Equal(t, 3, len(ci.StoreInfos), ci.StoreInfos) + checkContains(t, ci.StoreInfos, []string{"store1", "restore2", "store4"}) } func TestParsePath(t *testing.T) { diff --git a/x/upgrade/types/storeloader_test.go b/x/upgrade/types/storeloader_test.go index ec2bfa824d..680e4d3568 100644 --- a/x/upgrade/types/storeloader_test.go +++ b/x/upgrade/types/storeloader_test.go @@ -90,6 +90,7 @@ func TestSetLoader(t *testing.T) { loadStoreKey string }{ "don't set loader": { + setLoader: nil, origStoreKey: "foo", loadStoreKey: "foo", }, @@ -145,9 +146,13 @@ func TestSetLoader(t *testing.T) { res := app.Commit() require.NotNil(t, res.Data) + // checking the case of the store being renamed + if tc.setLoader != nil { + checkStore(t, db, upgradeHeight, tc.origStoreKey, k, nil) + } + // check db is properly updated checkStore(t, db, upgradeHeight, tc.loadStoreKey, k, v) - checkStore(t, db, upgradeHeight, tc.loadStoreKey, []byte("foo"), nil) }) } }