Fix batch tx send encoding

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

@@ -424,6 +424,7 @@ func ClassifySendError(err error, lggr logger.Logger, tx *types.Transaction, fro
 		return commonclient.Fatal, err
 	}
 	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
@@ -446,10 +447,12 @@ func ClassifySendError(err error, lggr logger.Logger, tx *types.Transaction, fro
 		return commonclient.Successful, err
 	}
 	if sendError.IsTerminallyUnderpriced() {
+		lggr.Errorw("Transaction terminally underpriced", "txHash", tx.Hash, "err", sendError)
 		return commonclient.Underpriced, err
 	}
 	if sendError.L2FeeTooLow() || sendError.IsL2FeeTooHigh() || sendError.IsL2Full() {
 		if isL2 {
+			lggr.Errorw("Transaction fee out of range", "err", sendError)
 			return commonclient.FeeOutOfValidRange, err
 		}
 		return commonclient.Unsupported, errors.Wrap(sendError, "this error type only handled for L2s")
@@ -469,7 +472,9 @@ func ClassifySendError(err error, lggr logger.Logger, tx *types.Transaction, fro
 		return commonclient.InsufficientFunds, err
 	}
 	if sendError.IsTimeout() {
-		return commonclient.Retryable, errors.Wrapf(sendError, "timeout while sending transaction %s", tx.Hash().Hex())
+		errorMsg := fmt.Sprintf("timeout while sending transaction %s", tx.Hash().Hex())
+		lggr.Errorw(errorMsg, "err", sendError)
+		return commonclient.Retryable, fmt.Errorf("%s: %w", errorMsg, sendError)
 	}
 	if sendError.IsTxFeeExceedsCap() {
 		logger.Criticalw(lggr, fmt.Sprintf("Sending transaction failed: %s", label.RPCTxFeeCapConfiguredIncorrectlyWarning),
@@ -479,6 +484,7 @@ func ClassifySendError(err error, lggr logger.Logger, tx *types.Transaction, fro
 		)
 		return commonclient.ExceedsMaxFee, err
 	}
+	lggr.Errorw("Unknown error encountered when sending transaction", "err", err)
 	return commonclient.Unknown, err
 }
 

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,7 +77,9 @@ 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())

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
 }

core/chains/evm/types/models_test.go

@@ -1,7 +1,6 @@
 package types_test
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"math"
@@ -12,7 +11,6 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
-	gethTypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -95,25 +93,24 @@ func TestEthTxAttempt_GetSignedTx(t *testing.T) {
 	cfg := configtest.NewGeneralConfig(t, nil)
 	ethKeyStore := cltest.NewKeyStore(t, db, cfg.Database()).Eth()
 	_, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore)
-	tx := gethTypes.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})
 
 	chainID := big.NewInt(3)
 
 	signedTx, err := ethKeyStore.SignTx(fromAddress, tx, chainID)
 	require.NoError(t, err)
-	signedTx.Size() // Needed to write the size for equality checking
-	rlp := new(bytes.Buffer)
-	require.NoError(t, signedTx.EncodeRLP(rlp))
+	encodedTx, err := signedTx.MarshalBinary()
+	require.NoError(t, err)
 
-	attempt := txmgr.TxAttempt{SignedRawTx: rlp.Bytes()}
+	attempt := txmgr.TxAttempt{SignedRawTx: encodedTx}
 
 	gotSignedTx, err := txmgr.GetGethSignedTx(attempt.SignedRawTx)
 	require.NoError(t, err)
-	decodedEncoded := new(bytes.Buffer)
-	require.NoError(t, gotSignedTx.EncodeRLP(decodedEncoded))
+	decodedTx, err2 := gotSignedTx.MarshalBinary()
+	require.NoError(t, err2)
 
 	require.Equal(t, signedTx.Hash(), gotSignedTx.Hash())
-	require.Equal(t, attempt.SignedRawTx, decodedEncoded.Bytes())
+	require.Equal(t, attempt.SignedRawTx, decodedTx)
 }
 
 func TestHead_ChainLength(t *testing.T) {

core/internal/cltest/factories.go

@@ -145,6 +145,18 @@ func NewEthTx(fromAddress common.Address) txmgr.Tx {
 	}
 }
 
+func NewLegacyTransaction(nonce uint64, to common.Address, value *big.Int, gasLimit uint32, gasPrice *big.Int, data []byte) *types.Transaction {
+	tx := types.LegacyTx{
+		Nonce:    nonce,
+		To:       &to,
+		Value:    value,
+		Gas:      uint64(gasLimit),
+		GasPrice: gasPrice,
+		Data:     data,
+	}
+	return types.NewTx(&tx)
+}
+
 func MustInsertUnconfirmedEthTx(t *testing.T, txStore txmgr.TestEvmTxStore, nonce int64, fromAddress common.Address, opts ...interface{}) txmgr.Tx {
 	broadcastAt := time.Now()
 	chainID := &FixtureChainID
@@ -172,7 +184,7 @@ func MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t *testing.T, txStore
 	etx := MustInsertUnconfirmedEthTx(t, txStore, nonce, fromAddress, opts...)
 	attempt := NewLegacyEthTxAttempt(t, etx.ID)
 
-	tx := types.NewTransaction(uint64(nonce), testutils.NewAddress(), big.NewInt(142), 242, big.NewInt(342), []byte{1, 2, 3})
+	tx := NewLegacyTransaction(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()