diff --git a/docs/INTEGRATIONS.md b/docs/INTEGRATIONS.md index 81e6ad4..f2188da 100644 --- a/docs/INTEGRATIONS.md +++ b/docs/INTEGRATIONS.md @@ -67,7 +67,7 @@ There are two ways to construct a transaction with `gasPrice`: ### Understanding Fee Deducted -The actual amount of fee deducted from the fee payer is based on gas consumed, not `gasLimit`. +The actual amount of fee deducted from the fee payer is based on gas consumed, not `gasLimit`. The total amount deducted (`fee + tip`) will be equal to the amount of fee specified on your transaction. The amount consumed is equal to the `inferredTip + gasPrice * gasConsumed`, where `inferredTip = feeAmount - gasLimit * gasPrice` (This may be different than the tip you specified when building the transaction because the `gasPrice` on chain may have changed since when you queried it.) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index a3b4546..84b944a 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -135,7 +135,6 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula } ctx = ctx.WithPriority(GetTxPriority(priorityFee, int64(gas), baseGasPrice)) - return next(ctx, tx, simulate) } return next(ctx, tx, simulate) } diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 2997579..18a5b24 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -2,6 +2,7 @@ package post import ( "bytes" + "cosmossdk.io/math" "fmt" errorsmod "cosmossdk.io/errors" @@ -54,11 +55,6 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - var ( - tip sdk.Coin - payCoin sdk.Coin - ) - // update fee market params params, err := dfd.feemarketKeeper.GetParams(ctx) if err != nil { @@ -89,6 +85,11 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul feeCoin := feeCoins[0] feeGas := int64(feeTx.GetGas()) + var ( + tip = sdk.NewCoin(feeCoin.Denom, math.ZeroInt()) + payCoin = feeCoin + ) + minGasPrice, err := dfd.feemarketKeeper.GetMinGasPrice(ctx, feeCoin.GetDenom()) if err != nil { return ctx, errorsmod.Wrapf(err, "unable to get min gas price for denom %s", feeCoins[0].GetDenom()) @@ -107,7 +108,7 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul } ctx.Logger().Info("fee deduct post handle", - "fee", feeCoins, + "fee", payCoin, "tip", tip, ) @@ -160,9 +161,11 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T if dfd.feegrantKeeper == nil { return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") } else if !bytes.Equal(feeGranter, feePayer) { - err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, sdk.NewCoins(fee), sdkTx.GetMsgs()) - if err != nil { - return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) + if !fee.IsNil() { + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, sdk.NewCoins(fee), sdkTx.GetMsgs()) + if err != nil { + return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer) + } } } @@ -177,7 +180,7 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T var events sdk.Events // deduct the fees and tip - if !fee.Amount.IsNil() && !fee.IsZero() { + if !fee.IsNil() { err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, sdk.NewCoins(fee), distributeFees) if err != nil { return err @@ -191,7 +194,7 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T } proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress) - if !tip.Amount.IsNil() && !tip.IsZero() { + if !tip.IsNil() { err := SendTip(dfd.bankKeeper, ctx, deductFeesFromAcc.GetAddress(), proposer, sdk.NewCoins(tip)) if err != nil { return err @@ -212,10 +215,6 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T // DeductCoins deducts coins from the given account. // Coins can be sent to the default fee collector (causes coins to be distributed to stakers) or sent to the feemarket fee collector account (causes coins to be burned). func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc sdk.AccountI, coins sdk.Coins, distributeFees bool) error { - if !coins.IsValid() { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins) - } - targetModuleAcc := feemarkettypes.FeeCollectorName if distributeFees { targetModuleAcc = authtypes.FeeCollectorName @@ -231,10 +230,6 @@ func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc sdk.AccountI, coins // SendTip sends a tip to the current block proposer. func SendTip(bankKeeper BankKeeper, ctx sdk.Context, acc, proposer sdk.AccAddress, coins sdk.Coins) error { - if !coins.IsValid() { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins) - } - err := bankKeeper.SendCoins(ctx, acc, proposer, coins) if err != nil { return err diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go index 48dc741..a23a5bd 100644 --- a/x/feemarket/post/fee_test.go +++ b/x/feemarket/post/fee_test.go @@ -18,10 +18,9 @@ import ( func TestDeductCoins(t *testing.T) { tests := []struct { - name string - coins sdk.Coins - wantErr bool - invalidCoin bool + name string + coins sdk.Coins + wantErr bool }{ { name: "valid", @@ -34,25 +33,16 @@ func TestDeductCoins(t *testing.T) { wantErr: false, }, { - name: "invalid coins negative amount", - coins: sdk.Coins{sdk.Coin{Denom: "test", Amount: math.NewInt(-1)}}, - wantErr: true, - invalidCoin: true, - }, - { - name: "invalid coins invalid denom", - coins: sdk.Coins{sdk.Coin{Amount: math.NewInt(1)}}, - wantErr: true, - invalidCoin: true, + name: "valid zero coin", + coins: sdk.NewCoins(sdk.NewCoin("test", math.ZeroInt())), + wantErr: false, }, } for _, tc := range tests { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { s := antesuite.SetupTestSuite(t, true) acc := s.CreateTestAccounts(1)[0] - if !tc.invalidCoin { - s.MockBankKeeper.On("SendCoinsFromAccountToModule", s.Ctx, acc.Account.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() - } + s.MockBankKeeper.On("SendCoinsFromAccountToModule", s.Ctx, acc.Account.GetAddress(), types.FeeCollectorName, tc.coins).Return(nil).Once() if err := post.DeductCoins(s.MockBankKeeper, s.Ctx, acc.Account, tc.coins, false); (err != nil) != tc.wantErr { s.Errorf(err, "DeductCoins() error = %v, wantErr %v", err, tc.wantErr) @@ -63,10 +53,9 @@ func TestDeductCoins(t *testing.T) { func TestDeductCoinsAndDistribute(t *testing.T) { tests := []struct { - name string - coins sdk.Coins - wantErr bool - invalidCoin bool + name string + coins sdk.Coins + wantErr bool }{ { name: "valid", @@ -79,25 +68,16 @@ func TestDeductCoinsAndDistribute(t *testing.T) { wantErr: false, }, { - name: "invalid coins negative amount", - coins: sdk.Coins{sdk.Coin{Denom: "test", Amount: math.NewInt(-1)}}, - wantErr: true, - invalidCoin: true, - }, - { - name: "invalid coins invalid denom", - coins: sdk.Coins{sdk.Coin{Amount: math.NewInt(1)}}, - wantErr: true, - invalidCoin: true, + name: "valid zero coin", + coins: sdk.NewCoins(sdk.NewCoin("test", math.ZeroInt())), + wantErr: false, }, } for _, tc := range tests { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { s := antesuite.SetupTestSuite(t, true) acc := s.CreateTestAccounts(1)[0] - if !tc.invalidCoin { - s.MockBankKeeper.On("SendCoinsFromAccountToModule", s.Ctx, acc.Account.GetAddress(), authtypes.FeeCollectorName, tc.coins).Return(nil).Once() - } + s.MockBankKeeper.On("SendCoinsFromAccountToModule", s.Ctx, acc.Account.GetAddress(), authtypes.FeeCollectorName, tc.coins).Return(nil).Once() if err := post.DeductCoins(s.MockBankKeeper, s.Ctx, acc.Account, tc.coins, true); (err != nil) != tc.wantErr { s.Errorf(err, "DeductCoins() error = %v, wantErr %v", err, tc.wantErr) @@ -108,10 +88,9 @@ func TestDeductCoinsAndDistribute(t *testing.T) { func TestSendTip(t *testing.T) { tests := []struct { - name string - coins sdk.Coins - wantErr bool - invalidCoin bool + name string + coins sdk.Coins + wantErr bool }{ { name: "valid", @@ -124,19 +103,16 @@ func TestSendTip(t *testing.T) { wantErr: false, }, { - name: "invalid coins", - coins: sdk.Coins{sdk.Coin{Amount: math.NewInt(-1)}}, - wantErr: true, - invalidCoin: true, + name: "valid zero coin", + coins: sdk.NewCoins(sdk.NewCoin("test", math.ZeroInt())), + wantErr: false, }, } for _, tc := range tests { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { s := antesuite.SetupTestSuite(t, true) accs := s.CreateTestAccounts(2) - if !tc.invalidCoin { - s.MockBankKeeper.On("SendCoins", s.Ctx, mock.Anything, mock.Anything, tc.coins).Return(nil).Once() - } + s.MockBankKeeper.On("SendCoins", s.Ctx, mock.Anything, mock.Anything, tc.coins).Return(nil).Once() if err := post.SendTip(s.MockBankKeeper, s.Ctx, accs[0].Account.GetAddress(), accs[1].Account.GetAddress(), tc.coins); (err != nil) != tc.wantErr { s.Errorf(err, "SendTip() error = %v, wantErr %v", err, tc.wantErr) @@ -180,10 +156,28 @@ func TestPostHandle(t *testing.T) { ExpPass: false, ExpErr: sdkerrors.ErrInsufficientFunds, }, + { + Name: "signer has no funds - simulate", + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) + s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(sdkerrors.ErrInsufficientFunds) + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validFee, + } + }, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: false, + ExpErr: sdkerrors.ErrInsufficientFunds, + }, { Name: "0 gas given should fail", - Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs { - accs := suite.CreateTestAccounts(1) + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) return antesuite.TestCaseArgs{ Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, @@ -197,11 +191,31 @@ func TestPostHandle(t *testing.T) { ExpPass: false, ExpErr: sdkerrors.ErrInvalidGasLimit, }, + { + Name: "0 gas given should pass - simulate", + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) + s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + s.MockBankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: 0, + FeeAmount: validFee, + } + }, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + }, { Name: "signer has enough funds, should pass, no tip", Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { accs := s.CreateTestAccounts(1) s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + s.MockBankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() return antesuite.TestCaseArgs{ Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, @@ -234,11 +248,31 @@ func TestPostHandle(t *testing.T) { ExpPass: true, ExpErr: nil, }, + { + Name: "signer has enough funds, should pass with tip - simulate", + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) + s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + s.MockBankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validFeeWithTip, + } + }, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + }, { Name: "signer has enough funds, should pass, no tip - resolvable denom", Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { accs := s.CreateTestAccounts(1) s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + s.MockBankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() return antesuite.TestCaseArgs{ Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, @@ -252,6 +286,25 @@ func TestPostHandle(t *testing.T) { ExpPass: true, ExpErr: nil, }, + { + Name: "signer has enough funds, should pass, no tip - resolvable denom - simulate", + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) + s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + s.MockBankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validResolvableFee, + } + }, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + }, { Name: "signer has enough funds, should pass with tip - resolvable denom", Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { @@ -271,6 +324,25 @@ func TestPostHandle(t *testing.T) { ExpPass: true, ExpErr: nil, }, + { + Name: "signer has enough funds, should pass with tip - resolvable denom - simulate", + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) + s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), types.FeeCollectorName, mock.Anything).Return(nil) + s.MockBankKeeper.On("SendCoins", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validResolvableFeeWithTip, + } + }, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + }, } for _, tc := range testCases {