From c061cc1c5754b7f44fb220785b8d330c854e7a0e Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 26 Jul 2021 17:39:58 +0200 Subject: [PATCH 1/4] docs: ADR-43: Aligning on interoperability (#9709) ## Description Following on: + NFT IBC [dependency](https://github.com/cosmos/cosmos-sdk/pull/9329#issuecomment-872415040) thread/issue + [interoperablity](https://github.com/cosmos/cosmos-sdk/pull/9329#pullrequestreview-704549373) with other NFT implementations + x/bank interoperability [discussion](https://github.com/cosmos/cosmos-sdk/discussions/9065#discussioncomment-873206) we need to be clear how the NFT token interoperability between modules and chains is achieved. --- ### 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... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] 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) --- docs/architecture/adr-043-nft-module.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/architecture/adr-043-nft-module.md b/docs/architecture/adr-043-nft-module.md index 814be6cb1898..8c2f967b1694 100644 --- a/docs/architecture/adr-043-nft-module.md +++ b/docs/architecture/adr-043-nft-module.md @@ -281,6 +281,15 @@ message QueryClassesResponse { } ``` + +### Interoperability + +Interoperability is all about reusing assets between modules and chains. The former one is achieved by ADR-33: Protobuf client - server communication. At the time of writing ADR-33 is not finalized. The latter is achieved by IBC. Here we will focus on the IBC side. +IBC is implemented per module. Here, we aligned that NFTs will be recorded and managed in the x/nft. This requires creation of a new IBC standard and implementation of it. + +For IBC interoperability, NFT custom modules MUST use the NFT object type understood by the IBC client. So, for x/nft interoperability, custom NFT implementations (example: x/cryptokitty) should use the canonical x/nft module and proxy all NFT balance keeping functionality to x/nft or else re-implement all functionality using the NFT object type understood by the IBC client. In other words: x/nft becomes the standard NFT registry for all Cosmos NFTs (example: x/cryptokitty will register a kitty NFT in x/nft and use x/nft for book keeping). This was [discussed](https://github.com/cosmos/cosmos-sdk/discussions/9065#discussioncomment-873206) in the context of using x/bank as a general asset balance book. Not using x/nft will require implementing another module for IBC. + + ## Consequences ### Backward Compatibility @@ -299,6 +308,7 @@ This specification conforms to the ERC-721 smart contract specification for NFT ### Negative ++ New IBC app is required for x/nft ### Neutral From 9ea1fee8b27a85d37540182f9c4c8ef2250530ff Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 26 Jul 2021 13:51:04 -0400 Subject: [PATCH 2/4] x/bank: create reverse prefix for denom<->address (#9611) --- CHANGELOG.md | 5 +++ simapp/app_test.go | 7 +++- x/auth/client/testutil/suite.go | 24 +++++++------ x/bank/keeper/genesis.go | 2 +- x/bank/keeper/grpc_query.go | 17 ++-------- x/bank/keeper/migrations.go | 6 ++++ x/bank/keeper/send.go | 30 ++++++++++++++-- x/bank/keeper/view.go | 7 +++- x/bank/migrations/v043/keys.go | 32 +++++++++++++++-- x/bank/migrations/v043/store.go | 4 +-- x/bank/migrations/v044/keys.go | 10 ++++++ x/bank/migrations/v044/store.go | 51 ++++++++++++++++++++++++++++ x/bank/migrations/v044/store_test.go | 45 ++++++++++++++++++++++++ x/bank/module.go | 10 ++++-- x/bank/spec/01_state.md | 17 +++++++--- x/bank/types/key.go | 19 +++++++++-- x/staking/client/testutil/suite.go | 9 +++-- 17 files changed, 248 insertions(+), 47 deletions(-) create mode 100644 x/bank/migrations/v044/keys.go create mode 100644 x/bank/migrations/v044/store.go create mode 100644 x/bank/migrations/v044/store_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 50af835d31d9..1849ceed13e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/simapp/app_test.go b/simapp/app_test.go index c84c99003053..76c1a423d1f9 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -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, }, { diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index dc843db9fac6..06e47e0164c3 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -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) diff --git a/x/bank/keeper/genesis.go b/x/bank/keeper/genesis.go index 24b5ef44f9fc..694fa6668e61 100644 --- a/x/bank/keeper/genesis.go +++ b/x/bank/keeper/genesis.go @@ -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 { diff --git a/x/bank/keeper/grpc_query.go b/x/bank/keeper/grpc_query.go index 53c192954f6a..6f53b5abba1b 100644 --- a/x/bank/keeper/grpc_query.go +++ b/x/bank/keeper/grpc_query.go @@ -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), }, ) } diff --git a/x/bank/keeper/migrations.go b/x/bank/keeper/migrations.go index 0ca384b64d9a..c081e1b0698b 100644 --- a/x/bank/keeper/migrations.go +++ b/x/bank/keeper/migrations.go @@ -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) +} diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 369fa631c447..1d43d1a0e1d3 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -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 diff --git a/x/bank/keeper/view.go b/x/bank/keeper/view.go index fe46ec9b6ec1..73334ef37035 100644 --- a/x/bank/keeper/view.go +++ b/x/bank/keeper/view.go @@ -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)) +} diff --git a/x/bank/migrations/v043/keys.go b/x/bank/migrations/v043/keys.go index fbef37c9885e..01a4aab2ab51 100644 --- a/x/bank/migrations/v043/keys.go +++ b/x/bank/migrations/v043/keys.go @@ -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 +} diff --git a/x/bank/migrations/v043/store.go b/x/bank/migrations/v043/store.go index be6d15255b42..2b3a084d0bf9 100644 --- a/x/bank/migrations/v043/store.go +++ b/x/bank/migrations/v043/store.go @@ -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()) diff --git a/x/bank/migrations/v044/keys.go b/x/bank/migrations/v044/keys.go new file mode 100644 index 000000000000..c06492658866 --- /dev/null +++ b/x/bank/migrations/v044/keys.go @@ -0,0 +1,10 @@ +package v044 + +var ( + DenomAddressPrefix = []byte{0x03} +) + +func CreateAddressDenomPrefix(denom string) []byte { + key := append(DenomAddressPrefix, []byte(denom)...) + return append(key, 0) +} diff --git a/x/bank/migrations/v044/store.go b/x/bank/migrations/v044/store.go new file mode 100644 index 000000000000..58966450f189 --- /dev/null +++ b/x/bank/migrations/v044/store.go @@ -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 +} diff --git a/x/bank/migrations/v044/store_test.go b/x/bank/migrations/v044/store_test.go new file mode 100644 index 000000000000..ac0958869f3e --- /dev/null +++ b/x/bank/migrations/v044/store_test.go @@ -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) + } +} diff --git a/x/bank/module.go b/x/bank/module.go index 55fda845c317..62038da8b7b8 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -103,7 +103,13 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterQueryServer(cfg.QueryServer(), am.keeper) m := keeper.NewMigrator(am.keeper.(keeper.BaseKeeper)) - cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2) + if err := cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2); err != nil { + panic(fmt.Sprintf("failed to migrate x/bank from version 1 to 2: %v", err)) + } + + if err := cfg.RegisterMigration(types.ModuleName, 2, m.Migrate2to3); err != nil { + panic(fmt.Sprintf("failed to migrate x/bank from version 2 to 3: %v", err)) + } } // NewAppModule creates a new AppModule object @@ -156,7 +162,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // ConsensusVersion implements AppModule/ConsensusVersion. -func (AppModule) ConsensusVersion() uint64 { return 2 } +func (AppModule) ConsensusVersion() uint64 { return 3 } // BeginBlock performs a no-op. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/x/bank/spec/01_state.md b/x/bank/spec/01_state.md index 5328e8b3caee..97584ca1abb6 100644 --- a/x/bank/spec/01_state.md +++ b/x/bank/spec/01_state.md @@ -4,9 +4,16 @@ order: 1 # State -The `x/bank` module keeps state of three primary objects, account balances, denom metadata and the -total supply of all balances. +The `x/bank` module keeps state of three primary objects: -- Supply: `0x0 | byte(denom) -> byte(amount)` -- Denom Metadata: `0x1 | byte(denom) -> ProtocolBuffer(Metadata)` -- Balances: `0x2 | byte(address length) | []byte(address) | []byte(balance.Denom) -> ProtocolBuffer(balance)` +1. Account balances +2. Denomination metadata +3. The total supply of all balances + +In addition, the `x/bank` module keeps the following indexes to manage the +aforementioned state: + +- Supply Index: `0x0 | byte(denom) -> byte(amount)` +- Denom Metadata Index: `0x1 | byte(denom) -> ProtocolBuffer(Metadata)` +- Balances Index: `0x2 | byte(address length) | []byte(address) | []byte(balance.Denom) -> ProtocolBuffer(balance)` +- Reverse Denomination to Address Index: `0x03 | byte(denom) | 0x00 | []byte(address) -> 0` diff --git a/x/bank/types/key.go b/x/bank/types/key.go index e91a38970e8b..be8f43f9aebe 100644 --- a/x/bank/types/key.go +++ b/x/bank/types/key.go @@ -22,11 +22,13 @@ const ( // KVStore keys var ( - // BalancesPrefix is the prefix for the account balances store. We use a byte - // (instead of `[]byte("balances")` to save some disk space). - BalancesPrefix = []byte{0x02} SupplyKey = []byte{0x00} DenomMetadataPrefix = []byte{0x1} + DenomAddressPrefix = []byte{0x03} + + // BalancesPrefix is the prefix for the account balances store. We use a byte + // (instead of `[]byte("balances")` to save some disk space). + BalancesPrefix = []byte{0x02} ) // DenomMetadataKey returns the denomination metadata key. @@ -44,12 +46,16 @@ func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) { if len(key) == 0 { return nil, ErrInvalidKey } + kv.AssertKeyAtLeastLength(key, 1) + addrLen := key[0] bound := int(addrLen) + if len(key)-1 < bound { return nil, ErrInvalidKey } + return key[1 : bound+1], nil } @@ -57,3 +63,10 @@ func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) { func CreateAccountBalancesPrefix(addr []byte) []byte { return append(BalancesPrefix, address.MustLengthPrefix(addr)...) } + +// CreateDenomAddressPrefix creates a prefix for a reverse index of denomination +// to account balance for that denomination. +func CreateDenomAddressPrefix(denom string) []byte { + key := append(DenomAddressPrefix, []byte(denom)...) + return append(key, 0) +} diff --git a/x/staking/client/testutil/suite.go b/x/staking/client/testutil/suite.go index 965c35823c57..4cbae20eba43 100644 --- a/x/staking/client/testutil/suite.go +++ b/x/staking/client/testutil/suite.go @@ -57,15 +57,18 @@ func (s *IntegrationTestSuite) SetupSuite() { val2 := s.network.Validators[1] // redelegate - _, err = MsgRedelegateExec( + out, err := MsgRedelegateExec( val.ClientCtx, val.Address, val.ValAddress, val2.ValAddress, unbond, - fmt.Sprintf("--%s=%d", flags.FlagGas, 202954), // 202954 is the required + fmt.Sprintf("--%s=%d", flags.FlagGas, 300000), ) s.Require().NoError(err) + var txRes sdk.TxResponse + s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txRes)) + s.Require().Equal(uint32(0), txRes.Code) _, err = s.network.WaitForHeight(1) s.Require().NoError(err) // unbonding @@ -1181,7 +1184,7 @@ func (s *IntegrationTestSuite) TestNewRedelegateCmd() { val2.ValAddress.String(), // dst-validator-addr sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(150)).String(), // amount fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), - fmt.Sprintf("--%s=%s", flags.FlagGas, "auto"), + fmt.Sprintf("--%s=%d", flags.FlagGas, 300000), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), From 3771ebdf2463aea04c2ebe85040e4891caf57c68 Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Mon, 26 Jul 2021 22:08:04 +0200 Subject: [PATCH 3/4] refactor: Improve txRaw decoder code (#9769) ## Description Apply post-merge reviews to a previous PR ref: https://github.com/cosmos/cosmos-sdk/pull/9754#pullrequestreview-713819730 --- ### 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... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] 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) --- x/auth/tx/decoder.go | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/x/auth/tx/decoder.go b/x/auth/tx/decoder.go index d93c7446db60..2fef6312b994 100644 --- a/x/auth/tx/decoder.go +++ b/x/auth/tx/decoder.go @@ -16,7 +16,7 @@ import ( func DefaultTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder { return func(txBytes []byte) (sdk.Tx, error) { // Make sure txBytes follow ADR-027. - err := rejectNonADR027(txBytes) + err := rejectNonADR027TxRaw(txBytes) if err != nil { return nil, sdkerrors.Wrap(sdkerrors.ErrTxDecode, err.Error()) } @@ -90,13 +90,14 @@ func DefaultJSONTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder { } } -// rejectNonADR027 rejects txBytes that do not follow ADR-027. This function +// rejectNonADR027TxRaw rejects txBytes that do not follow ADR-027. This is NOT +// a generic ADR-027 checker, it only applies decoding TxRaw. Specifically, it // only checks that: // - field numbers are in ascending order (1, 2, and potentially multiple 3s), -// - and varints as as short as possible. -// All other ADR-027 edge cases (e.g. TxRaw fields having default values) will -// not happen with TxRaw. -func rejectNonADR027(txBytes []byte) error { +// - and varints are as short as possible. +// All other ADR-027 edge cases (e.g. default values) are not applicable with +// TxRaw. +func rejectNonADR027TxRaw(txBytes []byte) error { // Make sure all fields are ordered in ascending order with this variable. prevTagNum := protowire.Number(0) @@ -105,21 +106,25 @@ func rejectNonADR027(txBytes []byte) error { if m < 0 { return fmt.Errorf("invalid length; %w", protowire.ParseError(m)) } + // TxRaw only has bytes fields. if wireType != protowire.BytesType { - return fmt.Errorf("expected %d wire type, got %d", protowire.VarintType, wireType) + return fmt.Errorf("expected %d wire type, got %d", protowire.BytesType, wireType) } + // Make sure fields are ordered in ascending order. if tagNum < prevTagNum { return fmt.Errorf("txRaw must follow ADR-027, got tagNum %d after tagNum %d", tagNum, prevTagNum) } prevTagNum = tagNum // All 3 fields of TxRaw have wireType == 2, so their next component - // is a varint. - // We make sure that the varint is as short as possible. + // is a varint, so we can safely call ConsumeVarint here. + // Byte structure: + // Inner fields are verified in `DefaultTxDecoder` lengthPrefix, m := protowire.ConsumeVarint(txBytes[m:]) if m < 0 { return fmt.Errorf("invalid length; %w", protowire.ParseError(m)) } + // We make sure that this varint is as short as possible. n := varintMinLength(lengthPrefix) if n != m { return fmt.Errorf("length prefix varint for tagNum %d is not as short as possible, read %d, only need %d", tagNum, m, n) @@ -141,23 +146,23 @@ func rejectNonADR027(txBytes []byte) error { func varintMinLength(n uint64) int { switch { // Note: 1< Date: Mon, 26 Jul 2021 22:51:22 +0200 Subject: [PATCH 4/4] test: Added tests for DecCoins to increase code coverage (#9752) ## Description Ref: #7031 Note/todo : 1. Not sure if [this](https://github.com/cosmos/cosmos-sdk/blob/spoorthi/7031-deccoins-adding-tests/types/dec_coin.go#L631) line is reachable. The condition is already checked in [this ](https://github.com/cosmos/cosmos-sdk/blob/spoorthi/7031-deccoins-adding-tests/types/dec_coin.go#L618) line. Therefore, 100% coverage for `ParseDecCoin` function likely not possible 2. Clarification needed [here](https://github.com/cosmos/cosmos-sdk/blob/spoorthi/7031-deccoins-adding-tests/types/dec_coin_test.go#L883-L884) 3. Clarification needed [here](https://github.com/cosmos/cosmos-sdk/blob/spoorthi/7031-deccoins-adding-tests/types/dec_coin_test.go#L1124-L1125) --- ### 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 - [ ] 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 - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] 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) --- container/option_test.go | 1 + types/dec_coin_test.go | 568 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 566 insertions(+), 3 deletions(-) create mode 100644 container/option_test.go diff --git a/container/option_test.go b/container/option_test.go new file mode 100644 index 000000000000..f447afa5a964 --- /dev/null +++ b/container/option_test.go @@ -0,0 +1 @@ +package container_test diff --git a/types/dec_coin_test.go b/types/dec_coin_test.go index 938f7dddffb4..c0cf44cd60d4 100644 --- a/types/dec_coin_test.go +++ b/types/dec_coin_test.go @@ -112,6 +112,7 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { input sdk.DecCoins original string expected string + panic bool }{ { name: "all greater than zero", @@ -124,6 +125,7 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { }, original: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", expected: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + panic: false, }, { name: "zero coin in middle", @@ -136,6 +138,7 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { }, original: "1.000000000000000000testa,2.000000000000000000testb,0.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", expected: "1.000000000000000000testa,2.000000000000000000testb,4.000000000000000000testd,5.000000000000000000teste", + panic: false, }, { name: "zero coin end (unordered)", @@ -148,13 +151,32 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { }, original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb", expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + panic: false, + }, + + { + name: "panic when same denoms in multiple coins", + input: sdk.DecCoins{ + {"testa", sdk.NewDec(5)}, + {"testa", sdk.NewDec(3)}, + {"testa", sdk.NewDec(1)}, + {"testd", sdk.NewDec(4)}, + {"testb", sdk.NewDec(2)}, + }, + original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb", + expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + panic: true, }, } for _, tt := range cases { - undertest := sdk.NewDecCoins(tt.input...) - s.Require().Equal(tt.expected, undertest.String(), "NewDecCoins must return expected results") - s.Require().Equal(tt.original, tt.input.String(), "input must be unmodified and match original") + if tt.panic { + s.Require().Panics(func() { sdk.NewDecCoins(tt.input...) }, "Should panic due to multiple coins with same denom") + } else { + undertest := sdk.NewDecCoins(tt.input...) + s.Require().Equal(tt.expected, undertest.String(), "NewDecCoins must return expected results") + s.Require().Equal(tt.original, tt.input.String(), "input must be unmodified and match original") + } } } @@ -580,3 +602,543 @@ func (s *decCoinTestSuite) TestDecCoins_AddDecCoinWithIsValid() { } } } + +func (s *decCoinTestSuite) TestDecCoins_Empty() { + testCases := []struct { + input sdk.DecCoins + expectedResult bool + msg string + }{ + {sdk.DecCoins{}, true, "No coins as expected."}, + {sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, false, "DecCoins is not empty"}, + } + + for _, tc := range testCases { + if tc.expectedResult { + s.Require().True(tc.input.Empty(), tc.msg) + } else { + s.Require().False(tc.input.Empty(), tc.msg) + } + } +} + +func (s *decCoinTestSuite) TestDecCoins_GetDenomByIndex() { + testCases := []struct { + name string + input sdk.DecCoins + index int + expectedResult string + expectedErr bool + }{ + { + "No DecCoins in Slice", + sdk.DecCoins{}, + 0, + "", + true, + }, + {"When index out of bounds", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, 2, "", true}, + {"When negative index", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, -1, "", true}, + { + "Appropriate index case", + sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(5)}, + sdk.DecCoin{testDenom2, sdk.NewDec(57)}, + }, 1, testDenom2, false, + }, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedErr { + s.Require().Panics(func() { tc.input.GetDenomByIndex(tc.index) }, "Test should have panicked") + } else { + res := tc.input.GetDenomByIndex(tc.index) + s.Require().Equal(tc.expectedResult, res, "Unexpected result for test case #%d, expected output: %s, input: %v", i, tc.expectedResult, tc.input) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_IsAllPositive() { + testCases := []struct { + name string + input sdk.DecCoins + expectedResult bool + }{ + {"No Coins", sdk.DecCoins{}, false}, + + {"One Coin - Zero value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(0)}}, false}, + + {"One Coin - Postive value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, true}, + + {"One Coin - Negative value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(-15)}}, false}, + + {"Multiple Coins - All positive value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(51)}, + sdk.DecCoin{testDenom1, sdk.NewDec(123)}, + sdk.DecCoin{testDenom1, sdk.NewDec(50)}, + sdk.DecCoin{testDenom1, sdk.NewDec(92233720)}, + }, true}, + + {"Multiple Coins - Some negative value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(51)}, + sdk.DecCoin{testDenom1, sdk.NewDec(-123)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(92233720)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedResult { + s.Require().True(tc.input.IsAllPositive(), "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(tc.input.IsAllPositive(), "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_IsLT() { + testCases := []struct { + name string + coin sdk.DecCoin + otherCoin sdk.DecCoin + expectedResult bool + expectedPanic bool + }{ + + {"Same Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom1, sdk.NewDec(19)}, true, false}, + + {"Same Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom1, sdk.NewDec(14)}, false, false}, + + {"Same Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom1, sdk.NewDec(20)}, false, false}, + + {"Different Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom2, sdk.NewDec(19)}, true, true}, + + {"Different Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom2, sdk.NewDec(14)}, true, true}, + + {"Different Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom2, sdk.NewDec(20)}, true, true}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coin.IsLT(tc.otherCoin) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coin.IsLT(tc.otherCoin) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_IsGTE() { + testCases := []struct { + name string + coin sdk.DecCoin + otherCoin sdk.DecCoin + expectedResult bool + expectedPanic bool + }{ + + {"Same Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom1, sdk.NewDec(19)}, false, false}, + + {"Same Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom1, sdk.NewDec(14)}, true, false}, + + {"Same Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom1, sdk.NewDec(20)}, true, false}, + + {"Different Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom2, sdk.NewDec(19)}, true, true}, + + {"Different Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom2, sdk.NewDec(14)}, true, true}, + + {"Different Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom2, sdk.NewDec(20)}, true, true}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coin.IsGTE(tc.otherCoin) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coin.IsGTE(tc.otherCoin) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_IsZero() { + testCases := []struct { + name string + coins sdk.DecCoins + expectedResult bool + }{ + {"No Coins", sdk.DecCoins{}, true}, + + {"One Coin - Zero value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(0)}}, true}, + + {"One Coin - Postive value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, false}, + + {"Multiple Coins - All zero value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + }, true}, + + {"Multiple Coins - Some positive value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(92233720)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedResult { + s.Require().True(tc.coins.IsZero(), "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(tc.coins.IsZero(), "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_MulDec() { + testCases := []struct { + name string + coins sdk.DecCoins + multiplier sdk.Dec + expectedResult sdk.DecCoins + }{ + {"No Coins", sdk.DecCoins{}, sdk.NewDec(1), sdk.DecCoins(nil)}, + + {"Multiple coins - zero multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(10)}, + sdk.DecCoin{testDenom1, sdk.NewDec(30)}, + }, sdk.NewDec(0), sdk.DecCoins(nil)}, + + {"Multiple coins - positive multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom1, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + }}, + + {"Multiple coins - negative multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom1, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(-2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(-20)}, + }}, + + {"Multiple coins - Different denom", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom2, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom2, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(8)}, + sdk.DecCoin{testDenom2, sdk.NewDec(12)}, + }}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + res := tc.coins.MulDec(tc.multiplier) + s.Require().Equal(tc.expectedResult, res, "Test case #%d: %s", i, tc.name) + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_MulDecTruncate() { + testCases := []struct { + name string + coins sdk.DecCoins + multiplier sdk.Dec + expectedResult sdk.DecCoins + expectedPanic bool + }{ + {"No Coins", sdk.DecCoins{}, sdk.NewDec(1), sdk.DecCoins(nil), false}, + + // TODO - Fix test - Function comment documentation for MulDecTruncate says if multiplier d is zero, it should panic. + // However, that is not the observed behaviour. Currently nil is returned. + {"Multiple coins - zero multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(10, 3)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(30, 2)}, + }, sdk.NewDec(0), sdk.DecCoins(nil), false}, + + {"Multiple coins - positive multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + }, sdk.NewDec(1), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(3, 0)}, + }, false}, + + {"Multiple coins - positive multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + }, sdk.NewDec(-2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(-6, 0)}, + }, false}, + + {"Multiple coins - Different denom", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom2, sdk.NewDecWithPrec(3333, 4)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom2, sdk.NewDecWithPrec(333, 4)}, + }, sdk.NewDec(10), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(30, 0)}, + sdk.DecCoin{testDenom2, sdk.NewDecWithPrec(3666, 3)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coins.MulDecTruncate(tc.multiplier) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coins.MulDecTruncate(tc.multiplier) + s.Require().Equal(tc.expectedResult, res, "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_QuoDec() { + + testCases := []struct { + name string + coins sdk.DecCoins + input sdk.Dec + expectedResult sdk.DecCoins + panics bool + }{ + {"No Coins", sdk.DecCoins{}, sdk.NewDec(1), sdk.DecCoins(nil), false}, + + {"Multiple coins - zero input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(10)}, + sdk.DecCoin{testDenom1, sdk.NewDec(30)}, + }, sdk.NewDec(0), sdk.DecCoins(nil), true}, + + {"Multiple coins - positive input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(35, 1)}, + }, false}, + + {"Multiple coins - negative input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(-2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(-35, 1)}, + }, false}, + + {"Multiple coins - Different input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom2, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom2, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(2)}, + sdk.DecCoin{testDenom2, sdk.NewDec(3)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.panics { + s.Require().Panics(func() { tc.coins.QuoDec(tc.input) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coins.QuoDec(tc.input) + s.Require().Equal(tc.expectedResult, res, "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_IsEqual() { + testCases := []struct { + name string + coin sdk.DecCoin + otherCoin sdk.DecCoin + expectedResult bool + expectedPanic bool + }{ + + {"Different Denom Same Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom2, sdk.NewDec(20)}, + false, true}, + + {"Different Denom Different Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom2, sdk.NewDec(10)}, + false, true}, + + {"Same Denom Different Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom1, sdk.NewDec(10)}, + false, false}, + + {"Same Denom Same Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + true, false}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coin.IsEqual(tc.otherCoin) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coin.IsEqual(tc.otherCoin) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_IsEqual() { + testCases := []struct { + name string + coinsA sdk.DecCoins + coinsB sdk.DecCoins + expectedResult bool + expectedPanic bool + }{ + {"Different length sets", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(35)}, + }, false, false}, + + {"Same length - different denoms", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom2, sdk.NewDec(3)}, + sdk.DecCoin{testDenom2, sdk.NewDec(4)}, + }, false, true}, + + {"Same length - different amounts", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(41)}, + sdk.DecCoin{testDenom1, sdk.NewDec(343)}, + }, false, false}, + + {"Same length - same amounts", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(33)}, + sdk.DecCoin{testDenom1, sdk.NewDec(344)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(33)}, + sdk.DecCoin{testDenom1, sdk.NewDec(344)}, + }, true, false}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coinsA.IsEqual(tc.coinsB) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coinsA.IsEqual(tc.coinsB) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_Validate() { + var empty sdk.DecCoin + testCases := []struct { + name string + input sdk.DecCoin + expectedPass bool + }{ + {"Uninitalized deccoin", empty, false}, + + {"Invalid denom string", sdk.DecCoin{"(){9**&})", sdk.NewDec(33)}, false}, + + {"Negative coin amount", sdk.DecCoin{testDenom1, sdk.NewDec(-33)}, false}, + + {"Valid coin", sdk.DecCoin{testDenom1, sdk.NewDec(33)}, true}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + err := tc.input.Validate() + if tc.expectedPass { + s.Require().NoError(err, "unexpected result for test case #%d %s, input: %v", i, tc.name, tc.input) + } else { + s.Require().Error(err, "unexpected result for test case #%d %s, input: %v", i, tc.name, tc.input) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_ParseDecCoin() { + var empty sdk.DecCoin + testCases := []struct { + name string + input string + expectedResult sdk.DecCoin + expectedErr bool + }{ + {"Empty input", "", empty, true}, + + {"Bad input", "✨🌟⭐", empty, true}, + + {"Invalid decimal coin", "9.3.0stake", empty, true}, + + {"Precision over limit", "9.11111111111111111111stake", empty, true}, + + // TODO - Clarify - According to error message for ValidateDenom call, supposed to + // throw error when upper case characters are used. Currently uppercase denoms are allowed. + {"Invalid denom", "9.3STAKE", sdk.DecCoin{"STAKE", sdk.NewDecWithPrec(93, 1)}, false}, + + {"Valid input - amount and denom seperated by space", "9.3 stake", sdk.DecCoin{"stake", sdk.NewDecWithPrec(93, 1)}, false}, + + {"Valid input - amount and denom concatenated", "9.3stake", sdk.DecCoin{"stake", sdk.NewDecWithPrec(93, 1)}, false}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + res, err := sdk.ParseDecCoin(tc.input) + if tc.expectedErr { + s.Require().Error(err, "expected error for test case #%d %s, input: %v", i, tc.name, tc.input) + } else { + s.Require().NoError(err, "unexpected error for test case #%d %s, input: %v", i, tc.name, tc.input) + s.Require().Equal(tc.expectedResult, res, "unexpected result for test case #%d %s, input: %v", i, tc.name, tc.input) + } + }) + } +}