Skip to content

Commit

Permalink
Refactor store keys for variable-length addresses (#8363)
Browse files Browse the repository at this point in the history
* Change account store key in x/bank

* Fix pagination test

* Fix merge master

* Fix staking keys.go

* Use bech32 in val state change map

* Fix sortNoLongerBonded

* Use length-prefix function

* Use length prefix function

* Fix test accountStore

* Fix ExamplePaginate

* Fix staking keys

* Use shorter balances prefix

* Do slashing keys

* Fix gov keys

* Fix x/gov tests

* Fix x/distrib

* Address reviews

* add change log entry

* Add changelog

* Fix failing tests

* Fix sim tests

* fix after-export sim

* Fix lint

* Address review

* Fix x/authz

* Fix global config in test

* Update x/staking/keeper/val_state_change.go

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Address comments

* Fix comments

* Address review

* Fix authz test

* Update comment

* Rename to LengthPrefixedAddressStoreKey

* Use variable

* Rename function

* Fix test build

* chore: update rosetta CI (#8453)

* Rename again

* Rename yet again

* Update feegrant keys

* Add function to create prefix

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Frojdi Dymylja <33157909+fdymylja@users.noreply.github.com>
Co-authored-by: Aaron Craelius <aaron@regen.network>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
5 people authored Feb 1, 2021
1 parent 17d7e9a commit c1b567f
Show file tree
Hide file tree
Showing 28 changed files with 411 additions and 271 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,13 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Client Breaking Changes

* [\#8363](https://github.com/cosmos/cosmos-sdk/issues/8363) Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address.

### State Machine Breaking

* (x/{bank,distrib,gov,slashing,staking}) [\#8363](https://github.com/cosmos/cosmos-sdk/issues/8363) Store keys have been modified to allow for variable-length addresses.
* (x/ibc) [\#8266](https://github.com/cosmos/cosmos-sdk/issues/8266) Add amino JSON for IBC messages in order to support Ledger text signing.

### Improvements
Expand Down
4 changes: 2 additions & 2 deletions contrib/rosetta/configuration/bootstrap.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
[
{
"account_identifier": {
"address":"cosmos1hdmjfmqmf8ck4pv4evu0s3up0ucm0yjjqfl87e"
"address":"cosmos158nkd0l9tyemv2crp579rmj8dg37qty8lzff88"
},
"currency":{
"symbol":"stake",
"decimals":0
},
"value": "999900000000"
"value": "999990000000"
}
]
5 changes: 3 additions & 2 deletions contrib/rosetta/configuration/data.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ simd init simd --chain-id testing
simd keys add fd --keyring-backend=test

addr=$(simd keys show fd -a --keyring-backend=test)
val_addr=$(simd keys show fd --keyring-backend=test --bech val -a)

# give the accounts some money
simd add-genesis-account "$addr" 1000000000000stake --keyring-backend=test

# save configs for the daemon
simd gentx fd --chain-id testing --keyring-backend=test
simd gentx fd 10000000stake --chain-id testing --keyring-backend=test

# input genTx to the genesis file
simd collect-gentxs
Expand Down Expand Up @@ -55,4 +56,4 @@ echo zipping data dir and saving to /tmp/data.tar.gz

tar -czvf /tmp/data.tar.gz /root/.simapp

echo new address for bootstrap.json "$addr"
echo new address for bootstrap.json "$addr" "$val_addr"
4 changes: 2 additions & 2 deletions contrib/rosetta/configuration/staking.ros
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ staking(1){
"account": {
"address": "staking_account",
"sub_account": {
"address" : "cosmosvaloper1hdmjfmqmf8ck4pv4evu0s3up0ucm0yjj9atjj2"
"address" : "cosmosvaloper158nkd0l9tyemv2crp579rmj8dg37qty86kaut5"
}
},
"amount":{
Expand Down Expand Up @@ -134,7 +134,7 @@ staking(1){
"account": {
"address": "staking_account",
"sub_account": {
"address" : "cosmosvaloper1hdmjfmqmf8ck4pv4evu0s3up0ucm0yjj9atjj2"
"address" : "cosmosvaloper158nkd0l9tyemv2crp579rmj8dg37qty86kaut5"
}
},
"amount":{
Expand Down
Binary file modified contrib/rosetta/node/data.tar.gz
Binary file not shown.
2 changes: 1 addition & 1 deletion simapp/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
counter := int16(0)

for ; iter.Valid(); iter.Next() {
addr := sdk.ValAddress(iter.Key()[1:])
addr := sdk.ValAddress(stakingtypes.AddressFromValidatorsKey(iter.Key()))
validator, found := app.StakingKeeper.GetValidator(ctx, addr)
if !found {
panic("expected validator, not found")
Expand Down
14 changes: 10 additions & 4 deletions types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (

"github.com/cosmos/cosmos-sdk/codec/legacy"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/types/address"
"github.com/cosmos/cosmos-sdk/types/bech32"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

const (
Expand All @@ -28,8 +30,6 @@ const (
// config.SetFullFundraiserPath(yourFullFundraiserPath)
// config.Seal()

// AddrLen defines a valid address length
AddrLen = 20
// Bech32MainPrefix defines the main SDK Bech32 prefix of an account's address
Bech32MainPrefix = "cosmos"

Expand Down Expand Up @@ -110,9 +110,15 @@ func VerifyAddressFormat(bz []byte) error {
if verifier != nil {
return verifier(bz)
}
if len(bz) != AddrLen {
return fmt.Errorf("incorrect address length (expected: %d, actual: %d)", AddrLen, len(bz))

if len(bz) == 0 {
return sdkerrors.Wrap(sdkerrors.ErrUnknownAddress, "addresses cannot be empty")
}

if len(bz) > address.MaxAddrLen {
return sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "address max length is %d, got %d", address.MaxAddrLen, len(bz))
}

return nil
}

Expand Down
33 changes: 33 additions & 0 deletions types/address/store_key.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package address

import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// MaxAddrLen is the maximum allowed length (in bytes) for an address.
const MaxAddrLen = 255

// LengthPrefix prefixes the address bytes with its length, this is used
// for example for variable-length components in store keys.
func LengthPrefix(bz []byte) ([]byte, error) {
bzLen := len(bz)
if bzLen == 0 {
return bz, nil
}

if bzLen > MaxAddrLen {
return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "address length should be max %d bytes, got %d", MaxAddrLen, bzLen)
}

return append([]byte{byte(bzLen)}, bz...), nil
}

// MustLengthPrefix is LengthPrefix with panic on error.
func MustLengthPrefix(bz []byte) []byte {
res, err := LengthPrefix(bz)
if err != nil {
panic(err)
}

return res
}
38 changes: 38 additions & 0 deletions types/address/store_key_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package address_test

import (
"testing"

"github.com/stretchr/testify/require"

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

func TestLengthPrefixedAddressStoreKey(t *testing.T) {
addr10byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
addr20byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}
addr256byte := make([]byte, 256)

tests := []struct {
name string
addr []byte
expStoreKey []byte
expErr bool
}{
{"10-byte address", addr10byte, append([]byte{byte(10)}, addr10byte...), false},
{"20-byte address", addr20byte, append([]byte{byte(20)}, addr20byte...), false},
{"256-byte address (too long)", addr256byte, nil, true},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
storeKey, err := address.LengthPrefix(tt.addr)
if tt.expErr {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, tt.expStoreKey, storeKey)
}
})
}
}
40 changes: 24 additions & 16 deletions types/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,15 +347,18 @@ func (s *addressTestSuite) TestVerifyAddressFormat() {
addr5 := make([]byte, 5)
addr20 := make([]byte, 20)
addr32 := make([]byte, 32)
addr256 := make([]byte, 256)

err := types.VerifyAddressFormat(addr0)
s.Require().EqualError(err, "incorrect address length 0")
s.Require().EqualError(err, "addresses cannot be empty: unknown address")
err = types.VerifyAddressFormat(addr5)
s.Require().EqualError(err, "incorrect address length 5")
s.Require().NoError(err)
err = types.VerifyAddressFormat(addr20)
s.Require().Nil(err)
s.Require().NoError(err)
err = types.VerifyAddressFormat(addr32)
s.Require().EqualError(err, "incorrect address length 32")
s.Require().NoError(err)
err = types.VerifyAddressFormat(addr256)
s.Require().EqualError(err, "address max length is 255, got 256: unknown address")
}

func (s *addressTestSuite) TestCustomAddressVerifier() {
Expand All @@ -364,34 +367,39 @@ func (s *addressTestSuite) TestCustomAddressVerifier() {
accBech := types.AccAddress(addr).String()
valBech := types.ValAddress(addr).String()
consBech := types.ConsAddress(addr).String()
// Verifiy that the default logic rejects this 10 byte address
// Verify that the default logic doesn't reject this 10 byte address
// The default verifier is nil, we're only checking address length is
// between 1-255 bytes.
err := types.VerifyAddressFormat(addr)
s.Require().NotNil(err)
s.Require().Nil(err)
_, err = types.AccAddressFromBech32(accBech)
s.Require().NotNil(err)
s.Require().Nil(err)
_, err = types.ValAddressFromBech32(valBech)
s.Require().NotNil(err)
s.Require().Nil(err)
_, err = types.ConsAddressFromBech32(consBech)
s.Require().NotNil(err)
s.Require().Nil(err)

// Set a custom address verifier that accepts 10 or 20 byte addresses
// Set a custom address verifier only accepts 20 byte addresses
types.GetConfig().SetAddressVerifier(func(bz []byte) error {
n := len(bz)
if n == 10 || n == types.AddrLen {
if n == 20 {
return nil
}
return fmt.Errorf("incorrect address length %d", n)
})

// Verifiy that the custom logic accepts this 10 byte address
// Verifiy that the custom logic rejects this 10 byte address
err = types.VerifyAddressFormat(addr)
s.Require().Nil(err)
s.Require().NotNil(err)
_, err = types.AccAddressFromBech32(accBech)
s.Require().Nil(err)
s.Require().NotNil(err)
_, err = types.ValAddressFromBech32(valBech)
s.Require().Nil(err)
s.Require().NotNil(err)
_, err = types.ConsAddressFromBech32(consBech)
s.Require().Nil(err)
s.Require().NotNil(err)

// Reinitialize the global config to default address verifier (nil)
types.GetConfig().SetAddressVerifier(nil)
}

func (s *addressTestSuite) TestBech32ifyAddressBytes() {
Expand Down
5 changes: 3 additions & 2 deletions types/query/filtered_pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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"
"github.com/cosmos/cosmos-sdk/types/query"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/cosmos-sdk/x/bank/types"
Expand Down Expand Up @@ -111,7 +112,7 @@ func ExampleFilteredPaginate() {
pageReq := &query.PageRequest{Key: nil, Limit: 1, CountTotal: true}
store := ctx.KVStore(app.GetKey(authtypes.StoreKey))
balancesStore := prefix.NewStore(store, types.BalancesPrefix)
accountStore := prefix.NewStore(balancesStore, addr1.Bytes())
accountStore := prefix.NewStore(balancesStore, address.MustLengthPrefix(addr1))

var balResult sdk.Coins
pageRes, err := query.FilteredPaginate(accountStore, pageReq, func(key []byte, value []byte, accumulate bool) (bool, error) {
Expand Down Expand Up @@ -143,7 +144,7 @@ func ExampleFilteredPaginate() {

func execFilterPaginate(store sdk.KVStore, pageReq *query.PageRequest, appCodec codec.Marshaler) (balances sdk.Coins, res *query.PageResponse, err error) {
balancesStore := prefix.NewStore(store, types.BalancesPrefix)
accountStore := prefix.NewStore(balancesStore, addr1.Bytes())
accountStore := prefix.NewStore(balancesStore, address.MustLengthPrefix(addr1))

var balResult sdk.Coins
res, err = query.FilteredPaginate(accountStore, pageReq, func(key []byte, value []byte, accumulate bool) (bool, error) {
Expand Down
3 changes: 2 additions & 1 deletion types/query/pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/cosmos/cosmos-sdk/store"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
"github.com/cosmos/cosmos-sdk/types/query"
authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand Down Expand Up @@ -193,7 +194,7 @@ func ExamplePaginate() {
balResult := sdk.NewCoins()
authStore := ctx.KVStore(app.GetKey(authtypes.StoreKey))
balancesStore := prefix.NewStore(authStore, types.BalancesPrefix)
accountStore := prefix.NewStore(balancesStore, addr1.Bytes())
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().UnmarshalBinaryBare(value, &tempRes)
Expand Down
19 changes: 15 additions & 4 deletions x/authz/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

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

const (
Expand All @@ -21,7 +22,7 @@ const (
// Keys for authz store
// Items are stored with the following key: values
//
// - 0x01<granterAddress_Bytes><granteeAddress_Bytes><msgType_Bytes>: Grant
// - 0x01<granterAddressLen (1 Byte)><granterAddress_Bytes><granteeAddressLen (1 Byte)><granteeAddress_Bytes><msgType_Bytes>: Grant

var (
// Keys for store prefixes
Expand All @@ -30,12 +31,22 @@ var (

// GetAuthorizationStoreKey - return authorization store key
func GetAuthorizationStoreKey(grantee sdk.AccAddress, granter sdk.AccAddress, msgType string) []byte {
return append(append(append(GrantKey, granter.Bytes()...), grantee.Bytes()...), []byte(msgType)...)
return append(append(append(
GrantKey,
address.MustLengthPrefix(granter)...),
address.MustLengthPrefix(grantee)...),
[]byte(msgType)...,
)
}

// ExtractAddressesFromGrantKey - split granter & grantee address from the authorization key
func ExtractAddressesFromGrantKey(key []byte) (granterAddr, granteeAddr sdk.AccAddress) {
granterAddr = sdk.AccAddress(key[1 : sdk.AddrLen+1])
granteeAddr = sdk.AccAddress(key[sdk.AddrLen+1 : sdk.AddrLen*2+1])
// key if of format:
// 0x01<granterAddressLen (1 Byte)><granterAddress_Bytes><granteeAddressLen (1 Byte)><granteeAddress_Bytes><msgType_Bytes>
granterAddrLen := key[1] // remove prefix key
granterAddr = sdk.AccAddress(key[2 : 2+granterAddrLen])
granteeAddrLen := int(key[2+granterAddrLen])
granteeAddr = sdk.AccAddress(key[3+granterAddrLen : 3+granterAddrLen+byte(granteeAddrLen)])

return granterAddr, granteeAddr
}
4 changes: 1 addition & 3 deletions x/bank/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ func (k BaseKeeper) AllBalances(ctx context.Context, req *types.QueryAllBalances
sdkCtx := sdk.UnwrapSDKContext(ctx)

balances := sdk.NewCoins()
store := sdkCtx.KVStore(k.storeKey)
balancesStore := prefix.NewStore(store, types.BalancesPrefix)
accountStore := prefix.NewStore(balancesStore, addr.Bytes())
accountStore := k.getAccountStore(sdkCtx, addr)

pageRes, err := query.Paginate(accountStore, req.Pagination, func(_, value []byte) error {
var result sdk.Coin
Expand Down
9 changes: 2 additions & 7 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ 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"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -233,9 +232,7 @@ func (k BaseSendKeeper) ClearBalances(ctx sdk.Context, addr sdk.AccAddress) {
return false
})

store := ctx.KVStore(k.storeKey)
balancesStore := prefix.NewStore(store, types.BalancesPrefix)
accountStore := prefix.NewStore(balancesStore, addr.Bytes())
accountStore := k.getAccountStore(ctx, addr)

for _, key := range keys {
accountStore.Delete(key)
Expand Down Expand Up @@ -264,9 +261,7 @@ func (k BaseSendKeeper) SetBalance(ctx sdk.Context, addr sdk.AccAddress, balance
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, balance.String())
}

store := ctx.KVStore(k.storeKey)
balancesStore := prefix.NewStore(store, types.BalancesPrefix)
accountStore := prefix.NewStore(balancesStore, addr.Bytes())
accountStore := k.getAccountStore(ctx, addr)

bz := k.cdc.MustMarshalBinaryBare(&balance)
accountStore.Set([]byte(balance.Denom), bz)
Expand Down
Loading

0 comments on commit c1b567f

Please sign in to comment.