Merge pull request #5044 from ethereum-optimism/jg/batcher_reuse_tx

op-service/txmgr: Return previous tx if no updates

op-service/txmgr/txmgr.go

@@ -122,6 +122,11 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa
 		gasTipCap = tip
 	}
 
+	// Return the same transaction if we don't update any fields.
+	// We do this because ethereum signatures are not deterministic and therefore the transaction hash will change
+	// when we re-sign the tx. We don't want to do that because we want to see ErrAlreadyKnown instead of ErrReplacementUnderpriced
+	var reusedTip, reusedFeeCap bool
+
 	// new = old * (100 + priceBump) / 100
 	// Enforce a min priceBump on the tip. Do this before the feeCap is calculated
 	thresholdTip := new(big.Int).Mul(priceBumpPercent, tx.GasTipCap())
@@ -129,6 +134,7 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa
 	if tx.GasTipCapIntCmp(gasTipCap) >= 0 {
 		m.l.Debug("Reusing the previous tip", "previous", tx.GasTipCap(), "suggested", gasTipCap)
 		gasTipCap = tx.GasTipCap()
+		reusedTip = true
 	} else if thresholdTip.Cmp(gasTipCap) > 0 {
 		m.l.Debug("Overriding the tip to enforce a price bump", "previous", tx.GasTipCap(), "suggested", gasTipCap, "new", thresholdTip)
 		gasTipCap = thresholdTip
@@ -150,11 +156,16 @@ func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transa
 	if tx.GasFeeCapIntCmp(gasFeeCap) >= 0 {
 		m.l.Debug("Reusing the previous fee cap", "previous", tx.GasFeeCap(), "suggested", gasFeeCap)
 		gasFeeCap = tx.GasFeeCap()
+		reusedFeeCap = true
 	} else if thresholdFeeCap.Cmp(gasFeeCap) > 0 {
 		m.l.Debug("Overriding the fee cap to enforce a price bump", "previous", tx.GasFeeCap(), "suggested", gasFeeCap, "new", thresholdFeeCap)
 		gasFeeCap = thresholdFeeCap
 	}
 
+	if reusedTip && reusedFeeCap {
+		return tx, nil
+	}
+
 	rawTx := &types.DynamicFeeTx{
 		ChainID:    tx.ChainId(),
 		Nonce:      tx.Nonce(),

op-service/txmgr/txmgr_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"math/big"
+	"math/rand"
 	"sync"
 	"testing"
 	"time"
@@ -11,9 +12,12 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/ethereum-optimism/optimism/op-node/testlog"
+	"github.com/ethereum-optimism/optimism/op-node/testutils"
+	opcrypto "github.com/ethereum-optimism/optimism/op-service/crypto"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/log"
 )
 
@@ -727,3 +731,42 @@ func TestIncreaseGasPriceUseLargeIncrease(t *testing.T) {
 	require.True(t, newTx.GasFeeCap().Cmp(feeCap) == 0, "new tx fee cap must be equal L1")
 	require.True(t, newTx.GasTipCap().Cmp(borkedBackend.gasTip) == 0, "new tx tip must be equal L1")
 }
+
+// TestIncreaseGasPriceReusesTransaction asserts that if the L1 basefee & tip remain the
+// same, the transaction is returned with the same signature values. The means that the error
+// when submitting the transaction to the network is ErrAlreadyKnown instead of ErrReplacementUnderpriced
+func TestIncreaseGasPriceReusesTransaction(t *testing.T) {
+	t.Parallel()
+
+	borkedBackend := failingBackend{
+		gasTip:  big.NewInt(10),
+		baseFee: big.NewInt(45),
+	}
+	pk := testutils.InsecureRandomKey(rand.New(rand.NewSource(123)))
+	signer := opcrypto.PrivateKeySignerFn(pk, big.NewInt(10))
+
+	mgr := &SimpleTxManager{
+		Config: Config{
+			ResubmissionTimeout:       time.Second,
+			ReceiptQueryInterval:      50 * time.Millisecond,
+			NumConfirmations:          1,
+			SafeAbortNonceTooLowCount: 3,
+			Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) {
+				return signer(from, tx)
+			},
+			From: crypto.PubkeyToAddress(pk.PublicKey),
+		},
+		name:    "TEST",
+		backend: &borkedBackend,
+		l:       testlog.Logger(t, log.LvlCrit),
+	}
+	tx := types.NewTx(&types.DynamicFeeTx{
+		GasTipCap: big.NewInt(10),
+		GasFeeCap: big.NewInt(100),
+	})
+
+	ctx := context.Background()
+	newTx, err := mgr.IncreaseGasPrice(ctx, tx)
+	require.NoError(t, err)
+	require.Equal(t, tx.Hash(), newTx.Hash())
+}