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: don't fail post handler on simulate tx with no fee #122

Merged
merged 7 commits into from
Jun 16, 2024
Merged
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
6 changes: 2 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# `x/feemarket` Specification
# `x/feemarket` Module

## Abstract

This document specifies the feemarket module.

The feemarket module is an implementation of the Additive Increase Multiplicative Decrease (AIMD) EIP-1559
feemarket. More information about the implementation can be found [here](./x/feemarket/README.md).

## Upgrading to FeeMarket
## Upgrading to Feemarket

More information about upgrading your chain to `x/feemarket` can be found in our dedicated [guide](docs/UPGRADING.md).

Expand Down
7 changes: 5 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ require (
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/skip-mev/chaintestutil v0.0.0-20240514161515-056d7ba45610
github.com/spf13/cast v1.6.0
github.com/spf13/cobra v1.8.0
github.com/spf13/cobra v1.8.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional / relevant? (Feel free to ignore)

github.com/spf13/viper v1.19.0
github.com/stretchr/testify v1.9.0
github.com/vektra/mockery/v2 v2.43.2
golang.org/x/tools v0.22.0
google.golang.org/genproto/googleapis/api v0.0.0-20240610135401-a8a62080eff3
google.golang.org/grpc v1.64.0
google.golang.org/protobuf v1.34.1
google.golang.org/protobuf v1.34.2
aljo242 marked this conversation as resolved.
Show resolved Hide resolved
mvdan.cc/gofumpt v0.6.0
pgregory.net/rapid v1.1.0
)
Expand Down Expand Up @@ -351,3 +351,6 @@ replace (
github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7
)

// simulation and fee UX
retract [v1.0.0, v1.0.2]
10 changes: 5 additions & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ github.com/cosmos/ledger-cosmos-go v0.13.3 h1:7ehuBGuyIytsXbd4MP43mLeoN2LTOEnk5n
github.com/cosmos/ledger-cosmos-go v0.13.3/go.mod h1:HENcEP+VtahZFw38HZ3+LS3Iv5XV6svsnkk9vdJtLr8=
github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE=
github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/creachadair/atomicfile v0.3.3 h1:yJlDq8qk9QmD/6ol+jq1X4bcoLNVdYq95+owOnauziE=
github.com/creachadair/atomicfile v0.3.3/go.mod h1:X1r9P4wigJlGkYJO1HXZREdkVn+b1yHrsBBMLSj7tak=
github.com/creachadair/mtest v0.0.0-20231015022703-31f2ea539dce h1:BFjvg2Oq88/2DOcUFu1ScIwKUn7KJYYvLr6AeuCJD54=
Expand Down Expand Up @@ -1236,8 +1236,8 @@ github.com/spf13/cast v1.6.0 h1:GEiTHELF+vaR5dhz3VqZfFSzZjYbgeKDpBxQVS4GYJ0=
github.com/spf13/cast v1.6.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo=
github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ=
github.com/spf13/cobra v0.0.5/go.mod h1:3K3wKZymM7VvHMDS9+Akkh4K60UwM26emMESw8tLCHU=
github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0=
github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho=
github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
github.com/spf13/pflag v1.0.1/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
Expand Down Expand Up @@ -2026,8 +2026,8 @@ google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ
google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg=
google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=
google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
2 changes: 1 addition & 1 deletion x/feemarket/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula

var feeCoin sdk.Coin
if simulate && len(feeCoins) == 0 {
feeCoin = sdk.NewCoin(params.FeeDenom, sdkmath.NewInt(0))
feeCoin = sdk.NewCoin(params.FeeDenom, sdkmath.ZeroInt())
} else {
feeCoin = feeCoins[0]
}
Expand Down
54 changes: 52 additions & 2 deletions x/feemarket/ante/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"

"cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -77,7 +76,7 @@ func TestAnteHandle(t *testing.T) {
ExpErr: sdkerrors.ErrOutOfGas,
},
{
Name: "0 gas given should pass in simulate",
Name: "0 gas given should pass in simulate - no fee",
Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := suite.CreateTestAccounts(1)

Expand All @@ -93,6 +92,23 @@ func TestAnteHandle(t *testing.T) {
ExpPass: true,
ExpErr: nil,
},
{
Name: "0 gas given should pass in simulate - fee",
Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := suite.CreateTestAccounts(1)

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 0,
FeeAmount: validFee,
}
},
RunAnte: true,
RunPost: false,
Simulate: true,
ExpPass: true,
ExpErr: nil,
},
{
Name: "signer has enough funds, should pass",
Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs {
Expand Down Expand Up @@ -125,6 +141,40 @@ func TestAnteHandle(t *testing.T) {
ExpPass: true,
ExpErr: nil,
},
{
Name: "no fee - fail",
Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := s.CreateTestAccounts(1)

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 1000000000,
FeeAmount: nil,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: false,
ExpErr: types.ErrNoFeeCoins,
},
{
Name: "no gas limit - fail",
Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := s.CreateTestAccounts(1)

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 0,
FeeAmount: nil,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: false,
ExpErr: sdkerrors.ErrInvalidGasLimit,
},
}

for _, tc := range testCases {
Expand Down
19 changes: 13 additions & 6 deletions x/feemarket/post/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package post

import (
"bytes"
"cosmossdk.io/math"
"fmt"

"cosmossdk.io/math"

errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -75,14 +76,20 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul
feeCoins := feeTx.GetFee()
gas := ctx.GasMeter().GasConsumed() // use context gas consumed

if len(feeCoins) != 1 {
if len(feeCoins) == 0 {
return ctx, errorsmod.Wrapf(feemarkettypes.ErrNoFeeCoins, "got length %d", len(feeCoins))
}
if len(feeCoins) == 0 && !simulate {
return ctx, errorsmod.Wrapf(feemarkettypes.ErrNoFeeCoins, "got length %d", len(feeCoins))
}
if len(feeCoins) > 1 {
return ctx, errorsmod.Wrapf(feemarkettypes.ErrTooManyFeeCoins, "got length %d", len(feeCoins))
}

feeCoin := feeCoins[0]
var feeCoin sdk.Coin
if simulate && len(feeCoins) == 0 {
feeCoin = sdk.NewCoin(params.FeeDenom, math.ZeroInt())
} else {
feeCoin = feeCoins[0]
}

feeGas := int64(feeTx.GetGas())

var (
Expand Down
68 changes: 68 additions & 0 deletions x/feemarket/post/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,74 @@ func TestPostHandle(t *testing.T) {
ExpPass: true,
ExpErr: nil,
},
{
Name: "0 gas given should pass in simulate - no fee",
Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := suite.CreateTestAccounts(1)

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 0,
FeeAmount: nil,
}
},
RunAnte: true,
RunPost: false,
Simulate: true,
ExpPass: true,
ExpErr: nil,
},
{
Name: "0 gas given should pass in simulate - fee",
Malleate: func(suite *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := suite.CreateTestAccounts(1)

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 0,
FeeAmount: validFee,
}
},
RunAnte: true,
RunPost: false,
Simulate: true,
ExpPass: true,
ExpErr: nil,
},
{
Name: "no fee - fail",
Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := s.CreateTestAccounts(1)

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 1000000000,
FeeAmount: nil,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: false,
ExpErr: types.ErrNoFeeCoins,
},
{
Name: "no gas limit - fail",
Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs {
accs := s.CreateTestAccounts(1)

return antesuite.TestCaseArgs{
Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())},
GasLimit: 0,
FeeAmount: nil,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: false,
ExpErr: sdkerrors.ErrInvalidGasLimit,
},
}

for _, tc := range testCases {
Expand Down
Loading