feat(bank): Add helper for v0.46 denom migration (cosmos#13891)

* feat(bank): Add helper for v0.46 denom migration * CL * Clearer name * Update x/bank/migrations/v046/store.go Co-authored-by: Likhita Polavarapu <[email protected]> * rename Co-authored-by: Likhita Polavarapu <[email protected]>
bianjieai · Nov 16, 2022 · 29cf4bc · 29cf4bc
1 parent 6b633ef
commit 29cf4bc
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -61,6 +61,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (store) [#13803](https://github.com/cosmos/cosmos-sdk/pull/13803) Add an error log if IAVL set operation failed.
 * [#13861](https://github.com/cosmos/cosmos-sdk/pull/13861) Allow `_` characters in tx event queries, i.e. `GetTxsEvent`.
 
+### Features
+
+* (x/bank) [#13891](https://github.com/cosmos/cosmos-sdk/pull/13891) Provide a helper function `Migrate_V0464_To_V0465` for migrating a chain **already on v0.46 with versions <=v0.46.4** to the latest v0.46.5 correct state.
+
 ## [v0.46.4](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.4) - 2022-11-01
 
 ### Features

diff --git a/x/bank/migrations/v046/keys.go b/x/bank/migrations/v046/keys.go
@@ -1,6 +1,9 @@
 package v046
 
-var DenomAddressPrefix = []byte{0x03}
+var (
+	DenomMetadataPrefix = []byte{0x1}
+	DenomAddressPrefix  = []byte{0x03}
+)
 
 // CreateDenomAddressPrefix creates a prefix for a reverse index of denomination
 // to account balance for that denomination.

diff --git a/x/bank/migrations/v046/store.go b/x/bank/migrations/v046/store.go
@@ -92,3 +92,39 @@ func migrateDenomMetadata(store sdk.KVStore) error {
 
 	return nil
 }
+
+// Migrate_V046_4_To_V046_5 is a helper function to migrate chains from <=v0.46.4
+// to v0.46.5 ONLY.
+//
+// IMPORTANT: Please do not use this function if you are upgrading to v0.46
+// from <=v0.45.
+//
+// This function migrates the store in-place by fixing the bank denom bug
+// discovered in https://github.com/cosmos/cosmos-sdk/pull/13821. It has been
+// fixed in v0.46.5, but if your chain had already migrated to v0.46, then you
+// can apply this patch (in a coordinated upgrade, e.g. in the upgrade handler)
+// to fix the bank denom state.
+//
+// The store is expected to be the bank store, and not any prefixed substore.
+func Migrate_V046_4_To_V046_5(store sdk.KVStore) error {
+	denomStore := prefix.NewStore(store, DenomMetadataPrefix)
+
+	iter := denomStore.Iterator(nil, nil)
+	defer iter.Close()
+
+	for ; iter.Valid(); iter.Next() {
+		oldKey := iter.Key()
+
+		// In the previous bugged version, we took one character too long,
+		// see this line diff:
+		// https://github.com/cosmos/cosmos-sdk/commit/62443b8c28a23efe43df2158aa2833c02c42af16#diff-d4d8a522eca0bd1fd052a756b80d0a50bff7bd8e487105221475eb78e232b46aR83
+		//
+		// Therefore we trim the last byte.
+		newKey := oldKey[:len(oldKey)-1]
+
+		denomStore.Set(newKey, iter.Value())
+		denomStore.Delete(oldKey)
+	}
+
+	return nil
+}
diff --git a/x/bank/migrations/v046/store_test.go b/x/bank/migrations/v046/store_test.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/simapp"
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	"github.com/cosmos/cosmos-sdk/testutil"
@@ -15,6 +16,35 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
+var (
+	metaData = []types.Metadata{
+		{
+			Name:        "Cosmos Hub Atom",
+			Symbol:      "ATOM",
+			Description: "The native staking token of the Cosmos Hub.",
+			DenomUnits: []*types.DenomUnit{
+				{"uatom", uint32(0), []string{"microatom"}},
+				{"matom", uint32(3), []string{"milliatom"}},
+				{"atom", uint32(6), nil},
+			},
+			Base:    "uatom",
+			Display: "atom",
+		},
+		{
+			Name:        "Token",
+			Symbol:      "TOKEN",
+			Description: "The native staking token of the Token Hub.",
+			DenomUnits: []*types.DenomUnit{
+				{"1token", uint32(5), []string{"decitoken"}},
+				{"2token", uint32(4), []string{"centitoken"}},
+				{"3token", uint32(7), []string{"dekatoken"}},
+			},
+			Base:    "utoken",
+			Display: "token",
+		},
+	}
+)
+
 func TestMigrateStore(t *testing.T) {
 	encCfg := simapp.MakeTestEncodingConfig()
 	bankKey := sdk.NewKVStoreKey("bank")
@@ -58,32 +88,6 @@ func TestMigrateDenomMetaData(t *testing.T) {
 	bankKey := sdk.NewKVStoreKey("bank")
 	ctx := testutil.DefaultContext(bankKey, sdk.NewTransientStoreKey("transient_test"))
 	store := ctx.KVStore(bankKey)
-	metaData := []types.Metadata{
-		{
-			Name:        "Cosmos Hub Atom",
-			Symbol:      "ATOM",
-			Description: "The native staking token of the Cosmos Hub.",
-			DenomUnits: []*types.DenomUnit{
-				{"uatom", uint32(0), []string{"microatom"}},
-				{"matom", uint32(3), []string{"milliatom"}},
-				{"atom", uint32(6), nil},
-			},
-			Base:    "uatom",
-			Display: "atom",
-		},
-		{
-			Name:        "Token",
-			Symbol:      "TOKEN",
-			Description: "The native staking token of the Token Hub.",
-			DenomUnits: []*types.DenomUnit{
-				{"1token", uint32(5), []string{"decitoken"}},
-				{"2token", uint32(4), []string{"centitoken"}},
-				{"3token", uint32(7), []string{"dekatoken"}},
-			},
-			Base:    "utoken",
-			Display: "token",
-		},
-	}
 	denomMetadataStore := prefix.NewStore(store, v043.DenomMetadataPrefix)
 
 	for i := range []int{0, 1} {
@@ -98,6 +102,74 @@ func TestMigrateDenomMetaData(t *testing.T) {
 	require.NoError(t, v046.MigrateStore(ctx, bankKey, encCfg.Codec))
 
 	denomMetadataStore = prefix.NewStore(store, v043.DenomMetadataPrefix)
+	assertCorrectDenomKeys(t, denomMetadataStore, encCfg.Codec)
+}
+
+// migrateDenomMetadataV0464 is the denom metadata migration function present
+// in v0.46.4. It is buggy, as discovered in https://github.com/cosmos/cosmos-sdk/pull/13821.
+// It is copied verbatim here to test the helper function Migrate_V046_4_To_V046_5
+// which aims to fix the bug on chains already on v0.46.
+//
+// Copied from:
+// https://github.com/cosmos/cosmos-sdk/blob/v0.46.4/x/bank/migrations/v046/store.go#L75-L94
+func migrateDenomMetadataV0464(store sdk.KVStore) error {
+	oldDenomMetaDataStore := prefix.NewStore(store, v043.DenomMetadataPrefix)
+
+	oldDenomMetaDataIter := oldDenomMetaDataStore.Iterator(nil, nil)
+	defer oldDenomMetaDataIter.Close()
+
+	for ; oldDenomMetaDataIter.Valid(); oldDenomMetaDataIter.Next() {
+		oldKey := oldDenomMetaDataIter.Key()
+		l := len(oldKey)/2 + 1
+
+		newKey := make([]byte, len(types.DenomMetadataPrefix)+l)
+		// old key: prefix_bytes | denom_bytes | denom_bytes
+		copy(newKey, types.DenomMetadataPrefix)
+		copy(newKey[len(types.DenomMetadataPrefix):], oldKey[:l])
+		store.Set(newKey, oldDenomMetaDataIter.Value())
+		oldDenomMetaDataStore.Delete(oldKey)
+	}
+
+	return nil
+}
+
+func TestMigrate_V046_4_To_V046_5(t *testing.T) {
+	// Step 1. Create a v0.43 state.
+	encCfg := simapp.MakeTestEncodingConfig()
+	bankKey := sdk.NewKVStoreKey("bank")
+	ctx := testutil.DefaultContext(bankKey, sdk.NewTransientStoreKey("transient_test"))
+	store := ctx.KVStore(bankKey)
+	denomMetadataStore := prefix.NewStore(store, v046.DenomMetadataPrefix)
+
+	for i := range []int{0, 1} {
+		// keys before 0.45 had denom two times in the key
+		key := append([]byte{}, []byte(metaData[i].Base)...)
+		key = append(key, []byte(metaData[i].Base)...)
+		bz, err := encCfg.Codec.Marshal(&metaData[i])
+		require.NoError(t, err)
+		denomMetadataStore.Set(key, bz)
+	}
+
+	// Step 2. Migrate to v0.46 using the BUGGED migration (present in<=v0.46.4).
+	require.NoError(t, migrateDenomMetadataV0464(store))
+
+	denomMetadataIter := denomMetadataStore.Iterator(nil, nil)
+	defer denomMetadataIter.Close()
+	for i := 0; denomMetadataIter.Valid(); denomMetadataIter.Next() {
+		newKey := denomMetadataIter.Key()
+		require.NotEqual(t, string(newKey), metaData[i].Base, "idx: %d", i) // not equal, because we had wrong keys
+		i++
+	}
+
+	// Step 3. Use the helper function to migrate to a correct v0.46.5 state.
+	require.NoError(t, v046.Migrate_V046_4_To_V046_5(store))
+
+	assertCorrectDenomKeys(t, denomMetadataStore, encCfg.Codec)
+}
+
+// assertCorrectDenomKeys makes sure the denom keys present in state are
+// correct and resolve to the correct metadata.
+func assertCorrectDenomKeys(t *testing.T, denomMetadataStore prefix.Store, cdc codec.Codec) {
 	denomMetadataIter := denomMetadataStore.Iterator(nil, nil)
 	defer denomMetadataIter.Close()
 	for i := 0; denomMetadataIter.Valid(); denomMetadataIter.Next() {
@@ -112,7 +184,7 @@ func TestMigrateDenomMetaData(t *testing.T) {
 		require.Equal(t, string(newKey), metaData[i].Base, "idx: %d", i)
 		bz = denomMetadataStore.Get(denomMetadataIter.Key())
 		require.NotNil(t, bz)
-		err := encCfg.Codec.Unmarshal(bz, &result)
+		err := cdc.Unmarshal(bz, &result)
 		require.NoError(t, err)
 		assertMetaDataEqual(t, metaData[i], result)
 		i++