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

check: fee distribution on failed tx #149

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 1 addition & 1 deletion tests/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestIntegrationTestSuite(t *testing.T) {
func (s *IntegrationTestSuite) SetupTest() {
s.encCfg = MakeTestEncodingConfig()

s.ctx, s.TestKeepers, s.TestMsgServers = testkeeper.NewTestSetup(s.T())
s.ctx, s.TestKeepers, s.TestMsgServers = testkeeper.NewTestSetup(s.T(), false)
}

func (s *IntegrationTestSuite) TestState() {
Expand Down
6 changes: 4 additions & 2 deletions testutils/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var additionalMaccPerms = map[string][]string{
}

// NewTestSetup returns initialized instances of all the keepers and message servers of the modules
func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context, TestKeepers, TestMsgServers) {
func NewTestSetup(t testing.TB, distributeFees bool, options ...testkeeper.SetupOption) (sdk.Context, TestKeepers, TestMsgServers) {
options = append(options, testkeeper.WithAdditionalModuleAccounts(additionalMaccPerms))

_, tk, tms := testkeeper.NewTestSetup(t, options...)
Expand All @@ -55,7 +55,9 @@ func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context,

err := feeMarketKeeper.SetState(ctx, feemarkettypes.DefaultState())
require.NoError(t, err)
err = feeMarketKeeper.SetParams(ctx, feemarkettypes.DefaultParams())
params := feemarkettypes.DefaultParams()
params.DistributeFees = distributeFees
err = feeMarketKeeper.SetParams(ctx, params)
require.NoError(t, err)

testKeepers := TestKeepers{
Expand Down
4 changes: 2 additions & 2 deletions x/feemarket/ante/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func TestAnteHandleMock(t *testing.T) {

for _, tc := range testCases {
t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, tc.Mock)
s := antesuite.SetupTestSuite(t, tc.Mock, false)
s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder()
args := tc.Malleate(s)

Expand Down Expand Up @@ -418,7 +418,7 @@ func TestAnteHandle(t *testing.T) {

for _, tc := range testCases {
t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, tc.Mock)
s := antesuite.SetupTestSuite(t, tc.Mock, false)
s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder()
args := tc.Malleate(s)

Expand Down
2 changes: 1 addition & 1 deletion x/feemarket/ante/feegrant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestEscrowFunds(t *testing.T) {
for name, stc := range cases {
tc := stc // to make scopelint happy
t.Run(name, func(t *testing.T) {
s := antesuite.SetupTestSuite(t, true)
s := antesuite.SetupTestSuite(t, true, false)
protoTxCfg := tx.NewTxConfig(codec.NewProtoCodec(s.EncCfg.InterfaceRegistry), tx.DefaultSignModes)
// this just tests our handler
dfd := feemarketante.NewFeeMarketCheckDecorator(s.AccountKeeper, s.MockBankKeeper, s.MockFeeGrantKeeper,
Expand Down
16 changes: 12 additions & 4 deletions x/feemarket/ante/suite/suite.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package suite

import (
"fmt"
"testing"

"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"

banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
Expand Down Expand Up @@ -106,11 +108,11 @@ func (s *TestSuite) SetAccountBalances(accounts []TestAccountBalance) {
}

// SetupTestSuite setups a new test, with new app, context, and anteHandler.
func SetupTestSuite(t *testing.T, mock bool) *TestSuite {
func SetupTestSuite(t *testing.T, mock, distributeFees bool) *TestSuite {
s := &TestSuite{}

s.EncCfg = MakeTestEncodingConfig()
ctx, testKeepers, _ := testkeeper.NewTestSetup(t)
ctx, testKeepers, _ := testkeeper.NewTestSetup(t, distributeFees)
s.Ctx = ctx

s.AccountKeeper = testKeepers.AccountKeeper
Expand Down Expand Up @@ -188,6 +190,7 @@ type TestCase struct {
ExpErr error
ExpectConsumedGas uint64
Mock bool
DistributeFees bool
}

type TestCaseArgs struct {
Expand Down Expand Up @@ -240,8 +243,13 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) {
tc.StateUpdate(s)
}

if tc.RunPost && anteErr == nil {
newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, true)
if tc.RunPost {
newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, anteErr == nil)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we should always run the post handler in these tests even if there is an ante handler error. https://docs.cosmos.network/v0.50/learn/advanced/baseapp#runmsgs

}

if tc.DistributeFees && !tc.Simulate && args.FeeAmount != nil {
postFeeBalance := s.BankKeeper.GetBalance(s.Ctx, s.AccountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName), args.FeeAmount.GetDenomByIndex(0))
require.True(t, postFeeBalance.Amount.Equal(math.ZeroInt()), fmt.Errorf("amounts not equal: %s, %s", postFeeBalance.Amount.String(), math.ZeroInt().String()))
}

if tc.ExpPass {
Expand Down
2 changes: 1 addition & 1 deletion x/feemarket/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (s *KeeperTestSuite) SetupTest() {
s.encCfg = MakeTestEncodingConfig()
s.authorityAccount = authtypes.NewModuleAddress(govtypes.ModuleName)
s.accountKeeper = mocks.NewAccountKeeper(s.T())
ctx, tk, tm := testkeeper.NewTestSetup(s.T())
ctx, tk, tm := testkeeper.NewTestSetup(s.T(), false)

s.ctx = ctx
s.feeMarketKeeper = tk.FeeMarketKeeper
Expand Down
1 change: 1 addition & 0 deletions x/feemarket/post/expected_keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type BankKeeper interface {
SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
SendCoinsFromModuleToModule(ctx context.Context, senderModule, recipientModule string, amt sdk.Coins) error
SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
GetAllBalances(ctx context.Context, addr sdk.AccAddress) sdk.Coins
}

// FeeMarketKeeper defines the expected feemarket keeper.
Expand Down
122 changes: 122 additions & 0 deletions x/feemarket/post/fee_distribute_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package post_test

import (
"fmt"

Check failure on line 4 in x/feemarket/post/fee_distribute_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `gofumpt`-ed (gofumpt)
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"testing"

Check failure on line 6 in x/feemarket/post/fee_distribute_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `gofumpt`-ed (gofumpt)

"cosmossdk.io/math"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/mock"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
antesuite "github.com/skip-mev/feemarket/x/feemarket/ante/suite"
"github.com/skip-mev/feemarket/x/feemarket/post"
"github.com/skip-mev/feemarket/x/feemarket/types"
)

func TestPostHandleDistributeFeesMock(t *testing.T) {
// Same data for every test case
const (
baseDenom = "stake"
resolvableDenom = "atom"
expectedConsumedGas = 11730
expectedConsumedSimGas = expectedConsumedGas + post.BankSendGasConsumption
gasLimit = expectedConsumedSimGas
)

validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit))
validFeeAmountWithTip := validFeeAmount.Add(math.LegacyNewDec(100))
validFeeWithTip := sdk.NewCoins(sdk.NewCoin(baseDenom, validFeeAmountWithTip.TruncateInt()))

testCases := []antesuite.TestCase{
{
Name: "signer has enough funds, should pass with 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).Once()
s.MockBankKeeper.On("SendCoinsFromModuleToModule", mock.Anything, types.FeeCollectorName, authtypes.FeeCollectorName, mock.Anything).Return(nil).Once()
s.MockBankKeeper.On("SendCoinsFromModuleToAccount", mock.Anything, types.FeeCollectorName, 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: false,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
Mock: true,
DistributeFees: true,
},
}

for _, tc := range testCases {
t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, tc.Mock, true)
s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder()
args := tc.Malleate(s)

s.RunTestCase(t, tc, args)
})
}
}

func TestPostHandleDistributeFees(t *testing.T) {
// Same data for every test case
const (
baseDenom = "stake"
resolvableDenom = "atom"
expectedConsumedGas = 65558

gasLimit = 100000
)

validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit))
validFeeAmountWithTip := validFeeAmount.Add(math.LegacyNewDec(100))
validFeeWithTip := sdk.NewCoins(sdk.NewCoin(baseDenom, validFeeAmountWithTip.TruncateInt()))

testCases := []antesuite.TestCase{
{
Name: "signer has enough funds, gaslimit is not enough to complete entire transaction, should pass",
Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := s.CreateTestAccounts(1)

balance := antesuite.TestAccountBalance{
TestAccount: accs[0],
Coins: validFeeWithTip,
}
s.SetAccountBalances([]antesuite.TestAccountBalance{balance})

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 35188,
FeeAmount: validFeeWithTip,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: false,
ExpErr: sdkerrors.ErrOutOfGas,
ExpectConsumedGas: 65558,
Mock: false,
DistributeFees: true,
},
}

for _, tc := range testCases {
t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, tc.Mock, true)
s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder()
args := tc.Malleate(s)

s.RunTestCase(t, tc, args)
})
}
}
10 changes: 5 additions & 5 deletions x/feemarket/post/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestDeductCoins(t *testing.T) {
}
for _, tc := range tests {
t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, true)
s := antesuite.SetupTestSuite(t, true, tc.distributeFees)
if tc.distributeFees {
s.MockBankKeeper.On("SendCoinsFromModuleToModule", s.Ctx, types.FeeCollectorName,
authtypes.FeeCollectorName,
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestDeductCoinsAndDistribute(t *testing.T) {
}
for _, tc := range tests {
t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, true)
s := antesuite.SetupTestSuite(t, true, false)
s.MockBankKeeper.On("SendCoinsFromModuleToModule", s.Ctx, types.FeeCollectorName, authtypes.FeeCollectorName,
tc.coins).Return(nil).Once()

Expand Down Expand Up @@ -136,7 +136,7 @@ func TestSendTip(t *testing.T) {
}
for _, tc := range tests {
t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, true)
s := antesuite.SetupTestSuite(t, true, false)
accs := s.CreateTestAccounts(2)
s.MockBankKeeper.On("SendCoinsFromModuleToAccount", s.Ctx, types.FeeCollectorName, mock.Anything,
tc.coins).Return(nil).Once()
Expand Down Expand Up @@ -525,7 +525,7 @@ func TestPostHandleMock(t *testing.T) {

for _, tc := range testCases {
t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, tc.Mock)
s := antesuite.SetupTestSuite(t, tc.Mock, false)
s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder()
args := tc.Malleate(s)

Expand Down Expand Up @@ -983,7 +983,7 @@ func TestPostHandle(t *testing.T) {

for _, tc := range testCases {
t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) {
s := antesuite.SetupTestSuite(t, tc.Mock)
s := antesuite.SetupTestSuite(t, tc.Mock, false)
s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder()
args := tc.Malleate(s)

Expand Down
11 changes: 6 additions & 5 deletions x/feemarket/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ var (
// KeyEnabledHeight is the store key for the feemarket module's enabled height.
KeyEnabledHeight = []byte{prefixEnableHeight}

EventTypeFeePay = "fee_pay"
EventTypeTipPay = "tip_pay"
AttributeKeyTip = "tip"
AttributeKeyTipPayer = "tip_payer"
AttributeKeyTipPayee = "tip_payee"
EventTypeFeePay = "fee_pay"
EventTypeTipPay = "tip_pay"
EventTypeDistributeStuckEscrowFees = "distribute_stuck_escrow_fees"
AttributeKeyTip = "tip"
AttributeKeyTipPayer = "tip_payer"
AttributeKeyTipPayee = "tip_payee"
)
Loading