From d9fc67769bcc4bf8629e2377a5ac93ce22e74f42 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Fri, 2 Dec 2022 04:38:34 -0800 Subject: [PATCH] chore(ante): deprecate legacy EIP-712 signature verification via AnteHandler (#1521) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: rename EIP-712 sig. verification to indicate Legacy status * Add changelog entry * Update changelog, refactor implementation, update comments * Apply suggestions from code review * address comments * changelog * Update CHANGELOG.md Co-authored-by: Vladislav Varadinov * fix test Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Vladislav Varadinov --- CHANGELOG.md | 1 + app/ante/ante.go | 4 ++-- app/ante/eip712.go | 43 +++++++++++++++++++++++++++++++------ app/ante/handler_options.go | 24 --------------------- 4 files changed, 39 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd4ff9d9e0..3b9f1840fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking +* (ante) [#1521](https://github.com/evmos/ethermint/pull/1521) Deprecate support for legacy EIP-712 signature verification implementation via AnteHandler decorator. * (ante) [#1214](https://github.com/evmos/ethermint/pull/1214) Set mempool priority to EVM transactions. * (evm) [#1405](https://github.com/evmos/ethermint/pull/1405) Add parameter `chainID` to evm keeper's `EVMConfig` method, so caller can choose to not use the cached `eip155ChainID`. diff --git a/app/ante/ante.go b/app/ante/ante.go index 88bb9d0ed8..c3a4b1bf8c 100644 --- a/app/ante/ante.go +++ b/app/ante/ante.go @@ -46,8 +46,8 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { // handle as *evmtypes.MsgEthereumTx anteHandler = newEthAnteHandler(options) case "/ethermint.types.v1.ExtensionOptionsWeb3Tx": - // handle as normal Cosmos SDK tx, except signature is checked for EIP712 representation - anteHandler = newCosmosAnteHandlerEip712(options) + // Deprecated: Handle as normal Cosmos SDK tx, except signature is checked for Legacy EIP712 representation + anteHandler = NewLegacyCosmosAnteHandlerEip712(options) case "/ethermint.types.v1.ExtensionOptionDynamicFeeTx": // cosmos-sdk tx with dynamic fee extension anteHandler = newCosmosAnteHandler(options) diff --git a/app/ante/eip712.go b/app/ante/eip712.go index 96f6e373a2..c773ba40ea 100644 --- a/app/ante/eip712.go +++ b/app/ante/eip712.go @@ -13,6 +13,7 @@ import ( authante "github.com/cosmos/cosmos-sdk/x/auth/ante" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + ibcante "github.com/cosmos/ibc-go/v5/modules/core/ante" ethcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/secp256k1" @@ -32,19 +33,47 @@ func init() { ethermintCodec = codec.NewProtoCodec(registry) } -// Eip712SigVerificationDecorator Verify all signatures for a tx and return an error if any are invalid. Note, -// the Eip712SigVerificationDecorator decorator will not get executed on ReCheck. +// Deprecated: NewLegacyCosmosAnteHandlerEip712 creates an AnteHandler to process legacy EIP-712 +// transactions, as defined by the presence of an ExtensionOptionsWeb3Tx extension. +func NewLegacyCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler { + return sdk.ChainAnteDecorators( + RejectMessagesDecorator{}, // reject MsgEthereumTxs + authante.NewSetUpContextDecorator(), + authante.NewValidateBasicDecorator(), + authante.NewTxTimeoutHeightDecorator(), + NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper), + authante.NewValidateMemoDecorator(options.AccountKeeper), + authante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), + authante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker), + // SetPubKeyDecorator must be called before all signature verification decorators + authante.NewSetPubKeyDecorator(options.AccountKeeper), + authante.NewValidateSigCountDecorator(options.AccountKeeper), + authante.NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer), + // Note: signature verification uses EIP instead of the cosmos signature validator + NewLegacyEip712SigVerificationDecorator(options.AccountKeeper, options.SignModeHandler), + authante.NewIncrementSequenceDecorator(options.AccountKeeper), + ibcante.NewRedundantRelayDecorator(options.IBCKeeper), + NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper), + ) +} + +// Deprecated: LegacyEip712SigVerificationDecorator Verify all signatures for a tx and return an error if any are invalid. Note, +// the LegacyEip712SigVerificationDecorator decorator will not get executed on ReCheck. +// NOTE: As of v0.20.0, EIP-712 signature verification is handled by the ethsecp256k1 public key (see ethsecp256k1.go) // // CONTRACT: Pubkeys are set in context for all signers before this decorator runs // CONTRACT: Tx must implement SigVerifiableTx interface -type Eip712SigVerificationDecorator struct { +type LegacyEip712SigVerificationDecorator struct { ak evmtypes.AccountKeeper signModeHandler authsigning.SignModeHandler } -// NewEip712SigVerificationDecorator creates a new Eip712SigVerificationDecorator -func NewEip712SigVerificationDecorator(ak evmtypes.AccountKeeper, signModeHandler authsigning.SignModeHandler) Eip712SigVerificationDecorator { - return Eip712SigVerificationDecorator{ +// Deprecated: NewLegacyEip712SigVerificationDecorator creates a new LegacyEip712SigVerificationDecorator +func NewLegacyEip712SigVerificationDecorator( + ak evmtypes.AccountKeeper, + signModeHandler authsigning.SignModeHandler, +) LegacyEip712SigVerificationDecorator { + return LegacyEip712SigVerificationDecorator{ ak: ak, signModeHandler: signModeHandler, } @@ -52,7 +81,7 @@ func NewEip712SigVerificationDecorator(ak evmtypes.AccountKeeper, signModeHandle // AnteHandle handles validation of EIP712 signed cosmos txs. // it is not run on RecheckTx -func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context, +func (svd LegacyEip712SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler, diff --git a/app/ante/handler_options.go b/app/ante/handler_options.go index 96387aa674..cacf289608 100644 --- a/app/ante/handler_options.go +++ b/app/ante/handler_options.go @@ -87,27 +87,3 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler { NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper), ) } - -func newCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler { - return sdk.ChainAnteDecorators( - RejectMessagesDecorator{}, // reject MsgEthereumTxs - ante.NewSetUpContextDecorator(), - // NOTE: extensions option decorator removed - // ante.NewRejectExtensionOptionsDecorator(), - ante.NewValidateBasicDecorator(), - ante.NewTxTimeoutHeightDecorator(), - NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper), - ante.NewValidateMemoDecorator(options.AccountKeeper), - ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), - ante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker), - // SetPubKeyDecorator must be called before all signature verification decorators - ante.NewSetPubKeyDecorator(options.AccountKeeper), - ante.NewValidateSigCountDecorator(options.AccountKeeper), - ante.NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer), - // Note: signature verification uses EIP instead of the cosmos signature validator - NewEip712SigVerificationDecorator(options.AccountKeeper, options.SignModeHandler), - ante.NewIncrementSequenceDecorator(options.AccountKeeper), - ibcante.NewRedundantRelayDecorator(options.IBCKeeper), - NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper), - ) -}