Merge pull request #7250 from ethereum-optimism/inphi/txmgr-nonce

txmgr: Do not update nonce on failed gas estimate

op-service/txmgr/txmgr.go

@@ -211,14 +211,8 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*
 	}
 	gasFeeCap := calcGasFeeCap(basefee, gasTipCap)
 
-	nonce, err := m.nextNonce(ctx)
-	if err != nil {
-		return nil, err
-	}
-
 	rawTx := &types.DynamicFeeTx{
 		ChainID:   m.chainID,
-		Nonce:     nonce,
 		To:        candidate.To,
 		GasTipCap: gasTipCap,
 		GasFeeCap: gasFeeCap,
@@ -247,6 +241,13 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*
 		rawTx.Gas = gas
 	}
 
+	// Avoid bumping the nonce if the gas estimation fails.
+	nonce, err := m.nextNonce(ctx)
+	if err != nil {
+		return nil, err
+	}
+	rawTx.Nonce = nonce
+
 	ctx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout)
 	defer cancel()
 	return m.cfg.Signer(ctx, m.cfg.From, types.NewTx(rawTx))

op-service/txmgr/txmgr_test.go

@@ -93,6 +93,7 @@ type gasPricer struct {
 	mineAtEpoch   int64
 	baseGasTipFee *big.Int
 	baseBaseFee   *big.Int
+	err           error
 	mu            sync.Mutex
 }
 
@@ -206,6 +207,9 @@ func (b *mockBackend) HeaderByNumber(ctx context.Context, number *big.Int) (*typ
 }
 
 func (b *mockBackend) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) {
+	if b.g.err != nil {
+		return 0, b.g.err
+	}
 	return b.g.basefee().Uint64(), nil
 }
 
@@ -420,6 +424,31 @@ func TestTxMgr_EstimateGas(t *testing.T) {
 	require.Equal(t, gasEstimate, tx.Gas())
 }
 
+func TestTxMgr_EstimateGasFails(t *testing.T) {
+	t.Parallel()
+	h := newTestHarness(t)
+	candidate := h.createTxCandidate()
+
+	// Set the gas limit to zero to trigger gas estimation.
+	candidate.GasLimit = 0
+
+	// Craft a successful transaction.
+	tx, err := h.mgr.craftTx(context.Background(), candidate)
+	require.Nil(t, err)
+	lastNonce := tx.Nonce()
+
+	// Mock gas estimation failure.
+	h.gasPricer.err = fmt.Errorf("execution error")
+	_, err = h.mgr.craftTx(context.Background(), candidate)
+	require.ErrorContains(t, err, "failed to estimate gas")
+
+	// Ensure successful craft uses the correct nonce
+	h.gasPricer.err = nil
+	tx, err = h.mgr.craftTx(context.Background(), candidate)
+	require.Nil(t, err)
+	require.Equal(t, lastNonce+1, tx.Nonce())
+}
+
 // TestTxMgrOnlyOnePublicationSucceeds asserts that the tx manager will return a
 // receipt so long as at least one of the publications is able to succeed with a
 // simulated rpc failure.