op-node+op-batcher: replace fallback-auth lead time with derivation-side enforcement delay#468
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3d97d07b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // authorization path is used and authenticatedHashes is unused. | ||
| var authenticatedHashes map[common.Hash]common.Address | ||
| if config.rollupCfg.IsEspresso(ref.Time) { | ||
| if isEspressoAuthEnforced(config.rollupCfg, ref.Time) { |
There was a problem hiding this comment.
Accept authenticated Espresso batches during grace
For chains that switch the BatchAuthenticator to a non-SystemConfig Espresso/TEE batcher at EspressoTime, this gate leaves authenticatedHashes nil for the whole grace window; isBatchTxAuthorized then takes the sender-only pre-enforcement branch and rejects otherwise valid authenticated batches from the Espresso batcher until EspressoTime + BatchAuthEnforcementDelaySecs. That turns the grace period into a 20-minute delay before the new authenticated batcher can derive, likely stalling the safe head during cutover; the grace window should allow authenticated batches while also allowing legacy sender-auth batches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I don't see this as a major issue, the espresso hardfork is a one off event and it's easy for us to co-ordinate when to enable the espresso batcher.
| // sender-based authorization. | ||
| return isAuthorizedBatchSender(tx, dsCfg.l1Signer, batcherAddr, logger) | ||
| } | ||
| // Post-fork: the commitment must have been authenticated within the lookback window. |
| // time (~20 minutes) — far above any realistic L1 inclusion delay. Under missed L1 | ||
| // slots the delay covers fewer than BatchAuthLookbackWindow blocks, which is fine: the | ||
| // bound that matters is time (inclusion delay), not block count. | ||
| const BatchAuthEnforcementDelaySecs uint64 = BatchAuthLookbackWindow * 12 |
There was a problem hiding this comment.
This same grace-period explanation now lives in four places: here, the isEspressoAuthEnforced doc in batch_authenticator.go, the isFallbackAuthRequired doc in espresso_active.go, and the TestDataAndHashesFromTxsForkBoundary comment. The copies are already drifting apart — most of them call the constant BatchAuthEnforcementDelay, which does not exist, the const is BatchAuthEnforcementDelaySecs — and any future change to the mechanism means keeping four blocks of prose in sync.
This constant seems like the natural home for the full story, the other three could be cut down to a line pointing here.
There was a problem hiding this comment.
Thanks for pointing this out, i renamed the constant after initially creating it and didnt up the comments. Let me know if this looks ok to you now
317aec3
Co-authored-by: piersy <pierspowlesland@gmail.com>
piersy
left a comment
There was a problem hiding this comment.
There's a couple of comments about comments, but apart from that it looks good!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23cd7d6ded
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Implements the suggestion from #458 (comment): instead of the batcher anticipating the fork with a configurable lead time, the derivation pipeline delays enforcement of event-based batch authentication until one full BatchAuthLookbackWindow after Espresso activation (BatchAuthEnforcementDelay = 100 blocks x 12s so ~20min).
During the grace window [EspressoTime, EspressoTime +
BatchAuthEnforcementDelaySecs), derivation keeps accepting sender-authenticated batches exactly as pre-fork this absorbs the skew between when the batcher decides a batch (L1 tip time) and when it lands (containing-block time), which the lead time previously compensated for. The batcher now switches to authenticated submission at plain activation (IsEspresso(l1Tip.Time)) with no configuration. By the time enforcement begins its auth events have been accumulating for a full lookback window, so the first enforced scan is fully populated.