Coinbase: 200ms blocks

Description

prepare_payload_attributes in stateful.rs fetches the parent system configuration using system_config_by_number, discarding the parent hash already available in l2_parent. The underlying provider caches blocks by number without invalidating those entries on reorg, so the lookup can return a block from the old branch rather than the parent the builder is actually extending.

If a reorg replaces a cached block with another block at the same height carrying different configuration, the builder can use stale gas limits or fee parameters when constructing payload attributes. The refresh logic immediately above this call forces this lookup around Denim activation, making correct parent identification particularly relevant at the upgrade boundary. Block numbers were already ambiguous across reorgs before Denim; the 200 ms cadence does not introduce that ambiguity.

The potential consequence is payload construction using configuration inconsistent with the selected parent, which could disrupt derivation or cause disagreement with nodes using the correct configuration. Full-node disruption or a chain split has not been demonstrated. Optimism addressed the same lookup pattern by retaining the parent hash when requesting system configuration: Optimism PR #22465.

Recommendation

Fetch the parent system configuration using l2_parent.block_info.hash and cache the result by hash. This binds the returned configuration to the exact parent being extended, regardless of changes to the canonical block at that height.

Coinbase: Acknowledged the issue and fixed the system-config lookups in Base PR #5274, according to the supplied known-issue record.

Description

The same-second ancestry check in batch_validator.rs calls l2_block_info_by_number to determine whether a batch’s claimed parent matches a recent canonical ancestor. A match causes the validator to skip that batch while preserving the remaining channel. An invalid parent instead causes the remaining channel to be discarded.

The underlying AlloyL2ChainProvider lookup uses a number-keyed cache without revalidating cached entries after a reorg. If a block at a cached height is replaced, the lookup can return the old branch’s block. Nodes with different cache contents could therefore classify the same batch differently: one could skip only that batch while another discards the remaining channel.

Recommendation

Ensure ancestry validation cannot consume stale number-cache entries. Fetch current canonical block information directly, or anchor the ancestry walk to the supplied safe head and follow its parent hashes backward.

The hash-based approach must start from the trusted safe head. Fetching the untrusted batch’s claimed parent by hash alone does not establish canonical ancestry.

Add regression coverage verifying that stale cached blocks cannot change the skip-versus-flush decision.

Coinbase: Acknowledged the concern. The specific stale-cache path is fixed on current main: l2_block_info_by_number now bypasses the number cache and fetches current canonical block information. Verified provider code. Base PR #5361 proposes additional hardening by anchoring ancestry to the safe head; it remained open and unmerged when checked. Production deployment has not been verified.

Description

The reviewed proof-history ExEx and manual prune command defaulted to a window of 1,296,000 blocks. At Denim's sustained rate of five blocks per second, this retains three days, despite the code describing it as a month at the old two-second cadence. The independent optional --full and full-snapshot preset retained 1,339,200 blocks of bodies, receipts, and state history, a configured distance of about 3.1 days at the Denim rate.

The dispute verifier can leave a one-proof game open for at least five days, while proof and witness tooling can request historical state from L2 nodes. An operator relying only on either affected retention setting, without an archive source or previously captured witness, could lack data needed for a late proof. This is a conditional availability risk, not a demonstrated failed deployed dispute or a consensus fault. Proof history and --full are independently enabled, and their deployed configurations were not established.

Recommendation

Set each retention default from the longest supported wall-clock proof workflow and the fastest supported block cadence, allowing for game creation lag and retries. Keep archive or captured-witness access for history already pruned, and test historical proof and witness retrieval after the configured pruning boundary.

Coinbase: Fixed for the reported defaults by Base PR #5362, merged as f76113c5, and Base PR #5367, merged as 90291981. The proof-history ExEx, node CLI, and manual prune default now use 6,480,000 blocks, or 15 days at five blocks per second. The separate --full preset now uses 13,392,000 blocks, or 31 days. The current main branch retains both values, and the merged build, unit-test, and system-test checks passed. Static review found no distinct code regression from these default changes. They do not restore previously pruned data, prevent an operator from choosing a shorter override, or prove that every possible late challenge fits within 15 days; a live pruning-to-dispute test was not run.

Description

BlockHandler::MAX_BLOCKS_TO_KEEP is set to 5 and documented as the maximum number of distinct blocks to keep per height. The enforcement check uses a strict greater than comparison, and the length is evaluated before the new hash is inserted. As a result a single height can hold 6 distinct hashes before the 7th is rejected, so the effective cap is 6 rather than the documented 5.

This bound exists to limit how many distinct block hashes are tracked per height, which protects against unbounded memory growth from gossip spam. A cap of 6 instead of 5 is a negligible difference in that resource bound and is not exploitable. The behavior also matches the op node reference implementation, so it reflects intended cross client parity rather than a Denim regression. Using a greater than or equal comparison would enforce a true cap of 5.

Recommendation

If a hard cap of 5 is desired, change the comparison to greater than or equal so the count is enforced before exceeding the documented limit. If parity with the op node reference is the priority, keep the current behavior and update the documentation to state that up to 6 distinct hashes may be retained per height.

Description

The millisecond L2 block timestamp is computed in three places, each with a different overflow behavior for the same value.

• RPC path, BaseTimeUpdateTx::extract_timestamp_ms: wrapping_mul then wrapping_add, so overflow silently wraps to a small value.
• Consensus path, RollupConfig::l2_block_timestamp_millis: saturating arithmetic, so overflow clamps to u64::MAX.
• Solidity predeploy, BaseTime.timestampMs: uint64(block.timestamp * 1000 + timestampMillisPart), so the uint64 cast truncates modulo 2^64.

For every realistically reachable input the three paths return the identical value. They diverge only once the seconds denominated timestamp exceeds u64::MAX / 1000, roughly 1.8e16 seconds or about 585 million years past epoch, a regime consensus bounds make unreachable.

The RPC path is read only and feeds cached timestamps into RPC responses, so the divergence has no reachable safety, liveness, or economic impact. The concern is consistency and defensive coding. Three overflow behaviors for one value is a latent footgun if the surrounding bounds ever change.

Recommendation

Align the three computations on a single overflow behavior across the RPC path, the consensus path, and the Solidity predeploy. Prefer plain checked arithmetic so an overflow that should never occur surfaces as a hard failure rather than being masked. Matching the saturating behavior of l2_block_timestamp_millis is an acceptable interim step.

Coinbase: Acknowledged. They intend to move all time related operations to plain arithmetic ops, but may do at later time.

Description

The fast block speedup activation is read from the same schedule value on two sides, but the two sides interpret a value of zero differently.

On the contract side, _firstFastBlock treats a schedule value of zero as unset and returns the never sentinel type(uint256).max. The verifier therefore stays on the slow cadence forever.

On the node side, denim_activation_block_number only returns None when the schedule key is absent. An explicit value of zero returns Some(0), which resolves to activation offset zero, so the sequencer runs the fast cadence from genesis.

Because an absent key is None but an explicit zero is a set value, and zero means from genesis elsewhere in the configs, a chain configured with the activation timestamp set to zero would split the sequencer and the verifier on the block to timestamp mapping. The sequencer would produce fast cadence blocks from genesis while the verifier would derive timestamps on the slow cadence, so honest proposals would fail verification.

This is not reachable on mainnet, where the speedup is scheduled at a real nonzero timestamp, and the team avoids a zero activation even on devnet. The impact is confined to a misconfiguration, but the two implementations do not agree on what not scheduled means.

Recommendation

Normalize a zero activation timestamp to the unset value at config load on the node so both sides agree, or reject a zero activation timestamp during config validation. This closes the divergence at the source rather than relying on operational discipline to never use zero.

Coinbase: Acknowledged as a known limitation. The contracts have no notion of None, so a schedule value of zero must be treated as unset. In practice, features are activated after zero, including on devnet, to avoid this case.

Description

The AggregateVerifier constructor validates the proposal intervals through _validateIntervals. The only cross check it performs is that the slow and fast interval pairs yield the same intermediate root count, at AggregateVerifier.sol:1207, because that count freezes the CWIA extraData layout for the lifetime of the implementation.

Nothing constrains the intermediate block interval itself to any specific value. It is a free deploy parameter stored as an immutable. intermediateOutputRootsCount and the challenge range built in _getStartingIntermediateRootAndL2SequenceNumbers are both derived from that interval dynamically.

The offchain ZK prover samples intermediate roots at a hardcoded cadence of 30 blocks, and the per game interval parameter that would let it honor the deployed value was dropped. Because the onchain interval and the prover cadence are set independently, a deploy that configures a different intermediate interval produces a root count and a challenge range that the prover does not match. Every ZK proof for that cadence then fails verification.

The one committed config in the repo, deploy-config/local.json, already sets these to 10 and 100 rather than 30, though it is a local development config that may run against a mock verifier and mask the mismatch in tests.

Recommendation

Make the onchain intermediate interval and the prover sampling cadence provably agree, in either direction. Either pin the onchain intervals to the prover constant so a mismatched config reverts at deploy time, or make the prover dynamic so it reads the deployed interval and samples accordingly. The second option preserves a configurable interval and is the stronger long term fix. In both cases the goal is that a configuration the prover cannot satisfy is rejected loudly rather than failing silently later.

Coinbase: Acknowledged. The ZK prover will be made dynamic so that the slow and fast counts equal each other and the prover honors the deployed intermediate interval.

Description

_firstFastBlock resolves the fast block activation timestamp by reading a single hardcoded position in the protocol versions schedule. That position is the constant FAST_BLOCK_UPGRADE_INDEX which is set to 13, and nothing on this path binds the slot to Denim by name or by hash. The verifier simply trusts that whatever value sits at index 13 is the Denim activation timestamp. If another upgrade were ever registered at index 13 before Denim, the verifier would silently read the wrong activation timestamp and compute an incorrect slow to fast block mapping with no error raised. Beyond the index itself, nothing here confirms that this timestamp, or the genesis timestamp and block time immutables, agree with the node config, so the entire correspondence between the contract and the node rests on deployment discipline rather than an enforced invariant. In practice this is not exploitable. Hard forks are scheduled one after another so index 13 will hold Denim on any correctly ordered deployment, and mainnet schedules Denim at a real slot. The concern is that a schedule layout change or a deployment mistake would fail quietly rather than loudly.

Recommendation

Add a deploy time assertion that the schedule slot really corresponds to Denim, for example by checking the schedule identifier for that index against an expected value, and add a runbook check that the contract immutables match the node config. This ensures the contract and the node cannot drift apart silently if the schedule layout ever changes.

Coinbase: Acknowledged the concern as reasonable but noted there is no current way to bind the slot by name or hash. They are relying on forks being scheduled sequentially so the index stays correct and will verify it at deployment, with no code change planned.

Description

The Denim predeploy activation in ensure_predeploy links the BaseTime implementation only after checking that the proxy account has non empty code and that its EIP 1967 admin slot equals the expected proxy admin. That is the full extent of the validation. It confirms that some contract exists at the BaseTime address and that the admin slot looks correct, but it never verifies that the deployed runtime is actually the canonical predeploy proxy.

The constant PROXY_CODE_HASH exists for exactly this purpose, yet it is only referenced from a unit test and never from the activation path. As a result the runtime bytecode behind the proxy is never compared against the pinned value, so a proxy that carries the expected admin slot but a different runtime would still pass activation and have the implementation linked behind it.

Recommendation

Consider verifying the proxy runtime against an expected code hash before linking the implementation. Where legitimate proxy runtimes differ between chains, use a chain-specific or explicitly configured expected hash. Alternatively, verify this invariant during genesis validation and document it as an activation prerequisite.

Coinbase: Acknowledged the hardening suggestion and elected not to implement the additional check. The scenario requires malformed genesis state, which the team does not consider applicable to its Sepolia or mainnet deployments. Enforcing the check would also require maintaining expected proxy runtime hashes across chains. No code change is planned.

Description

The Base execution consensus check validates the format of the BaseTime deposit at tx[1], but does not compare its millisecond value with the schedule for that block number. Staged backfill uses this check without the Engine post execution progression hook. In a synthetic full node test, an operator selected a malformed tip whose child claimed .400 when the configured schedule required .200. The target completed staged backfill and persisted that child and its BaseTime state, while the CL schedule helper rejected it.

An operator selected malformed target can therefore be persisted despite conflicting with the CL schedule. The test does not show that an untrusted peer can choose the target or that normal CL gossip or safe derivation accepts the block. No public trigger or safe chain effect was demonstrated.

Recommendation

Apply the configured absolute Denim timestamp schedule to staged imports as well as the Engine and locally built insertion paths. Test a valid parent followed by a child whose correctly formatted BaseTime deposit contains an off schedule millisecond value.

Coinbase: Provided the fix in the stacked PR #5332, PR #5334, and PR #5335. The PRs were open and unmerged at the time of verification.

Cantina Managed: Verified the complete proposed stack at commit f01fdfe. Consensus and execution now share the timestamp schedule. At Denim heights, header validation checks scheduled seconds, and body, pre execution, and staged post execution validation check the full timestamp and BaseTime metadata. The cached insertion wrapper applies the same check before delegating insertion of a locally executed block. The four relevant crate suites passed 433 tests, and targeted checks confirmed rejection of the reported .400 child where .200 was required, incorrect activation seconds, and missing metadata in staged post execution validation.

Description

The validator extracts the mandatory BaseTime deposit from tx[1] but does not reject another depositor authorized setter later in the block. A privileged unsafe producer can supply the scheduled .200 update at tx[1] and a .400 update at tx[2]. In a synthetic signed payload test, CL gossip checks accepted the envelope, a separate cold Engine accepted the same payload bytes, and the selected state stored .400 while block RPC reported the .200 claim. The CL insertion test used a mocked Engine response; normal safe derivation does not construct this duplicate.

Such an unsafe block can expose a different executed clock from its metadata claim and may need reorganization before normal progression. This requires a faulty or malicious privileged producer. An ordinary user cannot produce the necessary protocol deposit.

Recommendation

Consider rejecting additional BaseTime setter deposits outside tx[1] and checking the final executed BaseTime state against the accepted metadata. Test both the duplicate setter and a correctly formatted setter that does not update storage.

Coinbase: Acknowledged the scenario but elected not to add validation. The extra setter requires privileged payload production, and the team considers another scan for it low priority.

Description

The execution consensus helper requires BaseTime metadata after Denim activation but does not reject a privileged BaseTime setter before activation. In a synthetic chain with the BaseTime implementation already linked, a correctly timed pre Denim block with the setter at tx[1] was accepted by a cold Engine importer and selected by forkchoice. RPC reported a millisecond timestamp derived from that deposit even though the legacy schedule has no subsecond slot. A setter at a later position can also execute without appearing in this RPC field.

A privileged unsafe branch can therefore expose or execute premature BaseTime values. Normal safe derivation does not create these transactions, and the demonstrated state change depends on a privileged producer and synthetic preactivation state. This is not a public deposit attack or a demonstrated safe chain violation.

Recommendation

Consider rejecting depositor authorized BaseTime setters before activation without rejecting unrelated deposits, and gate RPC millisecond extraction on activation. Test setters at tx[1] and later positions with both linked and dormant predeploy states.

Coinbase: Acknowledged the scenario and elected not to change the code because it requires a malfunctioning or compromised sequencer.

Description

The execution consensus check validates the BaseTime deposit at tx[1] but does not constrain the recipient of the L1 info deposit at tx[0]. A privileged unsafe signer can redirect otherwise decodable L1 info calldata to a reader contract. That deposit executes before the valid BaseTime update, so the reader can observe the previous millisecond value while final state and metadata agree on the new value. A synthetic test passed signed gossip checks and cold Engine execution for the same payload bytes, with a mocked Engine response in the CL insertion portion.

A contract reached by this forged first deposit can observe stale time within an unsafe block. This requires privileged payload production; an ordinary user cannot use this route. Normal safe derivation creates the canonical first deposit and is not shown to preserve the forged branch.

Recommendation

Consider validating the canonical identity and recipient of tx[0] at a shared block boundary, or explicitly document reliance on the privileged producer. Test a redirected first deposit followed by a valid BaseTime update through gossip and Engine execution.

Coinbase: Acknowledged the faulty or compromised producer prerequisite and elected not to make a code change. The team relies on tests of the expected system transaction positions to prevent accidental production.

Description

At the review baseline, an indexed request for an executed pending transaction could omit blockTimestampMs on a cold BaseTime cache because the canonical transaction lookup could not find the pending body. Requesting the pending block first warmed the cache, causing a later indexed request for the same transaction to include the field. Thus the same pending transaction could expose different millisecond metadata depending on request order. This was an RPC consistency issue with no demonstrated security impact.

Recommendation

Ensure that an indexed request can derive blockTimestampMs from the matching executed pending block when its body is not yet canonical, so the result does not depend on a previous RPC request warming the cache. Add a regression test comparing cold and warm requests for the same pending transaction before forkchoice.

Coinbase: Fixed in PR #5287.

Cantina Managed: In merge commit 74a922c, BaseTimeCache::get falls back to pending_block() only when its hash matches the requested block. The added test checks that cold and warm indexed pending requests return the same timestamp metadata before forkchoice. The main CI test job passed. The merged test was not rerun locally.

Description

With the optional --proofs-history ExEx enabled, SyncTargetState::apply_next replaced a pending unwind with the newest one. If an A-to-B reorg beginning at block 5 was followed by a B-to-C reorg beginning at block 9 before the storage worker consumed either action, the worker unwound only from block 9. Old A-branch history at blocks 5 through 8 remained, while proof storage advanced its latest marker to the canonical C tip. This did not change the execution node's canonical chain, but it could cause historical eth_getProof responses to disagree with canonical state.

A two-node reproduction forced this notification overlap through the running ExEx worker and MDBX proof storage. All five affected eth_getProof responses returned the old storage value and had account proofs that failed verification against their canonical headers. Three non-overlapping controls returned valid proofs. A separate storage-level reproduction also showed stale rows on RocksDB, although the live RPC reproduction did not use RocksDB. The overlap was deliberately scheduled; its occurrence rate in production was not measured. See the full-node reproduction.

Recommendation

When combining queued reorg actions, retain the earliest required unwind height and its original parent anchor while updating the forward target to the latest canonical tip. Test overlapping notifications on both storage backends and verify historical proofs against canonical header roots.

Coinbase: Fixed in Base PR #5363, merged as da999ae2. The changed state transition retains the earlier complete BlockWithParent when a later pending revert begins higher, and its regression test covers the reported 5-then-9 sequence. The current main branch retains this logic. The saved full-node control with an equivalent correction returned five valid proofs from the same payloads; the exact merged PR binary was not rerun locally. The merged build, unit-test, and system-test checks passed. No distinct regression was identified in the changed merge logic; unforced occurrence and RocksDB RPC behavior remain unmeasured.