diff --git a/ante/evm/09_increment_sequence.go b/ante/evm/09_increment_sequence.go index 3440d7c61..db79fba99 100644 --- a/ante/evm/09_increment_sequence.go +++ b/ante/evm/09_increment_sequence.go @@ -3,36 +3,51 @@ package evm import ( "math" - anteinterfaces "github.com/cosmos/evm/ante/interfaces" - "github.com/cosmos/evm/mempool" - errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" errortypes "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/evm/mempool" ) // IncrementNonce increments the sequence of the account. -func IncrementNonce( +func (md MonoDecorator) IncrementNonce( ctx sdk.Context, - accountKeeper anteinterfaces.AccountKeeper, account sdk.AccountI, + tx sdk.Tx, txNonce uint64, ) error { + utx, ok := tx.(sdk.TxWithUnordered) + isUnordered := ok && utx.GetUnordered() + unorderedEnabled := md.accountKeeper.UnorderedTransactionsEnabled() + + if isUnordered && !unorderedEnabled { + return errorsmod.Wrap(sdkerrors.ErrNotSupported, "unordered transactions are not enabled") + } + accountNonce := account.GetSequence() - // we merged the accountNonce verification to accountNonce increment, so when tx includes multiple messages - // with same sender, they'll be accepted. - if txNonce != accountNonce { + + if isUnordered { + if err := md.verifyUnorderedNonce(ctx, account, utx); err != nil { + return err + } + } else { + // We've merged the accountNonce verification to accountNonce increment, so + // when tx includes multiple messages with same sender, they'll be accepted. if txNonce > accountNonce { return errorsmod.Wrapf( mempool.ErrNonceGap, "tx nonce: %d, account accountNonce: %d", txNonce, accountNonce, ) } - return errorsmod.Wrapf( - errortypes.ErrInvalidSequence, - "invalid nonce; got %d, expected %d", txNonce, accountNonce, - ) + + if txNonce < accountNonce { + return errorsmod.Wrapf( + errortypes.ErrInvalidSequence, + "invalid nonce; got %d, expected %d", txNonce, accountNonce, + ) + } } // EIP-2681 / state safety: refuse to overflow beyond 2^64-1. @@ -49,6 +64,60 @@ func IncrementNonce( return errorsmod.Wrapf(err, "failed to set sequence to %d", accountNonce) } - accountKeeper.SetAccount(ctx, account) + md.accountKeeper.SetAccount(ctx, account) + return nil +} + +// verifyUnorderedNonce verifies the unordered nonce of an unordered transaction. +// This checks that: +// 1. The unordered transaction's timeout timestamp is set. +// 2. The unordered transaction's timeout timestamp is not in the past. +// 3. The unordered transaction's timeout timestamp is not more than the max TTL. +// 4. The unordered transaction's nonce has not been used previously. +// +// If all the checks above pass, the nonce is marked as used for each signer of +// the transaction. +func (md MonoDecorator) verifyUnorderedNonce(ctx sdk.Context, account sdk.AccountI, unorderedTx sdk.TxWithUnordered) error { + blockTime := ctx.BlockTime() + timeoutTimestamp := unorderedTx.GetTimeoutTimeStamp() + + if timeoutTimestamp.IsZero() || timeoutTimestamp.Unix() == 0 { + return errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "unordered transaction must have timeout_timestamp set", + ) + } + + if timeoutTimestamp.Before(blockTime) { + return errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "unordered transaction has a timeout_timestamp that has already passed", + ) + } + + if timeoutTimestamp.After(blockTime.Add(md.maxTxTimeoutDuration)) { + return errorsmod.Wrapf( + sdkerrors.ErrInvalidRequest, + "unordered tx ttl exceeds %s", + md.maxTxTimeoutDuration.String(), + ) + } + + ctx.GasMeter().ConsumeGas(md.unorderedTxGasCost, "unordered tx") + + execMode := ctx.ExecMode() + if execMode == sdk.ExecModeSimulate { + return nil + } + + err := md.accountKeeper.TryAddUnorderedNonce( + ctx, + account.GetAddress().Bytes(), + unorderedTx.GetTimeoutTimeStamp(), + ) + if err != nil { + return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "failed to add unordered nonce: %s", err) + } + return nil } diff --git a/ante/evm/mono_decorator.go b/ante/evm/mono_decorator.go index 3971b071c..89c77bff7 100644 --- a/ante/evm/mono_decorator.go +++ b/ante/evm/mono_decorator.go @@ -2,29 +2,51 @@ package evm import ( "math/big" - - "github.com/ethereum/go-ethereum/common" - ethtypes "github.com/ethereum/go-ethereum/core/types" - - anteinterfaces "github.com/cosmos/evm/ante/interfaces" - evmkeeper "github.com/cosmos/evm/x/vm/keeper" - evmtypes "github.com/cosmos/evm/x/vm/types" + "time" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" + "github.com/ethereum/go-ethereum/common" + ethtypes "github.com/ethereum/go-ethereum/core/types" sdk "github.com/cosmos/cosmos-sdk/types" errortypes "github.com/cosmos/cosmos-sdk/types/errors" txtypes "github.com/cosmos/cosmos-sdk/types/tx" + authante "github.com/cosmos/cosmos-sdk/x/auth/ante" + anteinterfaces "github.com/cosmos/evm/ante/interfaces" + evmkeeper "github.com/cosmos/evm/x/vm/keeper" + evmtypes "github.com/cosmos/evm/x/vm/types" ) // MonoDecorator is a single decorator that handles all the prechecks for // ethereum transactions. type MonoDecorator struct { - accountKeeper anteinterfaces.AccountKeeper - feeMarketKeeper anteinterfaces.FeeMarketKeeper - evmKeeper anteinterfaces.EVMKeeper - maxGasWanted uint64 + accountKeeper anteinterfaces.AccountKeeper + feeMarketKeeper anteinterfaces.FeeMarketKeeper + evmKeeper anteinterfaces.EVMKeeper + maxGasWanted uint64 + maxTxTimeoutDuration time.Duration + unorderedTxGasCost uint64 +} + +type MonoDecoratorOption func(*MonoDecorator) + +// WithMaxUnorderedTxTimeoutDuration sets the maximum TTL a transaction can define +// for unordered transactions. +func WithMaxUnorderedTxTimeoutDuration(duration time.Duration) MonoDecoratorOption { + return func(md *MonoDecorator) { + md.maxTxTimeoutDuration = duration + } +} + +// WithUnorderedTxGasCost sets the gas cost for unordered transactions. +// We must charge extra gas for unordered transactions +// as they incur extra processing time for cleaning up the expired txs in x/auth PreBlocker. +// Note: this value was chosen by 2x-ing the cost of fetching and removing an unordered nonce entry. +func WithUnorderedTxGasCost(gasCost uint64) MonoDecoratorOption { + return func(md *MonoDecorator) { + md.unorderedTxGasCost = gasCost + } } // NewEVMMonoDecorator creates the 'mono' decorator, that is used to run the ante handle logic @@ -38,13 +60,22 @@ func NewEVMMonoDecorator( feeMarketKeeper anteinterfaces.FeeMarketKeeper, evmKeeper anteinterfaces.EVMKeeper, maxGasWanted uint64, + opts ...MonoDecoratorOption, ) MonoDecorator { - return MonoDecorator{ - accountKeeper: accountKeeper, - feeMarketKeeper: feeMarketKeeper, - evmKeeper: evmKeeper, - maxGasWanted: maxGasWanted, + md := MonoDecorator{ + accountKeeper: accountKeeper, + feeMarketKeeper: feeMarketKeeper, + evmKeeper: evmKeeper, + maxGasWanted: maxGasWanted, + maxTxTimeoutDuration: authante.DefaultMaxTimeoutDuration, + unorderedTxGasCost: authante.DefaultUnorderedTxGasCost, } + + for _, opt := range opts { + opt(&md) + } + + return md } // AnteHandle handles the entire decorator chain using a mono decorator. @@ -226,7 +257,7 @@ func (md MonoDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne ) } - if err := IncrementNonce(ctx, md.accountKeeper, acc, ethTx.Nonce()); err != nil { + if err := md.IncrementNonce(ctx, acc, tx, ethTx.Nonce()); err != nil { return ctx, err } diff --git a/ante/evm/nonce_limit_test.go b/ante/evm/nonce_limit_test.go index a6c3b8c36..545e1451c 100644 --- a/ante/evm/nonce_limit_test.go +++ b/ante/evm/nonce_limit_test.go @@ -6,14 +6,12 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - - evmante "github.com/cosmos/evm/ante/evm" - addresscodec "cosmossdk.io/core/address" + "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + evmante "github.com/cosmos/evm/ante/evm" ) // --- minimal codec to satisfy addresscodec.Codec (not used by these tests) --- @@ -54,8 +52,9 @@ func TestIncrementNonce_HappyPath(t *testing.T) { var ctx sdk.Context ak := &mockAccountKeeper{} acc := baseAcc(7) + md := evmante.NewEVMMonoDecorator(ak, nil, nil, 0) - err := evmante.IncrementNonce(ctx, ak, acc, 7) + err := md.IncrementNonce(ctx, acc, nil, 7) require.NoError(t, err) require.Equal(t, uint64(8), acc.GetSequence()) require.Equal(t, acc, ak.last) // SetAccount called @@ -65,8 +64,9 @@ func TestIncrementNonce_NonceMismatch(t *testing.T) { var ctx sdk.Context ak := &mockAccountKeeper{} acc := baseAcc(10) + md := evmante.NewEVMMonoDecorator(ak, nil, nil, 0) - err := evmante.IncrementNonce(ctx, ak, acc, 9) + err := md.IncrementNonce(ctx, acc, nil, 9) require.Error(t, err) require.Contains(t, err.Error(), "invalid nonce") require.Equal(t, uint64(10), acc.GetSequence()) // unchanged @@ -76,8 +76,9 @@ func TestIncrementNonce_OverflowGuard(t *testing.T) { var ctx sdk.Context ak := &mockAccountKeeper{} acc := baseAcc(math.MaxUint64) + md := evmante.NewEVMMonoDecorator(ak, nil, nil, 0) - err := evmante.IncrementNonce(ctx, ak, acc, math.MaxUint64) + err := md.IncrementNonce(ctx, acc, nil, math.MaxUint64) require.Error(t, err) require.Contains(t, err.Error(), "overflow") require.Equal(t, uint64(math.MaxUint64), acc.GetSequence()) // unchanged diff --git a/tests/integration/ante/test_evm_unit_09_increment_sequence.go b/tests/integration/ante/test_evm_unit_09_increment_sequence.go index d2b3e0b64..39fc33a88 100644 --- a/tests/integration/ante/test_evm_unit_09_increment_sequence.go +++ b/tests/integration/ante/test_evm_unit_09_increment_sequence.go @@ -56,6 +56,13 @@ func (s *EvmUnitAnteTestSuite) TestIncrementSequence() { }, } + md := evm.NewEVMMonoDecorator( + unitNetwork.App.GetAccountKeeper(), + unitNetwork.App.GetFeeMarketKeeper(), + unitNetwork.App.GetEVMKeeper(), + 0, + ) + for _, tc := range testCases { s.Run(tc.name, func() { account, err := grpcHandler.GetAccount(accAddr.String()) @@ -65,10 +72,10 @@ func (s *EvmUnitAnteTestSuite) TestIncrementSequence() { nonce := tc.malleate(account) // Function under test - err = evm.IncrementNonce( + err = md.IncrementNonce( unitNetwork.GetContext(), - unitNetwork.App.GetAccountKeeper(), account, + nil, nonce, ) diff --git a/x/vm/client/cli/tx.go b/x/vm/client/cli/tx.go index 8c9e0528f..9e0b0ce31 100644 --- a/x/vm/client/cli/tx.go +++ b/x/vm/client/cli/tx.go @@ -7,16 +7,12 @@ import ( "os" "strings" + "cosmossdk.io/core/address" "github.com/ethereum/go-ethereum/common/hexutil" ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/pkg/errors" "github.com/spf13/cobra" - "github.com/cosmos/evm/utils" - "github.com/cosmos/evm/x/vm/types" - - "cosmossdk.io/core/address" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/input" @@ -24,6 +20,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" errortypes "github.com/cosmos/cosmos-sdk/types/errors" types2 "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/cosmos/evm/utils" + "github.com/cosmos/evm/x/vm/types" ) // NewTxCmd returns a root CLI command handler for evm module transaction commands @@ -74,15 +72,18 @@ func NewRawTxCmd() *cobra.Command { return err } - baseDenom := types.GetEVMCoinDenom() + txf, err := tx.NewFactoryCLI(clientCtx, cmd.Flags()) + if err != nil { + return err + } - tx, err := msg.BuildTx(clientCtx.TxConfig.NewTxBuilder(), baseDenom) + signingTx, err := msg.BuildTxWithFactory(clientCtx.TxConfig.NewTxBuilder(), txf, types.GetEVMCoinDenom()) if err != nil { return err } if clientCtx.GenerateOnly { - json, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + json, err := clientCtx.TxConfig.TxJSONEncoder()(signingTx) if err != nil { return err } @@ -91,7 +92,7 @@ func NewRawTxCmd() *cobra.Command { } if !clientCtx.SkipConfirm { - out, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + out, err := clientCtx.TxConfig.TxJSONEncoder()(signingTx) if err != nil { return err } @@ -107,7 +108,7 @@ func NewRawTxCmd() *cobra.Command { } } - txBytes, err := clientCtx.TxConfig.TxEncoder()(tx) + txBytes, err := clientCtx.TxConfig.TxEncoder()(signingTx) if err != nil { return err } diff --git a/x/vm/types/msg.go b/x/vm/types/msg.go index 169a7c16a..c97f0e153 100644 --- a/x/vm/types/msg.go +++ b/x/vm/types/msg.go @@ -19,6 +19,7 @@ import ( txsigning "cosmossdk.io/x/tx/signing" "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/tx" codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/keyring" sdk "github.com/cosmos/cosmos-sdk/types" @@ -302,8 +303,9 @@ func (msg *MsgEthereumTx) Hash() common.Hash { return msg.AsTransaction().Hash() } -// BuildTx builds the canonical cosmos tx from ethereum msg -func (msg *MsgEthereumTx) BuildTx(b client.TxBuilder, evmDenom string) (signing.Tx, error) { +// BuildTxWithFactory builds the canonical cosmos tx from an ethereum msg using +// a provided tx.Factory to populate additional fields such as unordered values. +func (msg *MsgEthereumTx) BuildTxWithFactory(b client.TxBuilder, txf tx.Factory, evmDenom string) (signing.Tx, error) { builder, ok := b.(authtx.ExtensionOptionsTxBuilder) if !ok { return nil, errors.New("unsupported builder") @@ -323,7 +325,7 @@ func (msg *MsgEthereumTx) BuildTx(b client.TxBuilder, evmDenom string) (signing. builder.SetExtensionOptions(option) - // only keep the nessessary fields + // only keep the necessary fields err = builder.SetMsgs(&MsgEthereumTx{ From: msg.From, Raw: msg.Raw, @@ -331,10 +333,21 @@ func (msg *MsgEthereumTx) BuildTx(b client.TxBuilder, evmDenom string) (signing. if err != nil { return nil, err } + builder.SetFeeAmount(fees) builder.SetGasLimit(msg.GetGas()) - tx := builder.GetTx() - return tx, nil + builder.SetTimeoutHeight(txf.TimeoutHeight()) + builder.SetTimeoutTimestamp(txf.TimeoutTimestamp()) + builder.SetUnordered(txf.Unordered()) + + return builder.GetTx(), nil +} + +// BuildTx builds the canonical cosmos tx from an ethereum msg. +func (msg *MsgEthereumTx) BuildTx(b client.TxBuilder, evmDenom string) (signing.Tx, error) { + // Note: Using an empty Factory will result all accessed values having their + // native default value, which is acceptable given how txs are constructed. + return msg.BuildTxWithFactory(b, tx.Factory{}, evmDenom) } // ValidateBasic does a sanity check of the provided data