Fix batch tx send encoding

core/chains/evm/client/chain_client.go

@@ -216,7 +216,8 @@ func (c *chainClient) SendTransaction(ctx context.Context, tx *types.Transaction
 
 func (c *chainClient) SendTransactionReturnCode(ctx context.Context, tx *types.Transaction, fromAddress common.Address) (commonclient.SendTxReturnCode, error) {
 	err := c.SendTransaction(ctx, tx)
-	return ClassifySendError(err, c.logger, tx, fromAddress, c.IsL2())
+	returnCode := ClassifySendError(err, c.logger, tx, fromAddress, c.IsL2())
+	return returnCode, err
 }
 
 func (c *chainClient) SequenceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (evmtypes.Nonce, error) {

core/chains/evm/client/client.go

@@ -219,7 +219,8 @@ func (client *client) HeaderByHash(ctx context.Context, h common.Hash) (*types.H
 
 func (client *client) SendTransactionReturnCode(ctx context.Context, tx *types.Transaction, fromAddress common.Address) (commonclient.SendTxReturnCode, error) {
 	err := client.SendTransaction(ctx, tx)
-	return ClassifySendError(err, client.logger, tx, fromAddress, client.pool.ChainType().IsL2())
+	returnCode := ClassifySendError(err, client.logger, tx, fromAddress, client.pool.ChainType().IsL2())
+	return returnCode, err
 }
 
 // SendTransaction also uses the sendonly HTTP RPC URLs if set

core/chains/evm/client/client_test.go

@@ -417,7 +417,7 @@ func TestEthClient_HeaderByNumber(t *testing.T) {
 func TestEthClient_SendTransaction_NoSecondaryURL(t *testing.T) {
 	t.Parallel()
 
-	tx := types.NewTransaction(uint64(42), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
+	tx := cltest.NewLegacyTransaction(uint64(42), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
 
 	wsURL := testutils.NewWSServer(t, &cltest.FixtureChainID, func(method string, params gjson.Result) (resp testutils.JSONRPCResponse) {
 		switch method {
@@ -449,7 +449,7 @@ func TestEthClient_SendTransaction_NoSecondaryURL(t *testing.T) {
 func TestEthClient_SendTransaction_WithSecondaryURLs(t *testing.T) {
 	t.Parallel()
 
-	tx := types.NewTransaction(uint64(42), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
+	tx := cltest.NewLegacyTransaction(uint64(42), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
 
 	wsURL := testutils.NewWSServer(t, &cltest.FixtureChainID, func(method string, params gjson.Result) (resp testutils.JSONRPCResponse) {
 		switch method {
@@ -494,7 +494,7 @@ func TestEthClient_SendTransactionReturnCode(t *testing.T) {
 	t.Parallel()
 
 	fromAddress := testutils.NewAddress()
-	tx := types.NewTransaction(uint64(42), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
+	tx := cltest.NewLegacyTransaction(uint64(42), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
 
 	t.Run("returns Fatal error type when error message is fatal", func(t *testing.T) {
 		wsURL := testutils.NewWSServer(t, &cltest.FixtureChainID, func(method string, params gjson.Result) (resp testutils.JSONRPCResponse) {

core/chains/evm/client/errors.go

@@ -413,20 +413,21 @@ func ExtractRPCError(baseErr error) (*JsonError, error) {
 	return &jErr, nil
 }
 
-func ClassifySendError(err error, lggr logger.Logger, tx *types.Transaction, fromAddress common.Address, isL2 bool) (commonclient.SendTxReturnCode, error) {
+func ClassifySendError(err error, lggr logger.Logger, tx *types.Transaction, fromAddress common.Address, isL2 bool) commonclient.SendTxReturnCode {
 	sendError := NewSendError(err)
 	if sendError == nil {
-		return commonclient.Successful, err
+		return commonclient.Successful
 	}
 	if sendError.Fatal() {
 		logger.Criticalw(lggr, "Fatal error sending transaction", "err", sendError, "etx", tx)
 		// Attempt is thrown away in this case; we don't need it since it never got accepted by a node
-		return commonclient.Fatal, err
+		return commonclient.Fatal
 	}
 	if sendError.IsNonceTooLowError() || sendError.IsTransactionAlreadyMined() {
+		lggr.Debugw("Transaction already confirmed for this nonce: %d", tx.Nonce(), "err", sendError)
 		// Nonce too low indicated that a transaction at this nonce was confirmed already.
 		// Mark it as TransactionAlreadyKnown.
-		return commonclient.TransactionAlreadyKnown, err
+		return commonclient.TransactionAlreadyKnown
 	}
 	if sendError.IsReplacementUnderpriced() {
 		lggr.Errorw(fmt.Sprintf("Replacement transaction underpriced for eth_tx %x. "+
@@ -435,51 +436,56 @@ func ClassifySendError(err error, lggr logger.Logger, tx *types.Transaction, fro
 			tx.Hash(), err), "gasPrice", tx.GasPrice, "gasTipCap", tx.GasTipCap, "gasFeeCap", tx.GasFeeCap)
 
 		// Assume success and hand off to the next cycle.
-		return commonclient.Successful, err
+		return commonclient.Successful
 	}
 	if sendError.IsTransactionAlreadyInMempool() {
 		lggr.Debugw("Transaction already in mempool", "txHash", tx.Hash, "nodeErr", sendError.Error())
-		return commonclient.Successful, err
+		return commonclient.Successful
 	}
 	if sendError.IsTemporarilyUnderpriced() {
 		lggr.Infow("Transaction temporarily underpriced", "err", sendError.Error())
-		return commonclient.Successful, err
+		return commonclient.Successful
 	}
 	if sendError.IsTerminallyUnderpriced() {
-		return commonclient.Underpriced, err
+		lggr.Errorw("Transaction terminally underpriced", "txHash", tx.Hash, "err", sendError)
+		return commonclient.Underpriced
 	}
 	if sendError.L2FeeTooLow() || sendError.IsL2FeeTooHigh() || sendError.IsL2Full() {
 		if isL2 {
-			return commonclient.FeeOutOfValidRange, err
+			lggr.Errorw("Transaction fee out of range", "err", sendError)
+			return commonclient.FeeOutOfValidRange
 		}
-		return commonclient.Unsupported, errors.Wrap(sendError, "this error type only handled for L2s")
+		lggr.Errorw("this error type only handled for L2s", "err", sendError)
+		return commonclient.Unsupported
 	}
 	if sendError.IsNonceTooHighError() {
 		// This error occurs when the tx nonce is greater than current_nonce + tx_count_in_mempool,
 		// instead of keeping the tx in mempool. This can happen if previous transactions haven't
 		// reached the client yet. The correct thing to do is to mark it as retryable.
 		lggr.Warnw("Transaction has a nonce gap.", "err", err)
-		return commonclient.Retryable, err
+		return commonclient.Retryable
 	}
 	if sendError.IsInsufficientEth() {
 		logger.Criticalw(lggr, fmt.Sprintf("Tx %x with type 0x%d was rejected due to insufficient eth: %s\n"+
 			"ACTION REQUIRED: Chainlink wallet with address 0x%x is OUT OF FUNDS",
 			tx.Hash(), tx.Type(), sendError.Error(), fromAddress,
 		), "err", sendError)
-		return commonclient.InsufficientFunds, err
+		return commonclient.InsufficientFunds
 	}
 	if sendError.IsTimeout() {
-		return commonclient.Retryable, errors.Wrapf(sendError, "timeout while sending transaction %s", tx.Hash().Hex())
+		lggr.Errorw("timeout while sending transaction %s", tx.Hash().Hex(), "err", sendError)
+		return commonclient.Retryable
 	}
 	if sendError.IsTxFeeExceedsCap() {
 		logger.Criticalw(lggr, fmt.Sprintf("Sending transaction failed: %s", label.RPCTxFeeCapConfiguredIncorrectlyWarning),
 			"etx", tx,
 			"err", sendError,
 			"id", "RPCTxFeeCapExceeded",
 		)
-		return commonclient.ExceedsMaxFee, err
+		return commonclient.ExceedsMaxFee
 	}
-	return commonclient.Unknown, err
+	lggr.Errorw("Unknown error encountered when sending transaction", "err", err)
+	return commonclient.Unknown
 }
 
 // ClassifySendOnlyError handles SendOnly nodes error codes. In that case, we don't assume there is another transaction that will be correctly

core/chains/evm/client/send_only_node_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
 	evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client/mocks"
+	"github.com/smartcontractkit/chainlink/v2/core/internal/cltest"
 	"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
 )
 
@@ -95,7 +96,7 @@ func createSignedTx(t *testing.T, chainID *big.Int, nonce uint64, data []byte) *
 	require.NoError(t, err)
 	sender, err := bind.NewKeyedTransactorWithChainID(key, chainID)
 	require.NoError(t, err)
-	tx := types.NewTransaction(
+	tx := cltest.NewLegacyTransaction(
 		nonce, sender.From,
 		assets.Ether(100).ToInt(),
 		21000, big.NewInt(1000000000), data,

core/chains/evm/txmgr/attempts.go

@@ -1,8 +1,8 @@
 package txmgr
 
 import (
-	"bytes"
 	"context"
+	"fmt"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -119,9 +119,17 @@ func (c *evmTxAttemptBuilder) NewEmptyTxAttempt(nonce evmtypes.Nonce, feeLimit u
 		return attempt, errors.New("NewEmptyTranscation: legacy fee cannot be nil")
 	}
 
-	tx := types.NewTransaction(uint64(nonce), fromAddress, value, uint64(feeLimit), fee.Legacy.ToInt(), payload)
+	tx := newLegacyTransaction(
+		uint64(nonce),
+		fromAddress,
+		value,
+		uint32(feeLimit),
+		fee.Legacy,
+		payload,
+	)
 
-	hash, signedTxBytes, err := c.SignTx(fromAddress, tx)
+	transaction := types.NewTx(&tx)
+	hash, signedTxBytes, err := c.SignTx(fromAddress, transaction)
 	if err != nil {
 		return attempt, errors.Wrapf(err, "error using account %s to sign empty transaction", fromAddress.String())
 	}
@@ -295,14 +303,15 @@ func newLegacyTransaction(nonce uint64, to common.Address, value *big.Int, gasLi
 func (c *evmTxAttemptBuilder) SignTx(address common.Address, tx *types.Transaction) (common.Hash, []byte, error) {
 	signedTx, err := c.keystore.SignTx(address, tx, &c.chainID)
 	if err != nil {
-		return common.Hash{}, nil, errors.Wrap(err, "SignTx failed")
+		return common.Hash{}, nil, fmt.Errorf("failed to sign tx: %w", err)
 	}
-	rlp := new(bytes.Buffer)
-	if err := signedTx.EncodeRLP(rlp); err != nil {
-		return common.Hash{}, nil, errors.Wrap(err, "SignTx failed")
+	var txBytes []byte
+	txBytes, err = signedTx.MarshalBinary()
+	if err != nil {
+		return common.Hash{}, nil, fmt.Errorf("failed to marshal signed tx binary: %w", err)
 	}
 	txHash := signedTx.Hash()
-	return txHash, rlp.Bytes(), nil
+	return txHash, txBytes, nil
 }
 
 func newEvmPriorAttempts(attempts []TxAttempt) (prior []gas.EvmPriorAttempt) {

core/chains/evm/txmgr/attempts_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	gethcommon "github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/core/types"
 	gethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/pkg/errors"
@@ -85,6 +86,44 @@ func TestTxm_SignTx(t *testing.T) {
 		require.NotNil(t, rawBytes)
 		require.Equal(t, "0xdd68f554373fdea7ec6713a6e437e7646465d553a6aa0b43233093366cc87ef0", hash.String())
 	})
+	t.Run("can properly encoded and decode raw transaction for LegacyTx", func(t *testing.T) {
+		chainID := big.NewInt(1)
+		kst := ksmocks.NewEth(t)
+		kst.On("SignTx", to, tx, chainID).Return(tx, nil).Once()
+		cks := txmgr.NewEvmTxAttemptBuilder(*chainID, newFeeConfig(), kst, nil)
+
+		_, rawBytes, err := cks.SignTx(addr, tx)
+		require.NoError(t, err)
+		require.NotNil(t, rawBytes)
+		require.Equal(t, "0xe42a82015681f294b921f7763960b296b9cbad586ff066a18d749724818e83010203808080", hexutil.Encode(rawBytes))
+
+		var decodedTx *gethtypes.Transaction
+		decodedTx, err = txmgr.GetGethSignedTx(rawBytes)
+		require.NoError(t, err)
+		require.Equal(t, tx.Hash(), decodedTx.Hash())
+	})
+	t.Run("can properly encoded and decode raw transaction for DynamicFeeTx", func(t *testing.T) {
+		chainID := big.NewInt(1)
+		kst := ksmocks.NewEth(t)
+		typedTx := gethtypes.NewTx(&gethtypes.DynamicFeeTx{
+			Nonce: 42,
+			To:    &to,
+			Value: big.NewInt(142),
+			Gas:   242,
+			Data:  []byte{1, 2, 3},
+		})
+		kst.On("SignTx", to, typedTx, chainID).Return(typedTx, nil).Once()
+		cks := txmgr.NewEvmTxAttemptBuilder(*chainID, newFeeConfig(), kst, nil)
+		_, rawBytes, err := cks.SignTx(addr, typedTx)
+		require.NoError(t, err)
+		require.NotNil(t, rawBytes)
+		require.Equal(t, "0x02e5802a808081f294b921f7763960b296b9cbad586ff066a18d749724818e83010203c0808080", hexutil.Encode(rawBytes))
+
+		var decodedTx *gethtypes.Transaction
+		decodedTx, err = txmgr.GetGethSignedTx(rawBytes)
+		require.NoError(t, err)
+		require.Equal(t, typedTx.Hash(), decodedTx.Hash())
+	})
 }
 
 func TestTxm_NewDynamicFeeTx(t *testing.T) {

core/chains/evm/txmgr/client.go

@@ -77,10 +77,14 @@ func (c *evmTxmClient) BatchSendTransactions(
 			// convert to tx for logging purposes - exits early if error occurs
 			tx, signedErr := GetGethSignedTx(attempts[i].SignedRawTx)
 			if signedErr != nil {
-				processingErr[i] = fmt.Errorf("failed to process tx (index %d): %w", i, signedErr)
+				signedErrMsg := fmt.Sprintf("failed to process tx (index %d)", i)
+				lggr.Errorw(signedErrMsg, "err", signedErr)
+				processingErr[i] = fmt.Errorf("%s: %w", signedErrMsg, signedErr)
 				return
 			}
-			codes[i], txErrs[i] = client.ClassifySendError(reqs[i].Error, lggr, tx, attempts[i].Tx.FromAddress, c.client.IsL2())
+			sendErr := reqs[i].Error
+			codes[i] = client.ClassifySendError(sendErr, lggr, tx, attempts[i].Tx.FromAddress, c.client.IsL2())
+			txErrs[i] = sendErr
 		}(index)
 	}
 	wg.Wait()

core/chains/evm/txmgr/models.go

@@ -1,12 +1,10 @@
 package txmgr
 
 import (
-	"bytes"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/rlp"
 
 	"github.com/smartcontractkit/chainlink/v2/common/txmgr"
 	txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types"
@@ -65,9 +63,8 @@ const (
 
 // GetGethSignedTx decodes the SignedRawTx into a types.Transaction struct
 func GetGethSignedTx(signedRawTx []byte) (*types.Transaction, error) {
-	s := rlp.NewStream(bytes.NewReader(signedRawTx), 0)
 	signedTx := new(types.Transaction)
-	if err := signedTx.DecodeRLP(s); err != nil {
+	if err := signedTx.UnmarshalBinary(signedRawTx); err != nil {
 		return nil, err
 	}
 	return signedTx, nil

core/chains/evm/txmgr/txmgr_test.go

@@ -1,7 +1,6 @@
 package txmgr_test
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -651,14 +650,14 @@ func mustInsertUnconfirmedEthTxWithAttemptState(t *testing.T, txStore txmgr.Test
 	etx := cltest.MustInsertUnconfirmedEthTx(t, txStore, nonce, fromAddress, opts...)
 	attempt := cltest.NewLegacyEthTxAttempt(t, etx.ID)
 
-	tx := types.NewTransaction(uint64(nonce), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
-	rlp := new(bytes.Buffer)
-	require.NoError(t, tx.EncodeRLP(rlp))
-	attempt.SignedRawTx = rlp.Bytes()
+	tx := cltest.NewLegacyTransaction(uint64(nonce), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
+	encodedTx, err := tx.MarshalBinary()
+	require.NoError(t, err)
+	attempt.SignedRawTx = encodedTx
 
 	attempt.State = txAttemptState
 	require.NoError(t, txStore.InsertTxAttempt(&attempt))
-	etx, err := txStore.FindTxWithAttempts(etx.ID)
+	etx, err = txStore.FindTxWithAttempts(etx.ID)
 	require.NoError(t, err)
 	return etx
 }
@@ -679,13 +678,13 @@ func mustInsertUnconfirmedEthTxWithBroadcastDynamicFeeAttempt(t *testing.T, txSt
 		Data:      []byte{2, 3, 4},
 	}
 	tx := types.NewTx(&dtx)
-	rlp := new(bytes.Buffer)
-	require.NoError(t, tx.EncodeRLP(rlp))
-	attempt.SignedRawTx = rlp.Bytes()
+	encodedTx, err := tx.MarshalBinary()
+	require.NoError(t, err)
+	attempt.SignedRawTx = encodedTx
 
 	attempt.State = txmgrtypes.TxAttemptBroadcast
 	require.NoError(t, txStore.InsertTxAttempt(&attempt))
-	etx, err := txStore.FindTxWithAttempts(etx.ID)
+	etx, err = txStore.FindTxWithAttempts(etx.ID)
 	require.NoError(t, err)
 	return etx
 }
@@ -702,14 +701,14 @@ func mustInsertUnconfirmedEthTxWithInsufficientEthAttempt(t *testing.T, txStore
 	require.NoError(t, txStore.InsertTx(&etx))
 	attempt := cltest.NewLegacyEthTxAttempt(t, etx.ID)
 
-	tx := types.NewTransaction(uint64(nonce), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
-	rlp := new(bytes.Buffer)
-	require.NoError(t, tx.EncodeRLP(rlp))
-	attempt.SignedRawTx = rlp.Bytes()
+	tx := cltest.NewLegacyTransaction(uint64(nonce), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
+	encodedTx, err := tx.MarshalBinary()
+	require.NoError(t, err)
+	attempt.SignedRawTx = encodedTx
 
 	attempt.State = txmgrtypes.TxAttemptInsufficientFunds
 	require.NoError(t, txStore.InsertTxAttempt(&attempt))
-	etx, err := txStore.FindTxWithAttempts(etx.ID)
+	etx, err = txStore.FindTxWithAttempts(etx.ID)
 	require.NoError(t, err)
 	return etx
 }
@@ -740,13 +739,13 @@ func mustInsertInProgressEthTxWithAttempt(t *testing.T, txStore txmgr.TestEvmTxS
 	etx.State = txmgrcommon.TxInProgress
 	require.NoError(t, txStore.InsertTx(&etx))
 	attempt := cltest.NewLegacyEthTxAttempt(t, etx.ID)
-	tx := types.NewTransaction(uint64(nonce), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
-	rlp := new(bytes.Buffer)
-	require.NoError(t, tx.EncodeRLP(rlp))
-	attempt.SignedRawTx = rlp.Bytes()
+	tx := cltest.NewLegacyTransaction(uint64(nonce), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
+	encodedTx, err := tx.MarshalBinary()
+	require.NoError(t, err)
+	attempt.SignedRawTx = encodedTx
 	attempt.State = txmgrtypes.TxAttemptInProgress
 	require.NoError(t, txStore.InsertTxAttempt(&attempt))
-	etx, err := txStore.FindTxWithAttempts(etx.ID)
+	etx, err = txStore.FindTxWithAttempts(etx.ID)
 	require.NoError(t, err)
 	return etx
 }