fix(follow): decouple FCU head from finality by pepyakin · Pull Request #7343 · tempoxyz/tempo

pepyakin · 2026-08-27T23:21:11Z

A verified finalization certificate can reach the executor before marshal stores it durably, so advancing all FCU fields can move the execution layer finality beyond the certificate archive and make restart fail.

Track forkchoice head separately from safe and finalized: certificates advance head to guide execution sync, while durable block delivery advances safe and finalized. At startup, require the certificate archive to cover the execution layer finalized block.

pepyakin · 2026-08-27T23:21:58Z

This was renamed target.rs → fcu.rs because it now is more than just the target

A verified finalization certificate can reach the executor before marshal stores it durably, so advancing all FCU fields can move the execution layer finality beyond the certificate archive and make restart fail. Track forkchoice head separately from safe and finalized: certificates advance head to guide execution sync, while durable block delivery advances safe and finalized. At startup, require the certificate archive to cover the execution layer finalized block.

pepyakin · 2026-08-27T23:31:26Z

cyclops audit fast

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 39b5b9b04a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-27T23:34:46Z

+            let forkchoice = (self.forkchoice.requires_update() || heartbeat)
+                .then_some(self.forkchoice.latest());


Require a valid FCU before acknowledging delivered blocks

When a certificate has already advanced latest.head beyond the block being popped, this constructs an FCU whose head is still unknown to Reth while its finalized target is the delivered block. Reth can return SYNCING without applying that forkchoice, but submit_forkchoice_update accepts every non-INVALID response, after which execute_request acknowledges the block and records the tuple as submitted. If the node crashes before the future head becomes known, marshal may restart past the acknowledged block while the execution finalized marker remains behind, so the block should only be acknowledged after a VALID FCU that establishes its finality, or retained for retry.

Useful? React with 👍 / 👎.

tempoxyz-bot

👁️ Cyclops Review

This change correctly separates certificate-driven forkchoice head guidance from durability-backed safe/finalized advancement, but three verified issues remain in block canonicalization, acknowledgement safety, and archive coverage.

Reviewer Callouts

⚡ Reth backfill behavior: Non-OP backfill targets finalized_block_hash. Since finalized now names a block Reth already has, confirm followers retain an effective staged-sync path during initial catch-up.
⚡ Pre-TIP-1031 history: Target::from_block has no round for older headers, so verify replaying roundless history can still advance safe/finalized rather than leaving them pinned.

tempoxyz-bot · 2026-08-28T00:26:43Z

+            self.forkchoice
+                .advance_finalized(Target::from_block(&block));
+            let forkchoice = (self.forkchoice.requires_update() || heartbeat)
+                .then_some(self.forkchoice.latest());


🚨 [SECURITY] Certificate-led forkchoice prevents delivered blocks from being canonicalized

The forkchoice update accompanying a marshal-delivered block uses the latest certificate as its head. During catch-up that head can be unknown to Reth, so newPayload does not canonicalize the delivered block and the FCU returns SYNCING without applying safe/finalized. While the gap persists, the canonical head and marshal floor remain stale and in-memory executed state can grow until OOM.

Recommended Fix:
First send an FCU with the delivered block as head, safe, and finalized, requiring VALID; then restore the newest certificate in a separate head-guidance FCU while retaining the durable finalized target.

tempoxyz-bot · 2026-08-28T00:26:43Z

+            if let Some(forkchoice) = forkchoice {
                if let Err(error) =
-                    submit_forkchoice_update(&context, &execution_engine, &tip).await
+                    submit_forkchoice_update(&context, &execution_engine, &forkchoice).await


🚨 [SECURITY] SYNCING responses make block acknowledgements non-restart-safe

submit_forkchoice_update accepts every non-INVALID response. If this finality-bearing FCU returns SYNCING, execution has not applied safe/finalized, but the code still acknowledges the block and marshal durably marks it processed. A crash can then restart with stale execution finality while marshal skips the update needed to advance it.

Recommended Fix:
Require VALID before acknowledging a finality-bearing block FCU. On SYNCING, do not acknowledge or call note_submitted; retain the block for retry.

tempoxyz-bot · 2026-08-28T00:26:43Z

+            execution_height,
+        );
+
+        if archive_height == execution_height {


🛡️ [DEFENSE-IN-DEPTH] Archive-tip validation does not prove coverage at execution finality

This compares digests only when the archive tip equals the execution-finalized height. The immutable archive can be sparse, so a higher tip does not prove that an entry matching the execution-finalized block exists at that height.

Recommended Fix:
Read the archive entry at execution_height and verify its payload digest equals execution_digest, with an explicit floor-aware fallback only when that height is below the retained archive floor.

pepyakin · 2026-08-28T02:42:17Z

cyclops audit fast

tempoxyz-bot

👁️ Cyclops Review

This change separates certificate-driven head guidance from durability-backed safe/finalized advancement and adds startup archive validation. The design improves restart safety, but two verified integration issues remain: a VALID response does not always mean Reth applied finality, and a persistently SYNCING execution layer can silently wedge block delivery.

Reviewer Callouts

⚡ Archive floor reachability: validate_archive_tip does not reject a restored archive whose floor is above the execution tip plus one; marshal can then dispatch a block whose parent Reth lacks.
⚡ Existing-state migration: Nodes that used the old certificate-driven finality path may already have execution finality ahead of the durable archive and will now fail startup. Consider an explicit release note or migration check.
⚡ Head guidance retries: Head-only FCUs reported as SYNCING are still recorded as submitted, suppressing retries until another update or the heartbeat.
⚡ Engine/P2P churn: Catch-up can issue three FCUs per delivered block, and each SYNCING FCU can trigger a single-block download; measure the added load and RPC-visible head movement.

tempoxyz-bot · 2026-08-28T03:55:43Z

+    plan: FinalityPlan,
+) -> eyre::Result<ForkchoiceTargets> {
+    match submit_forkchoice_update(context, execution_engine, &plan.preferred).await? {
+        ForkchoiceOutcome::Valid => Ok(plan.preferred),


🚨 [SECURITY] Canonical-ancestor FCUs are acknowledged without applying finality

apply_finality treats every VALID response as proof that Reth applied the requested safe/finalized hashes. The pinned Reth engine also returns VALID when the requested head is a canonical ancestor of its current head, but that branch does not apply safe/finalized. The executor then acknowledges the marshal block even though execution finality remains stale, so marshal will not redeliver the update after restart.

Recommended Fix:
After a VALID finality-bearing FCU, read back Reth's effective finalized header and require it to equal the delivered block before acknowledging. Also update or patch Reth's canonical-ancestor path to apply the forkchoice state's safe/finalized fields, or submit the FCU with Reth's actual canonical head when the delivered block is its ancestor.

tempoxyz-bot · 2026-08-28T03:55:43Z

+    execution_engine: &E,
+    forkchoice: &ForkchoiceTargets,
+) -> eyre::Result<()> {
+    loop {


⚠️ [ISSUE] Unbounded SYNCING retry can wedge the follower

submit_until_valid retries forever at one-second intervals when the execution layer returns SYNCING, with no deadline or escalation. The in-flight block is not acknowledged until this loop returns; because follow mode permits only one pending marshal acknowledgement, persistent backfill or a missing ancestor stops all further block delivery while the process remains apparently healthy.

Recommended Fix:
Bound retries by attempt count or deadline and return an error through the existing execution-task failure path so supervision and alerting can react. If indefinite retry is required, move retry scheduling out of the blocking execution task and expose warning-level logs and a stall metric.

hamdiallam · 2026-08-28T14:46:23Z


-            ack.acknowledge();
-            ExecutionTaskResult::Completed(last_fcu)
+async fn submit_until_valid<TContext: Pacer, E: ExecutionEngine + ?Sized>(


Why do we have to loop instead of delivering once?

afaik this is only called on block updates which cant be synced (delivered in order). Even if was syncing it's fine? We can move onto the next block

#7343 (comment) this prompted the restructure.

So the problem is, when we are applying the finality FCU we cannot really accept the SYNCING as a successful response and thus cannot acknowledge the block.

For example, if we are:

Marshal processed height: 100

EL: 100

Then,

we submit newPayload(101)

fcu(.., finalized=101) → SYNCING (doesn't mean it updated the markers)

If we acknowledge here AND there is a crash, then we could end up in the state where

Marshal processed height: 101

EL: 100

AFAIU, this is unsafe, because the marshal won't redeliver the 101.

So we basically keep re-submitting here until we get VALID.

IOW, if it does return SYNCING then it may be unsound to acknowledge.
However, if we are ok to assert that it's not going to return SYNCING then we can avoid the loop!

hamdiallam · 2026-08-28T16:34:08Z

        }
    }

+    fn validate_archive_tip(


Is this needed? Now that the fix is solely scoped in the executor?

No, not strictly needed within the scope of this change

pepyakin requested review from SuperFluffy, hamdiallam and joshieDo as code owners August 27, 2026 23:21

pepyakin commented Aug 27, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 27, 2026

View reviewed changes

tempoxyz-bot reviewed Aug 28, 2026

View reviewed changes

pepyakin added 3 commits August 27, 2026 20:44

acknowledge only after VALID, keep trying on SYNCING

dc4bcab

wip

29ab04d

wip 2

2f6368c

pepyakin force-pushed the pep/decouple-fcu-head-2 branch from 39b5b9b to 2f6368c Compare August 28, 2026 02:42

tempoxyz-bot reviewed Aug 28, 2026

View reviewed changes

simplify

088ca6d

hamdiallam reviewed Aug 28, 2026

View reviewed changes

		let forkchoice = (self.forkchoice.requires_update() \|\| heartbeat)
		.then_some(self.forkchoice.latest());

Uh oh!

Conversation

pepyakin commented Aug 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pepyakin commented Aug 27, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

tempoxyz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pepyakin commented Aug 28, 2026

Uh oh!

tempoxyz-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants