Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: remove denom from DenomMetadata key #9890

Merged
merged 7 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/bank) [\#9611](https://github.com/cosmos/cosmos-sdk/pull/9611) Introduce a new index to act as a reverse index between a denomination and address allowing to query for
token holders of a specific denomination. `DenomOwners` is updated to use the new reverse index.
* (x/bank) [\#9832] (https://github.com/cosmos/cosmos-sdk/pull/9832) Account balance is stored as `sdk.Int` rather than `sdk.Coin`.
* (x/bank) [\#9890] (https://github.com/cosmos/cosmos-sdk/pull/9890) Remove duplicate denom from denom metadata key.

## [v0.43.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0) - 2021-08-10

Expand Down
4 changes: 2 additions & 2 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (k BaseKeeper) GetSupply(ctx sdk.Context, denom string) sdk.Coin {
// false otherwise.
func (k BaseKeeper) GetDenomMetaData(ctx sdk.Context, denom string) (types.Metadata, bool) {
store := ctx.KVStore(k.storeKey)
store = prefix.NewStore(store, types.DenomMetadataKey(denom))
store = prefix.NewStore(store, types.DenomMetadataPrefix)

bz := store.Get([]byte(denom))
if bz == nil {
Expand Down Expand Up @@ -265,7 +265,7 @@ func (k BaseKeeper) IterateAllDenomMetaData(ctx sdk.Context, cb func(types.Metad
// SetDenomMetaData sets the denominations metadata
func (k BaseKeeper) SetDenomMetaData(ctx sdk.Context, denomMetaData types.Metadata) {
store := ctx.KVStore(k.storeKey)
denomMetaDataStore := prefix.NewStore(store, types.DenomMetadataKey(denomMetaData.Base))
denomMetaDataStore := prefix.NewStore(store, types.DenomMetadataPrefix)

m := k.cdc.MustMarshal(&denomMetaData)
denomMetaDataStore.Set([]byte(denomMetaData.Base), m)
Expand Down
11 changes: 9 additions & 2 deletions x/bank/migrations/v043/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ const (
)

var (
SupplyKey = []byte{0x00}
BalancesPrefix = []byte{0x02}
SupplyKey = []byte{0x00}
BalancesPrefix = []byte{0x02}
DenomMetadataPrefix = []byte{0x1}

ErrInvalidKey = errors.New("invalid key")
)
Expand All @@ -36,3 +37,9 @@ func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) {

return key[1 : bound+1], nil
}

// DenomMetadataKey returns the denomination metadata key.
func DenomMetadataKey(denom string) []byte {
d := []byte(denom)
return append(DenomMetadataPrefix, d...)
}
26 changes: 25 additions & 1 deletion x/bank/migrations/v044/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@ import (
//
// - 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)
return addDenomReverseIndex(store, cdc)
err := addDenomReverseIndex(store, cdc)
if err != nil {
return err
}

return migrateDenomMetadata(store)
}

func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec) error {
Expand Down Expand Up @@ -64,3 +70,21 @@ func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec) error {

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()
// old key: prefix_bytes | denom_bytes | denom_bytes
newKey := append(types.DenomMetadataPrefix, oldKey[:len(oldKey)/2+1]...)

store.Set(newKey, oldDenomMetaDataIter.Value())
oldDenomMetaDataStore.Delete(oldDenomMetaDataIter.Key())
}

return nil
}
74 changes: 74 additions & 0 deletions x/bank/migrations/v044/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,77 @@ func TestMigrateStore(t *testing.T) {
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)...)
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)
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())
}
6 changes: 0 additions & 6 deletions x/bank/types/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ var (
BalancesPrefix = []byte{0x02}
)

// DenomMetadataKey returns the denomination metadata key.
func DenomMetadataKey(denom string) []byte {
d := []byte(denom)
return append(DenomMetadataPrefix, d...)
}

// AddressAndDenomFromBalancesStore returns an account address and denom from a balances prefix
// store. The key must not contain the prefix BalancesPrefix as the prefix store
// iterator discards the actual prefix.
Expand Down