Skip to content

Commit

Permalink
fix: x/bank/044 migrateDenomMetadata (#10239)
Browse files Browse the repository at this point in the history
* fix: x/bank/044 migrateDenomMetadata

* adding changelog entry

* comment update

* fix tests

(cherry picked from commit 16a953c)

# 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
  • Loading branch information
robert-zaremba authored and mergify-bot committed Sep 28, 2021
1 parent 37f5069 commit 299ab46
Show file tree
Hide file tree
Showing 6 changed files with 336 additions and 0 deletions.
72 changes: 72 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand All @@ -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)

Expand Down
16 changes: 16 additions & 0 deletions 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
}
93 changes: 93 additions & 0 deletions 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
}
129 changes: 129 additions & 0 deletions 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())
}
14 changes: 14 additions & 0 deletions x/bank/types/key.go
Expand Up @@ -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))
12 changes: 12 additions & 0 deletions x/bank/types/key_test.go
Expand Up @@ -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)
}

0 comments on commit 299ab46

Please sign in to comment.