ethereum-optimism / ethereum-optimism/optimism
op-node: sequencer LightCL performs needless engine reset on every restart (may rewind EL heads)
- Dominant language
- Go
- Stars
- 6.5k
- Forks
- 4k
- Avg merge
- 2d 15h
- Merged PRs (30d)
- 145
Description
## Summary
An op-node configured as a "sequencer LightCL" — i.e. with both `--sequencer.enabled` and `--l2.follow.source` — runs `TryInitialResetEngineForSequencer` → `FindL2Heads` → `forceReset` on every process restart. The guard intended to make this a one-shot is based on in-process memory state that is always zero at fresh process start, so it never trips on restart. The reset re-walks the L2 chain against the current L1 view on every boot.
In the happy case this is wasteful. Under a stale/slow/reorged L1 RPC at restart, `FindL2Heads` can return heads earlier than what the EL already has correctly persisted, and `forceReset` will drive the EL backwards — a needless walkback. A LightCL should not need to walk back on a simple restart.
This is based on code review; I haven't reproduced a specific walkback incident from the restart path described.
## Code path
**Config gating** — [`op-node/service.go:372`](https://github.com/ethereum-optimism/optimism/blob/develop/op-node/service.go#L372):
```go
NeedInitialResetEngine: ctx.Bool(flags.SequencerEnabledFlag.Name) && l2FollowSourceEndpoint != "",
```
**Entry** — [`op-node/rollup/driver/sync_deriver.go:238-244`](https://github.com/ethereum-optimism/optimism/blob/develop/op-node/rollup/driver/sync_deriver.go#L238-L244):
```go
if s.SyncCfg.FollowSourceEnabled() {
if s.SyncCfg.NeedInitialResetEngine {
// May need a single reset to trigger sequencer block building
s.Engine.TryInitialResetEngineForSequencer(s.Ctx)
}
return
}
```
**Insufficient guard** — [`op-node/rollup/engine/engine_controller.go:1147-1163`](https://github.com/ethereum-optimism/optimism/blob/develop/op-node/rollup/engine/engine_controller.go#L1147-L1163):
```go
func (e *EngineController) TryInitialResetEngineForSequencer(ctx context.Context) {
e.mu.Lock()
defer e.mu.Unlock()
if e.unsafeHead != (eth.L2BlockRef{}) {
return
}
// ... FindL2Heads + forceReset
}
```
`EngineController` is process-scoped — `e.unsafeHead` is zero at process start regardless of datadir/EL state. The guard is effective within a single process after the first call, but does not prevent the reset from re-running across restarts.
## Impact
- **Always on restart**: unnecessary FCU round-trip and L1-canonicality walk of the L2 chain.
- **Under L1 RPC flakiness or partial L1 view at restart**: `FindL2Heads` can return heads earlier than the EL's persisted unsafe head, and `forceReset` will send a `ForkchoiceUpdated` telling the EL to rewind blocks that were validly derived and persisted.
## Fix directions (not prescriptive)
- Check the EL for an existing unsafe head before the guard, so the guard reflects persisted state rather than in-memory state.
- Persistent "initialized" marker in the datadir, consulted before running the reset.
- Gate the reset on "EL reports genesis as latest" so it only runs on truly fresh state.
## Not affected
Pure LightCL followers (follow-source without `--sequencer.enabled`) do not hit this path — `NeedInitialResetEngine` is false for them.
Contributor guide
Research direction
Start with op-node/service.go:372, op-node/rollup/driver/sync_deriver.go:238-244, and op-node/rollup/engine/engine_controller.go:1147-1163; trace how NeedInitialResetEngine, unsafeHead, FindL2Heads, and forceReset interact across a process restart. Compare the EL's persisted unsafe head with the reset guard, then add coverage showing that a restart does not rerun the reset or rewind an existing head while a fresh state can still initialize.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- blockchain
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100