ethereum-optimism / ethereum-optimism/optimism
op-supernode: SuperAuthority head accessors conflate local vs cross-safe (root cause of pinned-at-genesis) — de-jank
- Dominant language
- Go
- Stars
- 6.5k
- Forks
- 4k
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 165
Description
## Correction (root cause was wrong)
This issue originally attributed the "finalized pinned at genesis" symptom to a derivation stall (unsafe/safe gap + deep L1 walkback). **That root cause was incorrect.** The walkback was an aggravating *trigger*, not the cause. Corrected analysis below.
## Incident
op-supernode (interop / SuperAuthority mode), SN2/SN3 on `interop-jnt-v2`. Chain rebuilt from scratch, CL sync on, reth syncing. Symptom:
```
reth latest: advancing
reth safe: advancing (nonzero)
reth finalized: 0 (pinned at genesis)
supernode log: super authority finalized a block ahead of local finalized; using local finalized
super_authority_finalized=...:44643 local_finalized=...:0
```
Safe/local-safe progressed fine; only **local finalized** was stuck at genesis, which forced published EL finalized to stay at 0.
## Actual root cause: layering confusion in the SuperAuthority head accessors
- Under SuperAuthority, `EngineController.SafeL2Head()` returns **cross-safe** (`superAuthority.FullyVerifiedL2Head`); without SuperAuthority it returns **local-safe** (`op-node/rollup/engine/engine_controller.go:202-224`). One method, two meanings, switched on a mode flag.
- The pre-fix `promoteFinalized(ref)` gated a **local** finality candidate against `SafeL2Head()` — i.e. against **cross-safe**:
```go
if ref.Number > e.SafeL2Head().Number {
e.log.Error("Block must be safe before it can be finalized", ...)
return // bails out before SetFinalizedHead
}
```
- Cross-safe is bounded by the **slowest chain in the interop dependency set**, and during verifier cold-start it fell back to the pre-activation anchor (block 99) or `crossSafeFallback`→`FinalizedHead()` (=0). So whenever local L1-derived finality ran **ahead of cross-safe** (another chain lagging / walking back, or cold-start backfill not done), the local finality candidate was **rejected before `SetFinalizedHead`**, and `localFinalizedHead` never left genesis.
- Downstream, `FinalizedHead()` returns `min(SA, local)` = `min(44643, 0)` = `0` → published EL finalized pinned at genesis, with the "SA finalized ahead of local finalized" warning as the visible symptom.
So, answering the original open question — local finality promotions were **generated but rejected**, by comparing a *local* head against a *cross-chain* head. A layering/abstraction bug, not a derivation stall.
## Acute symptom already hot-fixed
Addressed by three commits (in the `pr-21093` image):
- `a0bbd932a7` — cold-start: verifier returns `ErrNotStarted` (hold-previous) instead of resolving an empty verified DB to the anchor (block 99). `op-supernode/.../activity/interop/interop.go`
- `220aa24f84` — clamp SA-finalized down to SA-safe so EL's `finalized ≤ safe` invariant holds. `op-supernode/.../chain_container/super_authority.go`
- `0a2d66229b` — `promoteLocalFinalizedWithSuperAuthority`: advance `localFinalizedHead` from the local L1-derived signal **without** the cross-safe gate; apply the cross-safe gate only to the **published** head. `op-node/rollup/engine/engine_controller.go:1112`
Confirmed live: chain2 finalized 0→44643 under the fixed image **before** any DB clear — i.e. the code fix worked on its own.
## What this issue now tracks: structural fragility (the real root cause)
The hot-fix patched the instance; the structure that produced it remains and will keep generating this bug class:
- **Overloaded accessors:** `SafeL2Head()` / `FinalizedHead()` change meaning based on `superAuthority != nil`, checked in many call sites. Any caller written for the single-chain meaning is a latent SuperAuthority-mode bug.
- **Side-effectful getters:** `FinalizedHead()` performs engine RPC, canonicality checks, mutates the `superAuthorityFinalizedHead` cache, and emits warns — and runs inside the forkchoice-update path.
- **Zero-value sentinels:** `eth.L2BlockRef{}` doubles as "hold previous," indistinguishable from genesis.
- **`panic` on head conflicts** in resolution paths (e.g. same-height/different-hash).
- **`deprecatedFinalizedHead` / `deprecatedSafeHead`** are still load-bearing — a half-finished refactor leaking through the seams.
- **Untested failure surface:** only reproduces under divergent multi-chain progress; unit tests miss it (see #21096 for the sync-tester harness that could catch it).
## Proposed direction (design under discussion)
Separate the three head layers explicitly (**local / cross / published**); a **pure reconciliation function** mapping `(local, cross)` → published under the EL invariants; isolate impure SA/EL/cache I/O into a **resolver** that produces a typed snapshot; a typed **"unavailable"** instead of zero-ref sentinels; recoverable errors instead of panics; and **collapse single-chain into the degenerate case of the multi-chain path** (cross == local) to eliminate the mode-flag forks entirely. De-jank PR to follow.
## Separate operational issue (not this one)
chain102's deep L1 walkback / unsafe↔safe gap that needed a **manual DB clear** to recover is a **distinct** problem (cross-safe / sync-recovery progress), not the finality layering bug. Should be tracked separately.
Contributor guide
Assessment
This issue has not been assessed yet.