L1 watcher signer-refresh path is dead code (vendored rollup-node @c955480): on-chain signer rotation never picked up at runtime

[dghelm]

Summary

The L1 watcher's runtime "refresh the signer every new block" path in the vendored scroll-tech/rollup-node dependency is dead code: the guard that gates the authorized-signer refresh can never evaluate true, so an on-chain sequencer-signer rotation is never picked up by the watcher at runtime. The existing signer_rotation e2e test passes only because it injects the consensus notification downstream of the watcher, so it does not cover this path.

Note on repo: the defective code is in the scroll-tech/rollup-node dependency, not in this reth tree. We currently pin it via Cargo.lock at rev = c955480 (git+https://github.com/scroll-tech/rollup-node?rev=c955480). Filing here for DogeOS-side tracking; the fix is upstream (or a local patch/fork of rollup-node).

Where

crates/watcher/src/lib.rs in rollup-node @ c955480 (line numbers below are from that pinned rev; the same logic exists on later revs such as 591081f at shifted line numbers).

Root cause — ordering bug in step()

step() advances l1_state.head before the signer-refresh guard reads it:

// step()  (crates/watcher/src/lib.rs:333-334)
let latest = self.latest_block().await?;
self.handle_latest_block(&finalized.header, &latest.header).await?;   // sets l1_state.head

// ...later in the same step (:361-362)
if let Some(system_contract_update) =
    self.handle_system_contract_update(&latest).await?               // reads l1_state.head

handle_latest_block unconditionally sets head to the latest block number before returning:

// handle_latest_block (:483-484)
self.l1_state.head = latest.number;

The guard then compares that same latest against the value just written:

// handle_system_contract_update (:709-715)
// refresh the signer every new block.
if latest_block.header.number != self.l1_state.head {   // latest.number != latest.number  ⇒ always false
    let signer = self
        .execution_provider
        .authorized_signer(self.config.address_book.system_contract_address)
        .await?;
    return Ok(Some(L1Notification::Consensus(ConsensusUpdate::AuthorizedSigner(signer))));
}
Ok(None)

Because l1_state.head was just set to latest.number, the condition is latest.number != latest.number — always false. The authorized_signer() fetch and the AuthorizedSigner emission are unreachable in normal operation.

The comment ("refresh the signer every new block") plus the TODO(greg): update with logs once system contract emits logs indicate the guard was meant to compare the new block against the previous head to detect a new block; handle_latest_block defeats that by pre-advancing head.

Why there is no escape hatch

I checked for any path where head could lag latest.number at the guard:

• handle_latest_block early-returns at :443-444 (tail.hash == latest.hash) without re-setting head — but that only fires when latest is the current tail, meaning head was already set to latest.number in the iteration that added it. Still equal.
• In that early-return case, step() never reaches handle_system_contract_update anyway, because the outer guard if latest.header.number != self.current_block_number (:336) is also false.
• The reorg branch (:449-480) falls through to :484 and sets head as well.

There is no execution path where the guard is true.

Why the test doesn't catch it

signer_rotation (rollup-node crates/node/tests/e2e.rs:1699) rotates via fixture.l1().signer_update(...), which constructs the notification by hand and pushes it straight into the node's watcher-output channel — bypassing the real L1Watcher:

// crates/node/src/test_utils/l1_helpers.rs:52-55
pub async fn signer_update(self, new_signer: Address) -> eyre::Result<()> {
    let notification =
        Arc::new(L1Notification::Consensus(ConsensusUpdate::AuthorizedSigner(new_signer)));
    self.send_to_nodes(notification).await   // tx.send() on node.l1_watcher_tx
}

So the test proves the chain-orchestrator/consensus consumer of AuthorizedSigner works, but handle_system_contract_update is never executed.

Impact

In production the rollup-node never refreshes the authorized sequencer signer from the L1 system contract via this watcher — it would keep using the genesis/configured signer. Severity for DogeOS depends on whether any topology relies on dynamic L2-sequencer-signer rotation flowing through this watcher (distinct from the bridge's own Dogecoin RotateKey mechanism). To confirm before treating as cosmetic: check whether any DogeOS sequencer/follower flow depends on picking up an on-chain authorized_signer change.

Suggested fix (upstream)

Capture the prior head before handle_latest_block advances it and pass it to the guard (or have handle_system_contract_update compare against the previously-indexed current_block_number rather than the freshly-updated l1_state.head). Either way, add a watcher-level test that drives a real step() and asserts an AuthorizedSigner notification is emitted when the on-chain signer changes — the current e2e test cannot catch this regression.

Reproduction

Read-only static trace; no build required. Inspect crates/watcher/src/lib.rs in rollup-node c955480 (step at :327, handle_latest_block head write at :484, handle_system_contract_update guard at :710).