feat: add conf depth awareness to txmgr

.changeset/fuzzy-rivers-double.md

+---
+'@eth-optimism/batch-submitter-service': patch
+---
+
+Adds confirmation depth awareness to txmgr

go/batch-submitter/batch_submitter.go

@@ -164,6 +164,7 @@ func NewBatchSubmitter(cfg Config, gitVersion string) (*BatchSubmitter, error) {
 		GasRetryIncrement:    utils.GasPriceFromGwei(cfg.GasRetryIncrement),
 		ResubmissionTimeout:  cfg.ResubmissionTimeout,
 		ReceiptQueryInterval: time.Second,
+		NumConfirmations:     cfg.NumConfirmations,
 	}
 
 	var batchTxService *Service

go/batch-submitter/drivers/clear_pending_tx_test.go

@@ -103,11 +103,14 @@ func TestSignClearingTxEstimateGasFail(t *testing.T) {
 }
 
 type clearPendingTxHarness struct {
-	l1Client drivers.L1Client
+	l1Client *mock.L1Client
 	txMgr    txmgr.TxManager
 }
 
-func newClearPendingTxHarness(l1ClientConfig mock.L1ClientConfig) *clearPendingTxHarness {
+func newClearPendingTxHarnessWithNumConfs(
+	l1ClientConfig mock.L1ClientConfig,
+	numConfirmations uint64,
+) *clearPendingTxHarness {
 
 	if l1ClientConfig.BlockNumber == nil {
 		l1ClientConfig.BlockNumber = func(_ context.Context) (uint64, error) {
@@ -132,6 +135,7 @@ func newClearPendingTxHarness(l1ClientConfig mock.L1ClientConfig) *clearPendingT
 		GasRetryIncrement:    utils.GasPriceFromGwei(5),
 		ResubmissionTimeout:  time.Second,
 		ReceiptQueryInterval: 50 * time.Millisecond,
+		NumConfirmations:     numConfirmations,
 	}, l1Client)
 
 	return &clearPendingTxHarness{
@@ -140,6 +144,10 @@ func newClearPendingTxHarness(l1ClientConfig mock.L1ClientConfig) *clearPendingT
 	}
 }
 
+func newClearPendingTxHarness(l1ClientConfig mock.L1ClientConfig) *clearPendingTxHarness {
+	return newClearPendingTxHarnessWithNumConfs(l1ClientConfig, 1)
+}
+
 // TestClearPendingTxClearingTxÇonfirms asserts the happy path where our
 // clearing transactions confirms unobstructed.
 func TestClearPendingTxClearingTxConfirms(t *testing.T) {
@@ -149,7 +157,8 @@ func TestClearPendingTxClearingTxConfirms(t *testing.T) {
 		},
 		TransactionReceipt: func(_ context.Context, txHash common.Hash) (*types.Receipt, error) {
 			return &types.Receipt{
-				TxHash: txHash,
+				TxHash:      txHash,
+				BlockNumber: big.NewInt(int64(testBlockNumber)),
 			}, nil
 		},
 	})
@@ -197,3 +206,42 @@ func TestClearPendingTxTimeout(t *testing.T) {
 	)
 	require.Equal(t, txmgr.ErrPublishTimeout, err)
 }
+
+// TestClearPendingTxMultipleConfs tests we wait the appropriate number of
+// confirmations for the clearing transaction to confirm.
+func TestClearPendingTxMultipleConfs(t *testing.T) {
+	const numConfs = 2
+
+	// Instantly confirm transaction.
+	h := newClearPendingTxHarnessWithNumConfs(mock.L1ClientConfig{
+		SendTransaction: func(_ context.Context, _ *types.Transaction) error {
+			return nil
+		},
+		TransactionReceipt: func(_ context.Context, txHash common.Hash) (*types.Receipt, error) {
+			return &types.Receipt{
+				TxHash:      txHash,
+				BlockNumber: big.NewInt(int64(testBlockNumber)),
+			}, nil
+		},
+	}, numConfs)
+
+	// The txmgr should timeout waiting for the txn to confirm.
+	err := drivers.ClearPendingTx(
+		"test", context.Background(), h.txMgr, h.l1Client, testWalletAddr,
+		testPrivKey, testChainID,
+	)
+	require.Equal(t, txmgr.ErrPublishTimeout, err)
+
+	// Now set the chain height to the earliest the transaction will be
+	// considered sufficiently confirmed.
+	h.l1Client.SetBlockNumberFunc(func(_ context.Context) (uint64, error) {
+		return testBlockNumber + numConfs - 1, nil
+	})
+
+	// Publishing should succeed.
+	err = drivers.ClearPendingTx(
+		"test", context.Background(), h.txMgr, h.l1Client, testWalletAddr,
+		testPrivKey, testChainID,
+	)
+	require.Nil(t, err)
+}

go/batch-submitter/txmgr/txmgr.go

@@ -52,6 +52,10 @@ type Config struct {
 	// query the backend to check for confirmations after a tx at a
 	// specific gas price has been published.
 	ReceiptQueryInterval time.Duration
+
+	// NumConfirmations specifies how many blocks are need to consider a
+	// transaction confirmed.
+	NumConfirmations uint64
 }
 
 // TxManager is an interface that allows callers to reliably publish txs,
@@ -93,6 +97,10 @@ type SimpleTxManager struct {
 func NewSimpleTxManager(
 	name string, cfg Config, backend ReceiptSource) *SimpleTxManager {
 
+	if cfg.NumConfirmations == 0 {
+		panic("txmgr: NumConfirmations cannot be zero")
+	}
+
 	return &SimpleTxManager{
 		name:    name,
 		cfg:     cfg,
@@ -151,6 +159,7 @@ func (m *SimpleTxManager) Send(
 		// back to the main event loop if found.
 		receipt, err := WaitMined(
 			ctxc, m.backend, tx, m.cfg.ReceiptQueryInterval,
+			m.cfg.NumConfirmations,
 		)
 		if err != nil {
 			log.Debug(name+" send tx failed", "hash", txHash,
@@ -223,6 +232,7 @@ func WaitMined(
 	backend ReceiptSource,
 	tx *types.Transaction,
 	queryInterval time.Duration,
+	numConfirmations uint64,
 ) (*types.Receipt, error) {
 
 	queryTicker := time.NewTicker(queryInterval)
@@ -232,14 +242,42 @@ func WaitMined(
 
 	for {
 		receipt, err := backend.TransactionReceipt(ctx, txHash)
-		if receipt != nil {
-			return receipt, nil
-		}
+		switch {
+		case receipt != nil:
+			txHeight := receipt.BlockNumber.Uint64()
+			tipHeight, err := backend.BlockNumber(ctx)
+			if err != nil {
+				log.Error("Unable to fetch block number", "err", err)
+				break
+			}
 
-		if err != nil {
+			log.Trace("Transaction mined, checking confirmations",
+				"txHash", txHash, "txHeight", txHeight,
+				"tipHeight", tipHeight,
+				"numConfirmations", numConfirmations)
+
+			// The transaction is considered confirmed when
+			// txHeight+numConfirmations-1 <= tipHeight. Note that the -1 is
+			// needed to account for the fact that confirmations have an
+			// inherent off-by-one, i.e. when using 1 confirmation the
+			// transaction should be confirmed when txHeight is equal to
+			// tipHeight. The equation is rewritten in this form to avoid
+			// underflows.
+			if txHeight+numConfirmations <= tipHeight+1 {
+				log.Info("Transaction confirmed", "txHash", txHash)
+				return receipt, nil
+			}
+
+			// Safe to subtract since we know the LHS above is greater.
+			confsRemaining := (txHeight + numConfirmations) - (tipHeight + 1)
+			log.Info("Transaction not yet confirmed", "txHash", txHash,
+				"confsRemaining", confsRemaining)
+
+		case err != nil:
 			log.Trace("Receipt retrievel failed", "hash", txHash,
 				"err", err)
-		} else {
+
+		default:
 			log.Trace("Transaction not yet mined", "hash", txHash)
 		}
 

go/batch-submitter/txmgr/txmgr_test.go

@@ -95,13 +95,18 @@ func newTestHarnessWithConfig(cfg txmgr.Config) *testHarness {
 // newTestHarness initializes a testHarness with a defualt configuration that is
 // suitable for most tests.
 func newTestHarness() *testHarness {
-	return newTestHarnessWithConfig(txmgr.Config{
+	return newTestHarnessWithConfig(configWithNumConfs(1))
+}
+
+func configWithNumConfs(numConfirmations uint64) txmgr.Config {
+	return txmgr.Config{
 		MinGasPrice:          new(big.Int).SetUint64(5),
 		MaxGasPrice:          new(big.Int).SetUint64(50),
 		GasRetryIncrement:    new(big.Int).SetUint64(5),
 		ResubmissionTimeout:  time.Second,
 		ReceiptQueryInterval: 50 * time.Millisecond,
-	})
+		NumConfirmations:     numConfirmations,
+	}
 }
 
 type minedTxInfo struct {
@@ -392,7 +397,7 @@ func TestWaitMinedReturnsReceiptOnFirstSuccess(t *testing.T) {
 	h.backend.mine(&txHash, new(big.Int))
 
 	ctx := context.Background()
-	receipt, err := txmgr.WaitMined(ctx, h.backend, tx, 50*time.Millisecond)
+	receipt, err := txmgr.WaitMined(ctx, h.backend, tx, 50*time.Millisecond, 1)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	require.Equal(t, receipt.TxHash, txHash)
@@ -411,9 +416,54 @@ func TestWaitMinedCanBeCanceled(t *testing.T) {
 	// Create an unimined tx.
 	tx := types.NewTx(&types.LegacyTx{})
 
-	receipt, err := txmgr.WaitMined(ctx, h.backend, tx, 50*time.Millisecond)
+	receipt, err := txmgr.WaitMined(ctx, h.backend, tx, 50*time.Millisecond, 1)
+	require.Equal(t, err, context.DeadlineExceeded)
+	require.Nil(t, receipt)
+}
+
+// TestWaitMinedMultipleConfs asserts that WaitMiend will properly wait for more
+// than one confirmation.
+func TestWaitMinedMultipleConfs(t *testing.T) {
+	t.Parallel()
+
+	const numConfs = 2
+
+	h := newTestHarnessWithConfig(configWithNumConfs(numConfs))
+	ctxt, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	// Create an unimined tx.
+	tx := types.NewTx(&types.LegacyTx{})
+	txHash := tx.Hash()
+	h.backend.mine(&txHash, new(big.Int))
+
+	receipt, err := txmgr.WaitMined(ctxt, h.backend, tx, 50*time.Millisecond, numConfs)
 	require.Equal(t, err, context.DeadlineExceeded)
 	require.Nil(t, receipt)
+
+	ctxt, cancel = context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	// Mine an empty block, tx should now be confirmed.
+	h.backend.mine(nil, nil)
+	receipt, err = txmgr.WaitMined(ctxt, h.backend, tx, 50*time.Millisecond, numConfs)
+	require.Nil(t, err)
+	require.NotNil(t, receipt)
+	require.Equal(t, txHash, receipt.TxHash)
+}
+
+// TestManagerPanicOnZeroConfs ensures that the NewSimpleTxManager will panic
+// when attempting to configure with NumConfirmations set to zero.
+func TestManagerPanicOnZeroConfs(t *testing.T) {
+	t.Parallel()
+
+	defer func() {
+		if r := recover(); r == nil {
+			t.Fatal("NewSimpleTxManager should panic when using zero conf")
+		}
+	}()
+
+	_ = newTestHarnessWithConfig(configWithNumConfs(0))
 }
 
 // failingBackend implements txmgr.ReceiptSource, returning a failure on the
@@ -465,7 +515,7 @@ func TestWaitMinedReturnsReceiptAfterFailure(t *testing.T) {
 	txHash := tx.Hash()
 
 	ctx := context.Background()
-	receipt, err := txmgr.WaitMined(ctx, &borkedBackend, tx, 50*time.Millisecond)
+	receipt, err := txmgr.WaitMined(ctx, &borkedBackend, tx, 50*time.Millisecond, 1)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	require.Equal(t, receipt.TxHash, txHash)