-
Notifications
You must be signed in to change notification settings - Fork 12
Espresso 3a: Fallback batcher (re-hosted) #458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: celo-rebase-18
Are you sure you want to change the base?
Changes from all commits
7dd0a79
5e47299
e88cbe8
2704403
b99bcb9
dcff468
c102ced
011c7a7
d769ea2
5651e2a
ab161a1
400f1f5
e898f5c
0f4d001
ea1f014
c993c2a
09533af
2d634fd
e6d65e0
a99e175
483e8e4
2e5cc7d
59f5d38
5461fba
96dfd24
f132cbf
2354b89
70291e3
14210d4
68784e3
27f75a7
c87f184
d797a1b
1f3150f
9638afd
dd772e3
7a4a401
9b14f40
3a76b2d
182d84e
5aff9d1
7979391
8917fef
05cb149
25b8fa9
2782976
93fa0dc
1267e87
25a6c63
42382b6
49c4321
76b729b
8632b97
02831d8
9c9ba69
4671fec
98dbe45
cbb6942
4c437a2
25c5086
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package batcher | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| ) | ||
|
|
||
| // isFallbackAuthRequired reports whether the fallback (non-TEE) batcher must | ||
| // route its batch txs through BatchAuthenticator.authenticateBatchInfo before | ||
| // posting to the BatchInbox. It returns false if the rollup config has a | ||
| // zero BatchAuthenticatorAddress, indicating that the BatchAuthenticator-based | ||
| // authentication path is not in use. | ||
| // | ||
| // This decision must align with the verifier's per-L1-block fork gate | ||
| // (rollupCfg.IsEspresso(l1OriginTime) in data_source.go, which evaluates the | ||
| // hardfork activation predicate against the *containing* L1 block's timestamp). Since | ||
| // the tx is not yet mined at decision time, its eventual containing block | ||
| // has a strictly greater timestamp than the L1 tip the batcher observes: | ||
| // | ||
| // l1Tip.Time (batcher's view) < l1OriginTime (block containing the tx) | ||
| // | ||
| // Without compensation, in the window [forkTime − maxL1InclusionDelay, forkTime) | ||
| // the batcher would skip authenticateBatchInfo while the verifier — once the | ||
| // tx lands in a post-fork block — would require the resulting | ||
| // BatchInfoAuthenticated event, silently dropping the batch. | ||
| // | ||
| // To prevent this, we add Config.FallbackAuthLeadTime to the L1 tip's | ||
| // timestamp before evaluating the fork predicate. This makes the batcher | ||
| // start authenticating slightly before the verifier requires it. The reverse | ||
| // asymmetry (authenticated tx lands pre-fork) is harmless: pre-fork the | ||
| // verifier uses sender-based authorization and the auth event is just an | ||
| // unrelated L1 tx that does not affect derivation. | ||
| func (l *BatchSubmitter) isFallbackAuthRequired(ctx context.Context) (bool, error) { | ||
| if l.RollupConfig.BatchAuthenticatorAddress == (common.Address{}) { | ||
| return false, nil | ||
| } | ||
| tip, err := l.l1Tip(ctx) | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to fetch L1 tip for fallback-auth gate: %w", err) | ||
| } | ||
| leadSec := uint64(l.Config.FallbackAuthLeadTime / time.Second) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. M4 —
Recommend validating in
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added validation for anything
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually reading Piers comment below, i may look to remove this. |
||
| return l.RollupConfig.IsEspresso(tip.Time + leadSec), nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package batcher | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "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. | ||
| // | ||
| // The fallback batcher consults isFallbackAuthRequired to gate authentication | ||
| // behind the EspressoTime hardfork: pre-fork the verifier accepts plain | ||
| // sender-authenticated batches, and the BatchAuthenticator contract is | ||
| // irrelevant. | ||
| func (l *BatchSubmitter) dispatchAuthenticatedSendTx(txdata txData, isCancel bool, candidate *txmgr.TxCandidate, queue TxSender[txRef], receiptsCh chan txmgr.TxReceipt[txRef]) bool { | ||
| if isCancel { | ||
| return false | ||
| } | ||
| fallbackAuthRequired, err := l.isFallbackAuthRequired(l.killCtx) | ||
| if err != nil { | ||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: newTxRef(txdata, isCancel), | ||
| Err: fmt.Errorf("failed to evaluate fallback-auth gate: %w", err), | ||
| } | ||
| return true | ||
| } | ||
| if !fallbackAuthRequired { | ||
| return false | ||
| } | ||
| l.sendTxWithFallbackAuth(txdata, isCancel, candidate, queue, receiptsCh) | ||
| return true | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,163 @@ | ||
| package batcher | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "math/big" | ||
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/ethereum/go-ethereum/common/hexutil" | ||
| "github.com/ethereum/go-ethereum/core/types" | ||
|
|
||
| "github.com/ethereum-optimism/optimism/espresso/bindings" | ||
| "github.com/ethereum-optimism/optimism/op-node/rollup/derive" | ||
| "github.com/ethereum-optimism/optimism/op-service/eth" | ||
| "github.com/ethereum-optimism/optimism/op-service/txmgr" | ||
| ) | ||
|
|
||
| // computeCommitment computes the batch commitment hash from a transaction | ||
| // candidate. It delegates to the same functions the verifier uses so the two | ||
| // provably agree on the bytes that get authenticated: | ||
| // - calldata transactions: keccak256(calldata). | ||
| // - blob transactions: keccak256(concat(blobVersionedHashes)). | ||
| func computeCommitment(candidate *txmgr.TxCandidate) ([32]byte, error) { | ||
| if len(candidate.Blobs) == 0 { | ||
| return derive.ComputeCalldataBatchHash(candidate.TxData), nil | ||
| } | ||
|
|
||
| blobHashes := make([]common.Hash, len(candidate.Blobs)) | ||
| for i, blob := range candidate.Blobs { | ||
| blobCommitment, err := blob.ComputeKZGCommitment() | ||
| if err != nil { | ||
| return [32]byte{}, fmt.Errorf("failed to compute KZG commitment for blob: %w", err) | ||
| } | ||
| blobHashes[i] = eth.KZGToVersionedHash(blobCommitment) | ||
| } | ||
| return derive.ComputeBlobBatchHash(blobHashes), nil | ||
| } | ||
|
|
||
| // sendTxWithFallbackAuth authenticates a batch transaction via the BatchAuthenticator contract | ||
| // using the fallback batcher's sender identity (msg.sender check on-chain), then sends the | ||
| // batch data to the BatchInbox address. | ||
| // | ||
| // The contract's fallback path checks msg.sender against systemConfig.batcherHash(), so no | ||
| // 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]) { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mirrored from #448 (unresolved review thread) to keep tracking it here after the re-host. Original location: Original transcript: @palango — 2026-06-02:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed as part of the rework in 6f42f54 |
||
| transactionReference := newTxRef(txdata, isCancel) | ||
| l.Log.Debug("Sending fallback-authenticated L1 transaction", "txRef", transactionReference) | ||
|
|
||
| commitment, err := computeCommitment(candidate) | ||
| if err != nil { | ||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: transactionReference, | ||
| Err: fmt.Errorf("failed to compute commitment: %w", err), | ||
| } | ||
| return | ||
| } | ||
| l.Log.Debug("Computed fallback batch commitment", "txRef", transactionReference, "commitment", hexutil.Encode(commitment[:])) | ||
|
|
||
| batchAuthenticatorAbi, err := bindings.BatchAuthenticatorMetaData.GetAbi() | ||
| if err != nil { | ||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: transactionReference, | ||
| Err: fmt.Errorf("failed to get batch authenticator ABI: %w", err), | ||
| } | ||
| return | ||
| } | ||
|
|
||
| // Pass an empty signature — the contract checks msg.sender for the fallback path. | ||
| authenticateBatchCalldata, err := batchAuthenticatorAbi.Pack("authenticateBatchInfo", commitment, []byte{}) | ||
| if err != nil { | ||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: transactionReference, | ||
| Err: fmt.Errorf("failed to pack authenticateBatchInfo calldata: %w", err), | ||
| } | ||
| return | ||
| } | ||
|
|
||
| verifyCandidate := txmgr.TxCandidate{ | ||
| TxData: authenticateBatchCalldata, | ||
| 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) | ||
|
Comment on lines
+85
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. M2 — With It avoids deadlock only because these channels are buffered cap 1: the auth handler can write its receipt with no reader present (the watcher goroutine isn't started until after both Sends return, line 99). The comment should state that the buffering is what prevents the deadlock at the queue limit — switching these to unbuffered would hard-deadlock at the default config. Recommend documenting that
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the changes made in this #467 addressing |
||
|
|
||
| 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 | ||
| // nonces are assigned in submission order. Each Send blocks here when the queue is at its | ||
| // MaxPendingTransactions limit. | ||
| queue.Send(transactionReference, verifyCandidate, authReceiptCh) | ||
| queue.Send(transactionReference, *candidate, batchReceiptCh) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When blob DA is used with Useful? React with 👍 / 👎.
Comment on lines
+97
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. M3 — Nonce-gap hazard when the auth send fails asynchronously. These two Sends craft consecutive nonces synchronously (auth = N, batch = N+1). If the auth tx is crafted but its async This is the same class as the upstream single-tx path, but amplified here because this path deliberately fires two ordered txs and a failure/revert on the first is now an expected case (e.g. the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider the transactions for 2 subsequent batches of batches A1 - Auth tx batch 1 A2 - ... Assuming A1 fails and causes the nonce to be reset B1s nonce will be too high and prevent it from getting mined. However A2 should work because it would have a correct nonce, but the nonce of B2 would clash with B1. Assuming that B2 succeeds B1 will now fail due to it's nonce being too low. Once B1 is dropped the synthetic failure receipt will be sent on If A & B were both in the same channel then they will just be submitted again and the previously submitted parts of A & B will be lost, but if they are separate channels and now B has arrived before A, then the batcher will ultimately end up having to rewind to the last safe head when it decides that the safe head is not progressing appropriately. I think this can be pretty easily fixed by simply submitting and waiting for the auth tx before submitting and waiting for the batch tx and stopping and returning a synthetic error receipt at the first error encountered.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe I understand the issue, I will make a separate PR on top of this to address it so it is easier to review.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
lukeiannucci marked this conversation as resolved.
|
||
|
|
||
| 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) | ||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: transactionReference, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a fallback-auth blob batch hits Useful? React with 👍 / 👎. |
||
| Err: fmt.Errorf("failed to send fallback authenticateBatchInfo transaction: %w", authResult.Err), | ||
| } | ||
| return | ||
| } | ||
|
|
||
| // txmgr returns a receipt as soon as the tx is mined, regardless of execution status. A | ||
| // reverted authenticateBatchInfo call emits no BatchInfoAuthenticated event, so the verifier | ||
| // drops the batch and the safe head stalls; report failure so the frames are re-queued. The | ||
| // batch inbox tx needs no such check: derivation reads its data by L1 inclusion, not by | ||
| // execution status. | ||
| 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, | ||
| Err: fmt.Errorf("fallback authenticateBatchInfo transaction reverted: %s", authResult.Receipt.TxHash), | ||
| } | ||
| return | ||
| } | ||
|
|
||
| if batchResult.Err != nil { | ||
| l.Log.Error("Failed to send batch inbox transaction", "txRef", transactionReference, "err", batchResult.Err) | ||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: transactionReference, | ||
| Err: fmt.Errorf("failed to send batch inbox transaction: %w", batchResult.Err), | ||
| } | ||
| return | ||
| } | ||
|
|
||
| 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.Log.Error("authenticateBatchInfo transaction too far from batch inbox transaction", "txRef", transactionReference, "distance", distance) | ||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: transactionReference, | ||
| Err: fmt.Errorf("authenticateBatchInfo transaction too far from batch inbox transaction: %s", distance), | ||
| } | ||
| return | ||
| } | ||
|
|
||
| receiptsCh <- txmgr.TxReceipt[txRef]{ | ||
| ID: transactionReference, | ||
| Receipt: batchResult.Receipt, | ||
| Err: nil, | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly this lead time is meant to account for the time difference between the block at which a batch is authenticated and the block at which the batch is submitted to the batcher address.
If so, there is already an upper bound for that inclusion delay, it's the 100 block lookback window. So can't this just be set to a constant and removed from the config?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or simpler still, remove this entirely and have the derivation pipeline require espresso activation to be at least 100 blocks old before switching to espresso verification, thus allowing for a full lookback window. This keeps the 100 block window contained within derivation rather than leaking it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding this solution, i took a stab at it, but I think this changes some small code in the derivation pipeline, which is now being audited. So I am unsure if we want to make these changes now.