From 06ec3d31e3902d0c38408dbda10c51322e59ae19 Mon Sep 17 00:00:00 2001 From: billy rennekamp Date: Thu, 10 Nov 2022 11:02:17 -0500 Subject: [PATCH 1/2] fix: bank store migration (#13821) (cherry picked from commit d314a12e688060aab6afa63fa8b12bcd69ecb3bd) # Conflicts: # x/bank/migrations/v3/store.go # x/bank/migrations/v3/store_test.go --- x/bank/migrations/v3/store.go | 95 +++++++++++++++++++++ x/bank/migrations/v3/store_test.go | 131 +++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100644 x/bank/migrations/v3/store.go create mode 100644 x/bank/migrations/v3/store_test.go diff --git a/x/bank/migrations/v3/store.go b/x/bank/migrations/v3/store.go new file mode 100644 index 000000000000..c5db8553d2b7 --- /dev/null +++ b/x/bank/migrations/v3/store.go @@ -0,0 +1,95 @@ +package v3 + +import ( + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/store/prefix" + storetypes "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" + v2 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v2" + "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/tendermint/tendermint/libs/log" +) + +// MigrateStore performs in-place store migrations from v0.43 to v0.45. The +// migration includes: +// +// - Migrate coin storage to save only amount. +// - Add an additional reverse index from denomination to address. +// - Remove duplicate denom from denom metadata store key. +func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { + store := ctx.KVStore(storeKey) + err := addDenomReverseIndex(store, cdc, ctx.Logger()) + if err != nil { + return err + } + + return migrateDenomMetadata(store, ctx.Logger()) +} + +func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec, logger log.Logger) error { + oldBalancesStore := prefix.NewStore(store, v2.BalancesPrefix) + + oldBalancesIter := oldBalancesStore.Iterator(nil, nil) + defer sdk.LogDeferred(logger, func() error { return oldBalancesIter.Close() }) + + denomPrefixStores := make(map[string]prefix.Store) // memoize prefix stores + + for ; oldBalancesIter.Valid(); oldBalancesIter.Next() { + var balance sdk.Coin + if err := cdc.Unmarshal(oldBalancesIter.Value(), &balance); err != nil { + return err + } + + addr, err := v2.AddressFromBalancesStore(oldBalancesIter.Key()) + if err != nil { + return err + } + + var coin sdk.DecCoin + if err := cdc.Unmarshal(oldBalancesIter.Value(), &coin); err != nil { + return err + } + + bz, err := coin.Amount.Marshal() + if err != nil { + return err + } + + newStore := prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr)) + newStore.Set([]byte(coin.Denom), bz) + + denomPrefixStore, ok := denomPrefixStores[balance.Denom] + if !ok { + denomPrefixStore = prefix.NewStore(store, CreateDenomAddressPrefix(balance.Denom)) + denomPrefixStores[balance.Denom] = denomPrefixStore + } + + // Store a reverse index from denomination to account address with a + // sentinel value. + denomPrefixStore.Set(address.MustLengthPrefix(addr), []byte{0}) + } + + return nil +} + +func migrateDenomMetadata(store sdk.KVStore, logger log.Logger) error { + oldDenomMetaDataStore := prefix.NewStore(store, v2.DenomMetadataPrefix) + + oldDenomMetaDataIter := oldDenomMetaDataStore.Iterator(nil, nil) + defer sdk.LogDeferred(logger, func() error { return oldDenomMetaDataIter.Close() }) + + for ; oldDenomMetaDataIter.Valid(); oldDenomMetaDataIter.Next() { + oldKey := oldDenomMetaDataIter.Key() + l := len(oldKey) / 2 + + 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 +} diff --git a/x/bank/migrations/v3/store_test.go b/x/bank/migrations/v3/store_test.go new file mode 100644 index 000000000000..52e9efdcd324 --- /dev/null +++ b/x/bank/migrations/v3/store_test.go @@ -0,0 +1,131 @@ +package v3_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "cosmossdk.io/math" + + "github.com/cosmos/cosmos-sdk/store/prefix" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" + moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" + v2 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v2" + v3 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v3" + "github.com/cosmos/cosmos-sdk/x/bank/types" +) + +func TestMigrateStore(t *testing.T) { + encCfg := moduletestutil.MakeTestEncodingConfig() + bankKey := sdk.NewKVStoreKey("bank") + ctx := testutil.DefaultContext(bankKey, sdk.NewTransientStoreKey("transient_test")) + store := ctx.KVStore(bankKey) + + addr := sdk.AccAddress([]byte("addr________________")) + prefixAccStore := prefix.NewStore(store, v2.CreateAccountBalancesPrefix(addr)) + + balances := sdk.NewCoins( + sdk.NewCoin("foo", sdk.NewInt(10000)), + sdk.NewCoin("bar", sdk.NewInt(20000)), + ) + + for _, b := range balances { + bz, err := encCfg.Codec.Marshal(&b) + require.NoError(t, err) + + prefixAccStore.Set([]byte(b.Denom), bz) + } + + require.NoError(t, v3.MigrateStore(ctx, bankKey, encCfg.Codec)) + + for _, b := range balances { + addrPrefixStore := prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr)) + bz := addrPrefixStore.Get([]byte(b.Denom)) + var expected math.Int + require.NoError(t, expected.Unmarshal(bz)) + require.Equal(t, expected, b.Amount) + } + + for _, b := range balances { + denomPrefixStore := prefix.NewStore(store, v3.CreateDenomAddressPrefix(b.Denom)) + bz := denomPrefixStore.Get(address.MustLengthPrefix(addr)) + require.NotNil(t, bz) + } +} + +func TestMigrateDenomMetaData(t *testing.T) { + encCfg := moduletestutil.MakeTestEncodingConfig() + 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{ + {Denom: "uatom", Exponent: uint32(0), Aliases: []string{"microatom"}}, + {Denom: "matom", Exponent: uint32(3), Aliases: []string{"milliatom"}}, + {Denom: "atom", Exponent: uint32(6), Aliases: nil}, + }, + Base: "uatom", + Display: "atom", + }, + { + Name: "Token", + Symbol: "TOKEN", + Description: "The native staking token of the Token Hub.", + DenomUnits: []*types.DenomUnit{ + {Denom: "1token", Exponent: uint32(5), Aliases: []string{"decitoken"}}, + {Denom: "2token", Exponent: uint32(4), Aliases: []string{"centitoken"}}, + {Denom: "3token", Exponent: uint32(7), Aliases: []string{"dekatoken"}}, + }, + Base: "utoken", + Display: "token", + }, + } + denomMetadataStore := prefix.NewStore(store, v2.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) + } + + require.NoError(t, v3.MigrateStore(ctx, bankKey, encCfg.Codec)) + + denomMetadataStore = prefix.NewStore(store, v2.DenomMetadataPrefix) + denomMetadataIter := denomMetadataStore.Iterator(nil, nil) + defer denomMetadataIter.Close() + for i := 0; denomMetadataIter.Valid(); denomMetadataIter.Next() { + var result types.Metadata + newKey := denomMetadataIter.Key() + + // make sure old entry is deleted + oldKey := append(newKey, newKey[0:]...) + bz := denomMetadataStore.Get(oldKey) + require.Nil(t, bz) + + 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) + require.NoError(t, err) + assertMetaDataEqual(t, metaData[i], result) + i++ + } +} + +func assertMetaDataEqual(t *testing.T, expected, actual types.Metadata) { + require.Equal(t, expected.GetBase(), actual.GetBase()) + require.Equal(t, expected.GetDisplay(), actual.GetDisplay()) + require.Equal(t, expected.GetDescription(), actual.GetDescription()) + require.Equal(t, expected.GetDenomUnits()[1].GetDenom(), actual.GetDenomUnits()[1].GetDenom()) + require.Equal(t, expected.GetDenomUnits()[1].GetExponent(), actual.GetDenomUnits()[1].GetExponent()) + require.Equal(t, expected.GetDenomUnits()[1].GetAliases(), actual.GetDenomUnits()[1].GetAliases()) +} From 6c1832fbfb21c5bf081f5dafae459f14dd5295d3 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 10 Nov 2022 17:12:41 +0100 Subject: [PATCH 2/2] updates --- CHANGELOG.md | 1 + x/bank/migrations/v046/store.go | 2 +- x/bank/migrations/v046/store_test.go | 6 +- x/bank/migrations/v3/store.go | 95 ------------------- x/bank/migrations/v3/store_test.go | 131 --------------------------- 5 files changed, 5 insertions(+), 230 deletions(-) delete mode 100644 x/bank/migrations/v3/store.go delete mode 100644 x/bank/migrations/v3/store_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f71ecc87992..036e124ef9c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (x/bank) [#13821](https://github.com/cosmos/cosmos-sdk/pull/13821) Fix bank store migration of coin metadata. * (x/group) [#13808](https://github.com/cosmos/cosmos-sdk/pull/13808) Fix propagation of message events to the current context in `EndBlocker`. * (x/gov) [#13728](https://github.com/cosmos/cosmos-sdk/pull/13728) Fix propagation of message events to the current context in `EndBlocker`. * (store) [#13803](https://github.com/cosmos/cosmos-sdk/pull/13803) Add an error log if iavl set operation failed. diff --git a/x/bank/migrations/v046/store.go b/x/bank/migrations/v046/store.go index 8f4e628a11ea..8e5db7b6733f 100644 --- a/x/bank/migrations/v046/store.go +++ b/x/bank/migrations/v046/store.go @@ -80,7 +80,7 @@ func migrateDenomMetadata(store sdk.KVStore) error { for ; oldDenomMetaDataIter.Valid(); oldDenomMetaDataIter.Next() { oldKey := oldDenomMetaDataIter.Key() - l := len(oldKey)/2 + 1 + l := len(oldKey) / 2 newKey := make([]byte, len(types.DenomMetadataPrefix)+l) // old key: prefix_bytes | denom_bytes | denom_bytes diff --git a/x/bank/migrations/v046/store_test.go b/x/bank/migrations/v046/store_test.go index db44f2591fdc..4c43dec5b4e0 100644 --- a/x/bank/migrations/v046/store_test.go +++ b/x/bank/migrations/v046/store_test.go @@ -87,8 +87,8 @@ func TestMigrateDenomMetaData(t *testing.T) { denomMetadataStore := prefix.NewStore(store, v043.DenomMetadataPrefix) for i := range []int{0, 1} { - key := append(v043.DenomMetadataPrefix, []byte(metaData[i].Base)...) // 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) @@ -105,11 +105,11 @@ func TestMigrateDenomMetaData(t *testing.T) { newKey := denomMetadataIter.Key() // make sure old entry is deleted - oldKey := append(newKey, newKey[1:]...) + oldKey := append(newKey, newKey[0:]...) bz := denomMetadataStore.Get(oldKey) require.Nil(t, bz) - require.Equal(t, string(newKey)[1:], metaData[i].Base, "idx: %d", i) + 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) diff --git a/x/bank/migrations/v3/store.go b/x/bank/migrations/v3/store.go deleted file mode 100644 index c5db8553d2b7..000000000000 --- a/x/bank/migrations/v3/store.go +++ /dev/null @@ -1,95 +0,0 @@ -package v3 - -import ( - "github.com/cosmos/cosmos-sdk/codec" - "github.com/cosmos/cosmos-sdk/store/prefix" - storetypes "github.com/cosmos/cosmos-sdk/store/types" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/address" - v2 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v2" - "github.com/cosmos/cosmos-sdk/x/bank/types" - "github.com/tendermint/tendermint/libs/log" -) - -// MigrateStore performs in-place store migrations from v0.43 to v0.45. The -// migration includes: -// -// - Migrate coin storage to save only amount. -// - Add an additional reverse index from denomination to address. -// - Remove duplicate denom from denom metadata store key. -func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { - store := ctx.KVStore(storeKey) - err := addDenomReverseIndex(store, cdc, ctx.Logger()) - if err != nil { - return err - } - - return migrateDenomMetadata(store, ctx.Logger()) -} - -func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec, logger log.Logger) error { - oldBalancesStore := prefix.NewStore(store, v2.BalancesPrefix) - - oldBalancesIter := oldBalancesStore.Iterator(nil, nil) - defer sdk.LogDeferred(logger, func() error { return oldBalancesIter.Close() }) - - denomPrefixStores := make(map[string]prefix.Store) // memoize prefix stores - - for ; oldBalancesIter.Valid(); oldBalancesIter.Next() { - var balance sdk.Coin - if err := cdc.Unmarshal(oldBalancesIter.Value(), &balance); err != nil { - return err - } - - addr, err := v2.AddressFromBalancesStore(oldBalancesIter.Key()) - if err != nil { - return err - } - - var coin sdk.DecCoin - if err := cdc.Unmarshal(oldBalancesIter.Value(), &coin); err != nil { - return err - } - - bz, err := coin.Amount.Marshal() - if err != nil { - return err - } - - newStore := prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr)) - newStore.Set([]byte(coin.Denom), bz) - - denomPrefixStore, ok := denomPrefixStores[balance.Denom] - if !ok { - denomPrefixStore = prefix.NewStore(store, CreateDenomAddressPrefix(balance.Denom)) - denomPrefixStores[balance.Denom] = denomPrefixStore - } - - // Store a reverse index from denomination to account address with a - // sentinel value. - denomPrefixStore.Set(address.MustLengthPrefix(addr), []byte{0}) - } - - return nil -} - -func migrateDenomMetadata(store sdk.KVStore, logger log.Logger) error { - oldDenomMetaDataStore := prefix.NewStore(store, v2.DenomMetadataPrefix) - - oldDenomMetaDataIter := oldDenomMetaDataStore.Iterator(nil, nil) - defer sdk.LogDeferred(logger, func() error { return oldDenomMetaDataIter.Close() }) - - for ; oldDenomMetaDataIter.Valid(); oldDenomMetaDataIter.Next() { - oldKey := oldDenomMetaDataIter.Key() - l := len(oldKey) / 2 - - 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 -} diff --git a/x/bank/migrations/v3/store_test.go b/x/bank/migrations/v3/store_test.go deleted file mode 100644 index 52e9efdcd324..000000000000 --- a/x/bank/migrations/v3/store_test.go +++ /dev/null @@ -1,131 +0,0 @@ -package v3_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "cosmossdk.io/math" - - "github.com/cosmos/cosmos-sdk/store/prefix" - "github.com/cosmos/cosmos-sdk/testutil" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/address" - moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" - v2 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v2" - v3 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v3" - "github.com/cosmos/cosmos-sdk/x/bank/types" -) - -func TestMigrateStore(t *testing.T) { - encCfg := moduletestutil.MakeTestEncodingConfig() - bankKey := sdk.NewKVStoreKey("bank") - ctx := testutil.DefaultContext(bankKey, sdk.NewTransientStoreKey("transient_test")) - store := ctx.KVStore(bankKey) - - addr := sdk.AccAddress([]byte("addr________________")) - prefixAccStore := prefix.NewStore(store, v2.CreateAccountBalancesPrefix(addr)) - - balances := sdk.NewCoins( - sdk.NewCoin("foo", sdk.NewInt(10000)), - sdk.NewCoin("bar", sdk.NewInt(20000)), - ) - - for _, b := range balances { - bz, err := encCfg.Codec.Marshal(&b) - require.NoError(t, err) - - prefixAccStore.Set([]byte(b.Denom), bz) - } - - require.NoError(t, v3.MigrateStore(ctx, bankKey, encCfg.Codec)) - - for _, b := range balances { - addrPrefixStore := prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr)) - bz := addrPrefixStore.Get([]byte(b.Denom)) - var expected math.Int - require.NoError(t, expected.Unmarshal(bz)) - require.Equal(t, expected, b.Amount) - } - - for _, b := range balances { - denomPrefixStore := prefix.NewStore(store, v3.CreateDenomAddressPrefix(b.Denom)) - bz := denomPrefixStore.Get(address.MustLengthPrefix(addr)) - require.NotNil(t, bz) - } -} - -func TestMigrateDenomMetaData(t *testing.T) { - encCfg := moduletestutil.MakeTestEncodingConfig() - 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{ - {Denom: "uatom", Exponent: uint32(0), Aliases: []string{"microatom"}}, - {Denom: "matom", Exponent: uint32(3), Aliases: []string{"milliatom"}}, - {Denom: "atom", Exponent: uint32(6), Aliases: nil}, - }, - Base: "uatom", - Display: "atom", - }, - { - Name: "Token", - Symbol: "TOKEN", - Description: "The native staking token of the Token Hub.", - DenomUnits: []*types.DenomUnit{ - {Denom: "1token", Exponent: uint32(5), Aliases: []string{"decitoken"}}, - {Denom: "2token", Exponent: uint32(4), Aliases: []string{"centitoken"}}, - {Denom: "3token", Exponent: uint32(7), Aliases: []string{"dekatoken"}}, - }, - Base: "utoken", - Display: "token", - }, - } - denomMetadataStore := prefix.NewStore(store, v2.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) - } - - require.NoError(t, v3.MigrateStore(ctx, bankKey, encCfg.Codec)) - - denomMetadataStore = prefix.NewStore(store, v2.DenomMetadataPrefix) - denomMetadataIter := denomMetadataStore.Iterator(nil, nil) - defer denomMetadataIter.Close() - for i := 0; denomMetadataIter.Valid(); denomMetadataIter.Next() { - var result types.Metadata - newKey := denomMetadataIter.Key() - - // make sure old entry is deleted - oldKey := append(newKey, newKey[0:]...) - bz := denomMetadataStore.Get(oldKey) - require.Nil(t, bz) - - 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) - require.NoError(t, err) - assertMetaDataEqual(t, metaData[i], result) - i++ - } -} - -func assertMetaDataEqual(t *testing.T, expected, actual types.Metadata) { - require.Equal(t, expected.GetBase(), actual.GetBase()) - require.Equal(t, expected.GetDisplay(), actual.GetDisplay()) - require.Equal(t, expected.GetDescription(), actual.GetDescription()) - require.Equal(t, expected.GetDenomUnits()[1].GetDenom(), actual.GetDenomUnits()[1].GetDenom()) - require.Equal(t, expected.GetDenomUnits()[1].GetExponent(), actual.GetDenomUnits()[1].GetExponent()) - require.Equal(t, expected.GetDenomUnits()[1].GetAliases(), actual.GetDenomUnits()[1].GetAliases()) -}