From 8b780a7a7db01a2421e3ec2adbb9d2203288d05f Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 18:05:11 +0200
Subject: [PATCH 01/23] Added LoadLatestVersionAndUpgrade function

---
 server/mock/store.go     |  5 +++++
 store/rootmulti/store.go | 21 ++++++++++++++++++---
 store/types/store.go     | 16 ++++++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/server/mock/store.go b/server/mock/store.go
index cbf288b9ad62..16f6833f6a2e 100644
--- a/server/mock/store.go
+++ b/server/mock/store.go
@@ -5,6 +5,7 @@ import (
 
 	dbm "github.com/tendermint/tm-db"
 
+	store "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -70,6 +71,10 @@ func (ms multiStore) LoadLatestVersion() error {
 	return nil
 }
 
+func (ms multiStore) LoadLatestVersionAndUpgrade(upgrades *store.StoreUpgrades) error {
+	return nil
+}
+
 func (ms multiStore) LoadVersion(ver int64) error {
 	panic("not implemented")
 }
diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 496fdb0ef601..3d488ebd9c95 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -100,14 +100,24 @@ func (rs *Store) GetCommitKVStore(key types.StoreKey) types.CommitKVStore {
 	return rs.stores[key].(types.CommitKVStore)
 }
 
-// Implements CommitMultiStore.
+// LoadLatestVersionAndUpgrade implements CommitMultiStore
+func (rs *Store) LoadLatestVersionAndUpgrade(upgrades *types.StoreUpgrades) error {
+	ver := getLatestVersion(rs.db)
+	return rs.loadVersion(ver, upgrades)
+}
+
+// LoadLatestVersion implements CommitMultiStore.
 func (rs *Store) LoadLatestVersion() error {
 	ver := getLatestVersion(rs.db)
-	return rs.LoadVersion(ver)
+	return rs.loadVersion(ver, nil)
 }
 
-// Implements CommitMultiStore.
+// LoadVersion implements CommitMultiStore.
 func (rs *Store) LoadVersion(ver int64) error {
+	return rs.loadVersion(ver, nil)
+}
+
+func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 	if ver == 0 {
 		// Special logic for version 0 where there is no need to get commit
 		// information.
@@ -129,6 +139,11 @@ func (rs *Store) LoadVersion(ver int64) error {
 		return err
 	}
 
+	if upgrades != nil {
+		// TODO: run upgrades if present
+
+	}
+
 	// convert StoreInfos slice to map
 	infos := make(map[types.StoreKey]storeInfo)
 	for _, storeInfo := range cInfo.StoreInfos {
diff --git a/store/types/store.go b/store/types/store.go
index a85c93d548fa..d44a8fc57268 100644
--- a/store/types/store.go
+++ b/store/types/store.go
@@ -38,6 +38,17 @@ type Queryable interface {
 //----------------------------------------
 // MultiStore
 
+type StoreUpgrades struct {
+	New     []StoreKey
+	Renamed []StoreRename
+	Deleted []StoreKey
+}
+
+type StoreRename struct {
+	OldKey StoreKey
+	NewKey StoreKey
+}
+
 type MultiStore interface { //nolint
 	Store
 
@@ -94,6 +105,11 @@ type CommitMultiStore interface {
 	// Mount*Store() are complete.
 	LoadLatestVersion() error
 
+	// LoadLatestVersionAndUpgrade will load the latest version, but also
+	// rename/delete/create sub-store keys, before registering all the keys
+	// in order to handle breaking formats in migrations
+	LoadLatestVersionAndUpgrade(upgrades *StoreUpgrades) error
+
 	// Load a specific persisted version. When you load an old version, or when
 	// the last commit attempt didn't complete, the next commit after loading
 	// must be idempotent (return the same commit id). Otherwise the behavior is

From 5249cd2132faf8328d6e4443a7cce379ad23d8d7 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 18:09:14 +0200
Subject: [PATCH 02/23] Simplify LoadVersion logic

---
 store/rootmulti/store.go | 47 +++++++++++++++-------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 3d488ebd9c95..c7447a3e040d 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -118,44 +118,32 @@ func (rs *Store) LoadVersion(ver int64) error {
 }
 
 func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
-	if ver == 0 {
-		// Special logic for version 0 where there is no need to get commit
-		// information.
-		for key, storeParams := range rs.storesParams {
-			store, err := rs.loadCommitStoreFromParams(key, types.CommitID{}, storeParams)
-			if err != nil {
-				return fmt.Errorf("failed to load Store: %v", err)
-			}
+	infos := make(map[string]storeInfo)
+	var lastCommitID types.CommitID
 
-			rs.stores[key] = store
+	// load old data if we are not version 0
+	if ver != 0 {
+		cInfo, err := getCommitInfo(rs.db, ver)
+		if err != nil {
+			return err
 		}
 
-		rs.lastCommitID = types.CommitID{}
-		return nil
-	}
-
-	cInfo, err := getCommitInfo(rs.db, ver)
-	if err != nil {
-		return err
-	}
-
-	if upgrades != nil {
-		// TODO: run upgrades if present
-
-	}
+		if upgrades != nil {
+			// TODO: run upgrades if present
+		}
 
-	// convert StoreInfos slice to map
-	infos := make(map[types.StoreKey]storeInfo)
-	for _, storeInfo := range cInfo.StoreInfos {
-		infos[rs.nameToKey(storeInfo.Name)] = storeInfo
+		// convert StoreInfos slice to map
+		for _, storeInfo := range cInfo.StoreInfos {
+			infos[storeInfo.Name] = storeInfo
+		}
+		lastCommitID = cInfo.CommitID()
 	}
 
 	// load each Store
 	var newStores = make(map[types.StoreKey]types.CommitStore)
 	for key, storeParams := range rs.storesParams {
 		var id types.CommitID
-
-		info, ok := infos[key]
+		info, ok := infos[key.Name()]
 		if ok {
 			id = info.Core.CommitID
 		}
@@ -164,11 +152,10 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 		if err != nil {
 			return fmt.Errorf("failed to load Store: %v", err)
 		}
-
 		newStores[key] = store
 	}
 
-	rs.lastCommitID = cInfo.CommitID()
+	rs.lastCommitID = lastCommitID
 	rs.stores = newStores
 
 	return nil

From 5178339a12c46cfde9acb2d97919b8f7b97ec315 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 18:28:03 +0200
Subject: [PATCH 03/23] Wrote simple LoadLatestVersionAndUpgrade test cases

---
 store/rootmulti/store_test.go | 86 +++++++++++++++++++++++++++++++++++
 store/types/store.go          |  8 ++--
 2 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index 8574b60f843a..c723dd4c1e00 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -156,6 +156,70 @@ func TestMultistoreCommitLoad(t *testing.T) {
 	checkStore(t, store, commitID, commitID)
 }
 
+func TestMultistoreLoadWithUpgrade(t *testing.T) {
+	var db dbm.DB = dbm.NewMemDB()
+	store := newMultiStoreWithMounts(db)
+	err := store.LoadLatestVersion()
+	require.Nil(t, err)
+
+	// write some data in all stores
+	k1, v1 := []byte("first"), []byte("store")
+	s1, _ := store.getStoreByName("store1").(types.KVStore)
+	require.NotNil(t, s1)
+	s1.Set(k1, v1)
+
+	k2, v2 := []byte("second"), []byte("restore")
+	s2, _ := store.getStoreByName("store2").(types.KVStore)
+	require.NotNil(t, s2)
+	s2.Set(k2, v2)
+
+	k3, v3 := []byte("third"), []byte("dropped")
+	s3, _ := store.getStoreByName("store3").(types.KVStore)
+	require.NotNil(t, s3)
+	s3.Set(k3, v3)
+
+	// do one commit
+	commitID := store.Commit()
+	expectedCommitID := getExpectedCommitID(store, 1)
+	checkStore(t, store, expectedCommitID, commitID)
+
+	// Load without changes and make sure it is sensible
+	store = newMultiStoreWithMounts(db)
+	err = store.LoadLatestVersion()
+	require.Nil(t, err)
+	commitID = getExpectedCommitID(store, 1)
+	checkStore(t, store, commitID, commitID)
+
+	// let's query data to see it was saved properly
+	s2, _ = store.getStoreByName("store2").(types.KVStore)
+	require.NotNil(t, s2)
+	require.Equal(t, v2, s2.Get(k2))
+
+	// now, let's load with upgrades...
+	restore, upgrades := newMultiStoreWithModifiedMounts(db)
+	err = restore.LoadLatestVersionAndUpgrade(upgrades)
+	require.Nil(t, err)
+
+	// s1 was not changed
+	s1, _ = restore.getStoreByName("store1").(types.KVStore)
+	require.NotNil(t, s1)
+	require.Equal(t, v1, s1.Get(k1))
+
+	// store3 is mounted, but data deleted are gone
+	s3, _ = restore.getStoreByName("store3").(types.KVStore)
+	require.NotNil(t, s3)
+	require.Equal(t, nil, s3.Get(k3)) // data was deleted
+
+	// store2 is no longer mounted
+	st2 := restore.getStoreByName("store2")
+	require.Nil(t, st2)
+
+	// restore2 has the old data
+	rs1, _ := restore.getStoreByName("restore2").(types.KVStore)
+	require.NotNil(t, rs1)
+	require.Equal(t, v1, rs1.Get(k1))
+}
+
 func TestParsePath(t *testing.T) {
 	_, _, err := parsePath("foo")
 	require.Error(t, err)
@@ -262,6 +326,28 @@ func newMultiStoreWithMounts(db dbm.DB) *Store {
 	return store
 }
 
+// store2 -> restore2
+// store3 dropped data (but mount still there to test)
+func newMultiStoreWithModifiedMounts(db dbm.DB) (*Store, *types.StoreUpgrades) {
+	store := NewStore(db)
+	store.pruningOpts = types.PruneSyncable
+	store.MountStoreWithDB(
+		types.NewKVStoreKey("store1"), types.StoreTypeIAVL, nil)
+	store.MountStoreWithDB(
+		types.NewKVStoreKey("restore2"), types.StoreTypeIAVL, nil)
+	store.MountStoreWithDB(
+		types.NewKVStoreKey("store3"), types.StoreTypeIAVL, nil)
+
+	upgrades := &types.StoreUpgrades{
+		Renamed: []types.StoreRename{{
+			OldKey: "store2",
+			NewKey: "restore2",
+		}},
+		Deleted: []string{"store3"},
+	}
+	return store, upgrades
+}
+
 func checkStore(t *testing.T, store *Store, expect, got types.CommitID) {
 	require.Equal(t, expect, got)
 	require.Equal(t, expect, store.LastCommitID())
diff --git a/store/types/store.go b/store/types/store.go
index d44a8fc57268..1a0daef532c1 100644
--- a/store/types/store.go
+++ b/store/types/store.go
@@ -39,14 +39,14 @@ type Queryable interface {
 // MultiStore
 
 type StoreUpgrades struct {
-	New     []StoreKey
+	New     []string
 	Renamed []StoreRename
-	Deleted []StoreKey
+	Deleted []string
 }
 
 type StoreRename struct {
-	OldKey StoreKey
-	NewKey StoreKey
+	OldKey string
+	NewKey string
 }
 
 type MultiStore interface { //nolint

From cbf23f31a87fb0cc62caa62635af352c155bdfd0 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 19:00:28 +0200
Subject: [PATCH 04/23] Delete DB during upgrades

---
 store/rootmulti/store.go      | 37 ++++++++++++++++++++++++++++++-----
 store/rootmulti/store_test.go |  2 +-
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index c7447a3e040d..500bf823cb70 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -128,10 +128,6 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 			return err
 		}
 
-		if upgrades != nil {
-			// TODO: run upgrades if present
-		}
-
 		// convert StoreInfos slice to map
 		for _, storeInfo := range cInfo.StoreInfos {
 			infos[storeInfo.Name] = storeInfo
@@ -139,7 +135,7 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 		lastCommitID = cInfo.CommitID()
 	}
 
-	// load each Store
+	// load each Store (note this doesn't panic on unmounted keys now)
 	var newStores = make(map[types.StoreKey]types.CommitStore)
 	for key, storeParams := range rs.storesParams {
 		var id types.CommitID
@@ -155,12 +151,43 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 		newStores[key] = store
 	}
 
+	if upgrades != nil {
+		// NewStore is a no-op I believe
+
+		// DeleteStore
+		for _, deleted := range upgrades.Deleted {
+			// only worry about registered stores
+			if key, ok := rs.keysByName[deleted]; ok {
+				store := newStores[key].(types.KVStore)
+				deleteKVStore(store)
+			}
+		}
+
+		// TODO: run upgrades if present
+	}
+
 	rs.lastCommitID = lastCommitID
 	rs.stores = newStores
 
 	return nil
 }
 
+func deleteKVStore(kv types.KVStore) error {
+	// Note that we cannot write while iterating, so load all keys here, delete below
+	var keys [][]byte
+	itr := kv.Iterator(nil, nil)
+	for itr.Valid() {
+		keys = append(keys, itr.Key())
+		itr.Next()
+	}
+	itr.Close()
+
+	for _, k := range keys {
+		kv.Delete(k)
+	}
+	return nil
+}
+
 // SetTracer sets the tracer for the MultiStore that the underlying
 // stores will utilize to trace operations. A MultiStore is returned.
 func (rs *Store) SetTracer(w io.Writer) types.MultiStore {
diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index c723dd4c1e00..92a130e87847 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -208,7 +208,7 @@ func TestMultistoreLoadWithUpgrade(t *testing.T) {
 	// store3 is mounted, but data deleted are gone
 	s3, _ = restore.getStoreByName("store3").(types.KVStore)
 	require.NotNil(t, s3)
-	require.Equal(t, nil, s3.Get(k3)) // data was deleted
+	require.Nil(t, s3.Get(k3)) // data was deleted
 
 	// store2 is no longer mounted
 	st2 := restore.getStoreByName("store2")

From d8ba4a816fd472811c19262667297c2e972aaf53 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 19:26:48 +0200
Subject: [PATCH 05/23] Rename DB during upgrades

---
 store/rootmulti/store.go      | 70 +++++++++++++++++++++++++++--------
 store/rootmulti/store_test.go |  6 +--
 store/types/store.go          | 28 ++++++++++++++
 3 files changed, 86 insertions(+), 18 deletions(-)

diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 500bf823cb70..3c6136d676ac 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -139,31 +139,56 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 	var newStores = make(map[types.StoreKey]types.CommitStore)
 	for key, storeParams := range rs.storesParams {
 		var id types.CommitID
+
+		// handle renames specially
+		if oldName := upgrades.RenamedFrom(key.Name()); oldName != "" {
+			info, ok := infos[oldName]
+			if ok {
+				id = info.Core.CommitID
+			}
+			// make an unregistered key to satify loadCommitStore params
+			oldKey := types.NewKVStoreKey(oldName)
+			oldParams := storeParams
+			oldParams.key = oldKey
+
+			// load from the old name
+			oldStore, err := rs.loadCommitStoreFromParams(oldKey, id, oldParams)
+			if err != nil {
+				return fmt.Errorf("failed to load old Store '%s': %v", oldName, err)
+			}
+
+			// create in the new name
+			newStore, err := rs.loadCommitStoreFromParams(key, types.CommitID{}, storeParams)
+			if err != nil {
+				return fmt.Errorf("failed to load old Store '%s': %v", oldName, err)
+			}
+
+			// move all data
+			moveKVStoreData(oldStore.(types.KVStore), newStore.(types.KVStore))
+
+			// keep the new store
+			newStores[key] = newStore
+
+			// and skip the rest of this loop
+			continue
+		}
+
 		info, ok := infos[key.Name()]
 		if ok {
 			id = info.Core.CommitID
 		}
 
+		// Load it
 		store, err := rs.loadCommitStoreFromParams(key, id, storeParams)
 		if err != nil {
 			return fmt.Errorf("failed to load Store: %v", err)
 		}
 		newStores[key] = store
-	}
 
-	if upgrades != nil {
-		// NewStore is a no-op I believe
-
-		// DeleteStore
-		for _, deleted := range upgrades.Deleted {
-			// only worry about registered stores
-			if key, ok := rs.keysByName[deleted]; ok {
-				store := newStores[key].(types.KVStore)
-				deleteKVStore(store)
-			}
+		// If it was deleted, remove all data
+		if upgrades.IsDeleted(key.Name()) {
+			deleteKVStore(store.(types.KVStore))
 		}
-
-		// TODO: run upgrades if present
 	}
 
 	rs.lastCommitID = lastCommitID
@@ -188,6 +213,20 @@ func deleteKVStore(kv types.KVStore) error {
 	return nil
 }
 
+// we simulate move by a copy and delete
+func moveKVStoreData(oldDB types.KVStore, newDB types.KVStore) error {
+	// we read from one and write to another
+	itr := oldDB.Iterator(nil, nil)
+	for itr.Valid() {
+		newDB.Set(itr.Key(), itr.Value())
+		itr.Next()
+	}
+	itr.Close()
+
+	// then delete the old store
+	return deleteKVStore(oldDB)
+}
+
 // SetTracer sets the tracer for the MultiStore that the underlying
 // stores will utilize to trace operations. A MultiStore is returned.
 func (rs *Store) SetTracer(w io.Writer) types.MultiStore {
@@ -409,14 +448,15 @@ func parsePath(path string) (storeName string, subpath string, err errors.Error)
 }
 
 //----------------------------------------
-
+// Note: why do we use key and params.key in different places. Seems like there should be only one key used.
 func (rs *Store) loadCommitStoreFromParams(key types.StoreKey, id types.CommitID, params storeParams) (store types.CommitStore, err error) {
 	var db dbm.DB
 
 	if params.db != nil {
 		db = dbm.NewPrefixDB(params.db, []byte("s/_/"))
 	} else {
-		db = dbm.NewPrefixDB(rs.db, []byte("s/k:"+params.key.Name()+"/"))
+		prefix := "s/k:" + params.key.Name() + "/"
+		db = dbm.NewPrefixDB(rs.db, []byte(prefix))
 	}
 
 	switch params.typ {
diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index 92a130e87847..84ba4d776953 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -215,9 +215,9 @@ func TestMultistoreLoadWithUpgrade(t *testing.T) {
 	require.Nil(t, st2)
 
 	// restore2 has the old data
-	rs1, _ := restore.getStoreByName("restore2").(types.KVStore)
-	require.NotNil(t, rs1)
-	require.Equal(t, v1, rs1.Get(k1))
+	rs2, _ := restore.getStoreByName("restore2").(types.KVStore)
+	require.NotNil(t, rs2)
+	require.Equal(t, v2, rs2.Get(k2))
 }
 
 func TestParsePath(t *testing.T) {
diff --git a/store/types/store.go b/store/types/store.go
index 1a0daef532c1..ea78d7a6559f 100644
--- a/store/types/store.go
+++ b/store/types/store.go
@@ -49,6 +49,34 @@ type StoreRename struct {
 	NewKey string
 }
 
+// IsDeleted returns true if the given key should be deleted
+func (s *StoreUpgrades) IsDeleted(key string) bool {
+	if s == nil {
+		return false
+	}
+	for _, d := range s.Deleted {
+		if d == key {
+			return true
+		}
+	}
+	return false
+}
+
+// RenamedFrom returns the oldKey if it was renamed
+// Returns "" if it was not renamed
+func (s *StoreUpgrades) RenamedFrom(key string) string {
+	if s == nil {
+		return ""
+	}
+	for _, re := range s.Renamed {
+		if re.NewKey == key {
+			return re.OldKey
+		}
+	}
+	return ""
+
+}
+
 type MultiStore interface { //nolint
 	Store
 

From 62b6da5054c37237587d66f22f0466f9c85d92f4 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 20:12:47 +0200
Subject: [PATCH 06/23] Ensure reload after migrate and save works well

---
 store/rootmulti/store_test.go | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index 84ba4d776953..eb829c1402c9 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -218,6 +218,25 @@ func TestMultistoreLoadWithUpgrade(t *testing.T) {
 	rs2, _ := restore.getStoreByName("restore2").(types.KVStore)
 	require.NotNil(t, rs2)
 	require.Equal(t, v2, rs2.Get(k2))
+
+	// store this migrated data, and load it again without migrations
+	migratedID := restore.Commit()
+	require.Equal(t, migratedID.Version, int64(2))
+
+	reload, _ := newMultiStoreWithModifiedMounts(db)
+	err = reload.LoadLatestVersion()
+	require.Nil(t, err)
+	require.Equal(t, migratedID, reload.LastCommitID())
+
+	// query this new store
+	rl1, _ := reload.getStoreByName("store1").(types.KVStore)
+	require.NotNil(t, rl1)
+	require.Equal(t, v1, rl1.Get(k1))
+
+	rl2, _ := reload.getStoreByName("restore2").(types.KVStore)
+	require.NotNil(t, rl2)
+	require.Equal(t, v2, rl2.Get(k2))
+
 }
 
 func TestParsePath(t *testing.T) {

From 79c4380e805dd4872bd88e6350475a3cf41183fe Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 23:45:32 +0200
Subject: [PATCH 07/23] Add tests on commit info

---
 store/rootmulti/store_test.go | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index eb829c1402c9..9c29168808b6 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -183,6 +183,12 @@ func TestMultistoreLoadWithUpgrade(t *testing.T) {
 	expectedCommitID := getExpectedCommitID(store, 1)
 	checkStore(t, store, expectedCommitID, commitID)
 
+	ci, err := getCommitInfo(db, 1)
+	require.NoError(t, err)
+	require.Equal(t, int64(1), ci.Version)
+	require.Equal(t, 3, len(ci.StoreInfos))
+	checkContains(t, ci.StoreInfos, []string{"store1", "store2", "store3"})
+
 	// Load without changes and make sure it is sensible
 	store = newMultiStoreWithMounts(db)
 	err = store.LoadLatestVersion()
@@ -237,6 +243,13 @@ func TestMultistoreLoadWithUpgrade(t *testing.T) {
 	require.NotNil(t, rl2)
 	require.Equal(t, v2, rl2.Get(k2))
 
+	// check commitInfo in storage
+	ci, err = getCommitInfo(db, 2)
+	require.NoError(t, err)
+	require.Equal(t, int64(2), ci.Version)
+	require.Equal(t, 3, len(ci.StoreInfos), ci.StoreInfos)
+	checkContains(t, ci.StoreInfos, []string{"store1", "restore2", "store3"})
+	require.Equal(t, "restore2", ci.StoreInfos[1].Name)
 }
 
 func TestParsePath(t *testing.T) {
@@ -373,6 +386,24 @@ func checkStore(t *testing.T, store *Store, expect, got types.CommitID) {
 
 }
 
+func checkContains(t testing.TB, info []storeInfo, wanted []string) {
+	t.Helper()
+
+	for _, want := range wanted {
+		checkHas(t, info, want)
+	}
+}
+
+func checkHas(t testing.TB, info []storeInfo, want string) {
+	t.Helper()
+	for _, i := range info {
+		if i.Name == want {
+			return
+		}
+	}
+	t.Fatalf("storeInfo doesn't contain %s", want)
+}
+
 func getExpectedCommitID(store *Store, ver int64) types.CommitID {
 	return types.CommitID{
 		Version: ver,

From 347e03cf2826eb0c56fc3dde220a8f217c9324f8 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 15 Jul 2019 23:54:21 +0200
Subject: [PATCH 08/23] clean up loadVersionAndUpgrade logic

---
 store/rootmulti/store.go      | 60 +++++++++++++----------------------
 store/rootmulti/store_test.go |  1 -
 2 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 3c6136d676ac..d920fc9e3398 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -138,56 +138,32 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 	// load each Store (note this doesn't panic on unmounted keys now)
 	var newStores = make(map[types.StoreKey]types.CommitStore)
 	for key, storeParams := range rs.storesParams {
-		var id types.CommitID
 
-		// handle renames specially
-		if oldName := upgrades.RenamedFrom(key.Name()); oldName != "" {
-			info, ok := infos[oldName]
-			if ok {
-				id = info.Core.CommitID
-			}
+		// Load it
+		store, err := rs.loadCommitStoreFromParams(key, rs.getCommitID(infos, key.Name()), storeParams)
+		if err != nil {
+			return fmt.Errorf("failed to load Store: %v", err)
+		}
+		newStores[key] = store
+
+		// If it was deleted, remove all data
+		if upgrades.IsDeleted(key.Name()) {
+			deleteKVStore(store.(types.KVStore))
+		} else if oldName := upgrades.RenamedFrom(key.Name()); oldName != "" {
+			// handle renames specially
 			// make an unregistered key to satify loadCommitStore params
 			oldKey := types.NewKVStoreKey(oldName)
 			oldParams := storeParams
 			oldParams.key = oldKey
 
 			// load from the old name
-			oldStore, err := rs.loadCommitStoreFromParams(oldKey, id, oldParams)
-			if err != nil {
-				return fmt.Errorf("failed to load old Store '%s': %v", oldName, err)
-			}
-
-			// create in the new name
-			newStore, err := rs.loadCommitStoreFromParams(key, types.CommitID{}, storeParams)
+			oldStore, err := rs.loadCommitStoreFromParams(oldKey, rs.getCommitID(infos, oldName), oldParams)
 			if err != nil {
 				return fmt.Errorf("failed to load old Store '%s': %v", oldName, err)
 			}
 
 			// move all data
-			moveKVStoreData(oldStore.(types.KVStore), newStore.(types.KVStore))
-
-			// keep the new store
-			newStores[key] = newStore
-
-			// and skip the rest of this loop
-			continue
-		}
-
-		info, ok := infos[key.Name()]
-		if ok {
-			id = info.Core.CommitID
-		}
-
-		// Load it
-		store, err := rs.loadCommitStoreFromParams(key, id, storeParams)
-		if err != nil {
-			return fmt.Errorf("failed to load Store: %v", err)
-		}
-		newStores[key] = store
-
-		// If it was deleted, remove all data
-		if upgrades.IsDeleted(key.Name()) {
-			deleteKVStore(store.(types.KVStore))
+			moveKVStoreData(oldStore.(types.KVStore), store.(types.KVStore))
 		}
 	}
 
@@ -197,6 +173,14 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 	return nil
 }
 
+func (rs *Store) getCommitID(infos map[string]storeInfo, name string) types.CommitID {
+	info, ok := infos[name]
+	if !ok {
+		return types.CommitID{}
+	}
+	return info.Core.CommitID
+}
+
 func deleteKVStore(kv types.KVStore) error {
 	// Note that we cannot write while iterating, so load all keys here, delete below
 	var keys [][]byte
diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index 9c29168808b6..69fec842ee17 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -249,7 +249,6 @@ func TestMultistoreLoadWithUpgrade(t *testing.T) {
 	require.Equal(t, int64(2), ci.Version)
 	require.Equal(t, 3, len(ci.StoreInfos), ci.StoreInfos)
 	checkContains(t, ci.StoreInfos, []string{"store1", "restore2", "store3"})
-	require.Equal(t, "restore2", ci.StoreInfos[1].Name)
 }
 
 func TestParsePath(t *testing.T) {

From 49eac0269318b6b6e06ea9df2c46d3daa5195e69 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Tue, 16 Jul 2019 00:13:46 +0200
Subject: [PATCH 09/23] Provide configurable store loader in BaseApp to
 optionally do migrations

---
 baseapp/baseapp.go | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 5552d1c01c5f..75ed453ce391 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -1,8 +1,10 @@
 package baseapp
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"os"
 	"reflect"
 	"runtime/debug"
@@ -20,6 +22,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/store"
+	storeTypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -41,6 +44,35 @@ const (
 	MainStoreKey = "main"
 )
 
+type StoreLoader func(ms sdk.CommitMultiStore) error
+
+func DefaultStoreLoader(ms sdk.CommitMultiStore) error {
+	return ms.LoadLatestVersion()
+}
+
+func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
+	return func(ms sdk.CommitMultiStore) error {
+		_, err := os.Stat(upgradeInfoPath)
+		if os.IsNotExist(err) {
+			return DefaultStoreLoader(ms)
+		} else if err != nil {
+			return err
+		}
+
+		// there is a migration file, let's execute
+		data, err := ioutil.ReadFile(upgradeInfoPath)
+		if err != nil {
+			return fmt.Errorf("Cannot read upgrade file: %v", err)
+		}
+		var upgrades storeTypes.StoreUpgrades
+		err = json.Unmarshal(data, &upgrades)
+		if err != nil {
+			return fmt.Errorf("Cannot parse upgrade file: %v", err)
+		}
+		return ms.LoadLatestVersionAndUpgrade(&upgrades)
+	}
+}
+
 // BaseApp reflects the ABCI application implementation.
 type BaseApp struct {
 	// initialized on creation
@@ -48,6 +80,7 @@ type BaseApp struct {
 	name        string               // application name from abci.Info
 	db          dbm.DB               // common DB backend
 	cms         sdk.CommitMultiStore // Main (uncached) state
+	storeLoader StoreLoader          // function to handle store loading, may be overriden with SetStoreLoader()
 	router      sdk.Router           // handle any kind of message
 	queryRouter sdk.QueryRouter      // router for redirecting query calls
 	txDecoder   sdk.TxDecoder        // unmarshal []byte into sdk.Tx
@@ -106,6 +139,7 @@ func NewBaseApp(
 		name:           name,
 		db:             db,
 		cms:            store.NewCommitMultiStore(db),
+		storeLoader:    DefaultStoreLoader,
 		router:         NewRouter(),
 		queryRouter:    NewQueryRouter(),
 		txDecoder:      txDecoder,
@@ -139,6 +173,11 @@ func (app *BaseApp) SetCommitMultiStoreTracer(w io.Writer) {
 	app.cms.SetTracer(w)
 }
 
+// SetStoreLoader allows us to customize the rootMultiStore initialization.
+func (app *BaseApp) SetStoreLoader(loader StoreLoader) {
+	app.storeLoader = loader
+}
+
 // MountStores mounts all IAVL or DB stores to the provided keys in the BaseApp
 // multistore.
 func (app *BaseApp) MountStores(keys ...sdk.StoreKey) {
@@ -197,7 +236,7 @@ func (app *BaseApp) MountStore(key sdk.StoreKey, typ sdk.StoreType) {
 // LoadLatestVersion loads the latest application version. It will panic if
 // called more than once on a running BaseApp.
 func (app *BaseApp) LoadLatestVersion(baseKey *sdk.KVStoreKey) error {
-	err := app.cms.LoadLatestVersion()
+	err := app.storeLoader(app.cms)
 	if err != nil {
 		return err
 	}

From 14a0e8428ee2aec47f8bfba4f39b350c65719f21 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Tue, 16 Jul 2019 00:16:08 +0200
Subject: [PATCH 10/23] Fixed error handling thanks to golangci bot

---
 store/rootmulti/store.go | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index d920fc9e3398..8e25a96d043c 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -148,7 +148,9 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 
 		// If it was deleted, remove all data
 		if upgrades.IsDeleted(key.Name()) {
-			deleteKVStore(store.(types.KVStore))
+			if err := deleteKVStore(store.(types.KVStore)); err != nil {
+				return fmt.Errorf("failed to delete store %s: %v", key.Name(), err)
+			}
 		} else if oldName := upgrades.RenamedFrom(key.Name()); oldName != "" {
 			// handle renames specially
 			// make an unregistered key to satify loadCommitStore params
@@ -163,7 +165,9 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
 			}
 
 			// move all data
-			moveKVStoreData(oldStore.(types.KVStore), store.(types.KVStore))
+			if err := moveKVStoreData(oldStore.(types.KVStore), store.(types.KVStore)); err != nil {
+				return fmt.Errorf("failed to move store %s -> %s: %v", oldName, key.Name(), err)
+			}
 		}
 	}
 

From 9ea5cd99f00e8a9bc57868ce1562e87e4eed4809 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Tue, 16 Jul 2019 00:16:59 +0200
Subject: [PATCH 11/23] Fixed spelling error

---
 baseapp/baseapp.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 75ed453ce391..691a76370e4f 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -80,7 +80,7 @@ type BaseApp struct {
 	name        string               // application name from abci.Info
 	db          dbm.DB               // common DB backend
 	cms         sdk.CommitMultiStore // Main (uncached) state
-	storeLoader StoreLoader          // function to handle store loading, may be overriden with SetStoreLoader()
+	storeLoader StoreLoader          // function to handle store loading, may be overridden with SetStoreLoader()
 	router      sdk.Router           // handle any kind of message
 	queryRouter sdk.QueryRouter      // router for redirecting query calls
 	txDecoder   sdk.TxDecoder        // unmarshal []byte into sdk.Tx

From e3c7c00eb5519bfd2f4735048ffdd2223cf09acc Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Tue, 16 Jul 2019 00:21:10 +0200
Subject: [PATCH 12/23] Add comments to explain StoreLoader implementations in
 BaseApp

---
 baseapp/baseapp.go | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 691a76370e4f..1af447a6fd99 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -44,12 +44,22 @@ const (
 	MainStoreKey = "main"
 )
 
+// StoreLoader defines a customizable function to control how we load the CommitMultiStore
+// from disk
 type StoreLoader func(ms sdk.CommitMultiStore) error
 
+// DefaultStoreLoader will be used by default and loads the latest version
 func DefaultStoreLoader(ms sdk.CommitMultiStore) error {
 	return ms.LoadLatestVersion()
 }
 
+// UpgradeableStoreLoader can be configured by SetStoreLoader() to check for the
+// existence of a given upgrade file - json encoded StoreUpgrades data.
+// If the file if present, parse it and execute those upgrades
+// (rename or delete stores), while loading the data.
+//
+// This is useful for in place migrations when a store key is renamed between
+// two versions of the software.
 func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
 	return func(ms sdk.CommitMultiStore) error {
 		_, err := os.Stat(upgradeInfoPath)

From fecc2e8b9355fcae0a03def740a21e1cec639fe5 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Tue, 16 Jul 2019 15:19:16 +0200
Subject: [PATCH 13/23] Clean up StoreUpgrades type definition

---
 store/types/store.go | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/store/types/store.go b/store/types/store.go
index ea78d7a6559f..afdf4d688800 100644
--- a/store/types/store.go
+++ b/store/types/store.go
@@ -38,15 +38,18 @@ type Queryable interface {
 //----------------------------------------
 // MultiStore
 
+// StoreUpgrades defines a series of transformations to apply the multistore db upon load
 type StoreUpgrades struct {
-	New     []string
-	Renamed []StoreRename
-	Deleted []string
+	Renamed []StoreRename `json:"renamed"`
+	Deleted []string      `json:"deleted"`
 }
 
+// StoreRename defines a name change of a sub-store.
+// All data previously under a PrefixStore with OldKey will be copied
+// to a PrefixStore with NewKey, then deleted from OldKey store.
 type StoreRename struct {
-	OldKey string
-	NewKey string
+	OldKey string `json:"old"`
+	NewKey string `json:"new"`
 }
 
 // IsDeleted returns true if the given key should be deleted

From 94e454a2d2c2e3cab45dece92fdf50b586c1898e Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Tue, 16 Jul 2019 15:25:04 +0200
Subject: [PATCH 14/23] Clean up baseapp.StoreLoader implementations

---
 baseapp/baseapp.go | 87 +++++++++++++++++++++++++++-------------------
 1 file changed, 52 insertions(+), 35 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 1af447a6fd99..6ddcce541834 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -48,41 +48,6 @@ const (
 // from disk
 type StoreLoader func(ms sdk.CommitMultiStore) error
 
-// DefaultStoreLoader will be used by default and loads the latest version
-func DefaultStoreLoader(ms sdk.CommitMultiStore) error {
-	return ms.LoadLatestVersion()
-}
-
-// UpgradeableStoreLoader can be configured by SetStoreLoader() to check for the
-// existence of a given upgrade file - json encoded StoreUpgrades data.
-// If the file if present, parse it and execute those upgrades
-// (rename or delete stores), while loading the data.
-//
-// This is useful for in place migrations when a store key is renamed between
-// two versions of the software.
-func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
-	return func(ms sdk.CommitMultiStore) error {
-		_, err := os.Stat(upgradeInfoPath)
-		if os.IsNotExist(err) {
-			return DefaultStoreLoader(ms)
-		} else if err != nil {
-			return err
-		}
-
-		// there is a migration file, let's execute
-		data, err := ioutil.ReadFile(upgradeInfoPath)
-		if err != nil {
-			return fmt.Errorf("Cannot read upgrade file: %v", err)
-		}
-		var upgrades storeTypes.StoreUpgrades
-		err = json.Unmarshal(data, &upgrades)
-		if err != nil {
-			return fmt.Errorf("Cannot parse upgrade file: %v", err)
-		}
-		return ms.LoadLatestVersionAndUpgrade(&upgrades)
-	}
-}
-
 // BaseApp reflects the ABCI application implementation.
 type BaseApp struct {
 	// initialized on creation
@@ -253,6 +218,58 @@ func (app *BaseApp) LoadLatestVersion(baseKey *sdk.KVStoreKey) error {
 	return app.initFromMainStore(baseKey)
 }
 
+// DefaultStoreLoader will be used by default and loads the latest version
+func DefaultStoreLoader(ms sdk.CommitMultiStore) error {
+	return ms.LoadLatestVersion()
+}
+
+// StoreLoaderWithUpgrade is used to prepare baseapp with a fixed StoreLoader
+// pattern. This is useful in test cases, or with custom upgrade loading logic.
+func StoreLoaderWithUpgrade(upgrades *storeTypes.StoreUpgrades) StoreLoader {
+	return func(ms sdk.CommitMultiStore) error {
+		return ms.LoadLatestVersionAndUpgrade(upgrades)
+	}
+}
+
+// UpgradeableStoreLoader can be configured by SetStoreLoader() to check for the
+// existence of a given upgrade file - json encoded StoreUpgrades data.
+// If the file if present, parse it and execute those upgrades
+// (rename or delete stores), while loading the data.
+//
+// This is useful for in place migrations when a store key is renamed between
+// two versions of the software.
+func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
+	return func(ms sdk.CommitMultiStore) error {
+		_, err := os.Stat(upgradeInfoPath)
+		if os.IsNotExist(err) {
+			return DefaultStoreLoader(ms)
+		} else if err != nil {
+			return err
+		}
+
+		// there is a migration file, let's execute
+		data, err := ioutil.ReadFile(upgradeInfoPath)
+		if err != nil {
+			return fmt.Errorf("Cannot read upgrade file %s: %v", upgradeInfoPath, err)
+		}
+		var upgrades storeTypes.StoreUpgrades
+		err = json.Unmarshal(data, &upgrades)
+		if err != nil {
+			return fmt.Errorf("Cannot parse upgrade file: %v", err)
+		}
+		err = ms.LoadLatestVersionAndUpgrade(&upgrades)
+		if err != nil {
+			return fmt.Errorf("Load and upgrade database: %v", err)
+		}
+		// if we have a successful load, we delete the file
+		err = os.Remove(upgradeInfoPath)
+		if err != nil {
+			return fmt.Errorf("Deleting upgrade file %s: %v", upgradeInfoPath, err)
+		}
+		return nil
+	}
+}
+
 // LoadVersion loads the BaseApp application version. It will panic if called
 // more than once on a running baseapp.
 func (app *BaseApp) LoadVersion(version int64, baseKey *sdk.KVStoreKey) error {

From 7c721b73cd08a74abda09a6f7357f9a94235657f Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Wed, 17 Jul 2019 14:18:27 +0200
Subject: [PATCH 15/23] Add unit tests for StoreUpgrades

---
 store/types/store_test.go | 56 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 store/types/store_test.go

diff --git a/store/types/store_test.go b/store/types/store_test.go
new file mode 100644
index 000000000000..453bfa315aac
--- /dev/null
+++ b/store/types/store_test.go
@@ -0,0 +1,56 @@
+package types
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestStoreUpgrades(t *testing.T) {
+	type toDelete struct {
+		key    string
+		delete bool
+	}
+	type toRename struct {
+		newkey string
+		result string
+	}
+
+	cases := map[string]struct {
+		upgrades     *StoreUpgrades
+		expectDelete []toDelete
+		expectRename []toRename
+	}{
+		"empty upgrade": {
+			expectDelete: []toDelete{{"foo", false}},
+			expectRename: []toRename{{"foo", ""}},
+		},
+		"simple matches": {
+			upgrades: &StoreUpgrades{
+				Deleted: []string{"foo"},
+				Renamed: []StoreRename{{"bar", "baz"}},
+			},
+			expectDelete: []toDelete{{"foo", true}, {"bar", false}, {"baz", false}},
+			expectRename: []toRename{{"foo", ""}, {"bar", ""}, {"baz", "bar"}},
+		},
+		"many data points": {
+			upgrades: &StoreUpgrades{
+				Deleted: []string{"one", "two", "three", "four", "five"},
+				Renamed: []StoreRename{{"old", "new"}, {"white", "blue"}, {"black", "orange"}, {"fun", "boring"}},
+			},
+			expectDelete: []toDelete{{"four", true}, {"six", false}, {"baz", false}},
+			expectRename: []toRename{{"white", ""}, {"blue", "white"}, {"boring", "fun"}, {"missing", ""}},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			for _, d := range tc.expectDelete {
+				assert.Equal(t, tc.upgrades.IsDeleted(d.key), d.delete)
+			}
+			for _, r := range tc.expectRename {
+				assert.Equal(t, tc.upgrades.RenamedFrom(r.newkey), r.result)
+			}
+		})
+	}
+}

From 79ce8b6bb58f92655dadf56de8006f68a6d3fb52 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Wed, 17 Jul 2019 14:58:34 +0200
Subject: [PATCH 16/23] Add baseapp setloader test

---
 baseapp/baseapp_test.go | 144 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 1 deletion(-)

diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go
index efe628b08f35..5d2b450d2b10 100644
--- a/baseapp/baseapp_test.go
+++ b/baseapp/baseapp_test.go
@@ -4,10 +4,10 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"testing"
 
-	store "github.com/cosmos/cosmos-sdk/store/types"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -17,6 +17,8 @@ import (
 	dbm "github.com/tendermint/tm-db"
 
 	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store/rootmulti"
+	store "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -130,6 +132,146 @@ func TestLoadVersion(t *testing.T) {
 	testLoadVersionHelper(t, app, int64(2), commitID2)
 }
 
+func useDefaultLoader(app *BaseApp) {
+		app.SetStoreLoader(DefaultStoreLoader)
+}
+
+func useUpgradeLoader(upgrades *store.StoreUpgrades) func(*BaseApp) {
+	return func(app *BaseApp) {
+		app.SetStoreLoader(StoreLoaderWithUpgrade(upgrades))
+	}
+}
+
+func useFileUpgradeLoader(upgradeInfoPath string) func(*BaseApp) {
+	return func(app *BaseApp) {
+		app.SetStoreLoader(UpgradeableStoreLoader(upgradeInfoPath))
+	}
+}
+
+func initStore(t *testing.T, db dbm.DB, storeKey string, k, v []byte) {
+	rs := rootmulti.NewStore(db)
+	rs.SetPruning(store.PruneSyncable)
+	key := sdk.NewKVStoreKey(storeKey)
+	rs.MountStoreWithDB(key, store.StoreTypeIAVL, nil)
+	err := rs.LoadLatestVersion()
+	require.Nil(t, err)
+	require.Equal(t, int64(0), rs.LastCommitID().Version)
+
+	// write some data in substore
+	kv, _ := rs.GetStore(key).(store.KVStore)
+	require.NotNil(t, kv)
+	kv.Set(k, v)
+	commitID := rs.Commit()
+	require.Equal(t, int64(1), commitID.Version)	
+}
+
+func checkStore(t *testing.T, db dbm.DB, ver int64, storeKey string, k, v []byte) {
+	rs := rootmulti.NewStore(db)
+	rs.SetPruning(store.PruneSyncable)
+	key := sdk.NewKVStoreKey(storeKey)
+	rs.MountStoreWithDB(key, store.StoreTypeIAVL, nil)
+	err := rs.LoadLatestVersion()
+	require.Nil(t, err)
+	require.Equal(t, ver, rs.LastCommitID().Version)
+
+	// query data in substore
+	kv, _ := rs.GetStore(key).(store.KVStore)
+	require.NotNil(t, kv)
+	require.Equal(t, v, kv.Get(k))
+}
+
+
+// Test that we can make commits and then reload old versions.
+// Test that LoadLatestVersion actually does.
+func TestSetLoader(t *testing.T) {
+	// write a renamer to a file
+	f, err := ioutil.TempFile("", "upgrade-*.json")
+	require.NoError(t, err)
+	data := []byte(`{"renamed":[{"old": "bnk", "new": "banker"}]}`)
+	_, err = f.Write(data)
+	require.NoError(t, err)
+	configName := f.Name()
+	require.NoError(t, f.Close())
+
+	// make sure it exists before running everything
+	_, err = os.Stat(configName)
+	require.NoError(t, err)
+
+
+	cases := map[string]struct{
+		setLoader func(*BaseApp)
+		origStoreKey string
+		loadStoreKey string
+	}{
+		"don't set loader": {
+			origStoreKey: "foo",
+			loadStoreKey: "foo",
+		},
+		"default loader": {
+			setLoader: useDefaultLoader,
+			origStoreKey: "foo",
+			loadStoreKey: "foo",
+		},
+		"rename with inline opts": {
+			setLoader: useUpgradeLoader(&store.StoreUpgrades{
+				Renamed: []store.StoreRename{{
+					OldKey: "foo",
+					NewKey: "bar",
+				}},
+			}),
+			origStoreKey: "foo",
+			loadStoreKey: "bar",
+		},
+		"file loader with missing file": {
+			setLoader: useFileUpgradeLoader(configName + "randomchars"),
+			origStoreKey: "bnk",
+			loadStoreKey: "bnk",
+		},
+		"file loader with existing file": {
+			setLoader: useFileUpgradeLoader(configName),
+			origStoreKey: "bnk",
+			loadStoreKey: "banker",
+		},
+	}
+
+	k := []byte("key")
+	v := []byte("value")
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			// prepare a db with some data
+			db := dbm.NewMemDB()
+			initStore(t, db, tc.origStoreKey, k, v)
+		
+			// load the app with the existing db
+			opts := []func(*BaseApp){SetPruning(store.PruneSyncable)}
+			if tc.setLoader != nil {
+				opts = append(opts, tc.setLoader)
+			}	
+			app := NewBaseApp(t.Name(), defaultLogger(), db, nil, opts...)
+			capKey := sdk.NewKVStoreKey(MainStoreKey)
+			app.MountStores(capKey)
+			app.MountStores(sdk.NewKVStoreKey(tc.loadStoreKey))
+			err := app.LoadLatestVersion(capKey) 
+			require.Nil(t, err)
+
+			// "execute" one block
+			app.BeginBlock(abci.RequestBeginBlock{Header: abci.Header{Height: 2}})
+			res := app.Commit()
+			require.NotNil(t, res.Data)
+		
+			// check db is properly updated
+			checkStore(t, db, 2, tc.loadStoreKey, k, v)
+			checkStore(t, db, 2, tc.loadStoreKey, []byte("foo"), nil)
+		})
+	}
+
+	// ensure config file was deleted
+	_, err = os.Stat(configName)
+	require.True(t, os.IsNotExist(err))
+}
+
+
 func TestAppVersionSetterGetter(t *testing.T) {
 	logger := defaultLogger()
 	pruningOpt := SetPruning(store.PruneSyncable)

From a49e18e42c7ee26ee0f37ed69577a61f075f4ac9 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 29 Jul 2019 13:22:24 +0200
Subject: [PATCH 17/23] Addressed PR comments, rebased on newest master

---
 baseapp/baseapp.go | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 6ddcce541834..d3139d3fef56 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -22,7 +22,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/store"
-	storeTypes "github.com/cosmos/cosmos-sdk/store/types"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -45,7 +45,9 @@ const (
 )
 
 // StoreLoader defines a customizable function to control how we load the CommitMultiStore
-// from disk
+// from disk. This is useful for state migration, when loading a datastore writen with
+// an older version of the software. In particular, if a module changed the substore key name
+// (or removed a substore) between two versions of the software.
 type StoreLoader func(ms sdk.CommitMultiStore) error
 
 // BaseApp reflects the ABCI application implementation.
@@ -225,7 +227,7 @@ func DefaultStoreLoader(ms sdk.CommitMultiStore) error {
 
 // StoreLoaderWithUpgrade is used to prepare baseapp with a fixed StoreLoader
 // pattern. This is useful in test cases, or with custom upgrade loading logic.
-func StoreLoaderWithUpgrade(upgrades *storeTypes.StoreUpgrades) StoreLoader {
+func StoreLoaderWithUpgrade(upgrades *storetypes.StoreUpgrades) StoreLoader {
 	return func(ms sdk.CommitMultiStore) error {
 		return ms.LoadLatestVersionAndUpgrade(upgrades)
 	}
@@ -233,11 +235,17 @@ func StoreLoaderWithUpgrade(upgrades *storeTypes.StoreUpgrades) StoreLoader {
 
 // UpgradeableStoreLoader can be configured by SetStoreLoader() to check for the
 // existence of a given upgrade file - json encoded StoreUpgrades data.
-// If the file if present, parse it and execute those upgrades
-// (rename or delete stores), while loading the data.
+//
+// If not file is present, it will peform the default load (no upgrades to store).
+//
+// If the file is present, it will parse the file and execute those upgrades
+// (rename or delete stores), while loading the data. It will also delete the
+// upgrade file upon successful load, so that the upgrade is only applied once,
+// and not re-applied on next restart
 //
 // This is useful for in place migrations when a store key is renamed between
-// two versions of the software.
+// two versions of the software. (Note: this code will move to x/upgrades
+// when PR #4233 is merged, here mainly to help test the design)
 func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
 	return func(ms sdk.CommitMultiStore) error {
 		_, err := os.Stat(upgradeInfoPath)
@@ -252,15 +260,18 @@ func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
 		if err != nil {
 			return fmt.Errorf("Cannot read upgrade file %s: %v", upgradeInfoPath, err)
 		}
-		var upgrades storeTypes.StoreUpgrades
+
+		var upgrades storetypes.StoreUpgrades
 		err = json.Unmarshal(data, &upgrades)
 		if err != nil {
 			return fmt.Errorf("Cannot parse upgrade file: %v", err)
 		}
+
 		err = ms.LoadLatestVersionAndUpgrade(&upgrades)
 		if err != nil {
 			return fmt.Errorf("Load and upgrade database: %v", err)
 		}
+
 		// if we have a successful load, we delete the file
 		err = os.Remove(upgradeInfoPath)
 		if err != nil {

From 3c17955caf600f1f7fcb7c1d87fe6b02c2264e7e Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 29 Jul 2019 13:31:15 +0200
Subject: [PATCH 18/23] Add clog entry

---
 .pending/features/store/4724-Multistore-supp | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 .pending/features/store/4724-Multistore-supp

diff --git a/.pending/features/store/4724-Multistore-supp b/.pending/features/store/4724-Multistore-supp
new file mode 100644
index 000000000000..de83504e8c98
--- /dev/null
+++ b/.pending/features/store/4724-Multistore-supp
@@ -0,0 +1,4 @@
+4724 Multistore supports substore migrations upon load.
+New `rootmulti.Store.LoadLatestVersionAndUpgrade` method
+Baseapp supports `StoreLoader` to enable various upgrade strategies
+No longer panics if the store to load contains substores that we didn't explicitly mount.

From 55391f40c2b84d65b25e656989ab8357e01f77de Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 29 Jul 2019 13:33:31 +0200
Subject: [PATCH 19/23] Add LoadVersionAndUpgrade to CommitMultiStore

---
 server/mock/store.go     | 4 ++++
 store/rootmulti/store.go | 5 +++++
 store/types/store.go     | 5 +++++
 3 files changed, 14 insertions(+)

diff --git a/server/mock/store.go b/server/mock/store.go
index 16f6833f6a2e..9f0882023eed 100644
--- a/server/mock/store.go
+++ b/server/mock/store.go
@@ -75,6 +75,10 @@ func (ms multiStore) LoadLatestVersionAndUpgrade(upgrades *store.StoreUpgrades)
 	return nil
 }
 
+func (ms multiStore) LoadVersionAndUpgrade(ver int64, upgrades *store.StoreUpgrades) error {
+	panic("not implemented")
+}
+
 func (ms multiStore) LoadVersion(ver int64) error {
 	panic("not implemented")
 }
diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 8e25a96d043c..0856f02b7d0f 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -106,6 +106,11 @@ func (rs *Store) LoadLatestVersionAndUpgrade(upgrades *types.StoreUpgrades) erro
 	return rs.loadVersion(ver, upgrades)
 }
 
+// LoadVersionAndUpgrade allows us to rename substores while loading an older version
+func (rs *Store) LoadVersionAndUpgrade(ver int64, upgrades *types.StoreUpgrades) error {
+	return rs.loadVersion(ver, upgrades)
+}
+
 // LoadLatestVersion implements CommitMultiStore.
 func (rs *Store) LoadLatestVersion() error {
 	ver := getLatestVersion(rs.db)
diff --git a/store/types/store.go b/store/types/store.go
index afdf4d688800..b7c1493667bd 100644
--- a/store/types/store.go
+++ b/store/types/store.go
@@ -141,6 +141,11 @@ type CommitMultiStore interface {
 	// in order to handle breaking formats in migrations
 	LoadLatestVersionAndUpgrade(upgrades *StoreUpgrades) error
 
+	// LoadVersionAndUpgrade will load the named version, but also
+	// rename/delete/create sub-store keys, before registering all the keys
+	// in order to handle breaking formats in migrations
+	LoadVersionAndUpgrade(ver int64, upgrades *StoreUpgrades) error
+
 	// Load a specific persisted version. When you load an old version, or when
 	// the last commit attempt didn't complete, the next commit after loading
 	// must be idempotent (return the same commit id). Otherwise the behavior is

From 864f216f9fbd7ee73a8f8be1f98b759bc37772b0 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 29 Jul 2019 13:36:38 +0200
Subject: [PATCH 20/23] Fixed issues reported by make lint

---
 baseapp/baseapp.go       |  2 +-
 baseapp/baseapp_test.go  | 26 +++++++++++---------------
 store/rootmulti/store.go |  9 ---------
 3 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index d3139d3fef56..c40cdc3bff45 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -45,7 +45,7 @@ const (
 )
 
 // StoreLoader defines a customizable function to control how we load the CommitMultiStore
-// from disk. This is useful for state migration, when loading a datastore writen with
+// from disk. This is useful for state migration, when loading a datastore written with
 // an older version of the software. In particular, if a module changed the substore key name
 // (or removed a substore) between two versions of the software.
 type StoreLoader func(ms sdk.CommitMultiStore) error
diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go
index 5d2b450d2b10..9f0ffd7b89af 100644
--- a/baseapp/baseapp_test.go
+++ b/baseapp/baseapp_test.go
@@ -8,7 +8,6 @@ import (
 	"os"
 	"testing"
 
-
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -133,7 +132,7 @@ func TestLoadVersion(t *testing.T) {
 }
 
 func useDefaultLoader(app *BaseApp) {
-		app.SetStoreLoader(DefaultStoreLoader)
+	app.SetStoreLoader(DefaultStoreLoader)
 }
 
 func useUpgradeLoader(upgrades *store.StoreUpgrades) func(*BaseApp) {
@@ -162,7 +161,7 @@ func initStore(t *testing.T, db dbm.DB, storeKey string, k, v []byte) {
 	require.NotNil(t, kv)
 	kv.Set(k, v)
 	commitID := rs.Commit()
-	require.Equal(t, int64(1), commitID.Version)	
+	require.Equal(t, int64(1), commitID.Version)
 }
 
 func checkStore(t *testing.T, db dbm.DB, ver int64, storeKey string, k, v []byte) {
@@ -180,7 +179,6 @@ func checkStore(t *testing.T, db dbm.DB, ver int64, storeKey string, k, v []byte
 	require.Equal(t, v, kv.Get(k))
 }
 
-
 // Test that we can make commits and then reload old versions.
 // Test that LoadLatestVersion actually does.
 func TestSetLoader(t *testing.T) {
@@ -197,9 +195,8 @@ func TestSetLoader(t *testing.T) {
 	_, err = os.Stat(configName)
 	require.NoError(t, err)
 
-
-	cases := map[string]struct{
-		setLoader func(*BaseApp)
+	cases := map[string]struct {
+		setLoader    func(*BaseApp)
 		origStoreKey string
 		loadStoreKey string
 	}{
@@ -208,7 +205,7 @@ func TestSetLoader(t *testing.T) {
 			loadStoreKey: "foo",
 		},
 		"default loader": {
-			setLoader: useDefaultLoader,
+			setLoader:    useDefaultLoader,
 			origStoreKey: "foo",
 			loadStoreKey: "foo",
 		},
@@ -223,12 +220,12 @@ func TestSetLoader(t *testing.T) {
 			loadStoreKey: "bar",
 		},
 		"file loader with missing file": {
-			setLoader: useFileUpgradeLoader(configName + "randomchars"),
+			setLoader:    useFileUpgradeLoader(configName + "randomchars"),
 			origStoreKey: "bnk",
 			loadStoreKey: "bnk",
 		},
 		"file loader with existing file": {
-			setLoader: useFileUpgradeLoader(configName),
+			setLoader:    useFileUpgradeLoader(configName),
 			origStoreKey: "bnk",
 			loadStoreKey: "banker",
 		},
@@ -242,24 +239,24 @@ func TestSetLoader(t *testing.T) {
 			// prepare a db with some data
 			db := dbm.NewMemDB()
 			initStore(t, db, tc.origStoreKey, k, v)
-		
+
 			// load the app with the existing db
 			opts := []func(*BaseApp){SetPruning(store.PruneSyncable)}
 			if tc.setLoader != nil {
 				opts = append(opts, tc.setLoader)
-			}	
+			}
 			app := NewBaseApp(t.Name(), defaultLogger(), db, nil, opts...)
 			capKey := sdk.NewKVStoreKey(MainStoreKey)
 			app.MountStores(capKey)
 			app.MountStores(sdk.NewKVStoreKey(tc.loadStoreKey))
-			err := app.LoadLatestVersion(capKey) 
+			err := app.LoadLatestVersion(capKey)
 			require.Nil(t, err)
 
 			// "execute" one block
 			app.BeginBlock(abci.RequestBeginBlock{Header: abci.Header{Height: 2}})
 			res := app.Commit()
 			require.NotNil(t, res.Data)
-		
+
 			// check db is properly updated
 			checkStore(t, db, 2, tc.loadStoreKey, k, v)
 			checkStore(t, db, 2, tc.loadStoreKey, []byte("foo"), nil)
@@ -271,7 +268,6 @@ func TestSetLoader(t *testing.T) {
 	require.True(t, os.IsNotExist(err))
 }
 
-
 func TestAppVersionSetterGetter(t *testing.T) {
 	logger := defaultLogger()
 	pruningOpt := SetPruning(store.PruneSyncable)
diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 0856f02b7d0f..c3f011e200f4 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -475,15 +475,6 @@ func (rs *Store) loadCommitStoreFromParams(key types.StoreKey, id types.CommitID
 	}
 }
 
-func (rs *Store) nameToKey(name string) types.StoreKey {
-	for key := range rs.storesParams {
-		if key.Name() == name {
-			return key
-		}
-	}
-	panic("Unknown name " + name)
-}
-
 //----------------------------------------
 // storeParams
 

From ca59ca70479c6224ba7654a056dcadba88dcfc57 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 29 Jul 2019 13:41:24 +0200
Subject: [PATCH 21/23] Moved baseapp.Set* methods to options.go for
 consistency

---
 baseapp/baseapp.go | 12 ------------
 baseapp/options.go | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index c40cdc3bff45..8fbbb374f9bb 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -3,7 +3,6 @@ package baseapp
 import (
 	"encoding/json"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
 	"reflect"
@@ -144,17 +143,6 @@ func (app *BaseApp) Logger() log.Logger {
 	return app.logger
 }
 
-// SetCommitMultiStoreTracer sets the store tracer on the BaseApp's underlying
-// CommitMultiStore.
-func (app *BaseApp) SetCommitMultiStoreTracer(w io.Writer) {
-	app.cms.SetTracer(w)
-}
-
-// SetStoreLoader allows us to customize the rootMultiStore initialization.
-func (app *BaseApp) SetStoreLoader(loader StoreLoader) {
-	app.storeLoader = loader
-}
-
 // MountStores mounts all IAVL or DB stores to the provided keys in the BaseApp
 // multistore.
 func (app *BaseApp) MountStores(keys ...sdk.StoreKey) {
diff --git a/baseapp/options.go b/baseapp/options.go
index 18ee2fd42c84..15a825ee4a59 100644
--- a/baseapp/options.go
+++ b/baseapp/options.go
@@ -3,6 +3,7 @@ package baseapp
 
 import (
 	"fmt"
+	"io"
 
 	dbm "github.com/tendermint/tm-db"
 
@@ -110,3 +111,17 @@ func (app *BaseApp) SetFauxMerkleMode() {
 	}
 	app.fauxMerkleMode = true
 }
+
+// SetCommitMultiStoreTracer sets the store tracer on the BaseApp's underlying
+// CommitMultiStore.
+func (app *BaseApp) SetCommitMultiStoreTracer(w io.Writer) {
+	app.cms.SetTracer(w)
+}
+
+// SetStoreLoader allows us to customize the rootMultiStore initialization.
+func (app *BaseApp) SetStoreLoader(loader StoreLoader) {
+	if app.sealed {
+		panic("SetStoreLoader() on sealed BaseApp")
+	}
+	app.storeLoader = loader
+}

From 1c2f4726ed56e2bbf5704410cd4833d7f9da97bb Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Mon, 29 Jul 2019 14:56:50 +0200
Subject: [PATCH 22/23] Apply suggestions from code review

Some cleanup suggestions

Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
---
 .pending/features/store/4724-Multistore-supp | 2 +-
 baseapp/baseapp.go                           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/.pending/features/store/4724-Multistore-supp b/.pending/features/store/4724-Multistore-supp
index de83504e8c98..b1fa458f94ce 100644
--- a/.pending/features/store/4724-Multistore-supp
+++ b/.pending/features/store/4724-Multistore-supp
@@ -1,4 +1,4 @@
-4724 Multistore supports substore migrations upon load.
+#4724 Multistore supports substore migrations upon load.
 New `rootmulti.Store.LoadLatestVersionAndUpgrade` method
 Baseapp supports `StoreLoader` to enable various upgrade strategies
 No longer panics if the store to load contains substores that we didn't explicitly mount.
diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 8fbbb374f9bb..43a4ab63879f 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -232,7 +232,7 @@ func StoreLoaderWithUpgrade(upgrades *storetypes.StoreUpgrades) StoreLoader {
 // and not re-applied on next restart
 //
 // This is useful for in place migrations when a store key is renamed between
-// two versions of the software. (Note: this code will move to x/upgrades
+// two versions of the software. (TODO: this code will move to x/upgrades
 // when PR #4233 is merged, here mainly to help test the design)
 func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
 	return func(ms sdk.CommitMultiStore) error {
@@ -263,7 +263,7 @@ func UpgradeableStoreLoader(upgradeInfoPath string) StoreLoader {
 		// if we have a successful load, we delete the file
 		err = os.Remove(upgradeInfoPath)
 		if err != nil {
-			return fmt.Errorf("Deleting upgrade file %s: %v", upgradeInfoPath, err)
+			return fmt.Errorf("deleting upgrade file %s: %v", upgradeInfoPath, err)
 		}
 		return nil
 	}

From 6b2024cf643fc24d7dbd162e9c7710cc0f3bb2fc Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Tue, 6 Aug 2019 10:55:02 +0200
Subject: [PATCH 23/23] Change json encoding of rename and update tests

---
 baseapp/baseapp_test.go | 3 +--
 store/types/store.go    | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go
index 9f0ffd7b89af..44043a780986 100644
--- a/baseapp/baseapp_test.go
+++ b/baseapp/baseapp_test.go
@@ -185,7 +185,7 @@ func TestSetLoader(t *testing.T) {
 	// write a renamer to a file
 	f, err := ioutil.TempFile("", "upgrade-*.json")
 	require.NoError(t, err)
-	data := []byte(`{"renamed":[{"old": "bnk", "new": "banker"}]}`)
+	data := []byte(`{"renamed":[{"old_key": "bnk", "new_key": "banker"}]}`)
 	_, err = f.Write(data)
 	require.NoError(t, err)
 	configName := f.Name()
@@ -1133,7 +1133,6 @@ func TestMaxBlockGasLimits(t *testing.T) {
 	}
 
 	for i, tc := range testCases {
-		fmt.Printf("debug i: %v\n", i)
 		tx := tc.tx
 
 		// reset the block gas
diff --git a/store/types/store.go b/store/types/store.go
index b7c1493667bd..021cd36cbbae 100644
--- a/store/types/store.go
+++ b/store/types/store.go
@@ -48,8 +48,8 @@ type StoreUpgrades struct {
 // All data previously under a PrefixStore with OldKey will be copied
 // to a PrefixStore with NewKey, then deleted from OldKey store.
 type StoreRename struct {
-	OldKey string `json:"old"`
-	NewKey string `json:"new"`
+	OldKey string `json:"old_key"`
+	NewKey string `json:"new_key"`
 }
 
 // IsDeleted returns true if the given key should be deleted