From 3a629d9d04c370a9a068a199e2dac7824d95e2cd Mon Sep 17 00:00:00 2001 From: Luke Iannucci Date: Fri, 3 Jul 2026 14:10:52 -0400 Subject: [PATCH 1/2] address audit m3 --- op-batcher/batcher/driver.go | 15 ------------ op-batcher/batcher/espresso_driver.go | 17 ++++---------- op-batcher/batcher/fallback_auth.go | 29 ++++++------------------ op-batcher/batcher/fallback_auth_test.go | 18 +++++++-------- 4 files changed, 19 insertions(+), 60 deletions(-) diff --git a/op-batcher/batcher/driver.go b/op-batcher/batcher/driver.go index 1b0f4aa73b9..056e510684c 100644 --- a/op-batcher/batcher/driver.go +++ b/op-batcher/batcher/driver.go @@ -136,15 +136,6 @@ type BatchSubmitter struct { throttleController *throttler.ThrottleController publishSignal chan pubInfo - - // authGroup tracks the fallback batcher's receipt-watcher goroutines (one - // per auth+batch pair) so the publishing loop can drain them via - // waitForAuthGroup before closing receiptsCh. New watchers are back-pressured - // (not hard-bounded) by the txmgr Queue: queue.Send blocks at - // MaxPendingTransactions, so watchers are created no faster than txs drain, - // though a slow receipts loop can briefly leave more than that parked on their - // final receiptsCh send. - authGroup sync.WaitGroup } // NewBatchSubmitter initializes the BatchSubmitter driver from a preconfigured DriverSetup @@ -537,12 +528,6 @@ func (l *BatchSubmitter) publishingLoop(ctx context.Context, wg *sync.WaitGroup, } } - // Wait for all in-flight fallback-auth submissions to complete to prevent - // new transactions being queued. No-op when the rollup is not configured - // with a BatchAuthenticator or when the EspressoTime hardfork has not - // activated. - l.waitForAuthGroup() - // We _must_ wait for all senders on receiptsCh to finish before we can close it. if err := txQueue.Wait(); err != nil { if !errors.Is(err, context.Canceled) { diff --git a/op-batcher/batcher/espresso_driver.go b/op-batcher/batcher/espresso_driver.go index 3c2c6a31056..33512cb1a09 100644 --- a/op-batcher/batcher/espresso_driver.go +++ b/op-batcher/batcher/espresso_driver.go @@ -6,20 +6,11 @@ import ( "github.com/ethereum-optimism/optimism/op-service/txmgr" ) -// waitForAuthGroup blocks until all in-flight fallback-auth watcher goroutines -// have finished. publishingLoop calls it before closing receiptsCh: each watcher -// is a sender on receiptsCh, so the receipts loop must still be draining -// receiptsCh at this point or a watcher's final send would block forever. Each -// watcher always terminates because the txmgr Queue emits exactly one receipt per -// Send, even on context cancellation. -func (l *BatchSubmitter) waitForAuthGroup() { - l.authGroup.Wait() -} - // dispatchAuthenticatedSendTx routes sendTx through the fallback-batcher -// post-fork auth path, returning true when the tx has been handed off to -// authGroup. Returns false to mean "fall through to the upstream queue.Send -// path" — pre-fork operation and any cancel tx. +// post-fork auth path, returning true when the tx has been fully handled +// (sendTxWithFallbackAuth blocks until the pair is resolved and a receipt has +// been forwarded). Returns false to mean "fall through to the upstream +// queue.Send path" — pre-fork operation and any cancel tx. // // The fallback batcher consults isFallbackAuthRequired to gate authentication // behind the EspressoTime hardfork: pre-fork the verifier accepts plain diff --git a/op-batcher/batcher/fallback_auth.go b/op-batcher/batcher/fallback_auth.go index 61a3de51641..b5001e3b166 100644 --- a/op-batcher/batcher/fallback_auth.go +++ b/op-batcher/batcher/fallback_auth.go @@ -79,38 +79,18 @@ func (l *BatchSubmitter) sendTxWithFallbackAuth(txdata txData, isCancel bool, ca To: &l.RollupConfig.BatchAuthenticatorAddress, } - // Private buffered channels: queue.Send forwards exactly one receipt to each, so the watcher - // reads exactly once per channel (even on context cancellation, the queue still emits a - // ctx-error receipt). These never reach handleReceipt; only the synthetic receipt below does. - authReceiptCh := make(chan txmgr.TxReceipt[txRef], 1) - batchReceiptCh := make(chan txmgr.TxReceipt[txRef], 1) - l.Log.Debug( "Sending fallback authenticateBatchInfo transaction", "txRef", transactionReference, "commitment", hexutil.Encode(commitment[:]), "address", l.RollupConfig.BatchAuthenticatorAddress.String(), ) - // Submit the auth tx then the batch tx, in order, on the publishing-loop goroutine so their + // Submit the auth tx wait for receipt, then send the batch tx, on the publishing-loop goroutine so their // nonces are assigned in submission order. Each Send blocks here when the queue is at its // MaxPendingTransactions limit. + authReceiptCh := make(chan txmgr.TxReceipt[txRef], 1) queue.Send(transactionReference, verifyCandidate, authReceiptCh) - queue.Send(transactionReference, *candidate, batchReceiptCh) - - l.authGroup.Add(1) - go func() { - defer l.authGroup.Done() - l.watchFallbackAuthReceipts(transactionReference, authReceiptCh, batchReceiptCh, receiptsCh) - }() -} - -// watchFallbackAuthReceipts collects the auth and batch receipts for a fallback-auth pair, -// validates that the batch tx landed within the lookback window of the auth tx, and forwards a -// single synthetic receipt keyed to the batch txData onto receiptsCh. Any failure produces an -// error receipt so the channel manager rewinds and resubmits the frame set. -func (l *BatchSubmitter) watchFallbackAuthReceipts(transactionReference txRef, authReceiptCh, batchReceiptCh chan txmgr.TxReceipt[txRef], receiptsCh chan txmgr.TxReceipt[txRef]) { authResult := <-authReceiptCh - batchResult := <-batchReceiptCh if authResult.Err != nil { l.Log.Error("Failed to send fallback authenticateBatchInfo transaction", "txRef", transactionReference, "err", authResult.Err) @@ -135,6 +115,11 @@ func (l *BatchSubmitter) watchFallbackAuthReceipts(transactionReference txRef, a return } + // The auth tx is confirmed on L1. Now send the batch tx + batchReceiptCh := make(chan txmgr.TxReceipt[txRef], 1) + queue.Send(transactionReference, *candidate, batchReceiptCh) + batchResult := <-batchReceiptCh + if batchResult.Err != nil { l.Log.Error("Failed to send batch inbox transaction", "txRef", transactionReference, "err", batchResult.Err) receiptsCh <- txmgr.TxReceipt[txRef]{ diff --git a/op-batcher/batcher/fallback_auth_test.go b/op-batcher/batcher/fallback_auth_test.go index 0822f8a091f..c50dd6a69cd 100644 --- a/op-batcher/batcher/fallback_auth_test.go +++ b/op-batcher/batcher/fallback_auth_test.go @@ -80,7 +80,6 @@ func TestFallbackAuth_OrderingAndSuccess(t *testing.T) { receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) - l.authGroup.Wait() require.Len(t, queue.sends, 2) // First send must target the BatchAuthenticator (the auth tx), giving it the @@ -103,18 +102,19 @@ func TestFallbackAuth_AuthFailureRetried(t *testing.T) { queue := &fakeTxSender{ responses: []txmgr.TxReceipt[txRef]{ - {Err: errSendFailed}, // auth fails - {Receipt: receiptWithBlock(101)}, // batch lands anyway + {Err: errSendFailed}, // auth fails. No batch response, it must never be sent }, } receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) - l.authGroup.Wait() got := <-receiptsCh require.Error(t, got.Err) require.Equal(t, txdata.ID().String(), got.ID.id.String()) + // The batch tx must not be sent after an auth failure: it would be crafted at the + // next nonce while the failed auth tx resets the txmgr nonce, leaving a nonce gap. + require.Len(t, queue.sends, 1) } func TestFallbackAuth_BatchFailureRetried(t *testing.T) { @@ -131,7 +131,6 @@ func TestFallbackAuth_BatchFailureRetried(t *testing.T) { receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) - l.authGroup.Wait() got := <-receiptsCh require.Error(t, got.Err) @@ -148,18 +147,19 @@ func TestFallbackAuth_AuthRevertedRetried(t *testing.T) { queue := &fakeTxSender{ responses: []txmgr.TxReceipt[txRef]{ - {Receipt: revertedReceiptWithBlock(100)}, // auth mined but reverted - {Receipt: receiptWithBlock(101)}, // batch lands + {Receipt: revertedReceiptWithBlock(100)}, // auth mined but reverted. Batch must never be sent }, } receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) - l.authGroup.Wait() got := <-receiptsCh require.Error(t, got.Err) require.Equal(t, txdata.ID().String(), got.ID.id.String()) + // A reverted auth tx emits no BatchInfoAuthenticated event, so the batch data would be + // unverifiable so the batch tx must not be submitted at all. + require.Len(t, queue.sends, 1) } // TestFallbackAuth_WindowViolationRetried verifies that a batch tx landing @@ -180,7 +180,6 @@ func TestFallbackAuth_WindowViolationRetried(t *testing.T) { receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) - l.authGroup.Wait() got := <-receiptsCh require.Error(t, got.Err) @@ -207,7 +206,6 @@ func TestFallbackAuth_WindowBoundaryAccepted(t *testing.T) { receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) - l.authGroup.Wait() got := <-receiptsCh require.NoError(t, got.Err) From 5dbf252c8565c940e2bc107cae7f19e351db95da Mon Sep 17 00:00:00 2001 From: Luke Iannucci Date: Fri, 3 Jul 2026 14:15:03 -0400 Subject: [PATCH 2/2] update outdated comment --- op-batcher/batcher/fallback_auth.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/op-batcher/batcher/fallback_auth.go b/op-batcher/batcher/fallback_auth.go index b5001e3b166..50f87b0f2a9 100644 --- a/op-batcher/batcher/fallback_auth.go +++ b/op-batcher/batcher/fallback_auth.go @@ -85,9 +85,8 @@ func (l *BatchSubmitter) sendTxWithFallbackAuth(txdata txData, isCancel bool, ca "commitment", hexutil.Encode(commitment[:]), "address", l.RollupConfig.BatchAuthenticatorAddress.String(), ) - // Submit the auth tx wait for receipt, then send the batch tx, on the publishing-loop goroutine so their - // nonces are assigned in submission order. Each Send blocks here when the queue is at its - // MaxPendingTransactions limit. + // Submit the auth tx and wait for its receipt, then send the batch tx. Each Send + // blocks here when the queue is at its MaxPendingTransactions limit. authReceiptCh := make(chan txmgr.TxReceipt[txRef], 1) queue.Send(transactionReference, verifyCandidate, authReceiptCh) authResult := <-authReceiptCh