From 461f88ce3a57f6ec4f75b0b1bfcd0fee6f05980c Mon Sep 17 00:00:00 2001 From: Alex Johnson Date: Wed, 29 Nov 2023 11:18:52 -0500 Subject: [PATCH] test: boost coverage (#35) * go mod * add ante * set antehandler * cute * lint fix * setup and wire * wire fmk * clean * format * todo * lint fix * rename * wip * proto * add param * add get * test * format * re-wire * add ak * fee decorator * mock * always check and use fee * extend with tip * fix lint * refactor * utd * utd * setup test * attempt test * fmt * clean * add * fix * rename * set posthandler * set posthandler * setup post * finalize * add options * clean * comments * clean * rename * use names * add coverage to make * add case * wip * fix * clean * clean --- .gitignore | 3 + Makefile | 12 ++ x/feemarket/ante/fee.go | 4 +- x/feemarket/post/fee_test.go | 12 +- x/feemarket/post/feegrant_test.go | 220 ++++++++++++++++++++++++++++++ 5 files changed, 246 insertions(+), 5 deletions(-) create mode 100644 x/feemarket/post/feegrant_test.go diff --git a/.gitignore b/.gitignore index 72beac0..34c4e7c 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,6 @@ go.work* # editor .idea/ + +# coverage report +cover.html diff --git a/Makefile b/Makefile index 3b0213f..e21f5ba 100644 --- a/Makefile +++ b/Makefile @@ -9,6 +9,8 @@ BUILD_DIR ?= $(CURDIR)/build PROJECT_NAME = $(shell git remote get-url origin | xargs basename -s .git) HTTPS_GIT := https://github.com/skip-mev/feemarket.git DOCKER := $(shell which docker) +COVER_FILE := coverage.txt +COVER_HTML_FILE := cover.html ############################################################################### ## Workspaces ## @@ -127,6 +129,16 @@ test-integration: $(TEST_INTEGRATION_DEPS) test: @go test -v -race $(shell go list ./... | grep -v tests/) +## test-cover: Run the unit tests and create a coverage html report +test-cover: + @echo Running unit tests and creating coverage report... + @go test -mod=readonly -v -timeout 30m -coverprofile=$(COVER_FILE) -covermode=atomic $(shell go list ./... | grep -v tests/ | grep -v api/ | grep -v testutils/) + @sed -i '/.pb.go/d' $(COVER_FILE) + @sed -i '/.gw.go/d' $(COVER_FILE) + @go tool cover -html=$(COVER_FILE) -o $(COVER_HTML_FILE) + @rm $(COVER_FILE) + + .PHONY: test test-integration ############################################################################### diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 709a070..53dc0e9 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -38,7 +38,7 @@ func (dfd FeeMarketCheckDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } if !simulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 { - return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") + return ctx, sdkerrors.ErrInvalidGasLimit.Wrapf("must provide positive gas") } minGasPrices, err := dfd.feemarketKeeper.GetMinGasPrices(ctx) @@ -82,7 +82,7 @@ func CheckTxFees(minFees sdk.Coins, feeTx sdk.FeeTx, gas uint64) (feeCoins sdk.C } if !feeCoins.IsAnyGTE(requiredFees) { - return nil, nil, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + return nil, nil, sdkerrors.ErrInsufficientFee.Wrapf("got: %s required: %s", feeCoins, requiredFees) } tip = feeCoins.Sub(minFees...) // tip is the difference between feeCoins and the min fees diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go index 17b0e3a..d221ed9 100644 --- a/x/feemarket/post/fee_test.go +++ b/x/feemarket/post/fee_test.go @@ -33,8 +33,14 @@ func TestDeductCoins(t *testing.T) { wantErr: false, }, { - name: "invalid coins", - coins: sdk.Coins{sdk.Coin{Amount: sdk.NewInt(-1)}}, + name: "invalid coins negative amount", + coins: sdk.Coins{sdk.Coin{Denom: "test", Amount: sdk.NewInt(-1)}}, + wantErr: true, + invalidCoin: true, + }, + { + name: "invalid coins invalid denom", + coins: sdk.Coins{sdk.Coin{Amount: sdk.NewInt(1)}}, wantErr: true, invalidCoin: true, }, @@ -87,7 +93,7 @@ func TestSendTip(t *testing.T) { } if err := post.SendTip(s.BankKeeper, s.Ctx, accs[0].Account.GetAddress(), accs[1].Account.GetAddress(), tc.coins); (err != nil) != tc.wantErr { - s.Errorf(err, "SendCoins() error = %v, wantErr %v", err, tc.wantErr) + s.Errorf(err, "SendTip() error = %v, wantErr %v", err, tc.wantErr) } }) } diff --git a/x/feemarket/post/feegrant_test.go b/x/feemarket/post/feegrant_test.go new file mode 100644 index 0000000..ae5a1fb --- /dev/null +++ b/x/feemarket/post/feegrant_test.go @@ -0,0 +1,220 @@ +package post_test + +import ( + "math/rand" + "testing" + "time" + + "github.com/stretchr/testify/mock" + + "github.com/skip-mev/feemarket/x/feemarket/types" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/codec" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/simulation" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authsign "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/cosmos/cosmos-sdk/x/auth/tx" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/feegrant" + + antesuite "github.com/skip-mev/feemarket/x/feemarket/ante/suite" + feemarketpost "github.com/skip-mev/feemarket/x/feemarket/post" +) + +func TestDeductFeesNoDelegation(t *testing.T) { + cases := map[string]struct { + fee int64 + valid bool + err error + malleate func(*antesuite.TestSuite) (signer antesuite.TestAccount, feeAcc sdk.AccAddress) + }{ + "paying with insufficient fee": { + fee: 50, + valid: false, + err: sdkerrors.ErrInsufficientFee, + malleate: func(suite *antesuite.TestSuite) (antesuite.TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(1) + return accs[0], nil + }, + }, + "paying with good funds": { + fee: 24497000000, + valid: true, + malleate: func(suite *antesuite.TestSuite) (antesuite.TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(1) + suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil).Once() + suite.BankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + + return accs[0], nil + }, + }, + "paying with no account": { + fee: 24497000000, + valid: false, + err: sdkerrors.ErrUnknownAddress, + malleate: func(suite *antesuite.TestSuite) (antesuite.TestAccount, sdk.AccAddress) { + // Do not register the account + priv, _, addr := testdata.KeyTestPubAddr() + return antesuite.TestAccount{ + Account: authtypes.NewBaseAccountWithAddress(addr), + Priv: priv, + }, nil + }, + }, + "valid fee grant": { + // note: the original test said "valid fee grant with no account". + // this is impossible given that feegrant.GrantAllowance calls + // SetAccount for the grantee. + fee: 36630000000, + valid: true, + malleate: func(suite *antesuite.TestSuite) (antesuite.TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + suite.FeeGrantKeeper.On("UseGrantedFees", mock.Anything, accs[1].Account.GetAddress(), accs[0].Account.GetAddress(), mock.Anything, mock.Anything).Return(nil).Once() + suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[1].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil).Once() + suite.BankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + + return accs[0], accs[1].Account.GetAddress() + }, + }, + "no fee grant": { + fee: 36630000000, + valid: false, + err: sdkerrors.ErrNotFound, + malleate: func(suite *antesuite.TestSuite) (antesuite.TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + suite.FeeGrantKeeper.On( + "UseGrantedFees", mock.Anything, accs[1].Account.GetAddress(), accs[0].Account.GetAddress(), mock.Anything, mock.Anything). + Return(sdkerrors.ErrNotFound.Wrap("fee-grant not found")). + Once() + return accs[0], accs[1].Account.GetAddress() + }, + }, + "allowance smaller than requested fee": { + fee: 36630000000, + valid: false, + err: feegrant.ErrFeeLimitExceeded, + malleate: func(suite *antesuite.TestSuite) (antesuite.TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + suite.FeeGrantKeeper.On( + "UseGrantedFees", mock.Anything, accs[1].Account.GetAddress(), accs[0].Account.GetAddress(), mock.Anything, mock.Anything). + Return(feegrant.ErrFeeLimitExceeded.Wrap("basic allowance")). + Once() + return accs[0], accs[1].Account.GetAddress() + }, + }, + "granter cannot cover allowed fee grant": { + fee: 36630000000, + valid: false, + err: sdkerrors.ErrInsufficientFunds, + malleate: func(suite *antesuite.TestSuite) (antesuite.TestAccount, sdk.AccAddress) { + accs := suite.CreateTestAccounts(2) + suite.FeeGrantKeeper.On("UseGrantedFees", mock.Anything, accs[1].Account.GetAddress(), accs[0].Account.GetAddress(), mock.Anything, mock.Anything).Return(nil).Once() + suite.BankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[1].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds).Once() + return accs[0], accs[1].Account.GetAddress() + }, + }, + } + + for name, stc := range cases { + tc := stc // to make scopelint happy + t.Run(name, func(t *testing.T) { + suite := antesuite.SetupTestSuite(t) + protoTxCfg := tx.NewTxConfig(codec.NewProtoCodec(suite.EncCfg.InterfaceRegistry), tx.DefaultSignModes) + // this just tests our handler + dfd := feemarketpost.NewFeeMarketDeductDecorator(suite.AccountKeeper, suite.BankKeeper, suite.FeeGrantKeeper, suite.FeemarketKeeper) + feePostHandler := sdk.ChainPostDecorators(dfd) + + signer, feeAcc := stc.malleate(suite) + + fee := sdk.NewCoins(sdk.NewInt64Coin("stake", tc.fee)) + msgs := []sdk.Msg{testdata.NewTestMsg(signer.Account.GetAddress())} + + acc := suite.AccountKeeper.GetAccount(suite.Ctx, signer.Account.GetAddress()) + privs, accNums, seqs := []cryptotypes.PrivKey{signer.Priv}, []uint64{0}, []uint64{0} + if acc != nil { + accNums, seqs = []uint64{acc.GetAccountNumber()}, []uint64{acc.GetSequence()} + } + + var defaultGenTxGas uint64 = 10 + tx, err := genTxWithFeeGranter(protoTxCfg, msgs, fee, defaultGenTxGas, suite.Ctx.ChainID(), accNums, seqs, feeAcc, privs...) + require.NoError(t, err) + _, err = feePostHandler(suite.Ctx, tx, false, true) // tests only feegrant post + if tc.valid { + require.NoError(t, err) + } else { + require.ErrorIs(t, err, tc.err) + } + }) + } +} + +func genTxWithFeeGranter(gen client.TxConfig, msgs []sdk.Msg, feeAmt sdk.Coins, gas uint64, chainID string, accNums, + accSeqs []uint64, feeGranter sdk.AccAddress, priv ...cryptotypes.PrivKey, +) (sdk.Tx, error) { + sigs := make([]signing.SignatureV2, len(priv)) + + // create a random length memo + r := rand.New(rand.NewSource(time.Now().UnixNano())) + + memo := simulation.RandStringOfLength(r, simulation.RandIntBetween(r, 0, 100)) + + signMode := gen.SignModeHandler().DefaultMode() + + // 1st round: set SignatureV2 with empty signatures, to set correct + // signer infos. + for i, p := range priv { + sigs[i] = signing.SignatureV2{ + PubKey: p.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: signMode, + }, + Sequence: accSeqs[i], + } + } + + tx := gen.NewTxBuilder() + err := tx.SetMsgs(msgs...) + if err != nil { + return nil, err + } + err = tx.SetSignatures(sigs...) + if err != nil { + return nil, err + } + tx.SetMemo(memo) + tx.SetFeeAmount(feeAmt) + tx.SetGasLimit(gas) + tx.SetFeeGranter(feeGranter) + + // 2nd round: once all signer infos are set, every signer can sign. + for i, p := range priv { + signerData := authsign.SignerData{ + ChainID: chainID, + AccountNumber: accNums[i], + Sequence: accSeqs[i], + } + signBytes, err := gen.SignModeHandler().GetSignBytes(signMode, signerData, tx.GetTx()) + if err != nil { + panic(err) + } + sig, err := p.Sign(signBytes) + if err != nil { + panic(err) + } + sigs[i].Data.(*signing.SingleSignatureData).Signature = sig + err = tx.SetSignatures(sigs...) + if err != nil { + panic(err) + } + } + + return tx.GetTx(), nil +}