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

refactor(evm): clean up AnteHandler setup #1966

Merged
merged 11 commits into from
Jul 29, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#1962](https://github.com/NibiruChain/nibiru/pull/1962) - chore(evm): code cleanup, unused code, typos, styles, warnings
- [#1963](https://github.com/NibiruChain/nibiru/pull/1963) - feat(evm): Deduct a fee during the creation of a FunToken mapping. Implemented by `deductCreateFunTokenFee` inside of the `eth.evm.v1.MsgCreateFunToken` transaction.
- [#1965](https://github.com/NibiruChain/nibiru/pull/1965) - refactor(evm): remove evm post-processing hooks
- [#1966](https://github.com/NibiruChain/nibiru/pull/1966) - refactor(evm): clean up AnteHandler setup
- [#1967](https://github.com/NibiruChain/nibiru/pull/1967) - feat(evm): export genesis
- [#1968](https://github.com/NibiruChain/nibiru/pull/1968) - refactor(evm): funtoken events, cli commands and queries
- [#1970](https://github.com/NibiruChain/nibiru/pull/1970) - refactor(evm): move evm antehandlers to separate package. Remove "gosdk/sequence_test.go", which causes a race condition in CI.
Expand Down
69 changes: 20 additions & 49 deletions app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,64 +11,51 @@ import (

"github.com/NibiruChain/nibiru/app/ante"
"github.com/NibiruChain/nibiru/app/evmante"
"github.com/NibiruChain/nibiru/eth"
devgasante "github.com/NibiruChain/nibiru/x/devgas/v1/ante"
"github.com/NibiruChain/nibiru/x/evm"
)

// NewAnteHandler returns and AnteHandler that checks and increments sequence
// numbers, checks signatures and account numbers, and deducts fees from the
// first signer.
func NewAnteHandler(
keepers AppKeepers,
opts ante.AnteHandlerOptions,
options ante.AnteHandlerOptions,
) sdk.AnteHandler {
return func(
ctx sdk.Context, tx sdk.Tx, sim bool,
) (newCtx sdk.Context, err error) {
if err := opts.ValidateAndClean(); err != nil {
if err := options.ValidateAndClean(); err != nil {
return ctx, err
}

var anteHandler sdk.AnteHandler
hasExt, typeUrl := TxHasExtensions(tx)
if hasExt && typeUrl != "" {
anteHandler = AnteHandlerExtendedTx(typeUrl, keepers, opts)
return anteHandler(ctx, tx, sim)
txWithExtensions, ok := tx.(authante.HasExtensionOptionsTx)
if ok {
opts := txWithExtensions.GetExtensionOptions()
if len(opts) > 0 {
switch typeURL := opts[0].GetTypeUrl(); typeURL {
case "/eth.evm.v1.ExtensionOptionsEthereumTx":
// handle as *evmtypes.MsgEthereumTx
anteHandler = evmante.NewAnteHandlerEVM(options)
default:
return ctx, fmt.Errorf(
"rejecting tx with unsupported extension option: %s", typeURL)
}

return anteHandler(ctx, tx, sim)
}
Comment on lines +32 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test coverage for transaction extensions.

The logic for handling transactions with extension options is not covered by tests.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

Tools
GitHub Check: codecov/patch

[warning] 36-37: app/ante.go#L36-L37
Added lines #L36 - L37 were not covered by tests


[warning] 39-42: app/ante.go#L39 - L42
Added lines #L39 - L42 were not covered by tests


[warning] 45-45: app/ante.go#L45
Added line #L45 was not covered by tests

Tools
GitHub Check: codecov/patch

[warning] 36-37: app/ante.go#L36-L37
Added lines #L36 - L37 were not covered by tests


[warning] 39-42: app/ante.go#L39-L42
Added lines #L39 - L42 were not covered by tests


[warning] 45-45: app/ante.go#L45
Added line #L45 was not covered by tests

}

switch tx.(type) {
case sdk.Tx:
anteHandler = NewAnteHandlerNonEVM(opts)
anteHandler = NewAnteHandlerNonEVM(options)
default:
return ctx, fmt.Errorf("invalid tx type (%T) in AnteHandler", tx)
}
return anteHandler(ctx, tx, sim)
}
}

func AnteHandlerExtendedTx(
typeUrl string,
keepers AppKeepers,
opts ante.AnteHandlerOptions,
) (anteHandler sdk.AnteHandler) {
switch typeUrl {
case evm.TYPE_URL_ETHEREUM_TX:
anteHandler = evmante.NewAnteHandlerEVM(opts)
case eth.TYPE_URL_DYNAMIC_FEE_TX:
anteHandler = NewAnteHandlerNonEVM(opts)
default:
errUnsupported := fmt.Errorf(
`encountered tx with unsupported extension option, "%s"`, typeUrl)
return func(
ctx sdk.Context, tx sdk.Tx, simulate bool,
) (newCtx sdk.Context, err error) {
return ctx, errUnsupported
}
}
return anteHandler
}

// NewAnteHandlerNonEVM: Default ante handler for non-EVM transactions.
func NewAnteHandlerNonEVM(
opts ante.AnteHandlerOptions,
Expand All @@ -91,11 +78,9 @@ func NewAnteHandlerNonEVM(
authante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
// TODO: spike(security): Does minimum gas price of 0 pose a risk?
// ticket: https://github.com/NibiruChain/nibiru/issues/1916
authante.NewDeductFeeDecorator(
opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, opts.TxFeeChecker),
authante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, opts.TxFeeChecker),
// ----------- Ante Handlers: devgas
devgasante.NewDevGasPayoutDecorator(
opts.DevGasBankKeeper, opts.DevGasKeeper),
devgasante.NewDevGasPayoutDecorator(opts.DevGasBankKeeper, opts.DevGasKeeper),
// ----------- Ante Handlers: Keys and signatures
// NOTE: SetPubKeyDecorator must be called before all signature verification decorators
authante.NewSetPubKeyDecorator(opts.AccountKeeper),
Expand All @@ -107,17 +92,3 @@ func NewAnteHandlerNonEVM(
ante.AnteDecoratorGasWanted{},
)
}

func TxHasExtensions(tx sdk.Tx) (hasExt bool, typeUrl string) {
extensionTx, ok := tx.(authante.HasExtensionOptionsTx)
if !ok {
return false, ""
}

extOpts := extensionTx.GetExtensionOptions()
if len(extOpts) == 0 {
return false, ""
}

return true, extOpts[0].GetTypeUrl()
}
4 changes: 1 addition & 3 deletions eth/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ const (

// Transaction extension protobuf type URLs
const (
TYPE_URL_WEB3_TX = "/eth.types.v1.ExtensionOptionsWeb3Tx"
TYPE_URL_DYNAMIC_FEE_TX = "/eth.types.v1.ExtensionOptionDynamicFeeTx"
TYPE_URL_WEB3_TX = "/eth.types.v1.ExtensionOptionsWeb3Tx"
)

// RegisterInterfaces registers the tendermint concrete client-related
Expand Down Expand Up @@ -59,6 +58,5 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterImplementations(
(*sdktx.TxExtensionOptionI)(nil),
&ExtensionOptionsWeb3Tx{},
&ExtensionOptionDynamicFeeTx{},
)
}
1 change: 0 additions & 1 deletion eth/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ func (suite *CodecTestSuite) TestRegisterInterfaces() {
Interface: new(sdktx.TxExtensionOptionI),
WantImpls: []string{
TYPE_URL_WEB3_TX,
TYPE_URL_DYNAMIC_FEE_TX,
},
},
}
Expand Down
6 changes: 0 additions & 6 deletions x/evm/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ var (
AminoCdc = codec.NewAminoCodec(amino)
)

const (
// Protobuf type URL for a consensus tx holding Ethereum transaction msgs.
// Corresponds with [ExtensionOptionsEthereumTx].
TYPE_URL_ETHEREUM_TX = "/eth.evm.v1.ExtensionOptionsEthereumTx"
)

// NOTE: This is required for the GetSignBytes function
func init() {
RegisterLegacyAminoCodec(amino)
Expand Down
Loading