Skip to content

Commit 3e79c44

Browse files
vmarkushinclaude
andcommitted
fix(node): scope startup rewind to a non-fresh DB and require VALID (PR review pass 68)
Codex review pass 68 raised two P1s on the pass-66/67 startup head-reconciliation. P1a — do not apply the head-repair to a fresh rollup database. The pass-66 reconciliation fired whenever the execution node's head sat above the persisted L2 head, including when the persisted head is 0. A fresh rollup database against an already-synced execution datadir is a legitimate bootstrap (permitted by startup_refusal), and the zero anchor is not authoritative there: the repair would construct head=safe=0 and either fail startup with SafeBelowFinalized (when the execution node's finalized marker is above 0) or, via the pass-67 push, rewind the execution node's canonical head all the way back to genesis. Gate the reconciliation on a non-zero persisted head, so bootstrap adopts the provider forkchoice unchanged — exactly as the pre-existing repair loop already does (its `> finalized` bound is likewise never met at 0). P1b — treat a non-VALID startup rewind as a launch failure. The pass-67 push logged a warning and launched on SYNCING/ACCEPTED, on the assumption a later FCU would reassert the head. But the orchestrator starts L2-Synced with no l2_sync_recheck_target, and the periodic recheck only fires while L2 is syncing with a target, so on a quiescent node nothing reissues the FCU and the execution node keeps serving the discarded head. The rewind target is an ancestor the execution node already holds, so VALID is the only correct answer; any other status now fails startup so the supervisor restarts and retries, rather than launching on a head the execution node has not adopted. Both fixes are in the startup path already tracked for e2e coverage in .claude/vmark-pr-review-follow-ups.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f1b9f17 commit 3e79c44

1 file changed

Lines changed: 24 additions & 10 deletions

File tree

crates/node/src/args.rs

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,15 @@ impl ScrollRollupNodeConfig {
679679
// node never signed or announced — forking it from its peers. Drag the
680680
// engine head back down to the persisted head; the persisted head is at
681681
// or below finalized here, so it is guaranteed present in the EN.
682-
if fcs.head_block_info().number > l2_head_block_number {
682+
//
683+
// Excluded when the persisted head is 0: a fresh rollup database against
684+
// an already-synced execution datadir is a legitimate bootstrap that
685+
// `startup_refusal` permits, and the zero anchor is NOT authoritative
686+
// there — treating it as such would rewind the execution node to genesis
687+
// (or fail with SafeBelowFinalized when its finalized marker is above 0).
688+
// Bootstrap adopts the provider forkchoice unchanged, exactly as the
689+
// loop above (whose `> finalized` bound is likewise never met at 0) does.
690+
if l2_head_block_number > 0 && fcs.head_block_info().number > l2_head_block_number {
683691
if let Some(block) = l2_provider
684692
.get_block(l2_head_block_number.into())
685693
.full()
@@ -764,15 +772,21 @@ impl ScrollRollupNodeConfig {
764772
result.payload_status.status
765773
);
766774
}
767-
Ok(_result) => {
768-
// SYNCING/ACCEPTED: the execution node took the head but has
769-
// not finished adopting it. The mirror already holds it and
770-
// every later FCU reasserts it, so do not fail startup.
771-
tracing::warn!(
772-
target: "scroll::node::args",
773-
?head,
774-
"Execution node has not yet adopted the recovered startup head; it will \
775-
be reasserted"
775+
Ok(result) => {
776+
// Non-VALID (SYNCING/ACCEPTED): the execution node has NOT
777+
// adopted the rewind. The rewind target is an ANCESTOR the
778+
// execution node already holds, so VALID is the only correct
779+
// answer — anything else means it is in an unexpected state
780+
// (e.g. still snap-syncing). We cannot launch anyway: the
781+
// orchestrator starts L2-Synced with no recheck latch, and
782+
// its periodic recheck only fires while L2 is syncing WITH a
783+
// target, so nothing would reissue this FCU on a quiescent
784+
// node — it would keep serving the discarded head. Fail
785+
// startup so the supervisor restarts and retries.
786+
eyre::bail!(
787+
"execution node did not adopt the recovered startup head {head:?} \
788+
(status {:?}); refusing to launch on the discarded head",
789+
result.payload_status.status
776790
);
777791
}
778792
Err(err) => {

0 commit comments

Comments
 (0)