Skip to content

[Bank refactor]: finalize removal of setbalances from auth #8527

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Feb 8, 2021
Merged

[Bank refactor]: finalize removal of setbalances from auth #8527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b370b0c
add tests with is check tx
jgimeno Feb 5, 2021
5eedc1c
temp commit
jgimeno Feb 5, 2021
f15bbb1
fix test
jgimeno Feb 5, 2021
899f5ad
fix other test and remove setbalances
jgimeno Feb 5, 2021
fff229b
change(auth): remove usage of SetBalances is vesting tests
fdymylja Feb 8, 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
31 changes: 21 additions & 10 deletions x/auth/ante/ante_test.go
View file
Open in desktop
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"

"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
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) // TODO(fdymylja): IDK
err := suite.app.BankKeeper.MintCoins(suite.ctx, minttypes.ModuleName, feeAmount)
Copy link
Contributor Author

@fdymylja fdymylja Feb 5, 2021

Choose a reason for hiding this comment

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

@jgimeno can we use the simapp.FundAccount here?

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))) // TODO(fdymylja): IDK
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))) // TODO(fdymylja): IDK
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()) // TODO(fdymylja): IDK
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
11 changes: 7 additions & 4 deletions x/auth/ante/fee_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package ante_test

import (
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/testutil/testdata"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
)
Expand Down Expand Up @@ -61,7 +61,7 @@ func (suite *AnteTestSuite) TestEnsureMempoolFees() {
}

func (suite *AnteTestSuite) TestDeductFees() {
suite.SetupTest(true) // setup
suite.SetupTest(false) // setup
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder()

// keys and addresses
Expand All @@ -82,7 +82,9 @@ func (suite *AnteTestSuite) TestDeductFees() {
// Set account with insufficient funds
acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr1)
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
suite.app.BankKeeper.SetBalances(suite.ctx, addr1, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(10)))) // TODO(fdymylja): IDK
coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(10)))
err = simapp.FundAccount(suite.app, suite.ctx, addr1, coins)
suite.Require().NoError(err)

dfd := ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper)
antehandler := sdk.ChainAnteDecorators(dfd)
Expand All @@ -93,7 +95,8 @@ func (suite *AnteTestSuite) TestDeductFees() {

// Set account with sufficient funds
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
suite.app.BankKeeper.SetBalances(suite.ctx, addr1, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(200))))
err = simapp.FundAccount(suite.app, suite.ctx, addr1, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(200))))
suite.Require().NoError(err)

_, err = antehandler(suite.ctx, tx, false)

Expand Down
11 changes: 9 additions & 2 deletions x/auth/ante/testutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"testing"

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

"github.com/stretchr/testify/suite"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

Expand Down Expand Up @@ -75,9 +77,14 @@ func (suite *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount {
err := acc.SetAccountNumber(uint64(i))
suite.Require().NoError(err)
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
suite.app.BankKeeper.SetBalances(suite.ctx, addr, sdk.Coins{
someCoins := sdk.Coins{
sdk.NewInt64Coin("atom", 10000000),
}) // TODO(fdymylja): IDK
}
err = suite.app.BankKeeper.MintCoins(suite.ctx, minttypes.ModuleName, someCoins)
suite.Require().NoError(err)

err = suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, minttypes.ModuleName, addr, someCoins)
suite.Require().NoError(err)

accounts = append(accounts, TestAccount{acc, priv})
}
Expand Down
2 changes: 1 addition & 1 deletion x/auth/vesting/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (suite *HandlerTestSuite) TestMsgCreateVestingAccount() {

acc1 := suite.app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
suite.app.AccountKeeper.SetAccount(ctx, acc1)
suite.Require().NoError(suite.app.BankKeeper.SetBalances(ctx, addr1, balances))
suite.Require().NoError(simapp.FundAccount(suite.app, ctx, addr1, balances))

testCases := []struct {
name string
Expand Down