ethereum-optimism / ethereum-optimism/optimism

Formalize the ActivationTime/SetActivationTime fork-switch pattern as a shared design

Open
#21,267 0 comments 0 reactions 1 assignee Claimed by @sebastianst View on GitHub
Dominant language
Go
Stars
6.5k
Forks
4k
Avg merge
2d 15h
Merged PRs (30d)
145

Description

## Context

Two OP-Stack config types now express their hardfork schedule through the same two-method pattern: a single `ActivationTime(fork) *uint64` switch and a paired `SetActivationTime(fork, *uint64)` setter, with `IsForkActive(fork, t)` and every `Is` predicate delegating to `ActivationTime`. The fork→field mapping lives in exactly one place per type.

- **`rollup.Config`** — the consensus-layer rollup config:
- [`ActivationTime`](https://github.com/ethereum-optimism/optimism/blob/f176635fafcd5e7f6dc07b52e391bdc0c249fda6/op-node/rollup/types.go#L581-L614) / [`SetActivationTime`](https://github.com/ethereum-optimism/optimism/blob/f176635fafcd5e7f6dc07b52e391bdc0c249fda6/op-node/rollup/types.go#L616-L649)
- [`IsForkActive`](https://github.com/ethereum-optimism/optimism/blob/f176635fafcd5e7f6dc07b52e391bdc0c249fda6/op-node/rollup/types.go#L439-L442) and [`ActivateAt`](https://github.com/ethereum-optimism/optimism/blob/f176635fafcd5e7f6dc07b52e391bdc0c249fda6/op-node/rollup/types.go#L679-L695)
- **`op-core/params.ChainConfig`** — the execution-layer chain config that wraps upstream `params.ChainConfig`, added in #20260 (part of the op-geth decoupling, #20257).

## Problem

The pattern is currently **copy-pasted per type**, with a few sharp edges:

- **Two parallel `switch` statements that must stay in lockstep.** A fork mapped to the wrong field in `ActivationTime` but not `SetActivationTime` (or vice versa) is a silent bug; only a dedicated round-trip test catches it. Each new type re-introduces this risk.
- **Each type re-derives `IsForkActive` and the per-fork predicates**, so the same boilerplate is duplicated.
- **Implementations have already diverged.** `rollup.Config` covers `Delta` and `PectraBlobSchedule` and adds `ActivateAt` / `ActivateAtGenesis` / `IsActivationBlock`; `op-core/params.ChainConfig` omits `Delta` (no execution-layer field for it) and the activation-block helpers. The set of forks each type supports — and how it reports unknown forks — is decided independently.

## Proposal (to design)

Write up the pattern as a formal, reusable design. Options to evaluate:

1. **Shared generic helper / mixin** keyed on `forks.Name` that both types embed, so each type supplies only its fork→field binding once and gets `IsForkActive` + predicates for free.
2. **Code-generate the switches from `forks.All`**, eliminating the lockstep-drift risk entirely and making "add a fork" a single edit.
3. **Documented convention + a shared test helper** that asserts the `ActivationTime`/`SetActivationTime` round-trip for every fork a type claims to support, even if the switches stay hand-written.

Deliverable: a short design doc (e.g. under `docs/ai/` or a `design-docs` entry) capturing the chosen approach and the trade-offs, plus follow-up issues to converge the existing implementations onto it.

## References

- Pattern origin: `rollup.Config` (links above)
- New instance: `op-core/params.ChainConfig` (#20260)
- Epic: op-geth decoupling (#20257)

🤖 *Generated by Claude Code*

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.