From 7d2ee9ff6630fab84b2a3e4f6ea39bfdb6843a12 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 27 Sep 2021 17:08:33 +0200 Subject: [PATCH 1/4] fix: x/bank/044 migrateDenomMetadata --- x/bank/migrations/v044/keys.go | 7 +++++-- x/bank/migrations/v044/store.go | 9 ++++++--- x/bank/types/key.go | 7 +++++-- x/bank/types/key_test.go | 12 ++++++++++++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/x/bank/migrations/v044/keys.go b/x/bank/migrations/v044/keys.go index c0649265886..dc2a2dc95c8 100644 --- a/x/bank/migrations/v044/keys.go +++ b/x/bank/migrations/v044/keys.go @@ -5,6 +5,9 @@ var ( ) func CreateAddressDenomPrefix(denom string) []byte { - key := append(DenomAddressPrefix, []byte(denom)...) - return append(key, 0) + key := make([]byte, len(DenomAddressPrefix)+len(denom)+1) + copy(key, DenomAddressPrefix) + copy(key[len(DenomAddressPrefix):], denom) + // key[last] = 0 - not needed, because this is the default + return key } diff --git a/x/bank/migrations/v044/store.go b/x/bank/migrations/v044/store.go index b4340457312..c6b23924e58 100644 --- a/x/bank/migrations/v044/store.go +++ b/x/bank/migrations/v044/store.go @@ -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) + copy(newKey, types.DenomMetadataPrefix) + // old key: prefix_bytes | denom_bytes | denom_bytes + newKey = append(newKey, oldKey[:l]...) store.Set(newKey, oldDenomMetaDataIter.Value()) - oldDenomMetaDataStore.Delete(oldDenomMetaDataIter.Key()) + oldDenomMetaDataStore.Delete(oldKey) } return nil diff --git a/x/bank/types/key.go b/x/bank/types/key.go index dd01a4d4906..3c8bc9a4098 100644 --- a/x/bank/types/key.go +++ b/x/bank/types/key.go @@ -60,6 +60,9 @@ 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) + key := make([]byte, len(DenomAddressPrefix)+len(denom)+1) + copy(key, DenomAddressPrefix) + copy(key[len(DenomAddressPrefix):], denom) + // key[last] = 0 - not needed, because this is the default + return key } diff --git a/x/bank/types/key_test.go b/x/bank/types/key_test.go index fadadd02209..ba0dd5c63ee 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) +} From 265c6ccbc56d0dfbad79fa575abbc134438c3726 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 27 Sep 2021 17:13:32 +0200 Subject: [PATCH 2/4] adding changelog entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93c94e4923f..e5253478d9c 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`. @@ -137,6 +137,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. ### State Machine Breaking From a29673c2c256dbee964e4b5d577c726287e3fd10 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Tue, 28 Sep 2021 17:13:09 +0200 Subject: [PATCH 3/4] comment update --- x/bank/types/key.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/bank/types/key.go b/x/bank/types/key.go index 3c8bc9a4098..44decc3e716 100644 --- a/x/bank/types/key.go +++ b/x/bank/types/key.go @@ -60,9 +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 { + // 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) - // key[last] = 0 - not needed, because this is the default return key } From 2b9cb5aa03f7d8c0b07edf44cccf00e505650ece Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Tue, 28 Sep 2021 19:33:08 +0200 Subject: [PATCH 4/4] fix tests --- x/bank/migrations/v044/keys.go | 7 +++++-- x/bank/migrations/v044/store.go | 6 +++--- x/bank/migrations/v044/store_test.go | 5 +++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/x/bank/migrations/v044/keys.go b/x/bank/migrations/v044/keys.go index dc2a2dc95c8..d5a25393e63 100644 --- a/x/bank/migrations/v044/keys.go +++ b/x/bank/migrations/v044/keys.go @@ -4,10 +4,13 @@ var ( DenomAddressPrefix = []byte{0x03} ) -func CreateAddressDenomPrefix(denom string) []byte { +// 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) - // key[last] = 0 - not needed, because this is the default return key } diff --git a/x/bank/migrations/v044/store.go b/x/bank/migrations/v044/store.go index c6b23924e58..0ea07139835 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 } @@ -82,9 +82,9 @@ func migrateDenomMetadata(store sdk.KVStore) error { l := len(oldKey)/2 + 1 var newKey = make([]byte, len(types.DenomMetadataPrefix)+l) - copy(newKey, types.DenomMetadataPrefix) // old key: prefix_bytes | denom_bytes | denom_bytes - newKey = append(newKey, oldKey[:l]...) + copy(newKey, types.DenomMetadataPrefix) + copy(newKey[len(types.DenomMetadataPrefix):], oldKey[:l]) store.Set(newKey, oldDenomMetaDataIter.Value()) oldDenomMetaDataStore.Delete(oldKey) } diff --git a/x/bank/migrations/v044/store_test.go b/x/bank/migrations/v044/store_test.go index 37c268d3d70..e60fe4e231c 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)