Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Bank] Remove the unsafe balance changing API #8473

Merged
merged 40 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
af3df53
temp commit
jgimeno Jan 28, 2021
0763fea
setbalance now is internal
jgimeno Jan 29, 2021
8216752
remove set balances in genesis
jgimeno Jan 29, 2021
7fe6aeb
feedback test commit
jgimeno Jan 29, 2021
6a2a36d
update tests
jgimeno Jan 29, 2021
63f4a93
fix: genesis panic message
fdymylja Jan 29, 2021
bbc457f
fix not bonded pool
jgimeno Feb 1, 2021
3b91703
fix(staking): genesis test
fdymylja Feb 2, 2021
3a9e784
fix(simapp): rollback state fix change
fdymylja Feb 2, 2021
d15ff9e
fix(staking): genesis large val set test
fdymylja Feb 2, 2021
bf78e67
[Bank Refactor] Frojdi jonathan/remove setsupply (#8491)
jgimeno Feb 3, 2021
466279a
remove setbalances from genesis in gov
jgimeno Feb 3, 2021
29f5660
remove keyring
jgimeno Feb 3, 2021
6ad4503
add init genesis state
jgimeno Feb 3, 2021
1fa3c1e
change(staking): make genesis checks coherent and add tests
fdymylja Feb 3, 2021
37d7884
remove setbalances on distribution
jgimeno Feb 3, 2021
612472c
fix(staking): genesis tests
fdymylja Feb 4, 2021
c4bd11c
Merge branch 'jonathan-frojdi/refactor-bank' of github.com:cosmos/cos…
jgimeno Feb 4, 2021
f6718d8
[Bank Refactor]: Remove SetBalances usage from the code and tests (#8…
fdymylja Feb 10, 2021
5e7b1ad
change(bank): remove SubtractCoins from keeper public API
fdymylja Feb 10, 2021
58572ad
change(ibc/transfer): remove AddCoins from relay tests
fdymylja Feb 10, 2021
c3409ee
change(bank): remove AddCoins from public keeper API
fdymylja Feb 10, 2021
d2d930a
Merge branch 'master' into jonathan-frojdi/refactor-bank
jgimeno Feb 10, 2021
b0fab44
fix imports
jgimeno Feb 10, 2021
030bfbe
remove set balances
jgimeno Feb 10, 2021
5b236c6
fix fee test
jgimeno Feb 10, 2021
5fdc895
remove set balances
jgimeno Feb 10, 2021
a7d83b2
Merge branch 'master' of github.com:cosmos/cosmos-sdk into jonathan-f…
sahith-narahari Feb 11, 2021
aa665d3
Merge branch 'jonathan-frojdi/refactor-bank' of github.com:cosmos/cos…
sahith-narahari Feb 11, 2021
009a5ee
fix(staking): remove dependency on minter authorization for staking p…
fdymylja Feb 11, 2021
940cd4f
chore: update CHANGELOG.md
fdymylja Feb 11, 2021
cafdb01
Merge branch 'master' into jonathan-frojdi/refactor-bank
sahith-narahari Feb 12, 2021
442c1e5
Merge branch 'master' into jonathan-frojdi/refactor-bank
Feb 15, 2021
061e3e0
update: x/distribution/keeper/keeper_test.go
fdymylja Feb 16, 2021
72ea30d
Update simapp/test_helpers.go
fdymylja Feb 16, 2021
0e7477a
Update x/staking/genesis_test.go
fdymylja Feb 16, 2021
2aae3e2
fix(simapp): FundAccount amount variable name
fdymylja Feb 16, 2021
ece597e
Merge branch 'master' into jonathan-frojdi/refactor-bank
jgimeno Feb 17, 2021
1f0e2de
fix some PR issues
jgimeno Feb 17, 2021
f4bff7c
Merge branch 'master' into jonathan-frojdi/refactor-bank
jgimeno Feb 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/bank) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) Bank keeper does not expose unsafe balance changing methods such as `SetBalance`, `SetSupply` etc.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ADR-33 will solve it as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true that, but this change in tests had to be done even with adr-033. we really wanted to wait for it but we need to deliver rosetta ASAP, and the consensus on the previous calls we have had was that we can not wait for ADR-033 to be implemented.

* (x/staking) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) On genesis init, if non bonded pool and bonded pool balance, coming from the bank module, does not match what is saved in the staking state, the initialization will panic.
* (x/gov) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) On genesis init, if the gov module account balance, coming from bank module state, does not match the one in gov module state, the initialization will panic.
* (x/distribution) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) On genesis init, if the distribution module account balance, coming from bank module state, does not match the one in distribution module state, the initialization will panic.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a bug fix? Should this be ported to 0.41.1?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is, but it's a breaking change, idk if we can port although i'd imagine it not affecting any live chain.

* (client/keys) [\#8500](https://github.com/cosmos/cosmos-sdk/pull/8500) `InfoImporter` interface is removed from legacy keybase.

### State Machine Breaking
Expand All @@ -52,7 +56,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/gov) [\#7733](https://github.com/cosmos/cosmos-sdk/pull/7733) ADR 037 Implementation: Governance Split Votes

### Improvements

* (x/ibc) [\#8458](https://github.com/cosmos/cosmos-sdk/pull/8458) Add `packet_connection` attribute to ibc events to enable relayer filtering
* (x/bank) [\#8479](https://github.com/cosmos/cosmos-sdk/pull/8479) Adittional client denom metadata validation for `base` and `display` denoms.
* (x/ibc) [\#8404](https://github.com/cosmos/cosmos-sdk/pull/8404) Reorder IBC `ChanOpenAck` and `ChanOpenConfirm` handler execution to perform core handler first, followed by application callbacks.
Expand Down
54 changes: 54 additions & 0 deletions simapp/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
simappparams "github.com/cosmos/cosmos-sdk/simapp/params"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// AppStateFn returns the initial application state using a genesis or the simulation parameters.
Expand Down Expand Up @@ -68,6 +71,57 @@ func AppStateFn(cdc codec.JSONMarshaler, simManager *module.SimulationManager) s
appState, simAccs = AppStateRandomizedFn(simManager, r, cdc, accs, genesisTimestamp, appParams)
}

rawState := make(map[string]json.RawMessage)
err := json.Unmarshal(appState, &rawState)
if err != nil {
panic(err)
}

stakingStateBz, ok := rawState[stakingtypes.ModuleName]
if !ok {
panic("staking genesis state is missing")
}

stakingState := new(stakingtypes.GenesisState)
err = cdc.UnmarshalJSON(stakingStateBz, stakingState)
if err != nil {
panic(err)
}
// compute not bonded balance
notBondedTokens := sdk.ZeroInt()
for _, val := range stakingState.Validators {
if val.Status != stakingtypes.Unbonded {
continue
}
notBondedTokens = notBondedTokens.Add(val.GetTokens())
}
notBondedCoins := sdk.NewCoin(stakingState.Params.BondDenom, notBondedTokens)
// edit bank state to make it have the not bonded pool tokens
bankStateBz, ok := rawState[banktypes.ModuleName]
// TODO(fdymylja/jonathan): should we panic in this case
if !ok {
panic("bank genesis state is missing")
}
bankState := new(banktypes.GenesisState)
err = cdc.UnmarshalJSON(bankStateBz, bankState)
if err != nil {
panic(err)
}

bankState.Balances = append(bankState.Balances, banktypes.Balance{
Address: authtypes.NewModuleAddress(stakingtypes.NotBondedPoolName).String(),
Coins: sdk.NewCoins(notBondedCoins),
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How we know if this balance is not already there? Probably we should firstly check.


// change appState back
rawState[stakingtypes.ModuleName] = cdc.MustMarshalJSON(stakingState)
rawState[banktypes.ModuleName] = cdc.MustMarshalJSON(bankState)

// replace appstate
appState, err = json.Marshal(rawState)
if err != nil {
panic(err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed for migration? Or we need to do it every time? If for migration, then we have better solution with in-module migrations.

return appState, simAccs, chainID, genesisTimestamp
}
}
Expand Down
49 changes: 28 additions & 21 deletions simapp/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"testing"
"time"

minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move this import next to other cosmos-sdk imports


"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"
Expand Down Expand Up @@ -119,7 +121,6 @@ func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs
delegations = append(delegations, stakingtypes.NewDelegation(genAccs[0].GetAddress(), val.Address.Bytes(), sdk.OneDec()))

}

// set validators and delegations
stakingGenesis := stakingtypes.NewGenesisState(stakingtypes.DefaultParams(), validators, delegations)
genesisState[stakingtypes.ModuleName] = app.AppCodec().MustMarshalJSON(stakingGenesis)
Expand All @@ -130,6 +131,12 @@ func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs
totalSupply = totalSupply.Add(b.Coins.Add(sdk.NewCoin(sdk.DefaultBondDenom, bondAmt))...)
}

// add bonded amount to bonded pool module account
balances = append(balances, banktypes.Balance{
Address: authtypes.NewModuleAddress(stakingtypes.BondedPoolName).String(),
Coins: sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, bondAmt)},
})

// update total supply
bankGenesis := banktypes.NewGenesisState(banktypes.DefaultGenesisState().Params, balances, totalSupply, []banktypes.Metadata{})
genesisState[banktypes.ModuleName] = app.AppCodec().MustMarshalJSON(bankGenesis)
Expand Down Expand Up @@ -231,21 +238,11 @@ func createIncrementalAccounts(accNum int) []sdk.AccAddress {
func AddTestAddrsFromPubKeys(app *SimApp, ctx sdk.Context, pubKeys []cryptotypes.PubKey, accAmt sdk.Int) {
initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))

setTotalSupply(app, ctx, accAmt, len(pubKeys))
robert-zaremba marked this conversation as resolved.
Show resolved Hide resolved

// fill all the addresses with some coins, set the loose pool tokens simultaneously
for _, pubKey := range pubKeys {
saveAccount(app, ctx, sdk.AccAddress(pubKey.Address()), initCoins)
for _, pk := range pubKeys {
initAccountWithCoins(app, ctx, sdk.AccAddress(pk.Address()), initCoins)
}
}

// setTotalSupply provides the total supply based on accAmt * totalAccounts.
func setTotalSupply(app *SimApp, ctx sdk.Context, accAmt sdk.Int, totalAccounts int) {
totalSupply := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt.MulRaw(int64(totalAccounts))))
prevSupply := app.BankKeeper.GetSupply(ctx)
app.BankKeeper.SetSupply(ctx, banktypes.NewSupply(prevSupply.GetTotal().Add(totalSupply...)))
}

// AddTestAddrs constructs and returns accNum amount of accounts with an
// initial balance of accAmt in random order
func AddTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int) []sdk.AccAddress {
Expand All @@ -262,21 +259,21 @@ func addTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int, stra
testAddrs := strategy(accNum)

initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))
setTotalSupply(app, ctx, accAmt, accNum)

// fill all the addresses with some coins, set the loose pool tokens simultaneously
for _, addr := range testAddrs {
saveAccount(app, ctx, addr, initCoins)
initAccountWithCoins(app, ctx, addr, initCoins)
}

return testAddrs
}

// saveAccount saves the provided account into the simapp with balance based on initCoins.
func saveAccount(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, initCoins sdk.Coins) {
acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
app.AccountKeeper.SetAccount(ctx, acc)
err := app.BankKeeper.AddCoins(ctx, addr, initCoins)
func initAccountWithCoins(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, coins sdk.Coins) {
err := app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, coins)
if err != nil {
panic(err)
}

err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr, coins)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -440,3 +437,13 @@ type EmptyAppOptions struct{}
func (ao EmptyAppOptions) Get(o string) interface{} {
return nil
}

// FundAccount is a utility function that funds an account by minting and sending the coins to the address
// TODO(fdymylja): instead of using the mint module account, which has the permission of minting, create a "faucet" account
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for tests, why can't we use the bank/keeper.MintCoins?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's commented, mint coins needs a module account with permissions to mint coins. the correct flow to correctly track supply is that a module mints and after it sends the coins to the account.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but my question is why doing multiple minting is not good?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it's a mint and send, the PR size would've been 2x, and I think having a utility function it's consistent with we have had so far in simapp (AddAddrsAndCoins etc), and also does not leave other devs lost at what to do when they will need to set balance somewhere without using SetBalance

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't express my question clearly enough.

I know that we are doing mint+send here, and it's correct as you said. In the TODO you wrote that we should use faucet account and send tokens from that faucet account (instead of mint+send). It's not clear to me why faucet is superior to what you have implemented (mint+send).

func FundAccount(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, amounts sdk.Coins) error {
err := app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, amount)
if err != nil {
return err
}
return app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr, amount)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's move this function away. Ideally, simapp shouldn't be imported directly.

8 changes: 6 additions & 2 deletions types/query/filtered_pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
Expand All @@ -27,10 +28,11 @@ func (s *paginationTestSuite) TestFilteredPaginations() {
balances = append(balances, sdk.NewInt64Coin(denom, 250))
}

balances = balances.Sort()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to sort?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the send will fail if denoms are not sorted, before balance was set using SetBalance which bypassed this.

addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
app.AccountKeeper.SetAccount(ctx, acc1)
s.Require().NoError(app.BankKeeper.SetBalances(ctx, addr1, balances))
s.Require().NoError(simapp.FundAccount(app, ctx, addr1, balances))
store := ctx.KVStore(app.GetKey(types.StoreKey))

// verify pagination with limit > total values
Expand Down Expand Up @@ -100,10 +102,12 @@ func ExampleFilteredPaginate() {
denom := fmt.Sprintf("test%ddenom", i)
balances = append(balances, sdk.NewInt64Coin(denom, 250))
}

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
app.AccountKeeper.SetAccount(ctx, acc1)
err := app.BankKeeper.SetBalances(ctx, addr1, balances)
err := simapp.FundAccount(app, ctx, addr1, balances)
if err != nil { // should return no error
fmt.Println(err)
}
Expand Down
8 changes: 5 additions & 3 deletions types/query/pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,11 @@ func (s *paginationTestSuite) TestPagination() {
balances = append(balances, sdk.NewInt64Coin(denom, 100))
}

balances = balances.Sort()
addr1 := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
app.AccountKeeper.SetAccount(ctx, acc1)
s.Require().NoError(app.BankKeeper.SetBalances(ctx, addr1, balances))
s.Require().NoError(simapp.FundAccount(app, ctx, addr1, balances))

s.T().Log("verify empty page request results a max of defaultLimit records and counts total records")
pageReq := &query.PageRequest{}
Expand Down Expand Up @@ -178,11 +179,12 @@ func ExamplePaginate() {
balances = append(balances, sdk.NewInt64Coin(denom, 100))
}

balances = balances.Sort()
addr1 := sdk.AccAddress([]byte("addr1"))
acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
app.AccountKeeper.SetAccount(ctx, acc1)
err := app.BankKeeper.SetBalances(ctx, addr1, balances)
if err != nil {
err := simapp.FundAccount(app, ctx, addr1, balances)
if err != nil { // should return no error
fmt.Println(err)
}
// Paginate example
Expand Down
31 changes: 21 additions & 10 deletions x/auth/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import (
"strings"
"testing"

"github.com/cosmos/cosmos-sdk/simapp"

minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sdk imports should be at the end in one group.


"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
Expand All @@ -23,7 +27,7 @@ import (

// Test that simulate transaction accurately estimates gas cost
func (suite *AnteTestSuite) TestSimulateGasCost() {
suite.SetupTest(true) // reset
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we change this in all tests?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we've run a diff check on covered lines and they match either if setupTest is run with a true or fals isCheckTx param.

in reality what 'true' does is that it correctly inits genesis. which we require because if we do not the supply will be nil which will cause a panic when we mint coins.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, Why minting new coins requires a genesis?

Copy link
Contributor

@fdymylja fdymylja Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because supply is NIL unless initialized via genesis, this will cause a panic from the bank keeper if tries to get a nil supply. I wanted to address this but the consensus with aaron was that eventually when supply by denom PR is implemented this would ahve been automatically fixed.

suite.SetupTest(false) // reset

// Same data for every test cases
accounts := suite.CreateTestAccounts(3)
Expand Down Expand Up @@ -76,7 +80,7 @@ func (suite *AnteTestSuite) TestSimulateGasCost() {

// Test various error cases in the AnteHandler control flow.
func (suite *AnteTestSuite) TestAnteHandlerSigErrors() {
suite.SetupTest(true) // reset
suite.SetupTest(false) // reset

// Same data for every test cases
priv0, _, addr0 := testdata.KeyTestPubAddr()
Expand Down Expand Up @@ -137,7 +141,9 @@ func (suite *AnteTestSuite) TestAnteHandlerSigErrors() {
func() {
acc1 := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr0)
suite.app.AccountKeeper.SetAccount(suite.ctx, acc1)
err := suite.app.BankKeeper.SetBalances(suite.ctx, addr0, feeAmount)
err := suite.app.BankKeeper.MintCoins(suite.ctx, minttypes.ModuleName, feeAmount)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, minttypes.ModuleName, addr0, feeAmount)
suite.Require().NoError(err)
},
false,
Expand Down Expand Up @@ -435,7 +441,7 @@ func (suite *AnteTestSuite) TestAnteHandlerSequences() {

// Test logic around fee deduction.
func (suite *AnteTestSuite) TestAnteHandlerFees() {
suite.SetupTest(true) // setup
suite.SetupTest(false) // setup

// Same data for every test cases
priv0, _, addr0 := testdata.KeyTestPubAddr()
Expand Down Expand Up @@ -466,7 +472,8 @@ func (suite *AnteTestSuite) TestAnteHandlerFees() {
{
"signer does not have enough funds to pay the fee",
func() {
suite.app.BankKeeper.SetBalances(suite.ctx, addr0, sdk.NewCoins(sdk.NewInt64Coin("atom", 149)))
err := simapp.FundAccount(suite.app, suite.ctx, addr0, sdk.NewCoins(sdk.NewInt64Coin("atom", 149)))
suite.Require().NoError(err)
},
false,
false,
Expand All @@ -475,12 +482,14 @@ func (suite *AnteTestSuite) TestAnteHandlerFees() {
{
"signer as enough funds, should pass",
func() {
accNums = []uint64{7}
modAcc := suite.app.AccountKeeper.GetModuleAccount(suite.ctx, types.FeeCollectorName)

suite.Require().True(suite.app.BankKeeper.GetAllBalances(suite.ctx, modAcc.GetAddress()).Empty())
require.True(sdk.IntEq(suite.T(), suite.app.BankKeeper.GetAllBalances(suite.ctx, addr0).AmountOf("atom"), sdk.NewInt(149)))

suite.app.BankKeeper.SetBalances(suite.ctx, addr0, sdk.NewCoins(sdk.NewInt64Coin("atom", 150)))
err := simapp.FundAccount(suite.app, suite.ctx, addr0, sdk.NewCoins(sdk.NewInt64Coin("atom", 1)))
suite.Require().NoError(err)
},
false,
true,
Expand Down Expand Up @@ -960,7 +969,7 @@ func TestCountSubkeys(t *testing.T) {
}

func (suite *AnteTestSuite) TestAnteHandlerSigLimitExceeded() {
suite.SetupTest(true) // setup
suite.SetupTest(false) // setup

// Same data for every test cases
accounts := suite.CreateTestAccounts(8)
Expand Down Expand Up @@ -997,7 +1006,7 @@ func (suite *AnteTestSuite) TestAnteHandlerSigLimitExceeded() {

// Test custom SignatureVerificationGasConsumer
func (suite *AnteTestSuite) TestCustomSignatureVerificationGasConsumer() {
suite.SetupTest(true) // setup
suite.SetupTest(false) // setup

// setup an ante handler that only accepts PubKeyEd25519
suite.anteHandler = ante.NewAnteHandler(suite.app.AccountKeeper, suite.app.BankKeeper, func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error {
Expand Down Expand Up @@ -1047,7 +1056,7 @@ func (suite *AnteTestSuite) TestCustomSignatureVerificationGasConsumer() {
}

func (suite *AnteTestSuite) TestAnteHandlerReCheck() {
suite.SetupTest(true) // setup
suite.SetupTest(false) // setup
// Set recheck=true
suite.ctx = suite.ctx.WithIsReCheckTx(true)
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder()
Expand Down Expand Up @@ -1120,7 +1129,9 @@ func (suite *AnteTestSuite) TestAnteHandlerReCheck() {

// remove funds for account so antehandler fails on recheck
suite.app.AccountKeeper.SetAccount(suite.ctx, accounts[0].acc)
suite.app.BankKeeper.SetBalances(suite.ctx, accounts[0].acc.GetAddress(), sdk.NewCoins())
balances := suite.app.BankKeeper.GetAllBalances(suite.ctx, accounts[0].acc.GetAddress())
err = suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, accounts[0].acc.GetAddress(), minttypes.ModuleName, balances)
suite.Require().NoError(err)

_, err = suite.anteHandler(suite.ctx, tx, false)
suite.Require().NotNil(err, "antehandler on recheck did not fail once feePayer no longer has sufficient funds")
Expand Down
Loading