apollo_consensus_orchestrator: fail sync gracefully instead of panicking on missing recorder data

[gkaempfer]

Security scan finding

This is a fix for a vulnerability found while scanning #14826 (merged), which introduced fetching accessed-keys data from the centralized recorder in try_sync.

Vulnerability

In SequencerConsensusContext::try_sync (crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs), when fetch_accessed_keys_from_centralized is enabled, a None response from cende_ambassador.get_accessed_keys_input triggers a panic!:

Ok(None) => {
    panic!(
        "The accessed-keys data for synced block {block_number} is expected to be ready."
    );
}

get_accessed_keys_input is an HTTP GET to the recorder service, and its own doc comment states plainly: "Returns None when the recorder does not have the block." That is a documented, normal response from a network-reachable dependency (the recorder can legitimately lag behind state sync, restart, or be temporarily unreachable/partitioned) — not a programmer invariant. Every other failure path in the same function (Err(e) from the same call, block-not-found, timestamp validation) is handled by logging and return false, letting the caller retry. Only this branch crashes the process instead.

Impact: any node with fetch_accessed_keys_from_centralized: true can be crashed during sync whenever the recorder briefly doesn't have accessed-keys data for the block being synced — this can happen from ordinary recorder lag/restarts, and also gives a compromised or misbehaving recorder a trivial, repeatable way to take down every consensus node relying on it. This violates this repo's own mandatory practice ("Never panic on data reachable from requests" in .claude/rules/code-style.md).

Fix

Treat Ok(None) the same way as the Err(e) branch immediately below it: log and return false, so the sync attempt is retried instead of crashing the node.

Test

Updated try_sync_panics_when_recorder_has_no_accessed_keys (previously asserted a panic via #[should_panic]) into try_sync_fails_gracefully_when_recorder_has_no_accessed_keys, which asserts try_sync returns false and add_sync_block is never called, instead of panicking.

Verification

• cargo build -p apollo_consensus_orchestrator --tests — passes
• SEED=0 cargo test -p apollo_consensus_orchestrator --lib try_sync — 3/3 pass (including the new regression test)
• scripts/rust_fmt.sh --check — clean
• cargo clippy -p apollo_consensus_orchestrator --tests --all-features -- -D warnings — clean

---

Generated by Claude Code

[cursor]

PR Summary

Medium Risk
Touches consensus sync with centralized recorder data; behavior change is intentional (retry vs crash) and aligns with documented None from HTTP recorder, with low risk of incorrect advancement since sync still aborts before add_sync_block.

Overview
When fetch_accessed_keys_from_centralized is on, try_sync no longer **panic!**s if the recorder returns Ok(None) for accessed-keys input. It now logs an error and returns false, matching the existing Err handling so sync can be retried and consensus state is not advanced without batcher input.

The regression test was renamed and updated to expect try_sync → false and that add_sync_block is not called, instead of asserting a process panic.

^{Reviewed by Cursor Bugbot for commit 7ed3be5. Bugbot is set up for automated code reviews on this repo. Configure here.}

[reviewable-StarkWare]

This change is 

[itamar-starkware]

@itamar-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on yoavGrs).