From 299ab465f3a3bdcc10d2cee846b11f8f452ba93d 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 (cherry picked from commit 16a953cc97475ccd1cbf631055479abc81864cbe) # Conflicts: # CHANGELOG.md # x/bank/migrations/v044/keys.go # x/bank/migrations/v044/store.go # x/bank/migrations/v044/store_test.go # x/bank/types/key.go --- CHANGELOG.md | 72 +++++++++++++++ x/bank/migrations/v044/keys.go | 16 ++++ x/bank/migrations/v044/store.go | 93 +++++++++++++++++++ x/bank/migrations/v044/store_test.go | 129 +++++++++++++++++++++++++++ x/bank/types/key.go | 14 +++ x/bank/types/key_test.go | 12 +++ 6 files changed, 336 insertions(+) create mode 100644 x/bank/migrations/v044/keys.go create mode 100644 x/bank/migrations/v044/store.go create mode 100644 x/bank/migrations/v044/store_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ed748930b0e..d4be206dd16d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,9 +46,57 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +<<<<<<< HEAD * [\#9969](https://github.com/cosmos/cosmos-sdk/pull/9969) fix: use keyring in config for add-genesis-account cmd. * (x/genutil) [#10104](https://github.com/cosmos/cosmos-sdk/pull/10104) Ensure the `init` command reads the `--home` flag value correctly. * (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. +======= +* [\#9695](https://github.com/cosmos/cosmos-sdk/pull/9695) Migrate keys from `Info` -> `Record` + * Add new `codec.Codec` argument in: + * `keyring.NewInMemory` + * `keyring.New` + * Rename: + * `SavePubKey` to `SaveOfflineKey`. + * `NewMultiInfo`, `NewLedgerInfo` to `NewLegacyMultiInfo`, `newLegacyLedgerInfo` respectively. Move them into `legacy_info.go`. + * `NewOfflineInfo` to `newLegacyOfflineInfo` and move it to `migration_test.go`. + * Return: + *`keyring.Record, error` in `SaveOfflineKey`, `SaveLedgerKey`, `SaveMultiSig`, `Key` and `KeyByAddress`. + *`keyring.Record` instead of `Info` in `NewMnemonic` and `List`. + * Remove `algo` argument from : + * `SaveOfflineKey` + * Take `keyring.Record` instead of `Info` as first argument in: + * `MkConsKeyOutput` + * `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`. +* [\#9759](https://github.com/cosmos/cosmos-sdk/pull/9759) `NewAccountKeeeper` in `x/auth` now takes an additional `bech32Prefix` argument that represents `sdk.Bech32MainPrefix`. +* [\#9628](https://github.com/cosmos/cosmos-sdk/pull/9628) Rename `x/{mod}/legacy` to `x/{mod}/migrations`. +* [\#9571](https://github.com/cosmos/cosmos-sdk/pull/9571) Implemented error handling for staking hooks, which now return an error on failure. +* [\#9427](https://github.com/cosmos/cosmos-sdk/pull/9427) Move simapp `FundAccount` and `FundModuleAccount` to `x/bank/testutil` +* (client/tx) [\#9421](https://github.com/cosmos/cosmos-sdk/pull/9421/) `BuildUnsignedTx`, `BuildSimTx`, `PrintUnsignedStdTx` functions are moved to + the Tx Factory as methods. +* (client/keys) [\#9407](https://github.com/cosmos/cosmos-sdk/pull/9601) Added `keys rename` CLI command and `Keyring.Rename` interface method to rename a key in the keyring. +* (x/slashing) [\#9458](https://github.com/cosmos/cosmos-sdk/pull/9458) Coins burned from slashing is now returned from Slash function and included in Slash event. +* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error. +* [\#9519](https://github.com/cosmos/cosmos-sdk/pull/9519) `DeleteDeposits` renamed to `DeleteAndBurnDeposits`, `RefundDeposits` renamed to `RefundAndDeleteDeposits` +* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`. +* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Rename `EncodingConfig.Marshaler` to `Codec`. +* [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) `RESTHandlerFn` argument is removed from the `gov/NewProposalHandler`. +* [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) `types/rest` package moved to `testutil/rest`. +* [\#9432](https://github.com/cosmos/cosmos-sdk/pull/9432) `ConsensusParamsKeyTable` moved from `params/keeper` to `params/types` +* [\#9576](https://github.com/cosmos/cosmos-sdk/pull/9576) Add debug error message to `sdkerrors.QueryResult` when enabled +* [\#9650](https://github.com/cosmos/cosmos-sdk/pull/9650) Removed deprecated message handler implementation from the SDK modules. +* [\#10248](https://github.com/cosmos/cosmos-sdk/pull/10248) Remove unused `KeyPowerReduction` variable from x/staking types. +* (x/bank) [\#9832] (https://github.com/cosmos/cosmos-sdk/pull/9832) `AddressFromBalancesStore` renamed to `AddressAndDenomFromBalancesStore`. +* (tests) [\#9938](https://github.com/cosmos/cosmos-sdk/pull/9938) `simapp.Setup` accepts additional `testing.T` argument. +* (baseapp) [\#9920](https://github.com/cosmos/cosmos-sdk/pull/9920) BaseApp `{Check,Deliver,Simulate}Tx` methods are now replaced by a middleware stack. + * Replace the Antehandler interface with the `tx.Handler` and `tx.Middleware` interfaces. + * Replace `baseapp.SetAnteHandler` with `baseapp.SetTxHandler`. + * Move Msg routers from BaseApp to middlewares. + * Move Baseapp panic recovery into a middleware. + * Rename simulation helper methods `baseapp.{Check,Deliver}` to `baseapp.Sim{Check,Deliver}`. +>>>>>>> 16a953cc9 (fix: x/bank/044 migrateDenomMetadata (#10239)) ### Client Breaking Changes @@ -64,7 +112,31 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (deps) [\#9956](https://github.com/cosmos/cosmos-sdk/pull/9956) Bump Tendermint to [v0.34.12](https://github.com/tendermint/tendermint/releases/tag/v0.34.12). +<<<<<<< HEAD ### Deprecated +======= +### Bug Fixes +* [#10180](https://github.com/cosmos/cosmos-sdk/issues/10180) Documentation: make references to Cosmos SDK consistent +* (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking. +* (x/genutil) [#10104](https://github.com/cosmos/cosmos-sdk/pull/10104) Ensure the `init` command reads the `--home` flag value correctly. +* [\#9651](https://github.com/cosmos/cosmos-sdk/pull/9651) Change inconsistent limit of `0` to `MaxUint64` on InfiniteGasMeter and add GasRemaining func to GasMeter. +* [\#9639](https://github.com/cosmos/cosmos-sdk/pull/9639) Check store keys length before accessing them by making sure that `key` is of length `m+1` (for `key[n:m]`) +* (types) [\#9627](https://github.com/cosmos/cosmos-sdk/pull/9627) Fix nil pointer panic on `NewBigIntFromInt` +* (x/genutil) [\#9574](https://github.com/cosmos/cosmos-sdk/pull/9575) Actually use the `gentx` client tx flags (like `--keyring-dir`) +* (x/distribution) [\#9599](https://github.com/cosmos/cosmos-sdk/pull/9599) Withdraw rewards event now includes a value attribute even if there are 0 rewards (due to situations like 100% commission). +* (x/genutil) [\#9638](https://github.com/cosmos/cosmos-sdk/pull/9638) Added missing validator key save when recovering from mnemonic +* [\#9762](https://github.com/cosmos/cosmos-sdk/pull/9762) The init command uses the chain-id from the client config if --chain-id is not provided +* [\#9854](https://github.com/cosmos/cosmos-sdk/pull/9854) Fixed the `make proto-gen` to get dynamic container name based on project name for the cosmos based sdks. +* [\#9829](https://github.com/cosmos/cosmos-sdk/pull/9829) Fixed Coin denom sorting not being checked during `Balance.Validate` check. Refactored the Validation logic to use `Coins.Validate` for `Balance.Coins`. ++ [\#9965](https://github.com/cosmos/cosmos-sdk/pull/9965) Fixed `simd version` command output to report the right release tag. ++ [\#9980](https://github.com/cosmos/cosmos-sdk/pull/9980) Returning the error when the invalid argument is passed to bank query total supply cli. ++ [\#10061](https://github.com/cosmos/cosmos-sdk/pull/10061) Ensure that `LegacyAminoPubKey` struct correctly unmarshals from JSON +* (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 +>>>>>>> 16a953cc9 (fix: x/bank/044 migrateDenomMetadata (#10239)) * (x/upgrade) [\#9906](https://github.com/cosmos/cosmos-sdk/pull/9906) Deprecate `UpgradeConsensusState` gRPC query since this functionality is only used for IBC, which now has its own [IBC replacement](https://github.com/cosmos/ibc-go/blob/2c880a22e9f9cc75f62b527ca94aa75ce1106001/proto/ibc/core/client/v1/query.proto#L54) diff --git a/x/bank/migrations/v044/keys.go b/x/bank/migrations/v044/keys.go new file mode 100644 index 000000000000..d5a25393e635 --- /dev/null +++ b/x/bank/migrations/v044/keys.go @@ -0,0 +1,16 @@ +package v044 + +var ( + DenomAddressPrefix = []byte{0x03} +) + +// 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 new file mode 100644 index 000000000000..0ea071398352 --- /dev/null +++ b/x/bank/migrations/v044/store.go @@ -0,0 +1,93 @@ +package v044 + +import ( + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/store/prefix" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" + v043 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v043" + "github.com/cosmos/cosmos-sdk/x/bank/types" +) + +// MigrateStore performs in-place store migrations from v0.43 to v0.44. 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 sdk.StoreKey, cdc codec.BinaryCodec) error { + store := ctx.KVStore(storeKey) + err := addDenomReverseIndex(store, cdc) + if err != nil { + return err + } + + return migrateDenomMetadata(store) +} + +func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec) error { + oldBalancesStore := prefix.NewStore(store, v043.BalancesPrefix) + + oldBalancesIter := oldBalancesStore.Iterator(nil, nil) + defer 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 := v043.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) 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 + + 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(oldKey) + } + + return nil +} diff --git a/x/bank/migrations/v044/store_test.go b/x/bank/migrations/v044/store_test.go new file mode 100644 index 000000000000..e60fe4e231cf --- /dev/null +++ b/x/bank/migrations/v044/store_test.go @@ -0,0 +1,129 @@ +package v044_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/simapp" + "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" + v043 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v043" + v044 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v044" + "github.com/cosmos/cosmos-sdk/x/bank/types" +) + +func TestMigrateStore(t *testing.T) { + encCfg := simapp.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, v043.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, v044.MigrateStore(ctx, bankKey, encCfg.Codec)) + + for _, b := range balances { + addrPrefixStore := prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr)) + bz := addrPrefixStore.Get([]byte(b.Denom)) + var expected sdk.Int + require.NoError(t, expected.Unmarshal(bz)) + require.Equal(t, expected, b.Amount) + } + + for _, b := range balances { + denomPrefixStore := prefix.NewStore(store, v044.CreateDenomAddressPrefix(b.Denom)) + bz := denomPrefixStore.Get(address.MustLengthPrefix(addr)) + require.NotNil(t, bz) + } +} + +func TestMigrateDenomMetaData(t *testing.T) { + encCfg := simapp.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{ + {"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} { + 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) + denomMetadataStore.Set(key, bz) + } + + require.NoError(t, v044.MigrateStore(ctx, bankKey, encCfg.Codec)) + + denomMetadataStore = prefix.NewStore(store, v043.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[1:]...) + bz := denomMetadataStore.Get(oldKey) + require.Nil(t, bz) + + 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) + 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()) +} diff --git a/x/bank/types/key.go b/x/bank/types/key.go index 5d7f52baa531..0e754fe9cf95 100644 --- a/x/bank/types/key.go +++ b/x/bank/types/key.go @@ -55,3 +55,17 @@ func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) { func CreateAccountBalancesPrefix(addr []byte) []byte { return append(BalancesPrefix, address.MustLengthPrefix(addr)...) } +<<<<<<< HEAD +======= + +// 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 +} +>>>>>>> 16a953cc9 (fix: x/bank/044 migrateDenomMetadata (#10239)) diff --git a/x/bank/types/key_test.go b/x/bank/types/key_test.go index 9a7f457e45bd..1baa99482366 100644 --- a/x/bank/types/key_test.go +++ b/x/bank/types/key_test.go @@ -55,3 +55,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) +}