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

fix: stuck escrow fees #194

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,7 @@ func (app *App) setAnteHandler(txConfig client.TxConfig, wasmConfig wasmtypes.Wa

func (app *App) setPostHandler() {
postHandler := PostHandlerOptions{
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
FeeMarketKeeper: app.FeeMarketKeeper,
}
Expand Down
6 changes: 6 additions & 0 deletions app/post_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@ import (

// PostHandlerOptions are the options required for constructing a FeeMarket PostHandler.
type PostHandlerOptions struct {
AccountKeeper xfeemarketpost.AccountKeeper
BankKeeper xfeemarketpost.BankKeeper
FeeMarketKeeper xfeemarketpost.FeeMarketKeeper
}

// NewPostHandler returns a PostHandler chain with the fee deduct decorator.
func NewPostHandler(options PostHandlerOptions) (sdk.PostHandler, error) {
if options.AccountKeeper == nil {
return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "account keeper is required for post builder")
}

if options.BankKeeper == nil {
return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "bank keeper is required for post builder")
}
Expand All @@ -25,6 +30,7 @@ func NewPostHandler(options PostHandlerOptions) (sdk.PostHandler, error) {

postDecorators := []sdk.PostDecorator{
xfeemarketpost.NewFeeMarketDeductDecorator(
options.AccountKeeper,
options.BankKeeper,
options.FeeMarketKeeper,
),
Expand Down
10 changes: 10 additions & 0 deletions x/xfeemarket/post/expected_keepers.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
package post

import (
"context"

sdk "github.com/cosmos/cosmos-sdk/types"
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
feemarkettypes "github.com/skip-mev/feemarket/x/feemarket/types"
)

// AccountKeeper defines the contract needed for AccountKeeper related APIs.
// Interface provides support to use non-sdk AccountKeeper for AnteHandler's decorators.
//
//go:generate mockery --name AccountKeeper --filename mock_account_keeper.go
type AccountKeeper interface {
GetModuleAccount(ctx context.Context, name string) sdk.ModuleAccountI
}

// BankKeeper defines the contract needed for supply related APIs.
//
//go:generate mockery --name BankKeeper --filename mock_bank_keeper.go
Expand Down
46 changes: 32 additions & 14 deletions x/xfeemarket/post/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ const BankSendGasConsumption = 12490 + 37325
// Call next PostHandler if fees successfully deducted.
// CONTRACT: Tx must implement FeeTx interface
type FeeMarketDeductDecorator struct {
accountKeeper AccountKeeper
bankKeeper BankKeeper
feemarketKeeper FeeMarketKeeper
}

func NewFeeMarketDeductDecorator(bk BankKeeper, fmk FeeMarketKeeper) FeeMarketDeductDecorator {
func NewFeeMarketDeductDecorator(ak AccountKeeper, bk BankKeeper, fmk FeeMarketKeeper) FeeMarketDeductDecorator {
return FeeMarketDeductDecorator{
accountKeeper: ak,
bankKeeper: bk,
feemarketKeeper: fmk,
}
Expand Down Expand Up @@ -148,19 +150,6 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul
func (dfd FeeMarketDeductDecorator) BurnFeeAndRefund(ctx sdk.Context, fee, tip sdk.Coin, feePayer sdk.AccAddress, defaultFeeDenom string) error {
var events sdk.Events

// burn the fees if it is the default fee denom
if !fee.IsNil() && !fee.IsZero() && fee.Denom == defaultFeeDenom {
err := BurnCoins(dfd.bankKeeper, ctx, sdk.NewCoins(fee))
if err != nil {
return err
}

events = append(events, sdk.NewEvent(
feemarkettypes.EventTypeFeePay,
sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()),
))
}

// refund the tip if it is not nil and non zero
if !tip.IsNil() && !tip.IsZero() {
err := RefundTip(dfd.bankKeeper, ctx, feePayer, sdk.NewCoins(tip))
Expand All @@ -175,6 +164,35 @@ func (dfd FeeMarketDeductDecorator) BurnFeeAndRefund(ctx sdk.Context, fee, tip s
))
}

// fees from previous transactions may be stuck in escrow if transaction is out of gas
// burn both the fee and the stuck escrowed fees
feemarketCollector := dfd.accountKeeper.GetModuleAccount(ctx, feemarkettypes.FeeCollectorName)
burnCoin := dfd.bankKeeper.GetBalance(ctx, feemarketCollector.GetAddress(), defaultFeeDenom)
stuckEscrowedFee := sdk.NewCoin(defaultFeeDenom, math.ZeroInt())
if fee.Denom == defaultFeeDenom && burnCoin.Amount.GT(fee.Amount) {
stuckEscrowedFee = burnCoin.Sub(fee)
}
if burnCoin.IsPositive() {
err := BurnCoins(dfd.bankKeeper, ctx, sdk.NewCoins(burnCoin))
if err != nil {
return err
}

if !fee.IsNil() && !fee.IsZero() && fee.Denom == defaultFeeDenom {
events = append(events, sdk.NewEvent(
feemarkettypes.EventTypeFeePay,
sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()),
))
}

if stuckEscrowedFee.IsPositive() {
events = append(events, sdk.NewEvent(
xfeemarkettypes.EventTypeBurnStuckEscrowedFee,
sdk.NewAttribute(sdk.AttributeKeyFee, stuckEscrowedFee.String()),
))
}
}

ctx.EventManager().EmitEvents(events)
return nil
}
Expand Down
14 changes: 9 additions & 5 deletions x/xfeemarket/post/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,12 @@ func TestPostHandleMock(t *testing.T) {
const (
baseDenom = "stake"
resolvableDenom = "atom"
expectedConsumedGas = 9307
expectedConsumedGas = 25250
expectedConsumedSimGas = expectedConsumedGas + post.BankSendGasConsumption
gasLimit = expectedConsumedSimGas // 152800 12490 + 36385
)

zeroInt := math.ZeroInt()
validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit))
validFee := sdk.NewCoins(sdk.NewCoin(baseDenom, validFeeAmount.TruncateInt()))
validResolvableFee := sdk.NewCoins(sdk.NewCoin(resolvableDenom, validFeeAmount.TruncateInt()))
Expand Down Expand Up @@ -208,6 +209,7 @@ func TestPostHandleMock(t *testing.T) {
accs := s.CreateTestAccounts(1)
s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(),
types.FeeCollectorName, mock.Anything).Return(nil).Once()
s.MockBankKeeper.On("GetBalance", mock.Anything, mock.Anything, mock.Anything).Return(sdk.NewCoin(baseDenom, zeroInt)).Once()

return postsuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
Expand All @@ -229,7 +231,7 @@ func TestPostHandleMock(t *testing.T) {
accs := s.CreateTestAccounts(1)
s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(),
types.FeeCollectorName, mock.Anything).Return(nil)
s.MockBankKeeper.On("BurnCoins", mock.Anything, types.FeeCollectorName, mock.Anything).Return(nil).Once()
s.MockBankKeeper.On("GetBalance", mock.Anything, mock.Anything, mock.Anything).Return(sdk.NewCoin(baseDenom, zeroInt)).Once()
freeelancer marked this conversation as resolved.
Show resolved Hide resolved
s.MockBankKeeper.On("SendCoinsFromModuleToAccount", mock.Anything, types.FeeCollectorName,
accs[0].Account.GetAddress(), mock.Anything).Return(nil).Once()
return postsuite.TestCaseArgs{
Expand Down Expand Up @@ -296,6 +298,7 @@ func TestPostHandleMock(t *testing.T) {
accs := s.CreateTestAccounts(1)
s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(),
types.FeeCollectorName, mock.Anything).Return(nil).Once()
s.MockBankKeeper.On("GetBalance", mock.Anything, mock.Anything, mock.Anything).Return(sdk.NewCoin(baseDenom, zeroInt)).Once()
freeelancer marked this conversation as resolved.
Show resolved Hide resolved
s.MockBankKeeper.On("SendCoinsFromModuleToAccount", mock.Anything, types.FeeCollectorName,
accs[0].Account.GetAddress(), mock.Anything).Return(nil).Once()

Expand All @@ -319,6 +322,7 @@ func TestPostHandleMock(t *testing.T) {
accs := s.CreateTestAccounts(1)
s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(),
types.FeeCollectorName, mock.Anything).Return(nil).Once()
s.MockBankKeeper.On("GetBalance", mock.Anything, mock.Anything, mock.Anything).Return(sdk.NewCoin(baseDenom, zeroInt)).Once()
freeelancer marked this conversation as resolved.
Show resolved Hide resolved

return postsuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
Expand Down Expand Up @@ -428,9 +432,9 @@ func TestPostHandle(t *testing.T) {
const (
baseDenom = "stake"
resolvableDenom = "atom"
expectedConsumedGas = 59122
expectedConsumedGas = 61632

expectedConsumedGasResolve = 25360 // slight difference due to denom resolver
expectedConsumedGasResolve = 27870 // slight difference due to denom resolver

gasLimit = 100000
)
Expand Down Expand Up @@ -536,7 +540,7 @@ func TestPostHandle(t *testing.T) {
Simulate: false,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: 34836,
ExpectConsumedGas: 37325,
Mock: false,
},
{
Expand Down
50 changes: 50 additions & 0 deletions x/xfeemarket/post/mocks/mock_account_keeper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions x/xfeemarket/post/suite/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func SetupTestSuite(t *testing.T, mock bool) *TestSuite {
}

func (s *TestSuite) SetupHandlers(mock bool) {
accountkeeper := s.AccountKeeper
bankKeeper := s.BankKeeper
feeGrantKeeper := s.FeeGrantKeeper

Expand Down Expand Up @@ -165,6 +166,7 @@ func (s *TestSuite) SetupHandlers(mock bool) {
// create basic postHandler with the feemarket decorator
postDecorators := []sdk.PostDecorator{
feemarketpost.NewFeeMarketDeductDecorator(
accountkeeper,
bankKeeper,
s.FeeMarketKeeper,
),
Expand Down
3 changes: 2 additions & 1 deletion x/xfeemarket/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ var (
ParamsKey = collections.NewPrefix("p_xfeemarket")
PrefixDenomMultiplier = []byte{0x01}

EventTypeTipRefund = "tip_refund"
EventTypeTipRefund = "tip_refund"
EventTypeBurnStuckEscrowedFee = "burn_stuck_escrowed_fee"
)

func KeyPrefix(p string) []byte {
Expand Down
Loading