From 16a953cc97475ccd1cbf631055479abc81864cbe Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 29 Sep 2021 01:40:18 +0200 Subject: [PATCH] fix: x/bank/044 migrateDenomMetadata (#10239) * fix: x/bank/044 migrateDenomMetadata * adding changelog entry * comment update * fix tests --- CHANGELOG.md | 3 ++- x/bank/migrations/v044/keys.go | 12 +++++++++--- x/bank/migrations/v044/store.go | 11 +++++++---- x/bank/migrations/v044/store_test.go | 5 +++-- x/bank/types/key.go | 8 ++++++-- x/bank/types/key_test.go | 12 ++++++++++++ 6 files changed, 39 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32c849b6de53..449e04030ace 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * `SaveOfflineKey` * Take `keyring.Record` instead of `Info` as first argument in: * `MkConsKeyOutput` - * `MkValKeyOutput` + * `MkValKeyOutput` * `MkAccKeyOutput` * [\#10077](https://github.com/cosmos/cosmos-sdk/pull/10077) Remove telemetry on `GasKV` and `CacheKV` store Get/Set operations, significantly improving their performance. * [\#10022](https://github.com/cosmos/cosmos-sdk/pull/10022) `AuthKeeper` interface in `x/auth` now includes a function `HasAccount`. @@ -139,6 +139,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (server) [#10016](https://github.com/cosmos/cosmos-sdk/issues/10016) Fix marshaling of index-events into server config file. * (x/feegrant) [\#10049](https://github.com/cosmos/cosmos-sdk/issues/10049) Fixed the error message when `period` or `period-limit` flag is not set on a feegrant grant transaction. * [\#10184](https://github.com/cosmos/cosmos-sdk/pull/10184) Fixed CLI tx commands to no longer explicitly require the chain-id flag as this value can come from a user config. +* [\#10239](https://github.com/cosmos/cosmos-sdk/pull/10239) Fixed x/bank/044 migrateDenomMetadata. * (x/upgrade) [\#10189](https://github.com/cosmos/cosmos-sdk/issues/10189) Removed potential sources of non-determinism in upgrades ### State Machine Breaking diff --git a/x/bank/migrations/v044/keys.go b/x/bank/migrations/v044/keys.go index c06492658866..d5a25393e635 100644 --- a/x/bank/migrations/v044/keys.go +++ b/x/bank/migrations/v044/keys.go @@ -4,7 +4,13 @@ var ( DenomAddressPrefix = []byte{0x03} ) -func CreateAddressDenomPrefix(denom string) []byte { - key := append(DenomAddressPrefix, []byte(denom)...) - return append(key, 0) +// CreateDenomAddressPrefix creates a prefix for a reverse index of denomination +// to account balance for that denomination. +func CreateDenomAddressPrefix(denom string) []byte { + // we add a "zero" byte at the end - null byte terminator, to allow prefix denom prefix + // scan. Setting it is not needed (key[last] = 0) - because this is the default. + key := make([]byte, len(DenomAddressPrefix)+len(denom)+1) + copy(key, DenomAddressPrefix) + copy(key[len(DenomAddressPrefix):], denom) + return key } diff --git a/x/bank/migrations/v044/store.go b/x/bank/migrations/v044/store.go index b43404573121..0ea071398352 100644 --- a/x/bank/migrations/v044/store.go +++ b/x/bank/migrations/v044/store.go @@ -59,7 +59,7 @@ func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec) error { denomPrefixStore, ok := denomPrefixStores[balance.Denom] if !ok { - denomPrefixStore = prefix.NewStore(store, CreateAddressDenomPrefix(balance.Denom)) + denomPrefixStore = prefix.NewStore(store, CreateDenomAddressPrefix(balance.Denom)) denomPrefixStores[balance.Denom] = denomPrefixStore } @@ -79,11 +79,14 @@ func migrateDenomMetadata(store sdk.KVStore) error { for ; oldDenomMetaDataIter.Valid(); oldDenomMetaDataIter.Next() { oldKey := oldDenomMetaDataIter.Key() - // old key: prefix_bytes | denom_bytes | denom_bytes - newKey := append(types.DenomMetadataPrefix, oldKey[:len(oldKey)/2+1]...) + l := len(oldKey)/2 + 1 + var 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(oldDenomMetaDataIter.Key()) + oldDenomMetaDataStore.Delete(oldKey) } return nil diff --git a/x/bank/migrations/v044/store_test.go b/x/bank/migrations/v044/store_test.go index 37c268d3d702..e60fe4e231cf 100644 --- a/x/bank/migrations/v044/store_test.go +++ b/x/bank/migrations/v044/store_test.go @@ -47,7 +47,7 @@ func TestMigrateStore(t *testing.T) { } for _, b := range balances { - denomPrefixStore := prefix.NewStore(store, v044.CreateAddressDenomPrefix(b.Denom)) + denomPrefixStore := prefix.NewStore(store, v044.CreateDenomAddressPrefix(b.Denom)) bz := denomPrefixStore.Get(address.MustLengthPrefix(addr)) require.NotNil(t, bz) } @@ -88,6 +88,7 @@ func TestMigrateDenomMetaData(t *testing.T) { for i := range []int{0, 1} { key := append(v043.DenomMetadataPrefix, []byte(metaData[i].Base)...) + // keys before 0.44 had denom two times in the key key = append(key, []byte(metaData[i].Base)...) bz, err := encCfg.Codec.Marshal(&metaData[i]) require.NoError(t, err) @@ -108,7 +109,7 @@ func TestMigrateDenomMetaData(t *testing.T) { bz := denomMetadataStore.Get(oldKey) require.Nil(t, bz) - require.Equal(t, string(newKey)[1:], metaData[i].Base) + require.Equal(t, string(newKey)[1:], 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/types/key.go b/x/bank/types/key.go index dd01a4d49065..44decc3e7160 100644 --- a/x/bank/types/key.go +++ b/x/bank/types/key.go @@ -60,6 +60,10 @@ func CreateAccountBalancesPrefix(addr []byte) []byte { // CreateDenomAddressPrefix creates a prefix for a reverse index of denomination // to account balance for that denomination. func CreateDenomAddressPrefix(denom string) []byte { - key := append(DenomAddressPrefix, []byte(denom)...) - return append(key, 0) + // we add a "zero" byte at the end - null byte terminator, to allow prefix denom prefix + // scan. Setting it is not needed (key[last] = 0) - because this is the default. + key := make([]byte, len(DenomAddressPrefix)+len(denom)+1) + copy(key, DenomAddressPrefix) + copy(key[len(DenomAddressPrefix):], denom) + return key } diff --git a/x/bank/types/key_test.go b/x/bank/types/key_test.go index fadadd022097..ba0dd5c63ee4 100644 --- a/x/bank/types/key_test.go +++ b/x/bank/types/key_test.go @@ -56,3 +56,15 @@ func TestAddressFromBalancesStore(t *testing.T) { }) } } + +func TestCreateDenomAddressPrefix(t *testing.T) { + require := require.New(t) + + key := types.CreateDenomAddressPrefix("") + require.Len(key, len(types.DenomAddressPrefix)+1) + require.Equal(append(types.DenomAddressPrefix, 0), key) + + key = types.CreateDenomAddressPrefix("abc") + require.Len(key, len(types.DenomAddressPrefix)+4) + require.Equal(append(types.DenomAddressPrefix, 'a', 'b', 'c', 0), key) +}