diff --git a/CHANGELOG.md b/CHANGELOG.md index 59e7dd1ab..59dd8201d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - [\#471](https://github.com/cosmos/evm/pull/471) Notify new block for mempool in time - [\#492](https://github.com/cosmos/evm/pull/492) Duplicate case switch to avoid empty execution block +- [\#494](https://github.com/cosmos/evm/pull/494) Align temporary reject check for included tx from txpool. - [\#509](https://github.com/cosmos/evm/pull/509) Allow value with slashes when query token_pairs - [\#495](https://github.com/cosmos/evm/pull/495) Allow immediate SIGINT interrupt when mempool is not empty - [\#416](https://github.com/cosmos/evm/pull/416) Fix regression in CometBlockResultByNumber when height is 0 to use the latest block. This fixes eth_getFilterLogs RPC. diff --git a/mempool/txpool/legacypool/legacypool.go b/mempool/txpool/legacypool/legacypool.go index 0e2cc0cb6..e6a4853a2 100644 --- a/mempool/txpool/legacypool/legacypool.go +++ b/mempool/txpool/legacypool/legacypool.go @@ -18,7 +18,6 @@ package legacypool import ( - "errors" "maps" "math/big" "slices" @@ -31,6 +30,7 @@ import ( "github.com/ethereum/go-ethereum/common/prque" "github.com/ethereum/go-ethereum/consensus/misc/eip1559" "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/core/txpool/legacypool" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/crypto/kzg4844" @@ -58,25 +58,6 @@ const ( txMaxSize = 4 * txSlotSize // 128KB ) -var ( - // ErrTxPoolOverflow is returned if the transaction pool is full and can't accept - // another remote transaction. - ErrTxPoolOverflow = errors.New("txpool is full") - - // ErrOutOfOrderTxFromDelegated is returned when the transaction with gapped - // nonce received from the accounts with delegation or pending delegation. - ErrOutOfOrderTxFromDelegated = errors.New("gapped-nonce tx from delegated accounts") - - // ErrAuthorityReserved is returned if a transaction has an authorization - // signed by an address which already has in-flight transactions known to the - // pool. - ErrAuthorityReserved = errors.New("authority already reserved") - - // ErrFutureReplacePending is returned if a future transaction replaces a pending - // one. Future transactions should only be able to replace other future transactions. - ErrFutureReplacePending = errors.New("future transaction tries to replace pending") -) - var ( evictionInterval = time.Minute // Time interval to check for evictable transactions statsReportInterval = 8 * time.Second // Time interval to report transaction pool stats @@ -623,7 +604,7 @@ func (pool *LegacyPool) checkDelegationLimit(tx *types.Transaction) error { if pending == nil { // Transaction with gapped nonce is not supported for delegated accounts if pool.pendingNonces.get(from) != tx.Nonce() { - return ErrOutOfOrderTxFromDelegated + return legacypool.ErrOutOfOrderTxFromDelegated } return nil } @@ -654,7 +635,7 @@ func (pool *LegacyPool) validateAuth(tx *types.Transaction) error { count += queue.Len() } if count > 1 { - return ErrAuthorityReserved + return legacypool.ErrAuthorityReserved } // Because there is no exclusive lock held between different subpools // when processing transactions, the SetCode transaction may be accepted @@ -665,7 +646,7 @@ func (pool *LegacyPool) validateAuth(tx *types.Transaction) error { // that attackers cannot easily stack a SetCode transaction when the sender // is reserved by other pools. if pool.reserver.Has(auth) { - return ErrAuthorityReserved + return legacypool.ErrAuthorityReserved } } } @@ -730,7 +711,7 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) { // replacements to 25% of the slots if pool.changesSinceReorg > int(pool.config.GlobalSlots/4) { throttleTxMeter.Mark(1) - return false, ErrTxPoolOverflow + return false, legacypool.ErrTxPoolOverflow } // New transaction is better than our worse ones, make room for it. @@ -741,7 +722,7 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) { if !success { log.Trace("Discarding overflown transaction", "hash", hash) overflowedTxMeter.Mark(1) - return false, ErrTxPoolOverflow + return false, legacypool.ErrTxPoolOverflow } // If the new transaction is a future transaction it should never churn pending transactions @@ -760,7 +741,7 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) { pool.priced.Put(dropTx) } log.Trace("Discarding future transaction replacing pending tx", "hash", hash) - return false, ErrFutureReplacePending + return false, legacypool.ErrFutureReplacePending } } diff --git a/mempool/txpool/legacypool/legacypool_test.go b/mempool/txpool/legacypool/legacypool_test.go index 465d9f186..a63cb07b4 100644 --- a/mempool/txpool/legacypool/legacypool_test.go +++ b/mempool/txpool/legacypool/legacypool_test.go @@ -33,6 +33,7 @@ import ( "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/tracing" + "github.com/ethereum/go-ethereum/core/txpool/legacypool" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/crypto" @@ -109,11 +110,33 @@ func pricedTransaction(nonce uint64, gaslimit uint64, gasprice *big.Int, key *ec return tx } -func pricedDataTransaction(nonce uint64, gaslimit uint64, gasprice *big.Int, key *ecdsa.PrivateKey, bytes uint64) *types.Transaction { - data := make([]byte, bytes) - crand.Read(data) - - tx, _ := types.SignTx(types.NewTransaction(nonce, common.Address{}, big.NewInt(0), gaslimit, gasprice, data), types.HomesteadSigner{}, key) +// pricedDataTransaction generates a signed transaction with fixed-size data, +// and ensures that the resulting signature components (r and s) are exactly 32 bytes each, +// producing transactions with deterministic size. +// +// This avoids variability in transaction size caused by leading zeros being omitted in +// RLP encoding of r/s. Since r and s are derived from ECDSA, they occasionally have leading +// zeros and thus can be shorter than 32 bytes. +// +// For example: +// +// r: 0 leading zeros, bytesSize: 32, bytes: [221 ... 101] +// s: 1 leading zeros, bytesSize: 31, bytes: [0 75 ... 47] +func pricedDataTransaction(nonce uint64, gaslimit uint64, gasprice *big.Int, key *ecdsa.PrivateKey, dataBytes uint64) *types.Transaction { + var tx *types.Transaction + + // 10 attempts is statistically sufficient since leading zeros in ECDSA signatures are rare and randomly distributed. + var retryTimes = 10 + for i := 0; i < retryTimes; i++ { + data := make([]byte, dataBytes) + crand.Read(data) + + tx, _ = types.SignTx(types.NewTransaction(nonce, common.Address{}, big.NewInt(0), gaslimit, gasprice, data), types.HomesteadSigner{}, key) + _, r, s := tx.RawSignatureValues() + if len(r.Bytes()) == 32 && len(s.Bytes()) == 32 { + break + } + } return tx } @@ -1240,7 +1263,7 @@ func TestAllowedTxSize(t *testing.T) { const largeDataLength = txMaxSize - 200 // enough to have a 5 bytes RLP encoding of the data length number txWithLargeData := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, big.NewInt(1), key, largeDataLength) maxTxLengthWithoutData := txWithLargeData.Size() - largeDataLength // 103 bytes - maxTxDataLength := txMaxSize - maxTxLengthWithoutData // 131072 - 103 = 130953 bytes + maxTxDataLength := txMaxSize - maxTxLengthWithoutData // 131072 - 103 = 130969 bytes // Try adding a transaction with maximal allowed size tx := pricedDataTransaction(0, pool.currentHead.Load().GasLimit, big.NewInt(1), key, maxTxDataLength) @@ -1674,8 +1697,8 @@ func TestUnderpricing(t *testing.T) { t.Fatalf("failed to add well priced transaction: %v", err) } // Ensure that replacing a pending transaction with a future transaction fails - if err := pool.addRemoteSync(pricedTransaction(5, 100000, big.NewInt(6), keys[1])); !errors.Is(err, ErrFutureReplacePending) { - t.Fatalf("adding future replace transaction error mismatch: have %v, want %v", err, ErrFutureReplacePending) + if err := pool.addRemoteSync(pricedTransaction(5, 100000, big.NewInt(6), keys[1])); !errors.Is(err, legacypool.ErrFutureReplacePending) { + t.Fatalf("adding future replace transaction error mismatch: have %v, want %v", err, legacypool.ErrFutureReplacePending) } pending, queued = pool.Stats() if pending != 4 { @@ -2275,8 +2298,8 @@ func TestSetCodeTransactions(t *testing.T) { statedb.SetCode(aa, []byte{byte(vm.ADDRESS), byte(vm.PUSH0), byte(vm.SSTORE)}) // Send gapped transaction, it should be rejected. - if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keyA)); !errors.Is(err, ErrOutOfOrderTxFromDelegated) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrOutOfOrderTxFromDelegated, err) + if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keyA)); !errors.Is(err, legacypool.ErrOutOfOrderTxFromDelegated) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, legacypool.ErrOutOfOrderTxFromDelegated, err) } // Send transactions. First is accepted, second is rejected. if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1), keyA)); err != nil { @@ -2355,8 +2378,8 @@ func TestSetCodeTransactions(t *testing.T) { t.Fatalf("%s: failed to add with pending delegation: %v", name, err) } // Delegation rejected since two txs are already in-flight. - if err := pool.addRemoteSync(setCodeTx(0, keyA, []unsignedAuth{{0, keyB}})); !errors.Is(err, ErrAuthorityReserved) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrAuthorityReserved, err) + if err := pool.addRemoteSync(setCodeTx(0, keyA, []unsignedAuth{{0, keyB}})); !errors.Is(err, legacypool.ErrAuthorityReserved) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, legacypool.ErrAuthorityReserved, err) } }, }, diff --git a/rpc/backend/call_tx.go b/rpc/backend/call_tx.go index e444cc9b2..7f93daeba 100644 --- a/rpc/backend/call_tx.go +++ b/rpc/backend/call_tx.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "math/big" - "strings" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -16,7 +15,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "github.com/cosmos/evm/mempool" rpctypes "github.com/cosmos/evm/rpc/types" evmtypes "github.com/cosmos/evm/x/vm/types" @@ -147,19 +145,16 @@ func (b *Backend) SendRawTransaction(data hexutil.Bytes) (common.Hash, error) { } txHash := ethereumTx.AsTransaction().Hash() - syncCtx := b.ClientCtx.WithBroadcastMode(flags.BroadcastSync) rsp, err := syncCtx.BroadcastTx(txBytes) - if rsp != nil && rsp.Code != 0 { + if b.Mempool != nil && rsp != nil && rsp.Code != 0 { + if shouldSkip, msg := HandleBroadcastRawLog(rsp.RawLog, txHash); shouldSkip { + b.Logger.Debug(msg, "hash", txHash.Hex()) + return txHash, nil + } err = errorsmod.ABCIError(rsp.Codespace, rsp.Code, rsp.RawLog) } if err != nil { - // Check if this is a nonce gap error that was successfully queued - if b.Mempool != nil && strings.Contains(err.Error(), mempool.ErrNonceGap.Error()) { - // Transaction was successfully queued due to nonce gap, return success to client - b.Logger.Debug("transaction queued due to nonce gap", "hash", txHash.Hex()) - return txHash, nil - } b.Logger.Error("failed to broadcast tx", "error", err.Error()) return txHash, fmt.Errorf("failed to broadcast transaction: %w", err) } diff --git a/rpc/backend/sign_tx.go b/rpc/backend/sign_tx.go index 859c2c3a8..b95244a9a 100644 --- a/rpc/backend/sign_tx.go +++ b/rpc/backend/sign_tx.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "math/big" - "strings" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" @@ -13,7 +12,6 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/signer/core/apitypes" - "github.com/cosmos/evm/mempool" evmtypes "github.com/cosmos/evm/x/vm/types" errorsmod "cosmossdk.io/errors" @@ -105,16 +103,14 @@ func (b *Backend) SendTransaction(args evmtypes.TransactionArgs) (common.Hash, e // NOTE: If error is encountered on the node, the broadcast will not return an error syncCtx := b.ClientCtx.WithBroadcastMode(flags.BroadcastSync) rsp, err := syncCtx.BroadcastTx(txBytes) - if rsp != nil && rsp.Code != 0 { + if b.Mempool != nil && rsp != nil && rsp.Code != 0 { + if shouldSkip, msg := HandleBroadcastRawLog(rsp.RawLog, txHash); shouldSkip { + b.Logger.Debug(msg, "hash", txHash.Hex()) + return txHash, nil + } err = errorsmod.ABCIError(rsp.Codespace, rsp.Code, rsp.RawLog) } if err != nil { - // Check if this is a nonce gap error that was successfully queued - if b.Mempool != nil && strings.Contains(err.Error(), mempool.ErrNonceGap.Error()) { - // Transaction was successfully queued due to nonce gap, return success to client - b.Logger.Debug("transaction queued due to nonce gap", "hash", txHash.Hex()) - return txHash, nil - } b.Logger.Error("failed to broadcast tx", "error", err.Error()) return txHash, err } diff --git a/rpc/backend/utils.go b/rpc/backend/utils.go index 6fbfd3922..0154e35aa 100644 --- a/rpc/backend/utils.go +++ b/rpc/backend/utils.go @@ -9,6 +9,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/core/txpool/legacypool" ethtypes "github.com/ethereum/go-ethereum/core/types" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -17,6 +18,8 @@ import ( "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtrpctypes "github.com/cometbft/cometbft/rpc/core/types" + "github.com/cosmos/evm/mempool" + "github.com/cosmos/evm/mempool/txpool" "github.com/cosmos/evm/rpc/types" cosmosevmtypes "github.com/cosmos/evm/types" "github.com/cosmos/evm/utils" @@ -291,3 +294,23 @@ func GetHexProofs(proof *crypto.ProofOps) []string { } return proofs } + +// HandleBroadcastRawLog checks the RawLog for known temporary rejection or nonce gap error, +// since the error message only exists in rawLog of BroadcastTx response so we can only check against error string instead of error instance. +// NOTE: make sure it sync with the latest go-ethereum logic when upgrade: +// https://github.com/ethereum/go-ethereum/blob/master/core/txpool/locals/errors.go#L29 +func HandleBroadcastRawLog(rawLog string, txHash common.Hash) (bool, string) { + if strings.Contains(rawLog, mempool.ErrNonceGap.Error()) { + return true, "transaction queued due to nonce gap" + } + switch rawLog { + case legacypool.ErrOutOfOrderTxFromDelegated.Error(), + txpool.ErrInflightTxLimitReached.Error(), + legacypool.ErrAuthorityReserved.Error(), + txpool.ErrUnderpriced.Error(), + legacypool.ErrTxPoolOverflow.Error(), + legacypool.ErrFutureReplacePending.Error(): + return true, "transaction temporarily rejected or queued" + } + return false, "" +} diff --git a/rpc/backend/utils_test.go b/rpc/backend/utils_test.go new file mode 100644 index 000000000..09bb7f59e --- /dev/null +++ b/rpc/backend/utils_test.go @@ -0,0 +1,39 @@ +package backend + +import ( + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/txpool/legacypool" + "github.com/stretchr/testify/require" + + "github.com/cosmos/evm/mempool" + "github.com/cosmos/evm/mempool/txpool" +) + +func TestHandleBroadcastRawLog(t *testing.T) { + txHash := common.HexToHash("0x123") + tmpErrMsg := "transaction temporarily rejected or queued" + cases := []struct { + rawLog string + wantMsg string + }{ + {legacypool.ErrOutOfOrderTxFromDelegated.Error(), tmpErrMsg}, + {txpool.ErrInflightTxLimitReached.Error(), tmpErrMsg}, + {legacypool.ErrAuthorityReserved.Error(), tmpErrMsg}, + {txpool.ErrUnderpriced.Error(), tmpErrMsg}, + {legacypool.ErrTxPoolOverflow.Error(), tmpErrMsg}, + {legacypool.ErrFutureReplacePending.Error(), tmpErrMsg}, + {mempool.ErrNonceGap.Error(), "transaction queued due to nonce gap"}, + } + + for _, tc := range cases { + ok, msg := HandleBroadcastRawLog(tc.rawLog, txHash) + require.True(t, ok, "expected true for: %s", tc.rawLog) + require.Equal(t, tc.wantMsg, msg, "unexpected message for: %s", tc.rawLog) + } + + ok, msg := HandleBroadcastRawLog("some other error", txHash) + require.False(t, ok) + require.Equal(t, "", msg) +}