Skip to content

Commit

Permalink
x/bank: create reverse prefix for denom<->address (#9611)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderbez authored Jul 26, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent c061cc1 commit 9ea1fee
Showing 17 changed files with 248 additions and 47 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -87,6 +87,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/genutil) [\#9638](https://github.com/cosmos/cosmos-sdk/pull/9638) Added missing validator key save when recovering from mnemonic
* (server) [#9704](https://github.com/cosmos/cosmos-sdk/pull/9704) Start GRPCWebServer in goroutine, avoid blocking other services from starting.

### State Machine Breaking

* (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.

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

### Features
7 changes: 6 additions & 1 deletion simapp/app_test.go
Original file line number Diff line number Diff line change
@@ -123,8 +123,13 @@ func TestRunMigrations(t *testing.T) {
false, "", true, "no migrations found for module bank: not found", 0,
},
{
"can register and run migration handler for x/bank",
"can register 1->2 migration handler for x/bank, cannot run migration",
"bank", 1,
false, "", true, "no migration found for module bank from version 2 to version 3: not found", 0,
},
{
"can register 2->3 migration handler for x/bank, can run migration",
"bank", 2,
false, "", false, "", 1,
},
{
24 changes: 14 additions & 10 deletions x/auth/client/testutil/suite.go
Original file line number Diff line number Diff line change
@@ -1045,12 +1045,11 @@ func (s *IntegrationTestSuite) TestTxWithoutPublicKey() {
s.Require().NotEqual(0, res.Code)
}

// TestSignWithMultiSignersAminoJSON tests the case where a transaction with 2
// messages which has to be signed with 2 different keys. Sign and append the
// signatures using the CLI with Amino signing mode. Finally, send the
// transaction to the blockchain.
func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() {
// test case:
// Create a transaction with 2 messages which has to be signed with 2 different keys
// Sign and append the signatures using the CLI with Amino signing mode.
// Finally send the transaction to the blockchain. It should work.

require := s.Require()
val0, val1 := s.network.Validators[0], s.network.Validators[1]
val0Coin := sdk.NewCoin(fmt.Sprintf("%stoken", val0.Moniker), sdk.NewInt(10))
@@ -1067,7 +1066,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() {
banktypes.NewMsgSend(val1.Address, addr1, sdk.NewCoins(val1Coin)),
)
txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))))
txBuilder.SetGasLimit(testdata.NewTestGasLimit()) // min required is 101892
txBuilder.SetGasLimit(testdata.NewTestGasLimit() * 2)
require.Equal([]sdk.AccAddress{val0.Address, val1.Address}, txBuilder.GetTx().GetSigners())

// Write the unsigned tx into a file.
@@ -1083,14 +1082,19 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() {
// Then let val1 sign the file with signedByVal0.
val1AccNum, val1Seq, err := val0.ClientCtx.AccountRetriever.GetAccountNumberSequence(val0.ClientCtx, val1.Address)
require.NoError(err)

signedTx, err := TxSignExec(
val1.ClientCtx, val1.Address, signedByVal0File.Name(),
"--offline", fmt.Sprintf("--account-number=%d", val1AccNum), fmt.Sprintf("--sequence=%d", val1Seq), "--sign-mode=amino-json",
val1.ClientCtx,
val1.Address,
signedByVal0File.Name(),
"--offline",
fmt.Sprintf("--account-number=%d", val1AccNum),
fmt.Sprintf("--sequence=%d", val1Seq),
"--sign-mode=amino-json",
)
require.NoError(err)
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String())

// Now let's try to send this tx.
res, err := TxBroadcastExec(
val0.ClientCtx,
signedTxFile.Name(),
@@ -1100,7 +1104,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() {
require.NoError(err)
var txRes sdk.TxResponse
require.NoError(val0.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes))
require.Equal(uint32(0), txRes.Code)
require.Equal(uint32(0), txRes.Code, txRes.RawLog)

// Make sure the addr1's balance got funded.
queryResJSON, err := bankcli.QueryBalancesExec(val0.ClientCtx, addr1)
2 changes: 1 addition & 1 deletion x/bank/keeper/genesis.go
Original file line number Diff line number Diff line change
@@ -13,8 +13,8 @@ func (k BaseKeeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
k.SetParams(ctx, genState.Params)

totalSupply := sdk.Coins{}

genState.Balances = types.SanitizeGenesisBalances(genState.Balances)

for _, balance := range genState.Balances {
addr, err := sdk.AccAddressFromBech32(balance.Address)
if err != nil {
17 changes: 3 additions & 14 deletions x/bank/keeper/grpc_query.go
Original file line number Diff line number Diff line change
@@ -179,24 +179,13 @@ func (k BaseKeeper) DenomOwners(
}

ctx := sdk.UnwrapSDKContext(goCtx)

store := ctx.KVStore(k.storeKey)
balancesStore := prefix.NewStore(store, types.BalancesPrefix)
denomPrefixStore := k.getDenomAddressPrefixStore(ctx, req.Denom)

var denomOwners []*types.DenomOwner
pageRes, err := query.FilteredPaginate(
balancesStore,
denomPrefixStore,
req.Pagination,
func(key []byte, value []byte, accumulate bool) (bool, error) {
var balance sdk.Coin
if err := k.cdc.Unmarshal(value, &balance); err != nil {
return false, err
}

if req.Denom != balance.Denom {
return false, nil
}

if accumulate {
address, err := types.AddressFromBalancesStore(key)
if err != nil {
@@ -207,7 +196,7 @@ func (k BaseKeeper) DenomOwners(
denomOwners,
&types.DenomOwner{
Address: address.String(),
Balance: balance,
Balance: k.GetBalance(ctx, address, req.Denom),
},
)
}
6 changes: 6 additions & 0 deletions x/bank/keeper/migrations.go
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ package keeper
import (
sdk "github.com/cosmos/cosmos-sdk/types"
v043 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v043"
v044 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v044"
)

// Migrator is a struct for handling in-place store migrations.
@@ -19,3 +20,8 @@ func NewMigrator(keeper BaseKeeper) Migrator {
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
return v043.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
}

// Migrate2to3 migrates x/bank storage from version 2 to 3.
func (m Migrator) Migrate2to3(ctx sdk.Context) error {
return v044.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
}
30 changes: 28 additions & 2 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
@@ -2,8 +2,10 @@ package keeper

import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/bank/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
@@ -231,16 +233,31 @@ func (k BaseSendKeeper) addCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.C
// An error is returned upon failure.
func (k BaseSendKeeper) initBalances(ctx sdk.Context, addr sdk.AccAddress, balances sdk.Coins) error {
accountStore := k.getAccountStore(ctx, addr)
denomPrefixStores := make(map[string]prefix.Store) // memoize prefix stores

for i := range balances {
balance := balances[i]
if !balance.IsValid() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, balance.String())
}

// Bank invariants require to not store zero balances.
// x/bank invariants prohibit persistence of zero balances
if !balance.IsZero() {
bz := k.cdc.MustMarshal(&balance)
accountStore.Set([]byte(balance.Denom), bz)

denomPrefixStore, ok := denomPrefixStores[balance.Denom]
if !ok {
denomPrefixStore = k.getDenomAddressPrefixStore(ctx, balance.Denom)
denomPrefixStores[balance.Denom] = denomPrefixStore
}

// Store a reverse index from denomination to account address with a
// sentinel value.
denomAddrKey := address.MustLengthPrefix(addr)
if !denomPrefixStore.Has(denomAddrKey) {
denomPrefixStore.Set(denomAddrKey, []byte{0})
}
}
}

@@ -254,13 +271,22 @@ func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance
}

accountStore := k.getAccountStore(ctx, addr)
denomPrefixStore := k.getDenomAddressPrefixStore(ctx, balance.Denom)

// Bank invariants require to not store zero balances.
// x/bank invariants prohibit persistence of zero balances
if balance.IsZero() {
accountStore.Delete([]byte(balance.Denom))
denomPrefixStore.Delete(address.MustLengthPrefix(addr))
} else {
bz := k.cdc.MustMarshal(&balance)
accountStore.Set([]byte(balance.Denom), bz)

// Store a reverse index from denomination to account address with a
// sentinel value.
denomAddrKey := address.MustLengthPrefix(addr)
if !denomPrefixStore.Has(denomAddrKey) {
denomPrefixStore.Set(denomAddrKey, []byte{0})
}
}

return nil
7 changes: 6 additions & 1 deletion x/bank/keeper/view.go
Original file line number Diff line number Diff line change
@@ -228,6 +228,11 @@ func (k BaseViewKeeper) ValidateBalance(ctx sdk.Context, addr sdk.AccAddress) er
// getAccountStore gets the account store of the given address.
func (k BaseViewKeeper) getAccountStore(ctx sdk.Context, addr sdk.AccAddress) prefix.Store {
store := ctx.KVStore(k.storeKey)

return prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr))
}

// getDenomAddressPrefixStore returns a prefix store that acts as a reverse index
// between a denomination and account balance for that denomination.
func (k BaseViewKeeper) getDenomAddressPrefixStore(ctx sdk.Context, denom string) prefix.Store {
return prefix.NewStore(ctx.KVStore(k.storeKey), types.CreateDenomAddressPrefix(denom))
}
32 changes: 29 additions & 3 deletions x/bank/migrations/v043/keys.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,38 @@
package v043

import (
"errors"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
)

const (
// ModuleName is the name of the module
ModuleName = "bank"
)

// KVStore keys
var (
BalancesPrefix = []byte{0x02}
SupplyKey = []byte{0x00}
BalancesPrefix = []byte{0x02}

ErrInvalidKey = errors.New("invalid key")
)

func CreateAccountBalancesPrefix(addr []byte) []byte {
return append(BalancesPrefix, address.MustLengthPrefix(addr)...)
}

func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) {
if len(key) == 0 {
return nil, ErrInvalidKey
}

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

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

return key[1 : bound+1], nil
}
4 changes: 2 additions & 2 deletions x/bank/migrations/v043/store.go
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ func migrateSupply(store sdk.KVStore, cdc codec.BinaryCodec) error {
}

// We add a new key for each denom
supplyStore := prefix.NewStore(store, types.SupplyKey)
supplyStore := prefix.NewStore(store, SupplyKey)

// We're sure that SupplyI is a Supply struct, there's no other
// implementation.
@@ -61,7 +61,7 @@ func migrateBalanceKeys(store sdk.KVStore) {
for ; oldStoreIter.Valid(); oldStoreIter.Next() {
addr := v040bank.AddressFromBalancesStore(oldStoreIter.Key())
denom := oldStoreIter.Key()[v040auth.AddrLen:]
newStoreKey := append(types.CreateAccountBalancesPrefix(addr), denom...)
newStoreKey := append(CreateAccountBalancesPrefix(addr), denom...)

// Set new key on store. Values don't change.
store.Set(newStoreKey, oldStoreIter.Value())
10 changes: 10 additions & 0 deletions x/bank/migrations/v044/keys.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package v044

var (
DenomAddressPrefix = []byte{0x03}
)

func CreateAddressDenomPrefix(denom string) []byte {
key := append(DenomAddressPrefix, []byte(denom)...)
return append(key, 0)
}
51 changes: 51 additions & 0 deletions x/bank/migrations/v044/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
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"
)

// MigrateStore performs in-place store migrations from v0.43 to v0.44. The
// migration includes:
//
// - 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)
return addDenomReverseIndex(store, cdc)
}

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
}

denomPrefixStore, ok := denomPrefixStores[balance.Denom]
if !ok {
denomPrefixStore = prefix.NewStore(store, CreateAddressDenomPrefix(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
}
45 changes: 45 additions & 0 deletions x/bank/migrations/v044/store_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
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"
)

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 {
denomPrefixStore := prefix.NewStore(store, v044.CreateAddressDenomPrefix(b.Denom))
bz := denomPrefixStore.Get(address.MustLengthPrefix(addr))
require.NotNil(t, bz)
}
}
Loading

0 comments on commit 9ea1fee

Please sign in to comment.