ethereum-optimism / ethereum-optimism/optimism

kona: unify fork-activation-block predicates on the parent timestamp

Open
#22,683 0 comments 0 reactions 0 assignees View on GitHub
A-kona
Dominant language
Go
Stars
6.5k
Forks
4k
Avg merge
2d 15h
Merged PRs (30d)
145

Description

## Summary

Kona answers "is this block a fork's activation block?" in two inequivalent ways.

**Timestamp-only** — [`RollupConfig::is_first_*_block`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/genesis/src/rollup.rs#L167-L328) and [`is_fork_activation_block`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/genesis/src/rollup.rs#L304-L311) reconstruct the parent's timestamp from a config constant:

```rust
active(ts) && !active(ts.saturating_sub(self.block_time))
```

**Parent-relative** — [`alloy_op_hardforks::is_no_user_tx_activation_block`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/alloy-op-hardforks/src/lib.rs#L237-L247) and the upgrade-transaction injection in [`StatefulAttributesBuilder`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/derive/src/attributes/stateful.rs#L165-L215) use the parent's actual timestamp:

```rust
active(ts) && !active(parent_ts)
```

The parent-relative form is the correct one. The predicate is a statement about the chain — "the fork is active at this block and was not at its parent" — and `ts - block_time` is only a *reconstruction* of the parent's timestamp, valid exactly while every block satisfies `parent.timestamp == ts - block_time`. Where that holds the two forms are identical; where it does not, the timestamp-only form answers about a block that is not the parent.

## Where they diverge

**Multi-blocks ([#22671](https://github.com/ethereum-optimism/optimism/pull/22671))** is the concrete case: several L2 blocks share one timestamp, so `ts - block_time` names a block several positions back. Every sibling in the activating group answers `true` to `is_first_karst_block(ts)`, not just the first. The sequencer would build every sibling with `no_tx_pool`, `SingleBatch::check_batch` would drop every non-empty sibling, `L1BlockInfoTx::try_new` would emit the pre-fork variant for every sibling, and `upgrade_gas_to_strip` would strip the upgrade gas from every sibling instead of only from the block after the activation block. The parent-relative sites in the same flow would answer correctly, so the two halves of one block's construction disagree.

There is already a live asymmetry inside a single mechanism, independent of multi-blocks: the Karst/Lagoon upgrade gas is **added** parent-relatively in [`stateful.rs`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/derive/src/attributes/stateful.rs#L189-L215) but **stripped** timestamp-only in [`upgrade_gas_to_strip`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/protocol/src/utils.rs#L102-L120). The two have to agree exactly or the reconstructed `SystemConfig.gas_limit` diverges; today they agree only because the cadence is uniform.

## Every consumer of the timestamp-only helpers

14 production call sites, in 5 files.

| File | Lines | Enclosing fn | What it gates | Parent timestamp available? |
|---|---|---|---|---|
| [`node/service/src/actors/sequencer/actor.rs`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/node/service/src/actors/sequencer/actor.rs#L343-L408) | 358, 364, 370, 376, 382, 389, 396, 402 | `should_use_tx_pool` (ecotone, fjord, granite, holocene, isthmus, jovian, karst, interop) | whether the sequencer sets `no_tx_pool` | **Yes**, one frame up: `build_attributes`' `unsafe_head: L2BlockInfo` |
| [`protocol/protocol/src/batch/single.rs`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/protocol/src/batch/single.rs#L156-L166) | 157–159 | `SingleBatch::check_batch` | `BatchValidity::Drop(NonEmptyTransitionBlock)` — derivation-layer enforcement of the no-user-tx rule | **Yes**, `l2_safe_head: L2BlockInfo` already a parameter |
| [`protocol/protocol/src/info/variant.rs`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/protocol/src/info/variant.rs#L47-L155) | 59, 122, 151 | `L1BlockInfoTx::try_new` | which `L1BlockInfoTx` variant is emitted (the L1Block predeploy is not upgraded yet on the activation block) | **No** in `try_new`; yes two frames up at `stateful.rs:219` |
| [`protocol/protocol/src/utils.rs`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/protocol/src/utils.rs#L102-L120) | 109 | `upgrade_gas_to_strip` (via `to_system_config`) | how much one-time upgrade gas to subtract from the reconstructed `SystemConfig.gas_limit` | **No** — `to_system_config` sees one block; the parent costs an extra header fetch / preimage |
| [`protocol/interop/src/rules.rs`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/interop/src/rules.rs#L58-L62) | 61 | `MessageRules::interop_active_for_full_block` | `ExecutedTooEarly` / `InitiatedTooEarly` on cross-chain messages | **No** — bare timestamps from a message identifier and a remote chain |

`is_first_regolith_block`, `is_first_canyon_block`, `is_first_delta_block` and `is_first_pectra_blob_schedule_block` have no production consumers at all — only their own unit tests.

For contrast, the parent-relative sites: `is_no_user_tx_activation_block` is called from [`kona/crates/proof/executor/src/builder/core.rs:268`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/proof/executor/src/builder/core.rs#L268) and [`op-reth/crates/evm/src/lib.rs:217`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/op-reth/crates/evm/src/lib.rs#L217), and enforced at [`alloy-op-evm/src/block/mod.rs:876`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/alloy-op-evm/src/block/mod.rs#L876). The six inline blocks in `stateful.rs` inject upgrade transactions, and [`SystemConfig::eip_1559_params`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/rust/kona/crates/protocol/genesis/src/system/config.rs#L154-L170) takes an explicit `parent_timestamp` for the first-Holocene `B64::ZERO` signal. op-reth has no timestamp-only usage at all — it never sees a `RollupConfig`.

## Proposed refactor

1. Keep one predicate on `RollupConfig`:
```rust
pub fn is_fork_activation_block(&self, fork: OpHardfork, parent_timestamp: u64, timestamp: u64) -> bool
```
with the same parameter order as `is_no_user_tx_activation_block`.
2. Delete the 13 `is_first_*_block` helpers. Four have no production consumers and go straight away; the rest become `is_fork_activation_block(OpHardfork::X, parent_ts, ts)`. The `interop` and `sdm` feature gates alias Lagoon, so `is_interop_active` / `is_sdm_active` stay as feature gates and their activation check becomes `is_fork_activation_block(OpHardfork::Lagoon, ..)`.
3. Thread the parent timestamp to the call sites. Two are trivial (sequencer, `check_batch`), one is a signature change with the value in scope at the caller (`L1BlockInfoTx::try_new` / `try_new_with_deposit_tx` from `stateful.rs`).
4. Fold the inline parent-relative checks (`stateful.rs`, `SystemConfig::eip_1559_params`) onto the same predicate so there is one implementation.
5. While there: `single.rs` hardcodes Jovian/Karst/Lagoon and will silently miss the next fork, where `is_no_user_tx_activation_block` iterates `OpHardfork::Jovian.forks_from()`. Make it fork-generic.

### Open design questions

The two call sites without a parent in scope need a decision before anything is implemented:

- **`upgrade_gas_to_strip` / `to_system_config`.** Its three callers (`OracleL2ChainProvider::system_config_by_l2_hash`, `AlloyL2ChainProvider::system_config_by_l2_hash`, `providers-local/src/buffered.rs`) resolve a single block by hash. Threading a parent timestamp means an extra header fetch — and in the proof path, an extra preimage — on every system-config reconstruction. Alternatives: pass the parent timestamp down from the callers, or keep this one path cadence-based and document the constraint.
- **`MessageRules::interop_active_for_full_block`.** It compares timestamps from a message identifier against a remote chain's block, whose header the graph only fetches later. Either move the check after the header fetch or accept a timestamp-only rule here and say so in the spec.

## Scope

kona only. op-node has the same duality — [`IsXActivationBlock(l2BlockTime)`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/op-node/rollup/types.go#L546-L629) and [`IsActivationBlockForFork`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/op-node/rollup/types.go#L710-L714) versus [`IsActivationBlock(oldTime, newTime)`](https://github.com/ethereum-optimism/optimism/blob/68b009443f67cf830990e913bcbde8536cb15327/op-node/rollup/types.go#L699-L708) — but it is being deprecated, so there is no reason to churn it.

On every chain that exists today the cadence is uniform and the two forms agree, so this is a refactor with no consensus change on current chains. (One narrow exception: op-node's timestamp-only form guards with `l2BlockTime >= c.BlockTime` while kona uses `saturating_sub`, so the two clients can disagree for block timestamps below `block_time` — reachable only in tests near the Unix epoch.)

Not to be implemented before the two open questions above are settled.

🤖 *Co-created with Claude Fable 5*

Contributor guide

Open the contributing guide

Research direction

Start in rust/kona/crates/protocol/genesis/src/rollup.rs, then trace the listed consumers in stateful.rs, actor.rs, single.rs, variant.rs, utils.rs, and interop rules.rs. Resolve the parent-timestamp design questions for upgrade_gas_to_strip and MessageRules before implementation; done means one parent-relative activation predicate, updated call sites, and passing the affected unit tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, distributed-systems
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.