From 1d87a1d795f91c8888bc66319d45d8abbcce0a72 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 16 Nov 2022 16:12:41 +0100 Subject: [PATCH 1/5] feat(bank): Add helper for v0.46 denom migration --- x/bank/migrations/v046/keys.go | 5 +- x/bank/migrations/v046/store.go | 36 ++++++++ x/bank/migrations/v046/store_test.go | 126 +++++++++++++++++++++------ 3 files changed, 139 insertions(+), 28 deletions(-) diff --git a/x/bank/migrations/v046/keys.go b/x/bank/migrations/v046/keys.go index c86aae935805..c72de319d06d 100644 --- 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 index 8e5db7b6733f..82a6baab5f14 100644 --- 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 } + +// MigrateV0464ToV0465 is a helper function to migrate chains from <=v0.46.4 +// to v0.46.5 ONLY. +// +// IMPORTANT: Please de 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 MigrateV0464ToV0465(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 index 4c43dec5b4e0..ab70cac7087b 100644 --- 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 MigrateV0464ToV0465 +// 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 TestMigrateV0464ToV0465(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.MigrateV0464ToV0465(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++ From 2e331c4425602d133e1a2b1d0fd1e3b088c94a7a Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 16 Nov 2022 16:14:44 +0100 Subject: [PATCH 2/5] CL --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81c204f0b437..7d6c8c83ce9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features * (x/auth) [#13612](https://github.com/cosmos/cosmos-sdk/pull/13612) Add `Query/ModuleAccountByName` endpoint for accessing the module account info by module name. +* (feat) [#13891](https://github.com/cosmos/cosmos-sdk/pull/13891) Provide a helper function `MigrateV0464ToV0465` for migrating a chain **already on <=v0.46.4** to the latest v0.46.5 correct state. ### Improvements From ae793da99027093f6d3a3d0c6ae61fed317b704b Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 16 Nov 2022 16:50:08 +0100 Subject: [PATCH 3/5] Clearer name --- CHANGELOG.md | 5 ++++- x/bank/migrations/v046/store.go | 4 ++-- x/bank/migrations/v046/store_test.go | 6 +++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6c8c83ce9b..e3e90cff61cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,12 +53,15 @@ 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 * (x/auth) [#13612](https://github.com/cosmos/cosmos-sdk/pull/13612) Add `Query/ModuleAccountByName` endpoint for accessing the module account info by module name. -* (feat) [#13891](https://github.com/cosmos/cosmos-sdk/pull/13891) Provide a helper function `MigrateV0464ToV0465` for migrating a chain **already on <=v0.46.4** to the latest v0.46.5 correct state. ### Improvements diff --git a/x/bank/migrations/v046/store.go b/x/bank/migrations/v046/store.go index 82a6baab5f14..2be16c9f1278 100644 --- a/x/bank/migrations/v046/store.go +++ b/x/bank/migrations/v046/store.go @@ -93,7 +93,7 @@ func migrateDenomMetadata(store sdk.KVStore) error { return nil } -// MigrateV0464ToV0465 is a helper function to migrate chains from <=v0.46.4 +// Migrate_V0464_To_V0465 is a helper function to migrate chains from <=v0.46.4 // to v0.46.5 ONLY. // // IMPORTANT: Please de not use this function if you are upgrading to v0.46 @@ -106,7 +106,7 @@ func migrateDenomMetadata(store sdk.KVStore) error { // to fix the bank denom state. // // The store is expected to be the bank store, and not any prefixed substore. -func MigrateV0464ToV0465(store sdk.KVStore) error { +func Migrate_V0464_To_V0465(store sdk.KVStore) error { denomStore := prefix.NewStore(store, DenomMetadataPrefix) iter := denomStore.Iterator(nil, nil) diff --git a/x/bank/migrations/v046/store_test.go b/x/bank/migrations/v046/store_test.go index ab70cac7087b..1e6ea640f3de 100644 --- a/x/bank/migrations/v046/store_test.go +++ b/x/bank/migrations/v046/store_test.go @@ -107,7 +107,7 @@ func TestMigrateDenomMetaData(t *testing.T) { // 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 MigrateV0464ToV0465 +// It is copied verbatim here to test the helper function Migrate_V0464_To_V0465 // which aims to fix the bug on chains already on v0.46. // // Copied from: @@ -133,7 +133,7 @@ func migrateDenomMetadataV0464(store sdk.KVStore) error { return nil } -func TestMigrateV0464ToV0465(t *testing.T) { +func TestMigrate_V0464_To_V0465(t *testing.T) { // Step 1. Create a v0.43 state. encCfg := simapp.MakeTestEncodingConfig() bankKey := sdk.NewKVStoreKey("bank") @@ -162,7 +162,7 @@ func TestMigrateV0464ToV0465(t *testing.T) { } // Step 3. Use the helper function to migrate to a correct v0.46.5 state. - require.NoError(t, v046.MigrateV0464ToV0465(store)) + require.NoError(t, v046.Migrate_V0464_To_V0465(store)) assertCorrectDenomKeys(t, denomMetadataStore, encCfg.Codec) } From 13c8a93174ac31ba7ef8ebbc6b5d591c5be2731b Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Wed, 16 Nov 2022 16:50:34 +0100 Subject: [PATCH 4/5] Update x/bank/migrations/v046/store.go Co-authored-by: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com> --- x/bank/migrations/v046/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/bank/migrations/v046/store.go b/x/bank/migrations/v046/store.go index 2be16c9f1278..eafeb1448ddc 100644 --- a/x/bank/migrations/v046/store.go +++ b/x/bank/migrations/v046/store.go @@ -96,7 +96,7 @@ func migrateDenomMetadata(store sdk.KVStore) error { // Migrate_V0464_To_V0465 is a helper function to migrate chains from <=v0.46.4 // to v0.46.5 ONLY. // -// IMPORTANT: Please de not use this function if you are upgrading to v0.46 +// 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 From 3c714b7d346f0d468331ccb32bd8aa4ee50681e6 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 16 Nov 2022 17:23:54 +0100 Subject: [PATCH 5/5] rename --- x/bank/migrations/v046/store.go | 4 ++-- x/bank/migrations/v046/store_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/bank/migrations/v046/store.go b/x/bank/migrations/v046/store.go index 2be16c9f1278..6fff8faba7a7 100644 --- a/x/bank/migrations/v046/store.go +++ b/x/bank/migrations/v046/store.go @@ -93,7 +93,7 @@ func migrateDenomMetadata(store sdk.KVStore) error { return nil } -// Migrate_V0464_To_V0465 is a helper function to migrate chains from <=v0.46.4 +// Migrate_V046_4_To_V046_5 is a helper function to migrate chains from <=v0.46.4 // to v0.46.5 ONLY. // // IMPORTANT: Please de not use this function if you are upgrading to v0.46 @@ -106,7 +106,7 @@ func migrateDenomMetadata(store sdk.KVStore) error { // to fix the bank denom state. // // The store is expected to be the bank store, and not any prefixed substore. -func Migrate_V0464_To_V0465(store sdk.KVStore) error { +func Migrate_V046_4_To_V046_5(store sdk.KVStore) error { denomStore := prefix.NewStore(store, DenomMetadataPrefix) iter := denomStore.Iterator(nil, nil) diff --git a/x/bank/migrations/v046/store_test.go b/x/bank/migrations/v046/store_test.go index 1e6ea640f3de..0322f1e89228 100644 --- a/x/bank/migrations/v046/store_test.go +++ b/x/bank/migrations/v046/store_test.go @@ -107,7 +107,7 @@ func TestMigrateDenomMetaData(t *testing.T) { // 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_V0464_To_V0465 +// 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: @@ -133,7 +133,7 @@ func migrateDenomMetadataV0464(store sdk.KVStore) error { return nil } -func TestMigrate_V0464_To_V0465(t *testing.T) { +func TestMigrate_V046_4_To_V046_5(t *testing.T) { // Step 1. Create a v0.43 state. encCfg := simapp.MakeTestEncodingConfig() bankKey := sdk.NewKVStoreKey("bank") @@ -162,7 +162,7 @@ func TestMigrate_V0464_To_V0465(t *testing.T) { } // Step 3. Use the helper function to migrate to a correct v0.46.5 state. - require.NoError(t, v046.Migrate_V0464_To_V0465(store)) + require.NoError(t, v046.Migrate_V046_4_To_V046_5(store)) assertCorrectDenomKeys(t, denomMetadataStore, encCfg.Codec) }