diff --git a/op-batcher/batcher/config.go b/op-batcher/batcher/config.go index c6530742453..1aa99c098b9 100644 --- a/op-batcher/batcher/config.go +++ b/op-batcher/batcher/config.go @@ -191,6 +191,13 @@ func (c *CLIConfig) Check() error { if !flags.ValidDataAvailabilityType(c.DataAvailabilityType) { return fmt.Errorf("unknown data availability type: %q", c.DataAvailabilityType) } + // The auth→batch distance (num-confirmations + inclusion delay) must fit within + // BatchAuthLookbackWindow, so reserve room for the batch to land or the safe head stalls. + const fallbackAuthInclusionReserve = 75 // blocks (~15 min at 12s L1 slots) + if authLookback := derive.BatchAuthLookbackWindow; authLookback < c.TxMgrConfig.NumConfirmations+fallbackAuthInclusionReserve { + return fmt.Errorf("NumConfirmations (%d) too high for BatchAuthLookbackWindow (%d): need %d blocks of inclusion headroom", + c.TxMgrConfig.NumConfirmations, authLookback, fallbackAuthInclusionReserve) + } // Most chains' L1s still have only Cancun active, but we don't want to // overcomplicate this check with a dynamic L1 query, so we just use maxBlobsPerBlock. // We want to check for both, blobs and auto da-type. diff --git a/op-batcher/batcher/config_test.go b/op-batcher/batcher/config_test.go index 192ded7cb94..5031bbb33f5 100644 --- a/op-batcher/batcher/config_test.go +++ b/op-batcher/batcher/config_test.go @@ -106,6 +106,13 @@ func TestBatcherConfig(t *testing.T) { override: func(c *batcher.CLIConfig) { c.DataAvailabilityType = "foo" }, errString: "unknown data availability type: \"foo\"", }, + { + name: "num-confirmations leaves too little batch auth lookback window", + override: func(c *batcher.CLIConfig) { + c.TxMgrConfig.NumConfirmations = derive.BatchAuthLookbackWindow + }, + errString: "too high for BatchAuthLookbackWindow", + }, { name: "zero TargetNumFrames", override: func(c *batcher.CLIConfig) { c.TargetNumFrames = 0 }, diff --git a/op-batcher/batcher/fallback_auth.go b/op-batcher/batcher/fallback_auth.go index 61a3de51641..b27676e53cb 100644 --- a/op-batcher/batcher/fallback_auth.go +++ b/op-batcher/batcher/fallback_auth.go @@ -43,6 +43,13 @@ func computeCommitment(candidate *txmgr.TxCandidate) ([32]byte, error) { // separate signature is needed — the L1 transaction is already signed by the TxManager's key. func (l *BatchSubmitter) sendTxWithFallbackAuth(txdata txData, isCancel bool, candidate *txmgr.TxCandidate, queue TxSender[txRef], receiptsCh chan txmgr.TxReceipt[txRef]) { transactionReference := newTxRef(txdata, isCancel) + // The auth tx shares the batch txdata's identity (so a failure requeues the right frames) + // but is always a calldata tx. Auth failures must carry its real type: an ErrAlreadyReserved + // receipt labeled with the batch's blob type would make cancelBlockingTx cancel the wrong + // pool, leaving the reserving tx stuck. + authReference := transactionReference + authReference.isBlob = false + authReference.daType = DaTypeCalldata l.Log.Debug("Sending fallback-authenticated L1 transaction", "txRef", transactionReference) commitment, err := computeCommitment(candidate) @@ -92,30 +99,35 @@ func (l *BatchSubmitter) sendTxWithFallbackAuth(txdata txData, isCancel bool, ca "address", l.RollupConfig.BatchAuthenticatorAddress.String(), ) // Submit the auth tx then the batch tx, in order, 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. - queue.Send(transactionReference, verifyCandidate, authReceiptCh) + // nonces are assigned in submission order (auth = N, batch = N+1), guaranteeing the auth tx + // precedes the batch tx on chain. Each Send blocks here when the queue is at its + // MaxPendingTransactions limit, so the pair pipelines with subsequent pairs rather than + // stalling the loop for a full confirmation cycle. The auth tx uses authReference so its + // failures are typed as calldata (see above). + queue.Send(authReference, verifyCandidate, authReceiptCh) queue.Send(transactionReference, *candidate, batchReceiptCh) l.authGroup.Add(1) go func() { defer l.authGroup.Done() - l.watchFallbackAuthReceipts(transactionReference, authReceiptCh, batchReceiptCh, receiptsCh) + l.watchFallbackAuthReceipts(transactionReference, authReference, 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]) { +// error receipt so the channel manager rewinds and resubmits the frame set. Auth failures are +// reported under authReference so cancelBlockingTx targets the calldata pool, while the batch +// txData id is preserved so the right frames are requeued. +func (l *BatchSubmitter) watchFallbackAuthReceipts(transactionReference, authReference 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) receiptsCh <- txmgr.TxReceipt[txRef]{ - ID: transactionReference, + ID: authReference, Err: fmt.Errorf("failed to send fallback authenticateBatchInfo transaction: %w", authResult.Err), } return @@ -129,7 +141,7 @@ func (l *BatchSubmitter) watchFallbackAuthReceipts(transactionReference txRef, a if authResult.Receipt.Status != types.ReceiptStatusSuccessful { l.Log.Error("Fallback authenticateBatchInfo transaction reverted", "txRef", transactionReference, "txHash", authResult.Receipt.TxHash) receiptsCh <- txmgr.TxReceipt[txRef]{ - ID: transactionReference, + ID: authReference, Err: fmt.Errorf("fallback authenticateBatchInfo transaction reverted: %s", authResult.Receipt.TxHash), } return @@ -147,6 +159,7 @@ func (l *BatchSubmitter) watchFallbackAuthReceipts(transactionReference txRef, a distance := new(big.Int).Sub(batchResult.Receipt.BlockNumber, authResult.Receipt.BlockNumber) lookbackWindow := new(big.Int).SetUint64(derive.BatchAuthLookbackWindow) if distance.Sign() < 0 || distance.Cmp(lookbackWindow) > 0 { + l.Metr.RecordFallbackAuthWindowExceeded() l.Log.Error("authenticateBatchInfo transaction too far from batch inbox transaction", "txRef", transactionReference, "distance", distance) receiptsCh <- txmgr.TxReceipt[txRef]{ ID: transactionReference, diff --git a/op-batcher/batcher/fallback_auth_test.go b/op-batcher/batcher/fallback_auth_test.go index 0822f8a091f..6d140066b58 100644 --- a/op-batcher/batcher/fallback_auth_test.go +++ b/op-batcher/batcher/fallback_auth_test.go @@ -10,6 +10,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/stretchr/testify/require" + "github.com/ethereum-optimism/optimism/op-batcher/metrics" "github.com/ethereum-optimism/optimism/op-node/rollup" "github.com/ethereum-optimism/optimism/op-node/rollup/derive" "github.com/ethereum-optimism/optimism/op-service/eth" @@ -41,9 +42,17 @@ func (f *fakeTxSender) Send(id txRef, candidate txmgr.TxCandidate, receiptCh cha receiptCh <- resp } +type windowExceededSpy struct { + metrics.Metricer + count int +} + +func (s *windowExceededSpy) RecordFallbackAuthWindowExceeded() { s.count++ } + func newFallbackAuthSubmitter(t *testing.T) *BatchSubmitter { l := &BatchSubmitter{} l.Log = testlog.Logger(t, log.LevelDebug) + l.Metr = metrics.NoopMetrics l.RollupConfig = &rollup.Config{ BatchAuthenticatorAddress: common.HexToAddress("0x00000000000000000000000000000000000000aa"), } @@ -80,7 +89,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 @@ -96,6 +104,11 @@ func TestFallbackAuth_OrderingAndSuccess(t *testing.T) { require.Equal(t, txdata.ID().String(), got.ID.id.String()) } +// TestFallbackAuth_AuthFailureRetried verifies that an auth-tx send failure produces an error +// receipt keyed to the batch txData so the frames are re-queued. The pair is submitted +// back-to-back (pipelined), so the batch tx is also sent; if its auth never lands the batch is +// simply dropped by derivation — its commitment has no matching auth event in the lookback +// window — so the orphaned send is harmless to safety. func TestFallbackAuth_AuthFailureRetried(t *testing.T) { l := newFallbackAuthSubmitter(t) txdata := testFallbackTxData(t) @@ -104,17 +117,74 @@ func TestFallbackAuth_AuthFailureRetried(t *testing.T) { queue := &fakeTxSender{ responses: []txmgr.TxReceipt[txRef]{ {Err: errSendFailed}, // auth fails - {Receipt: receiptWithBlock(101)}, // batch lands anyway + {Receipt: receiptWithBlock(101)}, // batch send (result discarded on auth failure) + }, + } + receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) + + l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) + + got := <-receiptsCh + require.Error(t, got.Err) + require.Equal(t, txdata.ID().String(), got.ID.id.String()) + // Both txs are submitted back-to-back; the auth failure does not gate the batch send. + require.Len(t, queue.sends, 2) +} + +// TestFallbackAuth_AuthFailureTxRefType verifies that an auth-tx failure is +// reported under a calldata-typed txRef even when the batch txdata is blob. +// The auth tx is always calldata; if its ErrAlreadyReserved failure were +// labeled with the batch's blob type, cancelBlockingTx would send a calldata +// cancel against a blobpool reservation, which is rejected the same way, +// looping forever without ever displacing the stuck blob tx. +func TestFallbackAuth_AuthFailureTxRefType(t *testing.T) { + l := newFallbackAuthSubmitter(t) + txdata := testFallbackTxData(t) + txdata.daType = DaTypeBlob + candidate := &txmgr.TxCandidate{TxData: []byte("batch-calldata")} + + queue := &fakeTxSender{ + responses: []txmgr.TxReceipt[txRef]{ + {Err: errSendFailed}, // auth fails + {Receipt: receiptWithBlock(101)}, // batch send (result discarded on auth failure) }, } 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()) + require.Len(t, queue.sends, 2) + require.False(t, got.ID.isBlob) + require.Equal(t, DaTypeCalldata, got.ID.daType) +} + +// TestFallbackAuth_BatchFailureTxRefType verifies the converse: a batch-tx +// failure keeps the batch txdata's own type on the forwarded receipt. +func TestFallbackAuth_BatchFailureTxRefType(t *testing.T) { + l := newFallbackAuthSubmitter(t) + txdata := testFallbackTxData(t) + txdata.daType = DaTypeBlob + candidate := &txmgr.TxCandidate{TxData: []byte("batch-calldata")} + + queue := &fakeTxSender{ + responses: []txmgr.TxReceipt[txRef]{ + {Receipt: receiptWithBlock(100)}, // auth lands + {Err: errSendFailed}, // batch fails + }, + } + receiptsCh := make(chan txmgr.TxReceipt[txRef], 1) + + l.sendTxWithFallbackAuth(txdata, false, candidate, queue, receiptsCh) + + got := <-receiptsCh + require.Error(t, got.Err) + require.Equal(t, txdata.ID().String(), got.ID.id.String()) + require.Len(t, queue.sends, 2) + require.True(t, got.ID.isBlob) + require.Equal(t, DaTypeBlob, got.ID.daType) } func TestFallbackAuth_BatchFailureRetried(t *testing.T) { @@ -131,7 +201,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) @@ -149,17 +218,20 @@ 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: receiptWithBlock(101)}, // batch send (result discarded on auth revert) }, } 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 is unverifiable and + // an error receipt is forwarded to re-queue the frames. Both txs are still submitted + // (pipelined); the orphaned batch is dropped by derivation for lack of a matching auth event. + require.Len(t, queue.sends, 2) } // TestFallbackAuth_WindowViolationRetried verifies that a batch tx landing @@ -167,6 +239,8 @@ func TestFallbackAuth_AuthRevertedRetried(t *testing.T) { // channel manager rewinds and resubmits), rather than being confirmed. func TestFallbackAuth_WindowViolationRetried(t *testing.T) { l := newFallbackAuthSubmitter(t) + metr := &windowExceededSpy{Metricer: metrics.NoopMetrics} + l.Metr = metr txdata := testFallbackTxData(t) candidate := &txmgr.TxCandidate{TxData: []byte("batch-calldata")} @@ -180,11 +254,11 @@ 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) require.Equal(t, txdata.ID().String(), got.ID.id.String()) + require.Equal(t, 1, metr.count, "window violation should record the fallback_auth_window_exceeded metric") } // TestFallbackAuth_WindowBoundaryAccepted pins the inclusive bound of the lookback @@ -207,7 +281,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) diff --git a/op-batcher/metrics/metrics.go b/op-batcher/metrics/metrics.go index 93c8efd2808..84057122a0b 100644 --- a/op-batcher/metrics/metrics.go +++ b/op-batcher/metrics/metrics.go @@ -69,6 +69,8 @@ type Metricer interface { RecordBatchDataSizeBytes(daType string, size int) RecordFailoverToEthDA() + RecordFallbackAuthWindowExceeded() + Document() []opmetrics.DocumentedMetric PendingDABytes() float64 @@ -110,9 +112,10 @@ type Metrics struct { channelOutputBytesTotal prometheus.Counter channelQueueLength prometheus.Gauge - batchSentDATypeTotal prometheus.CounterVec - batchStoredDataSizeBytesTotal prometheus.CounterVec - altDaFailoverTotal prometheus.Counter + batchSentDATypeTotal prometheus.CounterVec + batchStoredDataSizeBytesTotal prometheus.CounterVec + altDaFailoverTotal prometheus.Counter + fallbackAuthWindowExceededTotal prometheus.Counter batcherTxEvs opmetrics.EventVec @@ -255,6 +258,11 @@ func NewMetrics(procName string) *Metrics { Name: "alt_da_failover_total", Help: "Total number of batches that could not be stored in AltDA and were sent to L1 instead", }), + fallbackAuthWindowExceededTotal: factory.NewCounter(prometheus.CounterOpts{ + Namespace: ns, + Name: "fallback_auth_window_exceeded_total", + Help: "Total number of fallback-auth submissions dropped because the batch tx landed beyond BatchAuthLookbackWindow blocks after its auth tx", + }), blobUsedBytes: factory.NewHistogram(prometheus.HistogramOpts{ Namespace: ns, Name: "blob_used_bytes", @@ -483,6 +491,10 @@ func (m *Metrics) RecordFailoverToEthDA() { m.altDaFailoverTotal.Inc() } +func (m *Metrics) RecordFallbackAuthWindowExceeded() { + m.fallbackAuthWindowExceededTotal.Inc() +} + func (m *Metrics) RecordChannelQueueLength(len int) { m.channelQueueLength.Set(float64(len)) } diff --git a/op-batcher/metrics/noop.go b/op-batcher/metrics/noop.go index 31002349fe2..b44bb8950f1 100644 --- a/op-batcher/metrics/noop.go +++ b/op-batcher/metrics/noop.go @@ -64,6 +64,7 @@ func (*noopMetrics) RecordBlobUsedBytes(int) {} func (*noopMetrics) RecordBatchDaType(string) {} func (*noopMetrics) RecordBatchDataSizeBytes(string, int) {} func (*noopMetrics) RecordFailoverToEthDA() {} +func (*noopMetrics) RecordFallbackAuthWindowExceeded() {} func (*noopMetrics) StartBalanceMetrics(log.Logger, *ethclient.Client, common.Address) io.Closer { return nil