Skip to content

Commit

Permalink
feat!: change Coin storage model (#9832)
Browse files Browse the repository at this point in the history
<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Closes: #9361 

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
aleem1314 committed Aug 5, 2021
1 parent bdf5aee commit eb79dd0
Show file tree
Hide file tree
Showing 12 changed files with 83 additions and 47 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#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.
* (x/capability) [\#9836](https://github.com/cosmos/cosmos-sdk/pull/9836) Removed `InitializeAndSeal(ctx sdk.Context)` and replaced with `Seal()`. App must add x/capability to begin blockers which will assure that the x/capability keeper is properly initialized. The x/capability begin blocker must be run before any other module which uses x/capability.
* (x/bank) [\#9832] (https://github.com/cosmos/cosmos-sdk/pull/9832) `AddressFromBalancesStore` renamed to `AddressAndDenomFromBalancesStore`.

### Client Breaking Changes

Expand Down Expand Up @@ -101,6 +102,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`.

## [v0.43.0-rc0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc0) - 2021-06-25

Expand Down
2 changes: 1 addition & 1 deletion contrib/rosetta/configuration/bootstrap.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[
{
"account_identifier": {
"address":"cosmos1kezmr2chzy7w00nhh7qxhpqphdwum3j0mgdaw0"
"address":"cosmos1y3awd3vl7g29q44uvz0yrevcduf2exvkwxk3uq"
},
"currency":{
"symbol":"stake",
Expand Down
Binary file modified contrib/rosetta/node/data.tar.gz
Binary file not shown.
22 changes: 11 additions & 11 deletions types/query/filtered_pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (s *paginationTestSuite) TestReverseFilteredPaginations() {
}

func ExampleFilteredPaginate() {
app, ctx, appCodec := setupTest()
app, ctx, _ := setupTest()

var balances sdk.Coins
for i := 0; i < numBalances; i++ {
Expand Down Expand Up @@ -200,16 +200,16 @@ func ExampleFilteredPaginate() {

var balResult sdk.Coins
pageRes, err := query.FilteredPaginate(accountStore, pageReq, func(key []byte, value []byte, accumulate bool) (bool, error) {
var bal sdk.Coin
err := appCodec.Unmarshal(value, &bal)
var amount sdk.Int
err := amount.Unmarshal(value)
if err != nil {
return false, err
}

// filter balances with amount greater than 100
if bal.Amount.Int64() > int64(100) {
// filter amount greater than 100
if amount.Int64() > int64(100) {
if accumulate {
balResult = append(balResult, bal)
balResult = append(balResult, sdk.NewCoin(string(key), amount))
}

return true, nil
Expand All @@ -232,16 +232,16 @@ func execFilterPaginate(store sdk.KVStore, pageReq *query.PageRequest, appCodec

var balResult sdk.Coins
res, err = query.FilteredPaginate(accountStore, pageReq, func(key []byte, value []byte, accumulate bool) (bool, error) {
var bal sdk.Coin
err := appCodec.Unmarshal(value, &bal)
var amount sdk.Int
err := amount.Unmarshal(value)
if err != nil {
return false, err
}

// filter balances with amount greater than 100
if bal.Amount.Int64() > int64(100) {
// filter amount greater than 100
if amount.Int64() > int64(100) {
if accumulate {
balResult = append(balResult, bal)
balResult = append(balResult, sdk.NewCoin(string(key), amount))
}

return true, nil
Expand Down
6 changes: 3 additions & 3 deletions types/query/pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,12 +319,12 @@ func ExamplePaginate() {
balancesStore := prefix.NewStore(authStore, types.BalancesPrefix)
accountStore := prefix.NewStore(balancesStore, address.MustLengthPrefix(addr1))
pageRes, err := query.Paginate(accountStore, request.Pagination, func(key []byte, value []byte) error {
var tempRes sdk.Coin
err := app.AppCodec().Unmarshal(value, &tempRes)
var amount sdk.Int
err := amount.Unmarshal(value)
if err != nil {
return err
}
balResult = append(balResult, tempRes)
balResult = append(balResult, sdk.NewCoin(string(key), amount))
return nil
})
if err != nil { // should return no error
Expand Down
11 changes: 5 additions & 6 deletions x/bank/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,12 @@ func (k BaseKeeper) AllBalances(ctx context.Context, req *types.QueryAllBalances
balances := sdk.NewCoins()
accountStore := k.getAccountStore(sdkCtx, addr)

pageRes, err := query.Paginate(accountStore, req.Pagination, func(_, value []byte) error {
var result sdk.Coin
err := k.cdc.Unmarshal(value, &result)
if err != nil {
pageRes, err := query.Paginate(accountStore, req.Pagination, func(key, value []byte) error {
var amount sdk.Int
if err := amount.Unmarshal(value); err != nil {
return err
}
balances = append(balances, result)
balances = append(balances, sdk.NewCoin(string(key), amount))
return nil
})

Expand Down Expand Up @@ -187,7 +186,7 @@ func (k BaseKeeper) DenomOwners(
req.Pagination,
func(key []byte, value []byte, accumulate bool) (bool, error) {
if accumulate {
address, err := types.AddressFromBalancesStore(key)
address, _, err := types.AddressAndDenomFromBalancesStore(key)
if err != nil {
return false, err
}
Expand Down
14 changes: 10 additions & 4 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,11 @@ func (k BaseSendKeeper) initBalances(ctx sdk.Context, addr sdk.AccAddress, balan

// x/bank invariants prohibit persistence of zero balances
if !balance.IsZero() {
bz := k.cdc.MustMarshal(&balance)
accountStore.Set([]byte(balance.Denom), bz)
amount, err := balance.Amount.Marshal()
if err != nil {
return err
}
accountStore.Set([]byte(balance.Denom), amount)

denomPrefixStore, ok := denomPrefixStores[balance.Denom]
if !ok {
Expand Down Expand Up @@ -278,8 +281,11 @@ func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance
accountStore.Delete([]byte(balance.Denom))
denomPrefixStore.Delete(address.MustLengthPrefix(addr))
} else {
bz := k.cdc.MustMarshal(&balance)
accountStore.Set([]byte(balance.Denom), bz)
amount, err := balance.Amount.Marshal()
if err != nil {
return err
}
accountStore.Set([]byte(balance.Denom), amount)

// Store a reverse index from denomination to account address with a
// sentinel value.
Expand Down
29 changes: 17 additions & 12 deletions x/bank/keeper/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,17 @@ func (k BaseViewKeeper) GetAccountsBalances(ctx sdk.Context) []types.Balance {
// by address.
func (k BaseViewKeeper) GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin {
accountStore := k.getAccountStore(ctx, addr)

amount := sdk.ZeroInt()
bz := accountStore.Get([]byte(denom))
if bz == nil {
return sdk.NewCoin(denom, sdk.ZeroInt())
return sdk.NewCoin(denom, amount)
}

var balance sdk.Coin
k.cdc.MustUnmarshal(bz, &balance)
if err := amount.Unmarshal(bz); err != nil {
panic(err)
}

return balance
return sdk.NewCoin(denom, amount)
}

// IterateAccountBalances iterates over the balances of a single account and
Expand All @@ -120,10 +121,12 @@ func (k BaseViewKeeper) IterateAccountBalances(ctx sdk.Context, addr sdk.AccAddr
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
var balance sdk.Coin
k.cdc.MustUnmarshal(iterator.Value(), &balance)
var amount sdk.Int
if err := amount.Unmarshal(iterator.Value()); err != nil {
panic(err)
}

if cb(balance) {
if cb(sdk.NewCoin(string(iterator.Key()), amount)) {
break
}
}
Expand All @@ -140,18 +143,20 @@ func (k BaseViewKeeper) IterateAllBalances(ctx sdk.Context, cb func(sdk.AccAddre
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
address, err := types.AddressFromBalancesStore(iterator.Key())
address, denom, err := types.AddressAndDenomFromBalancesStore(iterator.Key())
if err != nil {
k.Logger(ctx).With("key", iterator.Key(), "err", err).Error("failed to get address from balances store")
// TODO: revisit, for now, panic here to keep same behavior as in 0.42
// ref: https://github.com/cosmos/cosmos-sdk/issues/7409
panic(err)
}

var balance sdk.Coin
k.cdc.MustUnmarshal(iterator.Value(), &balance)
var amount sdk.Int
if err := amount.Unmarshal(iterator.Value()); err != nil {
panic(err)
}

if cb(address, balance) {
if cb(address, sdk.NewCoin(denom, amount)) {
break
}
}
Expand Down
15 changes: 15 additions & 0 deletions x/bank/migrations/v044/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import (
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.
func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, cdc codec.BinaryCodec) error {
store := ctx.KVStore(storeKey)
Expand All @@ -36,6 +38,19 @@ func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec) error {
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, CreateAddressDenomPrefix(balance.Denom))
Expand Down
9 changes: 9 additions & 0 deletions x/bank/migrations/v044/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"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) {
Expand All @@ -37,6 +38,14 @@ func TestMigrateStore(t *testing.T) {

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.CreateAddressDenomPrefix(b.Denom))
bz := denomPrefixStore.Get(address.MustLengthPrefix(addr))
Expand Down
17 changes: 8 additions & 9 deletions x/bank/types/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,25 @@ func DenomMetadataKey(denom string) []byte {
return append(DenomMetadataPrefix, d...)
}

// AddressFromBalancesStore returns an account address from a balances prefix
// 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.
//
// If invalid key is passed, AddressFromBalancesStore returns ErrInvalidKey.
func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) {
// If invalid key is passed, AddressAndDenomFromBalancesStore returns ErrInvalidKey.
func AddressAndDenomFromBalancesStore(key []byte) (sdk.AccAddress, string, error) {
if len(key) == 0 {
return nil, ErrInvalidKey
return nil, "", ErrInvalidKey
}

kv.AssertKeyAtLeastLength(key, 1)

addrLen := key[0]
bound := int(addrLen)
addrBound := int(key[0])

if len(key)-1 < bound {
return nil, ErrInvalidKey
if len(key)-1 < addrBound {
return nil, "", ErrInvalidKey
}

return key[1 : bound+1], nil
return key[1 : addrBound+1], string(key[addrBound+1:]), nil
}

// CreateAccountBalancesPrefix creates the prefix for an account's balances.
Expand Down
3 changes: 2 additions & 1 deletion x/bank/types/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestAddressFromBalancesStore(t *testing.T) {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
addr, err := types.AddressFromBalancesStore(tc.key)
addr, denom, err := types.AddressAndDenomFromBalancesStore(tc.key)
if tc.wantErr {
assert.Error(t, err)
assert.True(t, errors.Is(types.ErrInvalidKey, err))
Expand All @@ -51,6 +51,7 @@ func TestAddressFromBalancesStore(t *testing.T) {
}
if len(tc.expectedKey) > 0 {
assert.Equal(t, tc.expectedKey, addr)
assert.Equal(t, "stake", denom)
}
})
}
Expand Down

0 comments on commit eb79dd0

Please sign in to comment.