ethereum-optimism / ethereum-optimism/optimism

op-supernode: SuperAuthority head accessors conflate local vs cross-safe (root cause of pinned-at-genesis) — de-jank

Open
#21,097 0 comments 0 reactions 0 assignees View on GitHub
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

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.