feat(L1): fork-gate AggregateVerifier proposal intervals for Cobalt - #431
feat(L1): fork-gate AggregateVerifier proposal intervals for Cobalt#4310xth4nh wants to merge 4 commits into
Conversation
Denim shortens the L2 block time from 2s to 200ms, which multiplies the proposal intervals by 10 (600 -> 6000 blocks, 30 -> 300 intermediate) for the same 20-minute range. Carry both value sets on one implementation and select between them per game so no contract swap is needed at the fork. Intervals are selected on the game's *starting* block relative to the Denim activation block, which is re-derived from the existing ProtocolVersions schedule. Selecting on the start block keeps the game chain contiguous and produces exactly one straddling game. Both interval pairs must yield the same intermediate root count, which the constructor enforces, so the CWIA extraData layout and INITIALIZE_CALLDATA_SIZE are unchanged across the fork. Co-Authored-By: Claude <noreply@anthropic.com>
🟡 Heimdall Review Status
|
…vals live Review follow-ups on the fork-gated proposal intervals. - Test `challenge` under both sides of the activation. `_intervals` drives two fork-sensitive call sites and only the `initializeWithInitData` one was covered. The other feeds the journal the prover signs, so a stale interval there makes a valid challenge unconstructable instead of reverting. - Merge `_intervals` into `intervalsForStartingBlock` and resolve `_denimActivationBlock()` once in `initializeWithInitData`, threading it into both the interval selection and `_l2Timestamp` instead of reading `PROTOCOL_VERSIONS.getSchedule()` twice. - Document the deliberate start-block vs ending-block asymmetry between the interval selection and the `scheduleId` pin, and extend the `_denimActivationBlock` safety argument to cover the owner moving the activation earlier, not just delaying it. - Drop the `SystemDeploy` Denim interval requires. The constructor already reverts on zero, non-divisible, and mismatched-ratio pairs; restating a subset read as full validation while skipping the ratio check. - `SeedGames.s.sol` reads the block interval and intermediate root count off the deployed implementation rather than hardcoding 600/30, which would have seeded unopenable games on a devnet with Denim active. `generate-roots.sh` cannot see the chain, so its intervals are env-overridable. - `OptimismPortal2.t.sol` moves off `BLOCK_INTERVAL()`. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
The two proposal interval pairs differ because they are calibrated for different L2 block cadences: 600 blocks at 2s and 6,000 at 200ms are both a 20-minute range. Name them for that, rather than for the fork that happens to introduce the second one. This also fixes an asymmetry. The pre-Denim pair was unprefixed and the post-Denim pair prefixed, implying "default plus special case", when the fast pair is the permanent steady state and the slow one is the legacy. BLOCK_INTERVAL -> SLOW_BLOCK_INTERVAL INTERMEDIATE_BLOCK_INTERVAL -> SLOW_INTERMEDIATE_BLOCK_INTERVAL DENIM_BLOCK_INTERVAL -> FAST_BLOCK_INTERVAL DENIM_INTERMEDIATE_BLOCK_INTERVAL -> FAST_INTERMEDIATE_BLOCK_INTERVAL DENIM_BLOCKS_PER_SECOND -> FAST_BLOCKS_PER_SECOND DENIM_UPGRADE_INDEX -> FAST_BLOCK_UPGRADE_INDEX _denimActivationBlock() -> _firstFastBlock() _legacyL2Timestamp() -> _slowL2Timestamp() `intervalsForStartingBlock` and `L2_BLOCK_TIME` are unchanged. IntervalConfig fields and the multiproof* deploy-config keys move with the immutables. `BLOCK_INTERVAL()` and `INTERMEDIATE_BLOCK_INTERVAL()` therefore leave the 0.2.0 ABI. That is deliberate: a consumer calling them post-Denim gets a plausible-but-wrong number today, and a missing method is a better failure mode than a wrong answer. It forces the migration to `intervalsForStartingBlock()` that the offchain follow-ups already require. In-flight 0.1.0 clones keep exposing the old names. Denim is now named in exactly one place, on FAST_BLOCK_UPGRADE_INDEX, which is the only spot the contract is pinned to a specific hardfork. A later cadence change is a new index and a new interval pair, not new machinery. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
| /// which drops the L2 block time from 2s to 200ms. Everything downstream is expressed as | ||
| /// slow-vs-fast blocks, so a later cadence change is a new index and new interval pair | ||
| /// rather than new machinery. | ||
| uint256 private constant FAST_BLOCK_UPGRADE_INDEX = 13; |
There was a problem hiding this comment.
We need to change this to 12 now to target Cobalt
There was a problem hiding this comment.
Fixed in fa0584d: FAST_BLOCK_UPGRADE_INDEX is now 12, and the doc comment names Cobalt. Confirmed against test/L1/ProtocolVersions.t.sol — the Base mainnet schedule is 13 entries with schedule[12] = 0 (Cobalt unscheduled), so at 13 _firstFastBlock was hitting the schedule.length <= FAST_BLOCK_UPGRADE_INDEX guard and returning type(uint256).max unconditionally. Also renamed the remaining Denim mentions in scripts/multiproof.
There was a problem hiding this comment.
Following up on this one — the change is in (FAST_BLOCK_UPGRADE_INDEX = 12), but I want to flag something before it merges, because I think index 12 and the node disagree about which fork carries the speedup.
In base/base, the 200ms cadence is gated on Denim, not Cobalt, in ~26 production call sites:
RollupConfig::l2_block_timestamp_millisswitches toNATIVE_SUBSECOND_BLOCK_INTERVAL_MILLIS = 200atdenim_activation_block_number()—crates/common/genesis/src/rollup.rs:369,409-427- the sequencer's block interval:
if self.rollup_config.is_denim_active(...) { Duration::from_millis(NATIVE_SUBSECOND...) }—crates/consensus/service/src/actors/sequencer/actor.rs:345 - payload builder, engine API, execution consensus, builder multiplex all branch on
is_denim_active_at_timestamp BaseTimepredeploy is staged behind Denim —crates/common/evm/src/base_time.rs:107
Cobalt in the node is a different fork entirely — EIP-8130 system accounts (crates/common/evm/src/cobalt.rs:51) and B-20 permit metering (#4891). Nothing in it touches block time.
So as written, this contract switches proposals to FAST_BLOCK_INTERVAL / FAST_INTERMEDIATE_BLOCK_INTERVAL when Cobalt activates, while L2 is still producing 2s blocks until Denim. That's 6,000 blocks × 2s ≈ 3.3-hour game ranges over the whole Cobalt→Denim window, instead of the intended 20 minutes.
Two ways to read it, and I don't know which is right:
- The speedup really has moved from Denim to Cobalt at the protocol level. Then this constant is correct and
base/baseneeds its Denim gate re-pointed at Cobalt — a much bigger change that the node folks would own. - The concern was that index 13 doesn't exist yet —
test/L1/ProtocolVersions.t.sol:1145pins the Base mainnet schedule at 13 entries ending withschedule[12] = 0 // Cobalt is unscheduled, so at 13_firstFastBlock()hits theschedule.length <= FAST_BLOCK_UPGRADE_INDEXguard and returnstype(uint256).max. Worth noting that's fail-safe rather than broken: it just means the fast cadence can't activate until Denim is registered, which per the rollout plan happens before the timestamp is ever scheduled.
Which is it? Happy to move it back to 13 if it's (2), or leave it at 12 and get the node-side re-gating filed if it's (1) — but I don't think we want them pointing at different forks either way.
| uint256 blocksUntilFast; | ||
| if (fastActivationTimestamp > L2_GENESIS_TIMESTAMP) { | ||
| blocksUntilFast = FixedPointMathLib.divUp(fastActivationTimestamp - L2_GENESIS_TIMESTAMP, L2_BLOCK_TIME); | ||
| } | ||
| return L2_GENESIS_BLOCK_NUMBER + blocksUntilFast; | ||
| } |
There was a problem hiding this comment.
nit, prefer it like this:
| uint256 blocksUntilFast; | |
| if (fastActivationTimestamp > L2_GENESIS_TIMESTAMP) { | |
| blocksUntilFast = FixedPointMathLib.divUp(fastActivationTimestamp - L2_GENESIS_TIMESTAMP, L2_BLOCK_TIME); | |
| } | |
| return L2_GENESIS_BLOCK_NUMBER + blocksUntilFast; | |
| } | |
| if (fastActivationTimestamp <= L2_GENESIS_TIMESTAMP) { | |
| return L2_GENESIS_BLOCK_NUMBER; | |
| } | |
| return L2_GENESIS_BLOCK_NUMBER + FixedPointMathLib.divUp(fastActivationTimestamp - L2_GENESIS_TIMESTAMP, L2_BLOCK_TIME); | |
| } |
There was a problem hiding this comment.
Fixed in fa0584d: applied, with the return wrapped to stay inside the line limit.
Index 12 is Cobalt, the next unscheduled entry on the Base mainnet `ProtocolVersions` schedule; index 13 was Denim, which the schedule does not reach yet, so `_firstFastBlock` could only ever return `type(uint256).max`. Also folds the early-genesis branch of `_firstFastBlock` into an early return, per review. Co-Authored-By: Claude <noreply@anthropic.com>
Retargets the switchover from Denim (`ProtocolVersions` index 13, which the Base mainnet schedule does not reach) to Cobalt (index 12), matching base/contracts#431. The mock verifier's fields follow the contract's slow/fast vocabulary so a later cadence change needs no rename. Also drops the stray whitespace `cargo fmt` left in the `INTERMEDIATE_BLOCK_INTERVAL` divisibility error, flagged in review. Co-Authored-By: Claude <noreply@anthropic.com>
Follows base/contracts#431 and the proposer branch: the switchover is Cobalt (`ProtocolVersions` index 12), not Denim (index 13). Test fixtures and mock builders use the contract's slow/fast vocabulary. Co-Authored-By: Claude <noreply@anthropic.com>
Follows base/contracts#431 and the proposer branch: the switchover is Cobalt (`ProtocolVersions` index 12), not Denim (index 13). Test fixtures and mock builders use the contract's slow/fast vocabulary. Co-Authored-By: Claude <noreply@anthropic.com>
What changed?
AggregateVerifiernow carries both sets of proposal intervals and selects between them per game, instead of a single immutable pair.SLOW_BLOCK_INTERVAL/SLOW_INTERMEDIATE_BLOCK_INTERVAL(600/30 at 2s) andFAST_BLOCK_INTERVAL/FAST_INTERMEDIATE_BLOCK_INTERVAL(6,000/300 at 200ms). Both spans are the same 20 minutes, which is the point.IntervalConfigconstructor struct:slowBlockInterval,slowIntermediateBlockInterval,fastBlockInterval,fastIntermediateBlockInterval(replaces the two looseuint256params)._firstFastBlock(), extracted from_l2Timestamp()so the fork boundary is derived in exactly one place from the existingProtocolVersionsschedule (FAST_BLOCK_UPGRADE_INDEX = 13)._l2Timestamp()behaviour is unchanged._legacyL2Timestamp()becomes_slowL2Timestamp().intervalsForStartingBlock(uint256)drives the two fork-sensitive call sites — theUnexpectedBlockNumbercheck ininitializeWithInitData, and the sequence numbers in_getStartingIntermediateRootAndL2SequenceNumbers— and lets the proposer and challenger resolve a game's intervals without a clone.initializeWithInitDataresolves_firstFastBlock()once and threads it into both its interval selection and_l2Timestamp, so it readsPROTOCOL_VERSIONS.getSchedule()once rather than twice.MismatchedIntermediateRootCount. The constructor requires both pairs to yield the same intermediate root count.0.1.0→0.2.0; ABI and semver-lock snapshots regenerated.multiproofSlowBlockInterval/multiproofSlowIntermediateBlockInterval/multiproofFastBlockInterval/multiproofFastIntermediateBlockIntervalacrossDeployConfig,SystemDeploy, anddeploy-config/local.json; the two dev deploy virtuals collapse into one_intervalConfig(). Interval validation lives only in the constructor —SystemDeploydoes not restate it, because a partial copy there would read as full validation while skipping the ratio check.SeedGames.s.solreads the block interval and intermediate root count off the deployed implementation instead of hardcoding 600/30, which would otherwise seed unopenable games on a devnet with Cobalt active.generate-roots.shhas no L1 RPC to read from, so its two intervals became env-overridable with the slow-cadence values as defaults; seeding aborts on a mismatched roots file rather than creating bad games.BLOCK_INTERVAL()andINTERMEDIATE_BLOCK_INTERVAL()are gone from the 0.2.0 ABIThey are
SLOW_BLOCK_INTERVAL()andSLOW_INTERMEDIATE_BLOCK_INTERVAL()now. This is deliberate rather than incidental: a consumer that callsBLOCK_INTERVAL()on a fast-cadence game today gets a plausible-but-wrong number, and a missing method is a better failure mode than a wrong answer. It turns the migration tointervalsForStartingBlock()— which the offchain follow-ups below already require — into a compile error instead of a silent miscalculation.In-flight 0.1.0 clones keep exposing the old names, so tooling needs both paths during the transition regardless.
Cobalt is now named in exactly one place in the contract, on
FAST_BLOCK_UPGRADE_INDEX(ProtocolVersionsindex 12), which is the only spot pinned to a specific hardfork. A later cadence change is a new index and a new interval pair, not new machinery.Why?
Cobalt drops the L2 block time from 2s to 200ms. To keep the proposal range at 20 minutes, the block interval goes 600 → 6,000 and the intermediate interval 30 → 300. Those are immutable today, so the fork would need a contract swap timed to the activation.
Carrying both sets on one implementation means we deploy and upgrade before Cobalt and nothing has to happen during the fork. This is section 1 of the proof config switchover doc.
Two design points, both reviewed and now documented in-code:
Selection is on the game's starting block, not its ending block. Selecting on the end block would break the chain invariant
end == parent.end + blockIntervalfor the game spanning the activation. Selecting on the start block keeps the chain contiguous and produces exactly one straddling game — the one whose range contains the activation block — proven under the slow-block 600-interval. That is correct because provers apply fork rules per block by timestamp, not per game. This was the doc's open decision; it is resolved as acceptable.Note the deliberate asymmetry this creates inside
initializeWithInitData: intervals are selected from the starting block whilescheduleIdis pinned from the ending block. That is what makes the straddling game work — it spans the slow-block interval its start selects, while pinning the post-Cobalt schedule its end falls under, so the prover knows Cobalt is active for the blocks past the boundary. There is now a comment saying so.The activation block is re-derived live rather than pinned into storage. This is safe in both directions:
ProtocolVersions._assertNotFrozenrejects every mutation of a passed activation — fromsetTimestampanddelayTimestampalike. That selection can never be revoked.initializeWithInitDatarejects a game whose ending L2 timestamp L1 has not yet reached, so every initialized game satisfiesstartingTimestamp < endingTimestamp <= block.timestamp, while any new activation must clearblock.timestamp + MIN_NOTICE.It also keeps the diff free of a new storage slot —
AggregateVerifieris at 20,346 / 24,576 bytes (82.8%) after this change, up from 20,059.The intermediate root count is 20 on both sides (600/30 = 6000/300), so
intermediateOutputRootsCount(),INITIALIZE_CALLDATA_SIZE, and every CWIAextraDataoffset are byte-identical across the fork. The constructor enforces this rather than assuming it — otherwise parsingextraDatawould need the root count, which would need the interval, which would need the starting block, which lives inextraData.How to test?
The FFI-backed suites (
test/libraries/trie/*,test/libraries/Hashing.t.sol,test/L1/OptimismPortal2.t.sol) need the Go helper built first, otherwise they fail on a missing binary unrelated to this change:Targeted run for this change:
New tests, all in
test/L1/proofs/AggregateVerifier.t.sol(test fixtures use 100/10 slow and 1000/100 fast, same 10:1 ratio):test_intervalsForStartingBlock_selectsOnFirstFastBlock_succeedstest_intervalsForStartingBlock_speedupUnscheduled_succeedstest_initialize_fastIntervals_succeedsUnexpectedBlockNumber(1000, 100); a game ending at 1000 initializes.test_initialize_straddlingGame_usesSlowInterval_succeeds[0, 100)range: ending at 1000 revertsUnexpectedBlockNumber(100, 1000); ending at 100 initializes.test_challenge_fastIntermediateInterval_succeeds[0, 100), not[0, 10).test_challenge_straddlingGameUsesSlowIntermediateInterval_succeeds[0, 10).test_constructor_mismatchedIntermediateRootCount_reverts(100, 10, 1000, 200)→MismatchedIntermediateRootCount(10, 5).test_constructor_invalidFastBlockIntervals_revertsfastIntermediateBlockInterval = 0→InvalidBlockInterval(1000, 0).The two
test_challenge_*cases cover_getStartingIntermediateRootAndL2SequenceNumbers, the second fork-sensitive call site and the one whose output goes into the journal the prover signs — a stale interval there makes a valid challenge unconstructable rather than reverting loudly. They assert by matching the exactIVerifier.verifycall, since the mock verifier accepts any payload; both fail if the opposite interval is substituted.Snapshot and lint checks (
justrecipes, expanded so they run withoutjust):Two repo checks fail on
mainas well as here, neither involvingAggregateVerifier:go run ./scripts/checks/interfaces(IERC721,IDisputeGameFactory,FeeDisburser,BalanceTracker) andgo run ./scripts/checks/test-validation.Follow-ups (not in this PR)
The zero-arg interval views can no longer answer without game context, and the old names are gone. Offchain consumers must move to
intervalsForStartingBlock():base/basecrates/proof/contracts/src/aggregate_verifier.rs— the generated bindings. These will fail to compile against the new ABI, which is the intended forcing function.base/basecrates/proof/challenge/src/scanner.rsresolve_intermediate_block_interval()— caches the interval per implementation address, so once one implementation serves both sides it returns a single value for all games. The doc listsGameScanneras already correct; it is not, and the doc needs correcting.Per the doc's sequencing, the proposer and challenger changes must be deployed before this implementation is swapped in, even though this PR can merge first.
Update — retargeted to Cobalt
FAST_BLOCK_UPGRADE_INDEXwas 13 (Denim). It is now 12 (Cobalt), per review. The Base mainnetProtocolVersionsschedule is 13 entries long withschedule[12] = 0, so at index 13_firstFastBlock()was always short-circuiting on theschedule.length <= FAST_BLOCK_UPGRADE_INDEXguard and returningtype(uint256).max— the speedup could never activate. Also folded the early-genesis branch of_firstFastBlockinto an early return, and renamed the remaining Denim mentions underscripts/multiproof.The branch name still says
denim; renaming it would close and reopen the PR, so it stays.