From 29cf4bca75cdf3fa06f65eaca88ca89981681137 Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Wed, 16 Nov 2022 19:41:28 +0100 Subject: [PATCH] feat(bank): Add helper for v0.46 denom migration (#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 <78951027+likhita-809@users.noreply.github.com> * rename Co-authored-by: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com> --- CHANGELOG.md | 4 + x/bank/migrations/v046/keys.go | 5 +- x/bank/migrations/v046/store.go | 36 ++++++++ x/bank/migrations/v046/store_test.go | 126 +++++++++++++++++++++------ 4 files changed, 143 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8e9b024045f..bcf83b6b2aba 100644 --- 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 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..eb1b5ccaefc8 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 } + +// 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 index 4c43dec5b4e0..0322f1e89228 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 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++