fix(chain-orchestrator): refill pool + loopback parse; revert startup EL/DB reconciliation (PR review pass 69)

Codex review pass 69 raised two P1s, one P2, and one P3. The two P1s are both on
the pass-66..68 startup execution-node/database reconciliation, which has now
produced six findings across four passes without converging.

Revert the startup EL-vs-DB head reconciliation (the args.rs additions from
passes 66, 67, and 68), returning startup to the stable, pre-existing repair-loop
behavior. Root cause of the non-convergence: correctly reconciling a persisted
L2 head of 0 requires distinguishing a FRESH database (bootstrap against an
already-synced execution datadir — adopt the execution node) from a database
UNWOUND to genesis (authoritative at 0 — rewind the execution node), and no such
freshness signal exists today (`l2_head_block` is always seeded to 0, and
`startup_refusal` also treats 0 as bootstrap). Getting this right is a
self-contained crash-recovery hardening task (freshness marker, forkchoice push
to the execution node with a VALID gate, pool refill, e2e restart coverage) that
is disproportionate to this CI-stabilization PR and kept regressing when patched
incrementally. It is recorded in .claude/vmark-pr-review-follow-ups.md. This
resolves pass-67 P1, pass-68 P1a/P1b, and pass-69 P1a/P1b together. The
sign-failure fail-stop that motivated it is retained; restart re-converges via
the pre-existing loop with its documented limitations.

P2 — refill the transaction pool after the optimistic-head rewind to safe. The
recheck's recovery from an INVALID optimistic head (added pass 66) rewinds the
engine head down to the safe head but, because it is a target==mirror path, the
existing `rewound_backward` collection never ran, so the discarded blocks'
transactions were dropped from the pool. Collect them (safe+1 ..= mirror) before
the rewind FCU — while those blocks are still canonical — and reinsert on VALID,
matching every other rewind seam. (Their L1-message mappings were already cleared
when optimistic_sync purged all mappings on entry, so no purge is paired here.)

P3 — classify the remote-source plaintext-http loopback exemption by the PARSED
address, not a string prefix. `host.starts_with("127.")` treated DNS names like
`127.rpc.example` as loopback and silenced the MITM warning; parse the host as an
IP and use `is_loopback` (stripping IPv6 brackets), exempting only the exact
`localhost` domain.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: vmarkushin

.claude/vmark-pr-review-follow-ups.md

@@ -5,26 +5,40 @@ from every pass is either fixed in the PR or recorded here.
 
 ## Unresolved
 
-- **No e2e coverage for the startup rewind of an execution node that ran ahead
-  of the persisted head** (`crates/node/src/args.rs` startup head-repair + the
-  new post-`Engine::new` forkchoice push)
-  - Impact/evidence: Codex pass 67 P1 (2026-09-03) found that the pass-66
-    startup reconciliation only moved the in-memory mirror and the database, and
-    never pushed `engine_forkchoiceUpdated` to Reth, so a quiescent node would
-    keep serving — and followers keep importing — the discarded head. FIXED this
-    pass by issuing a checked FCU to the execution node after the engine is
-    built whenever startup repair lowered the mirror below the execution node's
-    live head (also closes the same latent gap in the pre-existing repair loop).
-    The new path has no automated test: it needs an e2e restart where the
-    execution node holds a block above the persisted L2 head (an unsigned
-    sequenced block left by a crash between commit and sign), asserting the
-    execution node's RPC head is rewound on restart.
-  - First/most-recent pass: Codex pass 67 (2026-09-03).
-  - Why unaddressed: exercising it requires a full node restart against a real
-    execution client with a head deliberately ahead of the database — an e2e
-    harness addition beyond this review loop's local-fix scope; the production
-    fix itself is landed and verified by build/clippy.
-  - Suggested Linear title: "rollup-node: e2e test for startup rewind when the execution node runs ahead of the persisted head"
+- **Startup reconciliation of an execution node whose head diverges from the
+  persisted L2 head is unsolved and was REVERTED** (`crates/node/src/args.rs`
+  startup head-repair loop; the sign-failure fail-stop in
+  `crates/chain-orchestrator/src/lib.rs`)
+  - Impact/evidence: the pre-existing startup repair loop only reconciles the
+    engine head down to the persisted head while the persisted head sits ABOVE
+    finalized, and (Codex pass 67) never pushes the resulting forkchoice to the
+    execution node at all — so a quiescent node keeps serving, and followers keep
+    importing, a head the node has locally discarded. This bites at least two
+    concrete cases: (a) a sequenced block committed to the engine but not signed
+    or persisted, left behind by the sign-failure fail-stop this PR adds, when
+    the prior block is already finalized (original Codex pass 66 P2); and (b) an
+    L1/administrative unwind that persists the L2 head at genesis and crashes
+    before its FCU, where the populated database is authoritative at 0 (Codex
+    pass 69 P1). Attempts to fix this inside the review loop (passes 66-68) each
+    surfaced a further edge — pushing the FCU (67), a fresh-DB-vs-authoritative-0
+    ambiguity that rewinds a bootstrapping node to genesis (68/69), requiring a
+    VALID answer, and failing on a missing target block (69) — because the fix
+    needs a reliable "is this database fresh or was it unwound to genesis"
+    signal that does NOT exist today (`l2_head_block` is always seeded to 0, and
+    `startup_refusal` also treats 0 as bootstrap), plus a proper transaction-pool
+    refill and an execution-node forkchoice push, all exercised by an e2e restart
+    harness. The pass-66..68 additions were REVERTED to the stable pre-existing
+    behavior to stop destabilizing this CI PR.
+  - First/most-recent pass: Codex pass 66 P2 (2026-09-03); Codex pass 69 P1x2
+    (2026-09-03).
+  - Why unaddressed: correct startup EL-vs-DB reconciliation is a self-contained
+    crash-recovery hardening task — it needs a database freshness/history marker
+    (likely a new migration), an execution-node forkchoice push with a VALID
+    gate, pool refill on the rewind, and e2e restart coverage — disproportionate
+    to this CI-stabilization PR and repeatedly regressing when patched
+    incrementally. The sign-failure fail-stop itself is retained (restart
+    re-converges via the pre-existing loop, with its documented limitations).
+  - Suggested Linear title: "rollup-node: startup reconciliation when the execution node head diverges from the persisted L2 head (fresh-vs-unwound DB signal, FCU push, pool refill, e2e)"
 
 - **Remote block source has no metrics** (`crates/node/src/add_ons/remote_block_source.rs`)
   - Impact/evidence: Claude pass 1 m6 and pass 5 m2 — the add-on exports no

crates/chain-orchestrator/src/lib.rs

@@ -2430,6 +2430,30 @@ impl<
                 // (derivation-validated, a finalized-descendant) and latch onto
                 // safe, so the next probe adopts it through the normal
                 // exit-sync path (which consolidates and persists the head).
+                //
+                // This is a backward head move (safe < mirror), so — like every
+                // other rewind seam — the transactions in the discarded blocks
+                // (safe+1 ..= mirror) must be refilled into the pool. Collect
+                // them BEFORE the FCU: on VALID it commits the mirror to `safe`,
+                // after which the canonical lookups find nothing above it. (The
+                // L1-message mappings for those blocks were already cleared when
+                // optimistic_sync purged all mappings on entry, so no purge is
+                // paired here.)
+                let reverted_transactions = match self
+                    .collect_reverted_txs_in_range(safe.number.saturating_add(1), mirror.number)
+                    .await
+                {
+                    Ok(txs) => txs,
+                    Err(err) => {
+                        tracing::warn!(
+                            target: "scroll::chain_orchestrator",
+                            %err,
+                            "Failed to collect reverted transactions for the pool refill before \
+                             an optimistic-head rewind to safe; continuing without them"
+                        );
+                        Vec::new()
+                    }
+                };
                 match self.engine.update_fcs_checked(Some(safe), None, None).await {
                     Ok(r) if r.is_valid() => {
                         tracing::warn!(
@@ -2439,6 +2463,7 @@ impl<
                             "Optimistic head rejected as INVALID; rewound the engine head to the \
                              safe head and re-latched onto it"
                         );
+                        self.reinsert_txs_into_pool(reverted_transactions).await;
                         self.l2_sync_recheck_target = Some(L2SyncRecheck {
                             target: safe,
                             latched_from: safe,
@@ -3811,8 +3836,14 @@ mod run_loop_policy_tests {
         engine_client
             .push_fork_choice_updated(ScriptedResponse::Ok(fcu(PayloadStatusEnum::Valid, None)));
         let asserter = Asserter::new();
-        asserter.push_success(&Option::<()>::None);
-        asserter.push_success(&Option::<()>::None);
+        // The rewind to safe collects reverted txs from the discarded blocks
+        // (safe+1 ..= mirror = 5 blocks) BEFORE the rewind FCU; answer each
+        // getBlockByNumber with a null block so the collection returns empty
+        // (the refill mechanics themselves are covered by the backward-adopt
+        // tests — here we only assert the rewind and re-latch).
+        for _ in 0..5 {
+            asserter.push_success(&Option::<()>::None);
+        }
         // Optimistic head M (height SAFE) sits ABOVE the derivation-validated
         // safe head S (height SAFE - 5) — the realistic optimistic-sync shape.
         let optimistic_head = info(SAFE, 0x11);

crates/node/src/args.rs

@@ -249,8 +249,18 @@ impl ScrollRollupNodeConfig {
             // fine.
             if url.scheme() == "http" {
                 let host = url.host_str().unwrap_or_default();
-                let loopback = matches!(host, "localhost" | "127.0.0.1" | "::1" | "[::1]") ||
-                    host.starts_with("127.");
+                // Classify by the PARSED address, not a string prefix: a prefix
+                // check treats DNS names like `127.rpc.example` as loopback even
+                // though they resolve to arbitrary addresses, silencing the very
+                // warning that matters. `host_str()` brackets IPv6 literals, so
+                // strip those before parsing; only the exact `localhost` name is
+                // exempt among domains.
+                let host_ip =
+                    host.strip_prefix('[').and_then(|h| h.strip_suffix(']')).unwrap_or(host);
+                let loopback = match host_ip.parse::<std::net::IpAddr>() {
+                    Ok(ip) => ip.is_loopback(),
+                    Err(_) => host.eq_ignore_ascii_case("localhost"),
+                };
                 if !loopback {
                     tracing::warn!(
                         target: "scroll::node::args",
@@ -582,13 +592,6 @@ impl ScrollRollupNodeConfig {
             }
         };
 
-        // The execution node's live head, before any startup head-repair below
-        // moves the in-memory mirror. If a repair rewinds the mirror BELOW this,
-        // the rewind must be pushed to the execution node (see the FCU after the
-        // engine is built); otherwise only the mirror and database move and the
-        // execution node keeps serving the discarded head.
-        let provider_head_number = fcs.head_block_info().number;
-
         let (l1_block_startup_info, mut l2_head_block_number) = db
             .tx_mut(move |tx| async move {
                 // On startup we replay the latest batch of blocks from the database as such we set
@@ -669,55 +672,6 @@ impl ScrollRollupNodeConfig {
             l2_head_block_number -= 1;
         }
 
-        // The loop above only runs while the persisted head sits ABOVE finalized,
-        // so it never fires when the persisted head is at or below finalized. But
-        // the execution node's head can sit ABOVE the persisted head there too: a
-        // sequenced block is committed to the engine BEFORE it is signed and its
-        // head persisted, so a crash between those steps (e.g. a signing failure
-        // that fail-stops the node) leaves the engine one block ahead of the
-        // database. Resuming on that block would sequence on top of a block this
-        // node never signed or announced — forking it from its peers. Drag the
-        // engine head back down to the persisted head; the persisted head is at
-        // or below finalized here, so it is guaranteed present in the EN.
-        //
-        // Excluded when the persisted head is 0: a fresh rollup database against
-        // an already-synced execution datadir is a legitimate bootstrap that
-        // `startup_refusal` permits, and the zero anchor is NOT authoritative
-        // there — treating it as such would rewind the execution node to genesis
-        // (or fail with SafeBelowFinalized when its finalized marker is above 0).
-        // Bootstrap adopts the provider forkchoice unchanged, exactly as the
-        // loop above (whose `> finalized` bound is likewise never met at 0) does.
-        if l2_head_block_number > 0 && fcs.head_block_info().number > l2_head_block_number {
-            if let Some(block) = l2_provider
-                .get_block(l2_head_block_number.into())
-                .full()
-                .await?
-                .map(|b| b.into_consensus().map_transactions(|tx| tx.inner.into_inner()))
-            {
-                let block_info: BlockInfo = (&block).into();
-                tracing::warn!(
-                    target: "scroll::node::args",
-                    engine_head = ?fcs.head_block_info(),
-                    persisted_head = ?block_info,
-                    "Execution node head sits above the persisted L2 head at startup; clamping \
-                     it down (a built block was likely committed to the engine without \
-                     completing signing and persistence)"
-                );
-                // Clamp safe too if it sits above the head we are resuming from,
-                // the same rule the loop above and the runtime rewind paths apply.
-                if fcs.safe_block_info().number > block_info.number {
-                    fcs = ForkchoiceState::new(block_info, block_info, *fcs.finalized_block_info());
-                } else {
-                    fcs.update(Some(block_info), None, None)?;
-                }
-                db.tx_mut(move |tx| async move {
-                    tx.set_l2_head_block_number(l2_head_block_number).await?;
-                    tx.purge_l1_message_to_l2_block_mappings(Some(l2_head_block_number + 1)).await
-                })
-                .await?;
-            }
-        }
-
         let chain_spec = Arc::new(chain_spec.clone());
 
         // Instantiate the network manager
@@ -738,65 +692,7 @@ impl ScrollRollupNodeConfig {
         ctx.task_executor.spawn_task(scroll_network_manager.run());
 
         tracing::info!(target: "scroll::node::args", fcs = ?fcs, payload_building_duration = ?self.sequencer_args.payload_building_duration, "Starting engine driver");
-        let mut engine = Engine::new(Arc::new(engine_api), fcs);
-
-        // If the startup head-repair above rewound the mirror BELOW the
-        // execution node's live head, the repair so far has only moved the
-        // in-memory mirror and the database — the execution node still holds the
-        // discarded head (e.g. an unsigned block committed to the engine by a
-        // crash between commit and sign, or an EL that simply ran ahead). Push
-        // and validate the rewind against the execution node now, before
-        // launching: otherwise a quiescent node keeps serving — and followers
-        // keep importing — the block this repair discarded, because nothing in
-        // the run loop reissues a head FCU on an idle chain. Skipped when the
-        // provider forkchoice could not be read (engine unreachable / genesis
-        // fallback), since there is no live head to rewind.
-        if !provider_fcs_missing && engine.fcs().head_block_info().number < provider_head_number {
-            let head = *engine.fcs().head_block_info();
-            match engine.update_fcs_checked(Some(head), None, None).await {
-                Ok(result) if result.is_valid() => {
-                    tracing::info!(
-                        target: "scroll::node::args",
-                        ?head,
-                        provider_head_number,
-                        "Rewound the execution node head to the recovered startup head"
-                    );
-                }
-                Ok(result) if result.is_invalid() => {
-                    // The execution node actively rejected a head the node
-                    // recovered from its own persisted state — a genuine
-                    // divergence that must not launch.
-                    eyre::bail!(
-                        "execution node rejected the recovered startup head {head:?} as INVALID: \
-                         {:?}",
-                        result.payload_status.status
-                    );
-                }
-                Ok(result) => {
-                    // Non-VALID (SYNCING/ACCEPTED): the execution node has NOT
-                    // adopted the rewind. The rewind target is an ANCESTOR the
-                    // execution node already holds, so VALID is the only correct
-                    // answer — anything else means it is in an unexpected state
-                    // (e.g. still snap-syncing). We cannot launch anyway: the
-                    // orchestrator starts L2-Synced with no recheck latch, and
-                    // its periodic recheck only fires while L2 is syncing WITH a
-                    // target, so nothing would reissue this FCU on a quiescent
-                    // node — it would keep serving the discarded head. Fail
-                    // startup so the supervisor restarts and retries.
-                    eyre::bail!(
-                        "execution node did not adopt the recovered startup head {head:?} \
-                         (status {:?}); refusing to launch on the discarded head",
-                        result.payload_status.status
-                    );
-                }
-                Err(err) => {
-                    eyre::bail!(
-                        "failed to push the recovered startup head {head:?} to the execution \
-                         node: {err}"
-                    );
-                }
-            }
-        }
+        let engine = Engine::new(Arc::new(engine_api), fcs);
 
         // Create the consensus.
         let authorized_signer = if let Some(provider) = l1_provider.as_ref() {