From f6b8443a8f8fe9b1dae6fb5e4c9f23f0bc8dac98 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Thu, 27 Aug 2026 19:10:00 -0400 Subject: [PATCH 01/14] fix(follow): decouple FCU head from finality 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. --- crates/consensus/src/alias.rs | 34 ++- crates/consensus/src/follow/executor/actor.rs | 124 ++++---- crates/consensus/src/follow/executor/fcu.rs | 274 ++++++++++++++++++ crates/consensus/src/follow/executor/mod.rs | 2 +- .../consensus/src/follow/executor/target.rs | 132 --------- .../consensus/src/follow/executor/test/mod.rs | 31 +- 6 files changed, 388 insertions(+), 209 deletions(-) create mode 100644 crates/consensus/src/follow/executor/fcu.rs delete mode 100644 crates/consensus/src/follow/executor/target.rs diff --git a/crates/consensus/src/alias.rs b/crates/consensus/src/alias.rs index 684bb04d17..22cffd8fb3 100644 --- a/crates/consensus/src/alias.rs +++ b/crates/consensus/src/alias.rs @@ -249,7 +249,10 @@ pub(crate) mod marshal { let execution_finalized = execution_finalized_point(execution_node); match archive_range { - Some((floor, tip)) => Ok(FinalizationRange { floor, tip }), + Some((floor, tip)) => { + validate_archive_tip(tip, execution_finalized)?; + Ok(FinalizationRange { floor, tip }) + } None if execution_finalized.0.is_zero() => Ok(FinalizationRange { floor: execution_finalized, // Genesis is not finalized in any round; the zero round @@ -270,6 +273,35 @@ pub(crate) mod marshal { } } + fn validate_archive_tip( + archive_tip: (Round, Height, Digest), + execution_finalized: (Height, Digest), + ) -> eyre::Result<()> { + let (_, archive_height, archive_digest) = archive_tip; + let (execution_height, execution_digest) = execution_finalized; + + ensure!( + archive_height >= execution_height, + "finalized certificate archive tip height `{}` is below execution finalized height \ + `{}`; restore consensus storage that covers execution state or reset execution state", + archive_height, + execution_height, + ); + + if archive_height == execution_height { + ensure!( + archive_digest == execution_digest, + "finalized certificate archive tip digest `{}` does not match execution finalized \ + digest `{}` at height `{}`; restore matching consensus and execution state", + archive_digest, + execution_digest, + archive_height, + ); + } + + Ok(()) + } + async fn finalized_archive_range( archive: &immutable::Archive< TContext, diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index fc999d21ee..1de1e4faf2 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -1,8 +1,8 @@ //! Execution-layer driver for follower nodes. //! -//! This actor sends verified finalized tips to Reth as head, safe, and finalized forkchoice -//! updates, periodically refreshes that forkchoice with a heartbeat, and advances marshal's floor -//! to one epoch behind Reth's finalized state. +//! This actor sends verified certificate targets to Reth as forkchoice heads. It advances the safe +//! and finalized targets as marshal delivers persisted finalized blocks. It also refreshes +//! forkchoice with a heartbeat and advances marshal's floor behind Reth's finalized state. //! //! Unlike the executor used by validator nodes, it does not build payloads, canonicalize proposal //! heads, or track blocks proposed by this node. Followers receive complete blocks from their @@ -11,7 +11,6 @@ use std::{collections::VecDeque, time::Duration}; -use alloy_rpc_types_engine::ForkchoiceState; use commonware_consensus::{ Heightable as _, marshal::Update, @@ -25,7 +24,9 @@ use tempo_node::TempoExecutionData; use tracing::{Level, debug, error, instrument}; use super::{ - Config, ExecutionEngine, FinalizedBlockProvider, Marshal, ingress::Message, target::Target, + Config, ExecutionEngine, FinalizedBlockProvider, Marshal, + fcu::{ForkchoiceTargets, ForkchoiceTracker, Target}, + ingress::Message, }; use crate::{consensus::block::Block, utils::OptionFuture}; @@ -40,8 +41,7 @@ pub(crate) struct Actor { epoch_strategy: FixedEpocher, floor: Height, - last_fcu: Target, - latest_tip: Target, + forkchoice: ForkchoiceTracker, block_queue: VecDeque<(Block, Exact)>, floor_candidate: Option, @@ -76,7 +76,7 @@ where let finalized_header = execution_provider .finalized_header() .expect("failed reading finalized execution header"); - let tip = Target::from_header(&finalized_header); + let forkchoice = ForkchoiceTracker::new(Target::from_header(&finalized_header)); Self { context: ContextCell::new(context), @@ -88,8 +88,7 @@ where execution_provider, execution_engine, - last_fcu: tip, - latest_tip: tip, + forkchoice, block_queue: VecDeque::new(), floor_candidate: None, execution_task: OptionFuture::none(), @@ -117,10 +116,9 @@ where result = &mut self.execution_task => { self.execution_task = OptionFuture::none(); match result { - ExecutionTaskResult::Completed(last_fcu) => { - self.last_fcu = last_fcu; - if last_fcu.supersedes(&self.latest_tip) { - self.latest_tip = last_fcu; + ExecutionTaskResult::Completed(submitted_fcu) => { + if let Some(submitted_fcu) = submitted_fcu { + self.forkchoice.note_submitted(submitted_fcu); } // Emits an event on error. @@ -136,25 +134,25 @@ where Some(message) = self.mailbox.next() => { match message { Message::Update(Update::Block(block, ack)) => { + // Marshal delivers persisted finalized blocks in height order. The + // executor submits each payload before using it as Reth's finalized + // forkchoice target. self.block_queue.push_back(((*block).clone(), ack)); } - // A Tip update has a persisted finalization, so it can - // start a new floor cycle. Message::Update(Update::Tip(round, height, digest)) => { + // Marshal reports known finalized tips before it completes gap-free + // block delivery. The certificate can guide the head while finalized + // follows delivered blocks. if self.floor_candidate.is_none() { self.floor_candidate = Some(height); } - let candidate = Target::from_finalization(round, digest); - if candidate.supersedes(&self.latest_tip) { - self.latest_tip = candidate; - } + self.forkchoice + .advance_head(Target::from_finalization(round, digest)); } Message::Finalization { round, digest } => { let candidate = Target::from_finalization(round, digest); - if candidate.supersedes(&self.latest_tip) { - self.latest_tip = candidate; - } + self.forkchoice.advance_head(candidate); } } } @@ -166,10 +164,6 @@ where } } - fn should_send_forkchoice(&self) -> bool { - self.latest_tip.digest != self.last_fcu.digest && self.latest_tip.supersedes(&self.last_fcu) - } - fn update_fcu_heartbeat_timer(&mut self) { if self.execution_task.is_none() && self.block_queue.is_empty() { if self.fcu_heartbeat_timer.is_none() { @@ -187,18 +181,25 @@ where } let request = if let Some((block, ack)) = self.block_queue.pop_front() { - ExecutionRequest::Block(block, ack) - } else if self.should_send_forkchoice() || heartbeat { - ExecutionRequest::Forkchoice(self.latest_tip) + self.forkchoice + .advance_finalized(Target::from_block(&block)); + let forkchoice = (self.forkchoice.requires_update() || heartbeat) + .then_some(self.forkchoice.latest()); + ExecutionRequest::Block { + block, + forkchoice, + ack, + } + } else if self.forkchoice.requires_update() || heartbeat { + ExecutionRequest::Forkchoice(self.forkchoice.latest()) } else { return; }; - let last_fcu = self.last_fcu; let context = self.context.child("execute_request"); let execution_engine = self.execution_engine.clone(); self.execution_task - .replace(execute_request(context, execution_engine, last_fcu, request).boxed()); + .replace(execute_request(context, execution_engine, request).boxed()); } #[instrument(skip_all, err(level = Level::WARN))] @@ -251,48 +252,50 @@ where } enum ExecutionRequest { - Forkchoice(Target), - Block(Block, Exact), + Forkchoice(ForkchoiceTargets), + Block { + block: Block, + forkchoice: Option, + ack: Exact, + }, } enum ExecutionTaskResult { - Completed(Target), + Completed(Option), Fatal(Report), } async fn execute_request( context: TContext, execution_engine: E, - last_fcu: Target, request: ExecutionRequest, ) -> ExecutionTaskResult { match request { - ExecutionRequest::Forkchoice(tip) => { - match submit_forkchoice_update(&context, &execution_engine, &tip).await { - Ok(()) => ExecutionTaskResult::Completed(tip), + ExecutionRequest::Forkchoice(forkchoice) => { + match submit_forkchoice_update(&context, &execution_engine, &forkchoice).await { + Ok(()) => ExecutionTaskResult::Completed(Some(forkchoice)), Err(error) => ExecutionTaskResult::Fatal(error), } } - ExecutionRequest::Block(block, ack) => { - let tip = Target::from_block(&block); - + ExecutionRequest::Block { + block, + forkchoice, + ack, + } => { if let Err(error) = submit_new_payload(&context, &execution_engine, block).await { return ExecutionTaskResult::Fatal(error); } - let last_fcu = if tip.supersedes(&last_fcu) { - if let Err(error) = - submit_forkchoice_update(&context, &execution_engine, &tip).await - { + if let Some(forkchoice) = forkchoice { + let result = + submit_forkchoice_update(&context, &execution_engine, &forkchoice).await; + if let Err(error) = result { return ExecutionTaskResult::Fatal(error); } - tip - } else { - last_fcu - }; + } ack.acknowledge(); - ExecutionTaskResult::Completed(last_fcu) + ExecutionTaskResult::Completed(forkchoice) } } } @@ -328,18 +331,21 @@ async fn submit_new_payload( Ok(()) } -#[instrument(skip_all, fields(round = ?tip.round, digest = %tip.digest))] +#[instrument( + skip_all, + fields( + head.round = ?forkchoice.head.round, + head.digest = %forkchoice.head.digest, + finalized.round = ?forkchoice.finalized.round, + finalized.digest = %forkchoice.finalized.digest, + ) +)] async fn submit_forkchoice_update( context: &TContext, execution_engine: &E, - tip: &Target, + forkchoice: &ForkchoiceTargets, ) -> eyre::Result<()> { - let hash = tip.digest.0; - let forkchoice = ForkchoiceState { - head_block_hash: hash, - safe_block_hash: hash, - finalized_block_hash: hash, - }; + let forkchoice = forkchoice.rpc_state(); let response = execution_engine .fork_choice_updated(forkchoice, None) diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs new file mode 100644 index 0000000000..f7463fe7e9 --- /dev/null +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -0,0 +1,274 @@ +use alloy_rpc_types_engine::ForkchoiceState; +use commonware_consensus::types::Round; +use reth_primitives_traits::SealedHeader; +use tempo_primitives::TempoHeader; + +use crate::consensus::{ + Digest, + block::{Block, round_from_context}, +}; + +/// A forkchoice target ordered by its consensus finalization round. +/// +/// Execution headers before TIP-1031 do not contain a consensus round. After +/// activation, every newly observed finalization has one. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) struct Target { + pub(super) round: Option, + pub(super) digest: Digest, +} + +impl Target { + pub(super) fn from_header(header: &SealedHeader) -> Self { + let tip = header.num_hash(); + Self { + round: header.consensus_context.map(round_from_context), + digest: Digest(tip.hash), + } + } + + pub(super) fn from_block(block: &Block) -> Self { + Self::from_header(block.block().sealed_header()) + } + + pub(super) const fn from_finalization(round: Round, digest: Digest) -> Self { + Self { + round: Some(round), + digest, + } + } + + /// Newly observed finalizations are post-TIP-1031 and always have a round. + /// They therefore supersede a roundless genesis or pre-activation target. + fn supersedes(&self, current: &Self) -> bool { + match (self.round, current.round) { + (Some(new), Some(old)) => new > old, + // TIP-1031 is active. In supported startup states, a roundless + // execution target is genesis; restoring partially synchronized + // pre-TIP execution state is unsupported. Any newly verified + // finalization therefore supersedes it. + (Some(_), None) => true, + _ => false, + } + } +} + +/// Forkchoice targets tracked by the follower executor. +/// +/// This represents the `(head, safe, finalized)` tuple accepted by Engine API +/// `forkchoiceUpdated`, with `safe` equal to `finalized`. +/// +/// `head` follows the latest verified finalization certificate and can refer to +/// a block that marshal has not delivered yet. Setting it tells Reth to sync +/// toward that block. `finalized` follows marshal's ordered block delivery after +/// the executor submits the block as a payload. +/// +/// The head target is always at least as new as the finalized target. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) struct ForkchoiceTargets { + pub(super) head: Target, + pub(super) finalized: Target, +} + +impl ForkchoiceTargets { + const fn new(target: Target) -> Self { + Self { + head: target, + finalized: target, + } + } + + fn advance_head(&mut self, candidate: Target) { + advance(&mut self.head, candidate); + } + + fn advance_finalized(&mut self, candidate: Target) { + advance(&mut self.finalized, candidate); + advance(&mut self.head, candidate); + } + + pub(super) fn rpc_state(self) -> ForkchoiceState { + ForkchoiceState { + head_block_hash: self.head.digest.0, + safe_block_hash: self.finalized.digest.0, + finalized_block_hash: self.finalized.digest.0, + } + } +} + +fn advance(current: &mut Target, candidate: Target) { + if candidate.supersedes(current) { + *current = candidate; + } +} + +/// Forkchoice progress across submitted and latest targets. +pub(super) struct ForkchoiceTracker { + submitted: ForkchoiceTargets, + latest: ForkchoiceTargets, +} + +impl ForkchoiceTracker { + /// Creates a new tracker with the given target as both submitted and latest. + pub(super) const fn new(target: Target) -> Self { + let targets = ForkchoiceTargets::new(target); + Self { + submitted: targets, + latest: targets, + } + } + + /// Moves the latest head to a later verified finalization. + /// + /// Finalizations can arrive out of order, so stale and equal-round targets are + /// ignored. + pub(super) fn advance_head(&mut self, candidate: Target) { + self.latest.advance_head(candidate); + } + + /// Moves the latest finalized target to a later block delivered by marshal. + /// + /// This also advances the head when needed, so finalized never moves ahead of + /// head. + pub(super) fn advance_finalized(&mut self, candidate: Target) { + self.latest.advance_finalized(candidate); + } + + /// Returns `true` if the latest targets have changed since the last known submitted state. + pub(super) fn requires_update(&self) -> bool { + self.latest != self.submitted + } + + /// Returns the targets selected by [`Self::advance_head`] and + /// [`Self::advance_finalized`] for the next forkchoice update. + pub(super) const fn latest(&self) -> ForkchoiceTargets { + self.latest + } + + /// Saves the given targets as the last known submitted state. + pub(super) fn note_submitted(&mut self, submitted: ForkchoiceTargets) { + self.submitted = submitted; + } +} + +#[cfg(test)] +mod tests { + use alloy_consensus::Header; + use commonware_consensus::types::{Epoch, View}; + use reth_primitives_traits::SealedHeader; + use tempo_primitives::{TempoConsensusContext, TempoHeader, ed25519::PublicKey}; + + use super::*; + + fn round(view: u64) -> Round { + Round::new(Epoch::zero(), View::new(view)) + } + + fn digest(byte: u8) -> Digest { + Digest(alloy_primitives::B256::with_last_byte(byte)) + } + + fn execution_header( + height: u64, + round: Option, + digest: Digest, + ) -> SealedHeader { + let consensus_context = round.map(|round| TempoConsensusContext { + epoch: round.epoch().get(), + view: round.view().get(), + parent_view: 0, + proposer: PublicKey::from_seed(0), + }); + SealedHeader::new( + TempoHeader { + inner: Header { + number: height, + ..Default::default() + }, + consensus_context, + ..Default::default() + }, + digest.0, + ) + } + + #[test] + fn later_round_supersedes_earlier_round() { + let newer = Target::from_finalization(round(9), digest(1)); + let older = Target::from_finalization(round(8), digest(2)); + assert!(newer.supersedes(&older)); + assert!(!older.supersedes(&newer)); + } + + #[test] + fn equal_round_does_not_supersede() { + let current = Target::from_finalization(round(8), digest(1)); + let conflicting = Target::from_finalization(round(8), digest(2)); + assert!(!conflicting.supersedes(¤t)); + } + + #[test] + fn finalized_target_supersedes_roundless_execution_target() { + let header = execution_header(100, None, digest(1)); + let prefork = Target::from_header(&header); + let finalized = Target::from_finalization(round(1), digest(2)); + assert!(finalized.supersedes(&prefork)); + } + + #[test] + fn roundless_target_never_supersedes() { + let roundless = Target::from_header(&execution_header(100, None, digest(1))); + let finalized = Target::from_finalization(round(1), digest(2)); + assert!(!roundless.supersedes(&finalized)); + assert!(!roundless.supersedes(&roundless)); + } + + #[test] + fn execution_target_uses_header_round() { + let header = execution_header(100, Some(round(2)), digest(1)); + assert_eq!(Target::from_header(&header).round, Some(round(2))); + } + + #[test] + fn certified_target_advances_only_head() { + let current = Target::from_finalization(round(1), digest(1)); + let mut tracker = ForkchoiceTracker::new(current); + let candidate = Target::from_finalization(round(2), digest(2)); + + tracker.advance_head(candidate); + + let targets = tracker.latest(); + assert_eq!(targets.head, candidate); + assert_eq!(targets.finalized, current); + assert!(tracker.requires_update()); + } + + #[test] + fn durable_target_advances_both_lanes() { + let current = Target::from_finalization(round(1), digest(1)); + let head = Target::from_finalization(round(3), digest(3)); + let finalized = Target::from_finalization(round(2), digest(2)); + let mut tracker = ForkchoiceTracker::new(current); + tracker.advance_head(head); + + tracker.advance_finalized(finalized); + + let targets = tracker.latest(); + assert_eq!(targets.head, head); + assert_eq!(targets.finalized, finalized); + } + + #[test] + fn finalized_lane_requires_update_when_head_is_unchanged() { + let current = Target::from_finalization(round(1), digest(1)); + let head = Target::from_finalization(round(2), digest(2)); + let mut tracker = ForkchoiceTracker::new(current); + tracker.advance_head(head); + tracker.note_submitted(tracker.latest()); + assert!(!tracker.requires_update()); + + tracker.advance_finalized(head); + + assert!(tracker.requires_update()); + } +} diff --git a/crates/consensus/src/follow/executor/mod.rs b/crates/consensus/src/follow/executor/mod.rs index 0f2f25ca9a..b1203abcfe 100644 --- a/crates/consensus/src/follow/executor/mod.rs +++ b/crates/consensus/src/follow/executor/mod.rs @@ -33,8 +33,8 @@ use tempo_primitives::TempoHeader; use crate::consensus::Digest; mod actor; +mod fcu; mod ingress; -mod target; #[cfg(test)] mod test; diff --git a/crates/consensus/src/follow/executor/target.rs b/crates/consensus/src/follow/executor/target.rs deleted file mode 100644 index db11635fad..0000000000 --- a/crates/consensus/src/follow/executor/target.rs +++ /dev/null @@ -1,132 +0,0 @@ -use commonware_consensus::types::Round; -use reth_primitives_traits::SealedHeader; -use tempo_primitives::TempoHeader; - -use crate::consensus::{ - Digest, - block::{Block, round_from_context}, -}; - -/// A possible finalized forkchoice target. -/// -/// Execution headers before TIP-1031 do not contain a consensus round. After -/// activation, every newly observed finalization has one. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) struct Target { - pub(super) round: Option, - pub(super) digest: Digest, -} - -impl Target { - pub(super) fn from_header(header: &SealedHeader) -> Self { - let tip = header.num_hash(); - Self { - round: header.consensus_context.map(round_from_context), - digest: Digest(tip.hash), - } - } - - pub(super) fn from_block(block: &Block) -> Self { - Self::from_header(block.block().sealed_header()) - } - - pub(super) const fn from_finalization(round: Round, digest: Digest) -> Self { - Self { - round: Some(round), - digest, - } - } - - /// Newly observed finalizations are post-TIP-1031 and always have a round. - /// They therefore supersede a roundless genesis or pre-activation target. - pub(super) fn supersedes(&self, current: &Self) -> bool { - match (self.round, current.round) { - (Some(new), Some(old)) => new > old, - // TIP-1031 is active. In supported startup states, a roundless - // execution target is genesis; restoring partially synchronized - // pre-TIP execution state is unsupported. Any newly verified - // finalization therefore supersedes it. - (Some(_), None) => true, - _ => false, - } - } -} - -#[cfg(test)] -mod tests { - use alloy_consensus::Header; - use commonware_consensus::types::{Epoch, View}; - use reth_primitives_traits::SealedHeader; - use tempo_primitives::{TempoConsensusContext, TempoHeader, ed25519::PublicKey}; - - use super::*; - - fn round(view: u64) -> Round { - Round::new(Epoch::zero(), View::new(view)) - } - - fn digest(byte: u8) -> Digest { - Digest(alloy_primitives::B256::with_last_byte(byte)) - } - - fn execution_header( - height: u64, - round: Option, - digest: Digest, - ) -> SealedHeader { - let consensus_context = round.map(|round| TempoConsensusContext { - epoch: round.epoch().get(), - view: round.view().get(), - parent_view: 0, - proposer: PublicKey::from_seed(0), - }); - SealedHeader::new( - TempoHeader { - inner: Header { - number: height, - ..Default::default() - }, - consensus_context, - ..Default::default() - }, - digest.0, - ) - } - - #[test] - fn later_round_supersedes_earlier_round() { - let newer = Target::from_finalization(round(9), digest(1)); - let older = Target::from_finalization(round(8), digest(2)); - assert!(newer.supersedes(&older)); - assert!(!older.supersedes(&newer)); - } - - #[test] - fn equal_round_does_not_supersede() { - let current = Target::from_finalization(round(8), digest(1)); - let conflicting = Target::from_finalization(round(8), digest(2)); - assert!(!conflicting.supersedes(¤t)); - } - - #[test] - fn finalized_target_supersedes_roundless_execution_target() { - let header = execution_header(100, None, digest(1)); - let prefork = Target::from_header(&header); - let finalized = Target::from_finalization(round(1), digest(2)); - assert!(finalized.supersedes(&prefork)); - } - - #[test] - fn roundless_target_never_supersedes() { - let roundless = Target::from_header(&execution_header(100, None, digest(1))); - let finalized = Target::from_finalization(round(1), digest(2)); - assert!(!roundless.supersedes(&finalized)); - assert!(!roundless.supersedes(&roundless)); - } - - #[test] - fn execution_target_uses_header_round() { - let header = execution_header(100, Some(round(2)), digest(1)); - assert_eq!(Target::from_header(&header).round, Some(round(2))); - } -} diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index a1aa2f41d7..9b4941330f 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -374,14 +374,13 @@ fn tips_are_monotonic_and_coalesced_while_forkchoice_is_in_flight() { let forkchoices = provider.forkchoices(); assert_eq!(forkchoices[0].head_block_hash, first_digest.0); assert_eq!(forkchoices[1].head_block_hash, highest_digest.0); - assert_eq!(forkchoices[1].safe_block_hash, highest_digest.0); - assert_eq!(forkchoices[1].finalized_block_hash, highest_digest.0); + assert_eq!(forkchoices[1].safe_block_hash, B256::ZERO); + assert_eq!(forkchoices[1].finalized_block_hash, B256::ZERO); }); } -// A finalization can advance forkchoice before execution receives its block. #[test_traced] -fn finalization_drives_forkchoice_by_round() { +fn finalization_waits_for_durable_block_before_advancing_finalized() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -398,23 +397,23 @@ fn finalization_drives_forkchoice_by_round() { ); actor.start(); - let first = Digest(B256::with_last_byte(1)); - let _ = mailbox.report(Update::Tip(round(1), Height::new(1), first)); + let block = make_block_at_round(1, B256::ZERO, round(2)); + let finalized = Digest(block.block_hash()); + mailbox.finalization(round(2), finalized); wait_until(&context, || provider.forkchoices().len() == 1).await; - let finalized = Digest(B256::with_last_byte(9)); - mailbox.finalization(round(2), finalized); - wait_until(&context, || provider.forkchoices().len() == 2).await; + let forkchoices = provider.forkchoices(); + assert_eq!(forkchoices.len(), 1); + assert_eq!(forkchoices[0].head_block_hash, finalized.0); + assert_eq!(forkchoices[0].safe_block_hash, B256::ZERO); + assert_eq!(forkchoices[0].finalized_block_hash, B256::ZERO); - let _ = mailbox.report(Update::Tip( - round(1), - Height::new(2), - Digest(B256::with_last_byte(2)), - )); - context.sleep(Duration::from_millis(5)).await; + let (ack, waiter) = Exact::handle(); + assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); + waiter.await.expect("durable block should be acknowledged"); + wait_until(&context, || provider.forkchoices().len() == 2).await; let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices.len(), 2); assert_eq!(forkchoices[1].head_block_hash, finalized.0); assert_eq!(forkchoices[1].safe_block_hash, finalized.0); assert_eq!(forkchoices[1].finalized_block_hash, finalized.0); From dc4bcabfd9f8f10ede25c1c190618fad8795473d Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Thu, 27 Aug 2026 20:44:01 -0400 Subject: [PATCH 02/14] acknowledge only after VALID, keep trying on SYNCING --- crates/consensus/src/follow/executor/actor.rs | 55 +++++++-- crates/consensus/src/follow/executor/fcu.rs | 20 ++-- .../consensus/src/follow/executor/test/mod.rs | 113 ++++++++++++++++++ .../src/follow/executor/test/utils.rs | 8 ++ 4 files changed, 175 insertions(+), 21 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index 1de1e4faf2..f493e1035a 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -30,6 +30,8 @@ use super::{ }; use crate::{consensus::block::Block, utils::OptionFuture}; +const FINALIZED_FCU_RETRY_INTERVAL: Duration = Duration::from_secs(1); + pub(crate) struct Actor { context: ContextCell, mailbox: mpsc::UnboundedReceiver, @@ -181,13 +183,15 @@ where } let request = if let Some((block, ack)) = self.block_queue.pop_front() { - self.forkchoice + let finalized_advanced = self + .forkchoice .advance_finalized(Target::from_block(&block)); let forkchoice = (self.forkchoice.requires_update() || heartbeat) .then_some(self.forkchoice.latest()); ExecutionRequest::Block { block, forkchoice, + finalized_advanced, ack, } } else if self.forkchoice.requires_update() || heartbeat { @@ -257,6 +261,9 @@ enum ExecutionRequest { block: Block, forkchoice: Option, ack: Exact, + /// Whether the block advances finality and must wait for a `VALID` FCU before + /// acknowledgement. + finalized_advanced: bool, }, } @@ -265,6 +272,11 @@ enum ExecutionTaskResult { Fatal(Report), } +enum ForkchoiceOutcome { + Valid, + Syncing, +} + async fn execute_request( context: TContext, execution_engine: E, @@ -272,14 +284,18 @@ async fn execute_request( ) -> ExecutionTaskResult { match request { ExecutionRequest::Forkchoice(forkchoice) => { - match submit_forkchoice_update(&context, &execution_engine, &forkchoice).await { - Ok(()) => ExecutionTaskResult::Completed(Some(forkchoice)), + let result = submit_forkchoice_update(&context, &execution_engine, &forkchoice).await; + match result { + Ok(ForkchoiceOutcome::Valid | ForkchoiceOutcome::Syncing) => { + ExecutionTaskResult::Completed(Some(forkchoice)) + } Err(error) => ExecutionTaskResult::Fatal(error), } } ExecutionRequest::Block { block, forkchoice, + finalized_advanced, ack, } => { if let Err(error) = submit_new_payload(&context, &execution_engine, block).await { @@ -287,10 +303,20 @@ async fn execute_request( } if let Some(forkchoice) = forkchoice { - let result = - submit_forkchoice_update(&context, &execution_engine, &forkchoice).await; - if let Err(error) = result { - return ExecutionTaskResult::Fatal(error); + loop { + let result = + submit_forkchoice_update(&context, &execution_engine, &forkchoice).await; + match result { + Ok(ForkchoiceOutcome::Valid) => break, + Ok(ForkchoiceOutcome::Syncing) if finalized_advanced => { + debug!( + "execution layer is syncing before applying finalized FCU; retrying" + ); + context.sleep(FINALIZED_FCU_RETRY_INTERVAL).await; + } + Ok(ForkchoiceOutcome::Syncing) => break, + Err(error) => return ExecutionTaskResult::Fatal(error), + } } } @@ -344,7 +370,7 @@ async fn submit_forkchoice_update( context: &TContext, execution_engine: &E, forkchoice: &ForkchoiceTargets, -) -> eyre::Result<()> { +) -> eyre::Result { let forkchoice = forkchoice.rpc_state(); let response = execution_engine @@ -355,10 +381,13 @@ async fn submit_forkchoice_update( debug!(payload_status = %response.payload_status, "execution layer reported FCU status"); - ensure!( - !response.is_invalid(), - Report::msg(response.payload_status).wrap_err("execution layer rejected fcu") - ); + if response.payload_status.is_valid() { + return Ok(ForkchoiceOutcome::Valid); + } - Ok(()) + if response.payload_status.is_syncing() { + return Ok(ForkchoiceOutcome::Syncing); + } + + Err(Report::msg(response.payload_status).wrap_err("execution layer rejected fcu")) } diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs index f7463fe7e9..dcc3c5fec5 100644 --- a/crates/consensus/src/follow/executor/fcu.rs +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -82,9 +82,10 @@ impl ForkchoiceTargets { advance(&mut self.head, candidate); } - fn advance_finalized(&mut self, candidate: Target) { - advance(&mut self.finalized, candidate); + fn advance_finalized(&mut self, candidate: Target) -> bool { + let advanced = advance(&mut self.finalized, candidate); advance(&mut self.head, candidate); + advanced } pub(super) fn rpc_state(self) -> ForkchoiceState { @@ -96,9 +97,12 @@ impl ForkchoiceTargets { } } -fn advance(current: &mut Target, candidate: Target) { +fn advance(current: &mut Target, candidate: Target) -> bool { if candidate.supersedes(current) { *current = candidate; + true + } else { + false } } @@ -129,9 +133,9 @@ impl ForkchoiceTracker { /// Moves the latest finalized target to a later block delivered by marshal. /// /// This also advances the head when needed, so finalized never moves ahead of - /// head. - pub(super) fn advance_finalized(&mut self, candidate: Target) { - self.latest.advance_finalized(candidate); + /// head. Returns whether the finalized target advanced. + pub(super) fn advance_finalized(&mut self, candidate: Target) -> bool { + self.latest.advance_finalized(candidate) } /// Returns `true` if the latest targets have changed since the last known submitted state. @@ -251,7 +255,7 @@ mod tests { let mut tracker = ForkchoiceTracker::new(current); tracker.advance_head(head); - tracker.advance_finalized(finalized); + assert!(tracker.advance_finalized(finalized)); let targets = tracker.latest(); assert_eq!(targets.head, head); @@ -267,7 +271,7 @@ mod tests { tracker.note_submitted(tracker.latest()); assert!(!tracker.requires_update()); - tracker.advance_finalized(head); + assert!(tracker.advance_finalized(head)); assert!(tracker.requires_update()); } diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index 9b4941330f..a9cf0f225b 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -420,6 +420,119 @@ fn finalization_waits_for_durable_block_before_advancing_finalized() { }); } +#[test_traced] +fn syncing_block_forkchoice_retries_before_acknowledging() { + deterministic::Runner::default().start(|context| async move { + let provider = StubExecutionProvider::default(); + let release_head_forkchoice = provider.pause_next_forkchoice(); + + let (actor, mut mailbox) = init( + context.child("follower_executor"), + Config { + execution_provider: provider.clone(), + execution_engine: provider.clone(), + marshal: StubMarshal::default(), + epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), + fcu_heartbeat_interval: Duration::from_secs(60), + }, + ); + actor.start(); + + let future_head = digest(9); + mailbox.finalization(round(2), future_head); + wait_until(&context, || provider.forkchoices().len() == 1).await; + + provider.set_forkchoices_syncing(true); + let block = make_block_at_round(1, B256::ZERO, round(1)); + let finalized = block.block_hash(); + let (ack, waiter) = Exact::handle(); + assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); + + release_head_forkchoice + .send(()) + .expect("the head FCU should still be waiting"); + wait_until(&context, || provider.forkchoices().len() == 2).await; + + let mut waiter = Box::pin(waiter); + tokio::select! { + result = &mut waiter => panic!("syncing FCU acknowledged the block: {result:?}"), + _ = context.sleep(Duration::from_millis(100)) => {} + } + + provider.set_forkchoices_syncing(false); + waiter + .await + .expect("valid retry should acknowledge the durable block"); + + let forkchoices = provider.forkchoices(); + assert_eq!(forkchoices.len(), 3); + assert_eq!(forkchoices[1], forkchoices[2]); + assert_eq!(forkchoices[2].head_block_hash, future_head.0); + assert_eq!(forkchoices[2].safe_block_hash, finalized); + assert_eq!(forkchoices[2].finalized_block_hash, finalized); + }); +} + +#[test_traced] +fn syncing_head_only_forkchoice_does_not_hold_stale_block_acknowledgement() { + deterministic::Runner::default().start(|context| async move { + let current = digest(10); + let provider = StubExecutionProvider::default(); + provider.set_finalized(100, current.0, round(10)); + let release_head_forkchoice = provider.pause_next_forkchoice(); + + let (actor, mut mailbox) = init( + context.child("follower_executor"), + Config { + execution_provider: provider.clone(), + execution_engine: provider.clone(), + marshal: StubMarshal::default(), + epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), + fcu_heartbeat_interval: Duration::from_secs(60), + }, + ); + actor.start(); + + mailbox.finalization(round(11), digest(11)); + wait_until(&context, || provider.forkchoices().len() == 1).await; + + provider.set_forkchoices_syncing(true); + let latest_head = digest(12); + mailbox.finalization(round(12), latest_head); + + let stale_block = make_block_at_round(99, digest(98).0, round(9)); + let (ack, waiter) = Exact::handle(); + assert!( + mailbox + .report(Update::Block(stale_block.into(), ack)) + .accepted() + ); + + release_head_forkchoice + .send(()) + .expect("the first head FCU should still be waiting"); + wait_until(&context, || provider.forkchoices().len() == 2).await; + + let mut waiter = Box::pin(waiter); + tokio::select! { + result = &mut waiter => { + result.expect("head-only FCU should acknowledge the stale block"); + } + _ = context.sleep(Duration::from_millis(100)) => { + panic!("head-only FCU held the stale block acknowledgement"); + } + } + + let forkchoices = provider.forkchoices(); + assert_eq!(forkchoices.len(), 2); + assert_eq!(forkchoices[1].head_block_hash, latest_head.0); + assert_eq!(forkchoices[1].safe_block_hash, current.0); + assert_eq!(forkchoices[1].finalized_block_hash, current.0); + }); +} + /// An older finalization received while a block FCU is in flight must not /// become the next forkchoice target. #[test_traced] diff --git a/crates/consensus/src/follow/executor/test/utils.rs b/crates/consensus/src/follow/executor/test/utils.rs index e57d1a1b8c..83f0d74d96 100644 --- a/crates/consensus/src/follow/executor/test/utils.rs +++ b/crates/consensus/src/follow/executor/test/utils.rs @@ -70,6 +70,7 @@ struct StubExecutionProviderInner { forkchoices: Mutex>, reject_payloads: AtomicBool, reject_forkchoices: AtomicBool, + sync_forkchoices: AtomicBool, forkchoice_gate: Mutex>>, } @@ -102,6 +103,10 @@ impl StubExecutionProvider { self.inner.reject_forkchoices.store(true, Ordering::SeqCst); } + pub(super) fn set_forkchoices_syncing(&self, syncing: bool) { + self.inner.sync_forkchoices.store(syncing, Ordering::SeqCst); + } + pub(super) fn pause_next_forkchoice(&self) -> oneshot::Sender<()> { let (release, gate) = oneshot::channel(); *self.inner.forkchoice_gate.lock() = Some(gate); @@ -178,6 +183,7 @@ impl ExecutionEngine for StubExecutionProvider { self.inner.forkchoices.lock().push(state); let gate = self.inner.forkchoice_gate.lock().take(); let rejected = self.inner.reject_forkchoices.load(Ordering::SeqCst); + let syncing = self.inner.sync_forkchoices.load(Ordering::SeqCst); async move { if let Some(gate) = gate { let _ = gate.await; @@ -186,6 +192,8 @@ impl ExecutionEngine for StubExecutionProvider { PayloadStatusEnum::Invalid { validation_error: "rejected by test engine".into(), } + } else if syncing { + PayloadStatusEnum::Syncing } else { PayloadStatusEnum::Valid }; From 29ab04df699998223f83e86ad253d60f2aa372e0 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Thu, 27 Aug 2026 22:22:11 -0400 Subject: [PATCH 03/14] wip --- crates/consensus/src/follow/executor/actor.rs | 87 +++++++++++++------ .../consensus/src/follow/executor/test/mod.rs | 64 ++++++++++++-- .../src/follow/executor/test/utils.rs | 8 +- 3 files changed, 124 insertions(+), 35 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index f493e1035a..be3eb3f174 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -183,15 +183,20 @@ where } let request = if let Some((block, ack)) = self.block_queue.pop_front() { - let finalized_advanced = self - .forkchoice - .advance_finalized(Target::from_block(&block)); - let forkchoice = (self.forkchoice.requires_update() || heartbeat) + let block_target = Target::from_block(&block); + let finality_anchor = + self.forkchoice + .advance_finalized(block_target) + .then_some(ForkchoiceTargets { + head: block_target, + finalized: block_target, + }); + let latest_forkchoice = (self.forkchoice.requires_update() || heartbeat) .then_some(self.forkchoice.latest()); ExecutionRequest::Block { block, - forkchoice, - finalized_advanced, + latest_forkchoice, + finality_anchor, ack, } } else if self.forkchoice.requires_update() || heartbeat { @@ -258,12 +263,14 @@ where enum ExecutionRequest { Forkchoice(ForkchoiceTargets), Block { + /// A finalized block delivered by marshal for execution. block: Block, - forkchoice: Option, + /// The latest combined head and finalized targets, if an update is due. + latest_forkchoice: Option, + /// The delivered block as both head and finalized when it advances finality. + finality_anchor: Option, + /// Signals marshal after the payload and required forkchoice update complete. ack: Exact, - /// Whether the block advances finality and must wait for a `VALID` FCU before - /// acknowledgement. - finalized_advanced: bool, }, } @@ -294,34 +301,64 @@ async fn execute_request( } ExecutionRequest::Block { block, - forkchoice, - finalized_advanced, + latest_forkchoice, + finality_anchor, ack, } => { if let Err(error) = submit_new_payload(&context, &execution_engine, block).await { return ExecutionTaskResult::Fatal(error); } - if let Some(forkchoice) = forkchoice { - loop { + let submitted_fcu = match latest_forkchoice { + Some(latest_forkchoice) => { let result = - submit_forkchoice_update(&context, &execution_engine, &forkchoice).await; + submit_forkchoice_update(&context, &execution_engine, &latest_forkchoice) + .await; match result { - Ok(ForkchoiceOutcome::Valid) => break, - Ok(ForkchoiceOutcome::Syncing) if finalized_advanced => { - debug!( - "execution layer is syncing before applying finalized FCU; retrying" - ); - context.sleep(FINALIZED_FCU_RETRY_INTERVAL).await; - } - Ok(ForkchoiceOutcome::Syncing) => break, + Ok(ForkchoiceOutcome::Valid) => Some(latest_forkchoice), + Ok(ForkchoiceOutcome::Syncing) => match finality_anchor { + Some(finality_anchor) => { + if let Err(error) = submit_finality_anchor( + &context, + &execution_engine, + &finality_anchor, + ) + .await + { + return ExecutionTaskResult::Fatal(error); + } + Some(finality_anchor) + } + None => Some(latest_forkchoice), + }, Err(error) => return ExecutionTaskResult::Fatal(error), } } - } + None => None, + }; ack.acknowledge(); - ExecutionTaskResult::Completed(forkchoice) + ExecutionTaskResult::Completed(submitted_fcu) + } + } +} + +async fn submit_finality_anchor( + context: &TContext, + execution_engine: &E, + finality_anchor: &ForkchoiceTargets, +) -> eyre::Result<()> { + loop { + let outcome = submit_forkchoice_update(context, execution_engine, finality_anchor).await?; + match outcome { + ForkchoiceOutcome::Valid => return Ok(()), + ForkchoiceOutcome::Syncing => { + debug!( + "execution layer is syncing before applying finalized block; retrying block \ + anchor FCU" + ); + context.sleep(FINALIZED_FCU_RETRY_INTERVAL).await; + } } } } diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index a9cf0f225b..f820b49529 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -421,7 +421,7 @@ fn finalization_waits_for_durable_block_before_advancing_finalized() { } #[test_traced] -fn syncing_block_forkchoice_retries_before_acknowledging() { +fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); let release_head_forkchoice = provider.pause_next_forkchoice(); @@ -443,6 +443,7 @@ fn syncing_block_forkchoice_retries_before_acknowledging() { mailbox.finalization(round(2), future_head); wait_until(&context, || provider.forkchoices().len() == 1).await; + provider.set_syncing_forkchoice_head(future_head.0); provider.set_forkchoices_syncing(true); let block = make_block_at_round(1, B256::ZERO, round(1)); let finalized = block.block_hash(); @@ -452,25 +453,70 @@ fn syncing_block_forkchoice_retries_before_acknowledging() { release_head_forkchoice .send(()) .expect("the head FCU should still be waiting"); - wait_until(&context, || provider.forkchoices().len() == 2).await; + wait_until(&context, || provider.forkchoices().len() == 3).await; let mut waiter = Box::pin(waiter); tokio::select! { - result = &mut waiter => panic!("syncing FCU acknowledged the block: {result:?}"), + result = &mut waiter => panic!("syncing block anchor acknowledged the block: {result:?}"), _ = context.sleep(Duration::from_millis(100)) => {} } provider.set_forkchoices_syncing(false); waiter .await - .expect("valid retry should acknowledge the durable block"); + .expect("valid block anchor should acknowledge the durable block"); + + wait_until(&context, || provider.forkchoices().len() == 5).await; let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices.len(), 3); - assert_eq!(forkchoices[1], forkchoices[2]); - assert_eq!(forkchoices[2].head_block_hash, future_head.0); - assert_eq!(forkchoices[2].safe_block_hash, finalized); - assert_eq!(forkchoices[2].finalized_block_hash, finalized); + assert_eq!(forkchoices[1].head_block_hash, future_head.0); + assert_eq!(forkchoices[1].safe_block_hash, finalized); + assert_eq!(forkchoices[1].finalized_block_hash, finalized); + + assert_eq!(forkchoices[2], forkchoices[3]); + assert_eq!(forkchoices[3].head_block_hash, finalized); + assert_eq!(forkchoices[3].safe_block_hash, finalized); + assert_eq!(forkchoices[3].finalized_block_hash, finalized); + + assert_eq!(forkchoices[4], forkchoices[1]); + }); +} + +#[test_traced] +fn known_certificate_head_finalizes_delivered_block_without_anchor() { + deterministic::Runner::default().start(|context| async move { + let provider = StubExecutionProvider::default(); + + let (actor, mut mailbox) = init( + context.child("follower_executor"), + Config { + execution_provider: provider.clone(), + execution_engine: provider.clone(), + marshal: StubMarshal::default(), + epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), + fcu_heartbeat_interval: Duration::from_secs(60), + }, + ); + actor.start(); + + let future_head = digest(9); + mailbox.finalization(round(2), future_head); + wait_until(&context, || provider.forkchoices().len() == 1).await; + + let block = make_block_at_round(1, B256::ZERO, round(1)); + let finalized = block.block_hash(); + let (ack, waiter) = Exact::handle(); + assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); + waiter + .await + .expect("known certificate head should finalize the delivered block"); + + let forkchoices = provider.forkchoices(); + assert_eq!(forkchoices.len(), 2); + assert_eq!(forkchoices[1].head_block_hash, future_head.0); + assert_eq!(forkchoices[1].safe_block_hash, finalized); + assert_eq!(forkchoices[1].finalized_block_hash, finalized); }); } diff --git a/crates/consensus/src/follow/executor/test/utils.rs b/crates/consensus/src/follow/executor/test/utils.rs index 83f0d74d96..6a37d79c46 100644 --- a/crates/consensus/src/follow/executor/test/utils.rs +++ b/crates/consensus/src/follow/executor/test/utils.rs @@ -71,6 +71,7 @@ struct StubExecutionProviderInner { reject_payloads: AtomicBool, reject_forkchoices: AtomicBool, sync_forkchoices: AtomicBool, + syncing_forkchoice_head: Mutex>, forkchoice_gate: Mutex>>, } @@ -107,6 +108,10 @@ impl StubExecutionProvider { self.inner.sync_forkchoices.store(syncing, Ordering::SeqCst); } + pub(super) fn set_syncing_forkchoice_head(&self, head: B256) { + *self.inner.syncing_forkchoice_head.lock() = Some(head); + } + pub(super) fn pause_next_forkchoice(&self) -> oneshot::Sender<()> { let (release, gate) = oneshot::channel(); *self.inner.forkchoice_gate.lock() = Some(gate); @@ -183,7 +188,8 @@ impl ExecutionEngine for StubExecutionProvider { self.inner.forkchoices.lock().push(state); let gate = self.inner.forkchoice_gate.lock().take(); let rejected = self.inner.reject_forkchoices.load(Ordering::SeqCst); - let syncing = self.inner.sync_forkchoices.load(Ordering::SeqCst); + let syncing = self.inner.sync_forkchoices.load(Ordering::SeqCst) + || *self.inner.syncing_forkchoice_head.lock() == Some(state.head_block_hash); async move { if let Some(gate) = gate { let _ = gate.await; From 2f6368c8309aecf7824c2b318a91fd72abf51257 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Thu, 27 Aug 2026 22:41:53 -0400 Subject: [PATCH 04/14] wip 2 --- crates/consensus/src/follow/engine.rs | 1 - crates/consensus/src/follow/executor/actor.rs | 167 +++----- crates/consensus/src/follow/executor/fcu.rs | 361 ++++++++++-------- crates/consensus/src/follow/executor/mod.rs | 1 - .../consensus/src/follow/executor/test/mod.rs | 77 ++-- .../src/follow/executor/test/utils.rs | 10 +- 6 files changed, 320 insertions(+), 297 deletions(-) diff --git a/crates/consensus/src/follow/engine.rs b/crates/consensus/src/follow/engine.rs index 63ce96a909..7891798d81 100644 --- a/crates/consensus/src/follow/engine.rs +++ b/crates/consensus/src/follow/engine.rs @@ -164,7 +164,6 @@ impl Config { .clone(), marshal: marshal_mailbox.clone(), epoch_strategy: epoch_strategy.clone(), - floor: last_finalized_height, fcu_heartbeat_interval: self.fcu_heartbeat_interval, }, ); diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index be3eb3f174..5117fd9dbf 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -25,7 +25,7 @@ use tracing::{Level, debug, error, instrument}; use super::{ Config, ExecutionEngine, FinalizedBlockProvider, Marshal, - fcu::{ForkchoiceTargets, ForkchoiceTracker, Target}, + fcu::{BlockForkchoice, FinalityPlan, ForkchoiceTargets, ForkchoiceTracker}, ingress::Message, }; use crate::{consensus::block::Block, utils::OptionFuture}; @@ -41,7 +41,6 @@ pub(crate) struct Actor { marshal: M, epoch_strategy: FixedEpocher, - floor: Height, forkchoice: ForkchoiceTracker, @@ -71,14 +70,13 @@ where execution_engine, marshal, epoch_strategy, - floor, fcu_heartbeat_interval, } = config; let finalized_header = execution_provider .finalized_header() .expect("failed reading finalized execution header"); - let forkchoice = ForkchoiceTracker::new(Target::from_header(&finalized_header)); + let forkchoice = ForkchoiceTracker::new(&finalized_header); Self { context: ContextCell::new(context), @@ -86,7 +84,6 @@ where mailbox, marshal, epoch_strategy, - floor, execution_provider, execution_engine, @@ -118,7 +115,7 @@ where result = &mut self.execution_task => { self.execution_task = OptionFuture::none(); match result { - ExecutionTaskResult::Completed(submitted_fcu) => { + Ok(submitted_fcu) => { if let Some(submitted_fcu) = submitted_fcu { self.forkchoice.note_submitted(submitted_fcu); } @@ -126,7 +123,7 @@ where // Emits an event on error. let _: Result<_, _> = self.try_advance_floor().await; } - ExecutionTaskResult::Fatal(error) => { + Err(error) => { error!(%error, "execution task failed"); break; } @@ -149,12 +146,10 @@ where if self.floor_candidate.is_none() { self.floor_candidate = Some(height); } - self.forkchoice - .advance_head(Target::from_finalization(round, digest)); + self.forkchoice.observe_certificate(round, digest); } Message::Finalization { round, digest } => { - let candidate = Target::from_finalization(round, digest); - self.forkchoice.advance_head(candidate); + self.forkchoice.observe_certificate(round, digest); } } } @@ -182,33 +177,19 @@ where return; } - let request = if let Some((block, ack)) = self.block_queue.pop_front() { - let block_target = Target::from_block(&block); - let finality_anchor = - self.forkchoice - .advance_finalized(block_target) - .then_some(ForkchoiceTargets { - head: block_target, - finalized: block_target, - }); - let latest_forkchoice = (self.forkchoice.requires_update() || heartbeat) - .then_some(self.forkchoice.latest()); - ExecutionRequest::Block { - block, - latest_forkchoice, - finality_anchor, - ack, - } - } else if self.forkchoice.requires_update() || heartbeat { - ExecutionRequest::Forkchoice(self.forkchoice.latest()) + let execution_engine = self.execution_engine.clone(); + let task = if let Some((block, ack)) = self.block_queue.pop_front() { + let forkchoice = self.forkchoice.plan_block(&block, heartbeat); + let context = self.context.child("execute_block"); + execute_block(context, execution_engine, block, forkchoice, ack).boxed() + } else if let Some(forkchoice) = self.forkchoice.plan_update(heartbeat) { + let context = self.context.child("execute_head_update"); + execute_head_update(context, execution_engine, forkchoice).boxed() } else { return; }; - let context = self.context.child("execute_request"); - let execution_engine = self.execution_engine.clone(); - self.execution_task - .replace(execute_request(context, execution_engine, request).boxed()); + self.execution_task.replace(task); } #[instrument(skip_all, err(level = Level::WARN))] @@ -253,103 +234,73 @@ where self.marshal.set_floor(finalization); - self.floor = floor_height; self.floor_candidate = None; Ok(()) } } -enum ExecutionRequest { - Forkchoice(ForkchoiceTargets), - Block { - /// A finalized block delivered by marshal for execution. - block: Block, - /// The latest combined head and finalized targets, if an update is due. - latest_forkchoice: Option, - /// The delivered block as both head and finalized when it advances finality. - finality_anchor: Option, - /// Signals marshal after the payload and required forkchoice update complete. - ack: Exact, - }, -} - -enum ExecutionTaskResult { - Completed(Option), - Fatal(Report), -} +type ExecutionTaskResult = eyre::Result>; enum ForkchoiceOutcome { Valid, Syncing, } -async fn execute_request( +async fn execute_head_update( context: TContext, execution_engine: E, - request: ExecutionRequest, + forkchoice: ForkchoiceTargets, ) -> ExecutionTaskResult { - match request { - ExecutionRequest::Forkchoice(forkchoice) => { - let result = submit_forkchoice_update(&context, &execution_engine, &forkchoice).await; - match result { - Ok(ForkchoiceOutcome::Valid | ForkchoiceOutcome::Syncing) => { - ExecutionTaskResult::Completed(Some(forkchoice)) - } - Err(error) => ExecutionTaskResult::Fatal(error), - } + submit_forkchoice_update(&context, &execution_engine, &forkchoice).await?; + Ok(Some(forkchoice)) +} + +async fn execute_block( + context: TContext, + execution_engine: E, + block: Block, + forkchoice: BlockForkchoice, + ack: Exact, +) -> ExecutionTaskResult { + submit_new_payload(&context, &execution_engine, block).await?; + + let submitted = match forkchoice { + BlockForkchoice::Guide(targets) => { + submit_forkchoice_update(&context, &execution_engine, &targets).await?; + Some(targets) } - ExecutionRequest::Block { - block, - latest_forkchoice, - finality_anchor, - ack, - } => { - if let Err(error) = submit_new_payload(&context, &execution_engine, block).await { - return ExecutionTaskResult::Fatal(error); - } + BlockForkchoice::Finalize(plan) => { + Some(apply_finality(&context, &execution_engine, plan).await?) + } + BlockForkchoice::None => None, + }; - let submitted_fcu = match latest_forkchoice { - Some(latest_forkchoice) => { - let result = - submit_forkchoice_update(&context, &execution_engine, &latest_forkchoice) - .await; - match result { - Ok(ForkchoiceOutcome::Valid) => Some(latest_forkchoice), - Ok(ForkchoiceOutcome::Syncing) => match finality_anchor { - Some(finality_anchor) => { - if let Err(error) = submit_finality_anchor( - &context, - &execution_engine, - &finality_anchor, - ) - .await - { - return ExecutionTaskResult::Fatal(error); - } - Some(finality_anchor) - } - None => Some(latest_forkchoice), - }, - Err(error) => return ExecutionTaskResult::Fatal(error), - } - } - None => None, - }; + ack.acknowledge(); + Ok(submitted) +} - ack.acknowledge(); - ExecutionTaskResult::Completed(submitted_fcu) +async fn apply_finality( + context: &TContext, + execution_engine: &E, + plan: FinalityPlan, +) -> eyre::Result { + match submit_forkchoice_update(context, execution_engine, &plan.preferred).await? { + ForkchoiceOutcome::Valid => Ok(plan.preferred), + ForkchoiceOutcome::Syncing => { + submit_until_valid(context, execution_engine, &plan.anchor).await?; + Ok(plan.anchor) } } } -async fn submit_finality_anchor( +async fn submit_until_valid( context: &TContext, execution_engine: &E, - finality_anchor: &ForkchoiceTargets, + forkchoice: &ForkchoiceTargets, ) -> eyre::Result<()> { loop { - let outcome = submit_forkchoice_update(context, execution_engine, finality_anchor).await?; + let outcome = submit_forkchoice_update(context, execution_engine, forkchoice).await?; match outcome { ForkchoiceOutcome::Valid => return Ok(()), ForkchoiceOutcome::Syncing => { @@ -397,10 +348,8 @@ async fn submit_new_payload( #[instrument( skip_all, fields( - head.round = ?forkchoice.head.round, - head.digest = %forkchoice.head.digest, - finalized.round = ?forkchoice.finalized.round, - finalized.digest = %forkchoice.finalized.digest, + head.digest = %forkchoice.head, + finalized.digest = %forkchoice.finalized, ) )] async fn submit_forkchoice_update( diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs index dcc3c5fec5..ff131509d5 100644 --- a/crates/consensus/src/follow/executor/fcu.rs +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -1,5 +1,5 @@ use alloy_rpc_types_engine::ForkchoiceState; -use commonware_consensus::types::Round; +use commonware_consensus::types::{Height, Round}; use reth_primitives_traits::SealedHeader; use tempo_primitives::TempoHeader; @@ -8,159 +8,187 @@ use crate::consensus::{ block::{Block, round_from_context}, }; -/// A forkchoice target ordered by its consensus finalization round. -/// -/// Execution headers before TIP-1031 do not contain a consensus round. After -/// activation, every newly observed finalization has one. +/// A certified head, ordered by consensus round. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) struct Target { - pub(super) round: Option, - pub(super) digest: Digest, +struct CertifiedHead { + round: Round, + digest: Digest, } -impl Target { - pub(super) fn from_header(header: &SealedHeader) -> Self { - let tip = header.num_hash(); - Self { - round: header.consensus_context.map(round_from_context), - digest: Digest(tip.hash), - } +impl CertifiedHead { + fn from_header(header: &SealedHeader) -> Option { + let round = header.consensus_context.map(round_from_context)?; + Some(Self { + round, + digest: Digest(header.hash()), + }) } - pub(super) fn from_block(block: &Block) -> Self { + fn from_block(block: &Block) -> Option { Self::from_header(block.block().sealed_header()) } - pub(super) const fn from_finalization(round: Round, digest: Digest) -> Self { - Self { - round: Some(round), - digest, - } + const fn from_finalization(round: Round, digest: Digest) -> Self { + Self { round, digest } } - /// Newly observed finalizations are post-TIP-1031 and always have a round. - /// They therefore supersede a roundless genesis or pre-activation target. - fn supersedes(&self, current: &Self) -> bool { - match (self.round, current.round) { - (Some(new), Some(old)) => new > old, - // TIP-1031 is active. In supported startup states, a roundless - // execution target is genesis; restoring partially synchronized - // pre-TIP execution state is unsupported. Any newly verified - // finalization therefore supersedes it. - (Some(_), None) => true, - _ => false, - } + fn supersedes(self, current: Self) -> bool { + self.round > current.round } } -/// Forkchoice targets tracked by the follower executor. -/// -/// This represents the `(head, safe, finalized)` tuple accepted by Engine API -/// `forkchoiceUpdated`, with `safe` equal to `finalized`. -/// -/// `head` follows the latest verified finalization certificate and can refer to -/// a block that marshal has not delivered yet. Setting it tells Reth to sync -/// toward that block. `finalized` follows marshal's ordered block delivery after -/// the executor submits the block as a payload. -/// -/// The head target is always at least as new as the finalized target. +/// A persisted block, ordered by execution height. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) struct ForkchoiceTargets { - pub(super) head: Target, - pub(super) finalized: Target, +struct DurableFinality { + height: Height, + digest: Digest, } -impl ForkchoiceTargets { - const fn new(target: Target) -> Self { +impl DurableFinality { + fn from_header(header: &SealedHeader) -> Self { + let tip = header.num_hash(); Self { - head: target, - finalized: target, + height: Height::new(tip.number), + digest: Digest(tip.hash), } } - fn advance_head(&mut self, candidate: Target) { - advance(&mut self.head, candidate); + fn from_block(block: &Block) -> Self { + Self::from_header(block.block().sealed_header()) } - fn advance_finalized(&mut self, candidate: Target) -> bool { - let advanced = advance(&mut self.finalized, candidate); - advance(&mut self.head, candidate); - advanced + fn supersedes(self, current: Self) -> bool { + self.height > current.height + } +} + +/// Hashes sent in one Engine API forkchoice update. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) struct ForkchoiceTargets { + pub(super) head: Digest, + pub(super) finalized: Digest, +} + +impl ForkchoiceTargets { + fn anchored(target: DurableFinality) -> Self { + Self { + head: target.digest, + finalized: target.digest, + } } pub(super) fn rpc_state(self) -> ForkchoiceState { ForkchoiceState { - head_block_hash: self.head.digest.0, - safe_block_hash: self.finalized.digest.0, - finalized_block_hash: self.finalized.digest.0, + head_block_hash: self.head.0, + safe_block_hash: self.finalized.0, + finalized_block_hash: self.finalized.0, } } } -fn advance(current: &mut Target, candidate: Target) -> bool { - if candidate.supersedes(current) { - *current = candidate; - true - } else { - false - } +/// FCUs needed after a block advances durable finality. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) struct FinalityPlan { + pub(super) preferred: ForkchoiceTargets, + pub(super) anchor: ForkchoiceTargets, +} + +/// Forkchoice work associated with one delivered block. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum BlockForkchoice { + None, + Guide(ForkchoiceTargets), + Finalize(FinalityPlan), } -/// Forkchoice progress across submitted and latest targets. +/// Forkchoice progress across certified heads and persisted blocks. pub(super) struct ForkchoiceTracker { submitted: ForkchoiceTargets, - latest: ForkchoiceTargets, + head: Option, + finalized: DurableFinality, } impl ForkchoiceTracker { - /// Creates a new tracker with the given target as both submitted and latest. - pub(super) const fn new(target: Target) -> Self { - let targets = ForkchoiceTargets::new(target); + pub(super) fn new(header: &SealedHeader) -> Self { + let head = CertifiedHead::from_header(header); + let finalized = DurableFinality::from_header(header); + let submitted = Self::targets(head, finalized); Self { - submitted: targets, - latest: targets, + submitted, + head, + finalized, } } - /// Moves the latest head to a later verified finalization. - /// - /// Finalizations can arrive out of order, so stale and equal-round targets are - /// ignored. - pub(super) fn advance_head(&mut self, candidate: Target) { - self.latest.advance_head(candidate); + /// Moves the head to a later verified certificate. + pub(super) fn observe_certificate(&mut self, round: Round, digest: Digest) { + let candidate = CertifiedHead::from_finalization(round, digest); + if self + .head + .is_none_or(|current| candidate.supersedes(current)) + { + self.head = Some(candidate); + } } - /// Moves the latest finalized target to a later block delivered by marshal. - /// - /// This also advances the head when needed, so finalized never moves ahead of - /// head. Returns whether the finalized target advanced. - pub(super) fn advance_finalized(&mut self, candidate: Target) -> bool { - self.latest.advance_finalized(candidate) - } + /// Plans the forkchoice work for one delivered block. + pub(super) fn plan_block(&mut self, block: &Block, force: bool) -> BlockForkchoice { + let candidate = DurableFinality::from_block(block); + let advances_finality = candidate.supersedes(self.finalized); + if advances_finality { + self.finalized = candidate; + if let Some(head) = CertifiedHead::from_block(block) { + self.observe_certificate(head.round, head.digest); + } + } - /// Returns `true` if the latest targets have changed since the last known submitted state. - pub(super) fn requires_update(&self) -> bool { - self.latest != self.submitted + let preferred = self.desired(); + if advances_finality { + return BlockForkchoice::Finalize(FinalityPlan { + preferred, + anchor: ForkchoiceTargets::anchored(candidate), + }); + } + + if preferred != self.submitted || force { + BlockForkchoice::Guide(preferred) + } else { + BlockForkchoice::None + } } - /// Returns the targets selected by [`Self::advance_head`] and - /// [`Self::advance_finalized`] for the next forkchoice update. - pub(super) const fn latest(&self) -> ForkchoiceTargets { - self.latest + /// Returns the current targets when an update or heartbeat is due. + pub(super) fn plan_update(&self, force: bool) -> Option { + let desired = self.desired(); + (desired != self.submitted || force).then_some(desired) } - /// Saves the given targets as the last known submitted state. + /// Saves the targets from the last completed forkchoice request. pub(super) fn note_submitted(&mut self, submitted: ForkchoiceTargets) { self.submitted = submitted; } + + fn desired(&self) -> ForkchoiceTargets { + Self::targets(self.head, self.finalized) + } + + fn targets(head: Option, finalized: DurableFinality) -> ForkchoiceTargets { + ForkchoiceTargets { + head: head.map_or(finalized.digest, |target| target.digest), + finalized: finalized.digest, + } + } } #[cfg(test)] mod tests { use alloy_consensus::Header; use commonware_consensus::types::{Epoch, View}; + use reth_node_core::primitives::SealedBlock; use reth_primitives_traits::SealedHeader; - use tempo_primitives::{TempoConsensusContext, TempoHeader, ed25519::PublicKey}; + use tempo_primitives::{ + Block as TempoBlock, BlockBody, TempoConsensusContext, TempoHeader, ed25519::PublicKey, + }; use super::*; @@ -177,102 +205,119 @@ mod tests { round: Option, digest: Digest, ) -> SealedHeader { + SealedHeader::new(tempo_header(height, round), digest.0) + } + + fn execution_block(height: u64, round: Option) -> Block { + let block = TempoBlock { + header: tempo_header(height, round), + body: BlockBody::default(), + }; + Block::from_execution_block(SealedBlock::seal_slow(block), None) + .expect("test block should not contain BAL side data") + } + + fn tempo_header(height: u64, round: Option) -> TempoHeader { let consensus_context = round.map(|round| TempoConsensusContext { epoch: round.epoch().get(), view: round.view().get(), parent_view: 0, proposer: PublicKey::from_seed(0), }); - SealedHeader::new( - TempoHeader { - inner: Header { - number: height, - ..Default::default() - }, - consensus_context, + TempoHeader { + inner: Header { + number: height, ..Default::default() }, - digest.0, - ) + consensus_context, + ..Default::default() + } } #[test] fn later_round_supersedes_earlier_round() { - let newer = Target::from_finalization(round(9), digest(1)); - let older = Target::from_finalization(round(8), digest(2)); - assert!(newer.supersedes(&older)); - assert!(!older.supersedes(&newer)); + let newer = CertifiedHead::from_finalization(round(9), digest(1)); + let older = CertifiedHead::from_finalization(round(8), digest(2)); + assert!(newer.supersedes(older)); + assert!(!older.supersedes(newer)); } #[test] fn equal_round_does_not_supersede() { - let current = Target::from_finalization(round(8), digest(1)); - let conflicting = Target::from_finalization(round(8), digest(2)); - assert!(!conflicting.supersedes(¤t)); + let current = CertifiedHead::from_finalization(round(8), digest(1)); + let conflicting = CertifiedHead::from_finalization(round(8), digest(2)); + assert!(!conflicting.supersedes(current)); } #[test] - fn finalized_target_supersedes_roundless_execution_target() { + fn roundless_header_has_no_certified_head() { let header = execution_header(100, None, digest(1)); - let prefork = Target::from_header(&header); - let finalized = Target::from_finalization(round(1), digest(2)); - assert!(finalized.supersedes(&prefork)); - } - - #[test] - fn roundless_target_never_supersedes() { - let roundless = Target::from_header(&execution_header(100, None, digest(1))); - let finalized = Target::from_finalization(round(1), digest(2)); - assert!(!roundless.supersedes(&finalized)); - assert!(!roundless.supersedes(&roundless)); + assert_eq!(CertifiedHead::from_header(&header), None); } #[test] - fn execution_target_uses_header_round() { + fn execution_head_uses_header_round() { let header = execution_header(100, Some(round(2)), digest(1)); - assert_eq!(Target::from_header(&header).round, Some(round(2))); + assert_eq!(CertifiedHead::from_header(&header).unwrap().round, round(2)); } #[test] - fn certified_target_advances_only_head() { - let current = Target::from_finalization(round(1), digest(1)); - let mut tracker = ForkchoiceTracker::new(current); - let candidate = Target::from_finalization(round(2), digest(2)); - - tracker.advance_head(candidate); - - let targets = tracker.latest(); - assert_eq!(targets.head, candidate); - assert_eq!(targets.finalized, current); - assert!(tracker.requires_update()); + fn later_durable_height_supersedes() { + let current = DurableFinality::from_header(&execution_header(100, None, digest(1))); + let conflicting = DurableFinality::from_header(&execution_header(100, None, digest(3))); + let newer = DurableFinality::from_header(&execution_header(101, None, digest(2))); + assert!(newer.supersedes(current)); + assert!(!current.supersedes(newer)); + assert!(!conflicting.supersedes(current)); } #[test] - fn durable_target_advances_both_lanes() { - let current = Target::from_finalization(round(1), digest(1)); - let head = Target::from_finalization(round(3), digest(3)); - let finalized = Target::from_finalization(round(2), digest(2)); - let mut tracker = ForkchoiceTracker::new(current); - tracker.advance_head(head); - - assert!(tracker.advance_finalized(finalized)); - - let targets = tracker.latest(); - assert_eq!(targets.head, head); - assert_eq!(targets.finalized, finalized); + fn certified_target_advances_only_head() { + let current = execution_header(100, Some(round(1)), digest(1)); + let mut tracker = ForkchoiceTracker::new(¤t); + + tracker.observe_certificate(round(2), digest(2)); + + assert_eq!( + tracker.plan_update(false), + Some(ForkchoiceTargets { + head: digest(2), + finalized: digest(1), + }) + ); } #[test] - fn finalized_lane_requires_update_when_head_is_unchanged() { - let current = Target::from_finalization(round(1), digest(1)); - let head = Target::from_finalization(round(2), digest(2)); - let mut tracker = ForkchoiceTracker::new(current); - tracker.advance_head(head); - tracker.note_submitted(tracker.latest()); - assert!(!tracker.requires_update()); + fn roundless_durable_block_keeps_certified_head_and_builds_anchor() { + let current = execution_header(100, None, digest(1)); + let mut tracker = ForkchoiceTracker::new(¤t); + let certified = digest(3); + tracker.observe_certificate(round(3), certified); + + let block = execution_block(101, None); + let durable = block.digest(); + let BlockForkchoice::Finalize(plan) = tracker.plan_block(&block, false) else { + panic!("a later durable block should require finality work"); + }; + + assert_eq!( + plan, + FinalityPlan { + preferred: ForkchoiceTargets { + head: certified, + finalized: durable, + }, + anchor: ForkchoiceTargets { + head: durable, + finalized: durable, + }, + } + ); - assert!(tracker.advance_finalized(head)); + tracker.note_submitted(plan.anchor); + assert_eq!(tracker.plan_update(false), Some(plan.preferred)); - assert!(tracker.requires_update()); + tracker.note_submitted(plan.preferred); + assert_eq!(tracker.plan_update(false), None); } } diff --git a/crates/consensus/src/follow/executor/mod.rs b/crates/consensus/src/follow/executor/mod.rs index b1203abcfe..4456ed8879 100644 --- a/crates/consensus/src/follow/executor/mod.rs +++ b/crates/consensus/src/follow/executor/mod.rs @@ -47,7 +47,6 @@ pub(crate) struct Config { pub(crate) execution_engine: E, pub(crate) marshal: M, pub(crate) epoch_strategy: FixedEpocher, - pub(crate) floor: Height, pub(crate) fcu_heartbeat_interval: std::time::Duration, } diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index f820b49529..6ec958744f 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -13,10 +13,13 @@ use commonware_consensus::{ use commonware_macros::test_traced; use commonware_runtime::{Clock as _, Runner as _, Supervisor as _, deterministic}; use commonware_utils::{Acknowledgement as _, acknowledgement::Exact}; +use futures::FutureExt as _; use super::{Config, init}; use crate::consensus::Digest; -use utils::{StubExecutionProvider, StubMarshal, make_block, make_block_at_round}; +use utils::{ + StubExecutionProvider, StubMarshal, make_block, make_block_at_round, make_prefork_block, +}; const EPOCH_LENGTH: NonZeroU64 = NonZeroU64::new(10).expect("epoch length is nonzero"); const HEARTBEAT_INTERVAL: Duration = Duration::from_millis(5); @@ -61,7 +64,6 @@ fn block_is_executed_canonicalized_acknowledged_and_advances_floor_to_deep_candi execution_engine: provider.clone(), marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -116,7 +118,6 @@ fn floor_candidate_uses_execution_depth_and_next_tip_starts_new_cycle() { execution_engine: provider.clone(), marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -177,7 +178,7 @@ fn floor_candidate_uses_execution_depth_and_next_tip_starts_new_cycle() { } #[test_traced] -fn block_at_or_below_finalized_tip_does_not_regress_forkchoice() { +fn stale_block_with_higher_round_does_not_regress_forkchoice() { deterministic::Runner::default().start(|context| async move { let finalized_height = EPOCH_LENGTH.get(); let provider = StubExecutionProvider::default(); @@ -190,14 +191,13 @@ fn block_at_or_below_finalized_tip_does_not_regress_forkchoice() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); actor.start(); - let block = make_block(finalized_height - 1, B256::with_last_byte(10)); + let block = make_block_at_round(finalized_height - 1, B256::with_last_byte(10), round(1)); let (ack, waiter) = Exact::handle(); assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); waiter.await.expect("valid payload should be acknowledged"); @@ -207,6 +207,45 @@ fn block_at_or_below_finalized_tip_does_not_regress_forkchoice() { }); } +#[test_traced] +fn roundless_prefork_block_advances_finality_by_height() { + deterministic::Runner::default().start(|context| async move { + let current = B256::with_last_byte(100); + let provider = StubExecutionProvider::default(); + provider.set_prefork_finalized(100, current); + + let (actor, mut mailbox) = init( + context.child("follower_executor"), + Config { + execution_provider: provider.clone(), + execution_engine: provider.clone(), + marshal: StubMarshal::default(), + epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + fcu_heartbeat_interval: Duration::from_secs(60), + }, + ); + actor.start(); + + let block = make_prefork_block(101, current); + let block_hash = block.block_hash(); + let (ack, waiter) = Exact::handle(); + assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); + waiter + .await + .expect("valid pre-TIP block should be acknowledged"); + + assert_eq!(provider.payload_count(), 1); + assert_eq!( + provider.forkchoices(), + vec![alloy_rpc_types_engine::ForkchoiceState { + head_block_hash: block_hash, + safe_block_hash: block_hash, + finalized_block_hash: block_hash, + }] + ); + }); +} + #[test_traced] fn floor_does_not_advance_until_its_execution_block_is_durable() { deterministic::Runner::default().start(|context| async move { @@ -223,7 +262,6 @@ fn floor_does_not_advance_until_its_execution_block_is_durable() { execution_engine: provider, marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -273,7 +311,6 @@ fn invalid_payload_exits_without_acknowledging_or_canonicalizing() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -307,7 +344,6 @@ fn forkchoice_failure_exits_without_acknowledging_block() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -340,7 +376,6 @@ fn tips_are_monotonic_and_coalesced_while_forkchoice_is_in_flight() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -391,7 +426,6 @@ fn finalization_waits_for_durable_block_before_advancing_finalized() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -433,7 +467,6 @@ fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -456,10 +489,10 @@ fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { wait_until(&context, || provider.forkchoices().len() == 3).await; let mut waiter = Box::pin(waiter); - tokio::select! { - result = &mut waiter => panic!("syncing block anchor acknowledged the block: {result:?}"), - _ = context.sleep(Duration::from_millis(100)) => {} - } + assert!( + waiter.as_mut().now_or_never().is_none(), + "syncing block anchor must hold the acknowledgement" + ); provider.set_forkchoices_syncing(false); waiter @@ -483,7 +516,7 @@ fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { } #[test_traced] -fn known_certificate_head_finalizes_delivered_block_without_anchor() { +fn valid_certificate_head_finalizes_delivered_block_in_one_fcu() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -494,7 +527,6 @@ fn known_certificate_head_finalizes_delivered_block_without_anchor() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -535,7 +567,6 @@ fn syncing_head_only_forkchoice_does_not_hold_stale_block_acknowledgement() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -596,7 +627,6 @@ fn delayed_finalization_does_not_regress_newer_block_forkchoice() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); @@ -643,7 +673,6 @@ fn execution_tip_round_orders_finalizations_after_restart() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -673,7 +702,6 @@ fn finalization_supersedes_roundless_prefork_execution_tip() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -701,7 +729,6 @@ fn finalization_is_driven_to_from_genesis() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -732,7 +759,6 @@ fn heartbeat_resubmits_latest_tip_after_interval() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); @@ -767,7 +793,6 @@ fn heartbeat_waits_for_in_flight_execution() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); @@ -809,7 +834,6 @@ fn durable_block_read_failure_does_not_exit_actor() { execution_engine: provider.clone(), marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -851,7 +875,6 @@ fn startup_uses_execution_finalized_tip_without_immediate_forkchoice() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); diff --git a/crates/consensus/src/follow/executor/test/utils.rs b/crates/consensus/src/follow/executor/test/utils.rs index 6a37d79c46..d031132f09 100644 --- a/crates/consensus/src/follow/executor/test/utils.rs +++ b/crates/consensus/src/follow/executor/test/utils.rs @@ -32,14 +32,22 @@ pub(super) fn make_block(height: u64, parent_hash: B256) -> Block { make_block_at_round(height, parent_hash, Round::zero()) } +pub(super) fn make_prefork_block(height: u64, parent_hash: B256) -> Block { + make_block_with_round(height, parent_hash, None) +} + pub(super) fn make_block_at_round(height: u64, parent_hash: B256, round: Round) -> Block { + make_block_with_round(height, parent_hash, Some(round)) +} + +fn make_block_with_round(height: u64, parent_hash: B256, round: Option) -> Block { let header = TempoHeader { inner: Header { parent_hash, number: height, ..Default::default() }, - consensus_context: Some(TempoConsensusContext { + consensus_context: round.map(|round| TempoConsensusContext { epoch: round.epoch().get(), view: round.view().get(), parent_view: 0, From 088ca6d814b55c0e74934ac9516e2f20f202e5f4 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 28 Aug 2026 00:23:49 -0400 Subject: [PATCH 05/14] simplify --- crates/consensus/src/follow/executor/actor.rs | 63 +++--- crates/consensus/src/follow/executor/fcu.rs | 153 ++++++------- crates/consensus/src/follow/executor/mod.rs | 7 +- .../consensus/src/follow/executor/test/mod.rs | 214 ++++++++---------- .../src/follow/executor/test/utils.rs | 12 +- 5 files changed, 186 insertions(+), 263 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index 5117fd9dbf..817089ac42 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -1,13 +1,8 @@ //! Execution-layer driver for follower nodes. //! -//! This actor sends verified certificate targets to Reth as forkchoice heads. It advances the safe -//! and finalized targets as marshal delivers persisted finalized blocks. It also refreshes -//! forkchoice with a heartbeat and advances marshal's floor behind Reth's finalized state. -//! -//! Unlike the executor used by validator nodes, it does not build payloads, canonicalize proposal -//! heads, or track blocks proposed by this node. Followers receive complete blocks from their -//! upstream, submit them to Reth as finalized payloads, and rely on Reth's sync machinery plus -//! marshal gap repair to fill history. +//! This actor imports marshal-delivered blocks, applies finality for advancing heights, and +//! refreshes forkchoice on a timer. It moves marshal's floor behind Reth's finalized state. Reth +//! sync and marshal gap repair fill missing history. use std::{collections::VecDeque, time::Duration}; @@ -25,12 +20,12 @@ use tracing::{Level, debug, error, instrument}; use super::{ Config, ExecutionEngine, FinalizedBlockProvider, Marshal, - fcu::{BlockForkchoice, FinalityPlan, ForkchoiceTargets, ForkchoiceTracker}, + fcu::{FinalityPlan, ForkchoiceTargets, ForkchoiceTracker}, ingress::Message, }; use crate::{consensus::block::Block, utils::OptionFuture}; -const FINALIZED_FCU_RETRY_INTERVAL: Duration = Duration::from_secs(1); +const FINALITY_FCU_RETRY_INTERVAL: Duration = Duration::from_secs(1); pub(crate) struct Actor { context: ContextCell, @@ -120,7 +115,7 @@ where self.forkchoice.note_submitted(submitted_fcu); } - // Emits an event on error. + // Floor advancement retries after the next completed execution task. let _: Result<_, _> = self.try_advance_floor().await; } Err(error) => { @@ -133,9 +128,6 @@ where Some(message) = self.mailbox.next() => { match message { Message::Update(Update::Block(block, ack)) => { - // Marshal delivers persisted finalized blocks in height order. The - // executor submits each payload before using it as Reth's finalized - // forkchoice target. self.block_queue.push_back(((*block).clone(), ack)); } @@ -146,10 +138,10 @@ where if self.floor_candidate.is_none() { self.floor_candidate = Some(height); } - self.forkchoice.observe_certificate(round, digest); + self.forkchoice.observe_finalization(round, digest); } Message::Finalization { round, digest } => { - self.forkchoice.observe_certificate(round, digest); + self.forkchoice.observe_finalization(round, digest); } } } @@ -179,10 +171,10 @@ where let execution_engine = self.execution_engine.clone(); let task = if let Some((block, ack)) = self.block_queue.pop_front() { - let forkchoice = self.forkchoice.plan_block(&block, heartbeat); + let finality = self.forkchoice.observe_block(&block); let context = self.context.child("execute_block"); - execute_block(context, execution_engine, block, forkchoice, ack).boxed() - } else if let Some(forkchoice) = self.forkchoice.plan_update(heartbeat) { + execute_block(context, execution_engine, block, finality, ack).boxed() + } else if let Some(forkchoice) = self.forkchoice.next_head_update(heartbeat) { let context = self.context.child("execute_head_update"); execute_head_update(context, execution_engine, forkchoice).boxed() } else { @@ -252,6 +244,8 @@ async fn execute_head_update( execution_engine: E, forkchoice: ForkchoiceTargets, ) -> ExecutionTaskResult { + // `SYNCING` is safe because no block acknowledgement depends on head guidance. The heartbeat + // resubmits the targets. submit_forkchoice_update(&context, &execution_engine, &forkchoice).await?; Ok(Some(forkchoice)) } @@ -260,20 +254,14 @@ async fn execute_block( context: TContext, execution_engine: E, block: Block, - forkchoice: BlockForkchoice, + finality: Option, ack: Exact, ) -> ExecutionTaskResult { submit_new_payload(&context, &execution_engine, block).await?; - let submitted = match forkchoice { - BlockForkchoice::Guide(targets) => { - submit_forkchoice_update(&context, &execution_engine, &targets).await?; - Some(targets) - } - BlockForkchoice::Finalize(plan) => { - Some(apply_finality(&context, &execution_engine, plan).await?) - } - BlockForkchoice::None => None, + let submitted = match finality { + Some(plan) => Some(apply_finality(&context, &execution_engine, plan).await?), + None => None, }; ack.acknowledge(); @@ -288,8 +276,8 @@ async fn apply_finality( match submit_forkchoice_update(context, execution_engine, &plan.preferred).await? { ForkchoiceOutcome::Valid => Ok(plan.preferred), ForkchoiceOutcome::Syncing => { - submit_until_valid(context, execution_engine, &plan.anchor).await?; - Ok(plan.anchor) + submit_until_valid(context, execution_engine, &plan.block_anchor).await?; + Ok(plan.block_anchor) } } } @@ -305,10 +293,10 @@ async fn submit_until_valid( ForkchoiceOutcome::Valid => return Ok(()), ForkchoiceOutcome::Syncing => { debug!( - "execution layer is syncing before applying finalized block; retrying block \ - anchor FCU" + "execution layer is syncing before applying finality; retrying block anchor \ + FCU" ); - context.sleep(FINALIZED_FCU_RETRY_INTERVAL).await; + context.sleep(FINALITY_FCU_RETRY_INTERVAL).await; } } } @@ -329,16 +317,17 @@ async fn submit_new_payload( .new_payload(TempoExecutionData { block, block_access_list, - // can be omitted for finalized blocks + // Marshal delivers blocks after consensus finality checks, so this payload needs no + // validator set. validator_set: None, }) .pace(context, Duration::from_millis(20)) .await - .wrap_err("failed sending finalized payload")?; + .wrap_err("failed sending delivered payload")?; ensure!( payload_status.is_valid() || payload_status.is_syncing(), - "payload status of finalized block was neither valid nor syncing: \ + "payload status of delivered block was neither valid nor syncing: \ `{payload_status}`" ); diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs index ff131509d5..4ff350c990 100644 --- a/crates/consensus/src/follow/executor/fcu.rs +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -1,3 +1,7 @@ +//! Forkchoice policy for follower execution. +//! +//! Certificates order the head by round. Marshal-delivered blocks order finality by height. + use alloy_rpc_types_engine::ForkchoiceState; use commonware_consensus::types::{Height, Round}; use reth_primitives_traits::SealedHeader; @@ -8,7 +12,6 @@ use crate::consensus::{ block::{Block, round_from_context}, }; -/// A certified head, ordered by consensus round. #[derive(Clone, Copy, Debug, PartialEq, Eq)] struct CertifiedHead { round: Round, @@ -24,11 +27,7 @@ impl CertifiedHead { }) } - fn from_block(block: &Block) -> Option { - Self::from_header(block.block().sealed_header()) - } - - const fn from_finalization(round: Round, digest: Digest) -> Self { + const fn from_certificate(round: Round, digest: Digest) -> Self { Self { round, digest } } @@ -37,14 +36,13 @@ impl CertifiedHead { } } -/// A persisted block, ordered by execution height. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -struct DurableFinality { +struct FinalityTarget { height: Height, digest: Digest, } -impl DurableFinality { +impl FinalityTarget { fn from_header(header: &SealedHeader) -> Self { let tip = header.num_hash(); Self { @@ -53,16 +51,12 @@ impl DurableFinality { } } - fn from_block(block: &Block) -> Self { - Self::from_header(block.block().sealed_header()) - } - fn supersedes(self, current: Self) -> bool { self.height > current.height } } -/// Hashes sent in one Engine API forkchoice update. +/// Engine API targets. Safe always follows finalized. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(super) struct ForkchoiceTargets { pub(super) head: Digest, @@ -70,7 +64,7 @@ pub(super) struct ForkchoiceTargets { } impl ForkchoiceTargets { - fn anchored(target: DurableFinality) -> Self { + fn anchored(target: FinalityTarget) -> Self { Self { head: target.digest, finalized: target.digest, @@ -86,93 +80,76 @@ impl ForkchoiceTargets { } } -/// FCUs needed after a block advances durable finality. +/// Preferred targets keep the certified head. If Reth reports `SYNCING`, the executor retries the +/// block anchor until `VALID` before it acknowledges the block. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(super) struct FinalityPlan { pub(super) preferred: ForkchoiceTargets, - pub(super) anchor: ForkchoiceTargets, -} - -/// Forkchoice work associated with one delivered block. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) enum BlockForkchoice { - None, - Guide(ForkchoiceTargets), - Finalize(FinalityPlan), + pub(super) block_anchor: ForkchoiceTargets, } -/// Forkchoice progress across certified heads and persisted blocks. pub(super) struct ForkchoiceTracker { submitted: ForkchoiceTargets, - head: Option, - finalized: DurableFinality, + certified_head: Option, + finalized: FinalityTarget, } impl ForkchoiceTracker { pub(super) fn new(header: &SealedHeader) -> Self { - let head = CertifiedHead::from_header(header); - let finalized = DurableFinality::from_header(header); - let submitted = Self::targets(head, finalized); + let certified_head = CertifiedHead::from_header(header); + let finalized = FinalityTarget::from_header(header); + let submitted = Self::targets(certified_head, finalized); Self { submitted, - head, + certified_head, finalized, } } - /// Moves the head to a later verified certificate. - pub(super) fn observe_certificate(&mut self, round: Round, digest: Digest) { - let candidate = CertifiedHead::from_finalization(round, digest); + pub(super) fn observe_finalization(&mut self, round: Round, digest: Digest) { + let candidate = CertifiedHead::from_certificate(round, digest); if self - .head + .certified_head .is_none_or(|current| candidate.supersedes(current)) { - self.head = Some(candidate); + self.certified_head = Some(candidate); } } - /// Plans the forkchoice work for one delivered block. - pub(super) fn plan_block(&mut self, block: &Block, force: bool) -> BlockForkchoice { - let candidate = DurableFinality::from_block(block); - let advances_finality = candidate.supersedes(self.finalized); - if advances_finality { - self.finalized = candidate; - if let Some(head) = CertifiedHead::from_block(block) { - self.observe_certificate(head.round, head.digest); - } + pub(super) fn observe_block(&mut self, block: &Block) -> Option { + let header = block.block().sealed_header(); + let candidate = FinalityTarget::from_header(header); + if !candidate.supersedes(self.finalized) { + // Replayed blocks cannot move the certified head, even when their header has a later + // round. + return None; } - let preferred = self.desired(); - if advances_finality { - return BlockForkchoice::Finalize(FinalityPlan { - preferred, - anchor: ForkchoiceTargets::anchored(candidate), - }); + self.finalized = candidate; + if let Some(head) = CertifiedHead::from_header(header) { + self.observe_finalization(head.round, head.digest); } - if preferred != self.submitted || force { - BlockForkchoice::Guide(preferred) - } else { - BlockForkchoice::None - } + Some(FinalityPlan { + preferred: self.desired(), + block_anchor: ForkchoiceTargets::anchored(candidate), + }) } - /// Returns the current targets when an update or heartbeat is due. - pub(super) fn plan_update(&self, force: bool) -> Option { + pub(super) fn next_head_update(&self, heartbeat_due: bool) -> Option { let desired = self.desired(); - (desired != self.submitted || force).then_some(desired) + (desired != self.submitted || heartbeat_due).then_some(desired) } - /// Saves the targets from the last completed forkchoice request. pub(super) fn note_submitted(&mut self, submitted: ForkchoiceTargets) { self.submitted = submitted; } fn desired(&self) -> ForkchoiceTargets { - Self::targets(self.head, self.finalized) + Self::targets(self.certified_head, self.finalized) } - fn targets(head: Option, finalized: DurableFinality) -> ForkchoiceTargets { + fn targets(head: Option, finalized: FinalityTarget) -> ForkchoiceTargets { ForkchoiceTargets { head: head.map_or(finalized.digest, |target| target.digest), finalized: finalized.digest, @@ -236,16 +213,16 @@ mod tests { #[test] fn later_round_supersedes_earlier_round() { - let newer = CertifiedHead::from_finalization(round(9), digest(1)); - let older = CertifiedHead::from_finalization(round(8), digest(2)); + let newer = CertifiedHead::from_certificate(round(9), digest(1)); + let older = CertifiedHead::from_certificate(round(8), digest(2)); assert!(newer.supersedes(older)); assert!(!older.supersedes(newer)); } #[test] fn equal_round_does_not_supersede() { - let current = CertifiedHead::from_finalization(round(8), digest(1)); - let conflicting = CertifiedHead::from_finalization(round(8), digest(2)); + let current = CertifiedHead::from_certificate(round(8), digest(1)); + let conflicting = CertifiedHead::from_certificate(round(8), digest(2)); assert!(!conflicting.supersedes(current)); } @@ -256,30 +233,30 @@ mod tests { } #[test] - fn execution_head_uses_header_round() { + fn certified_head_uses_header_round() { let header = execution_header(100, Some(round(2)), digest(1)); assert_eq!(CertifiedHead::from_header(&header).unwrap().round, round(2)); } #[test] - fn later_durable_height_supersedes() { - let current = DurableFinality::from_header(&execution_header(100, None, digest(1))); - let conflicting = DurableFinality::from_header(&execution_header(100, None, digest(3))); - let newer = DurableFinality::from_header(&execution_header(101, None, digest(2))); + fn later_block_height_supersedes_finality_target() { + let current = FinalityTarget::from_header(&execution_header(100, None, digest(1))); + let conflicting = FinalityTarget::from_header(&execution_header(100, None, digest(3))); + let newer = FinalityTarget::from_header(&execution_header(101, None, digest(2))); assert!(newer.supersedes(current)); assert!(!current.supersedes(newer)); assert!(!conflicting.supersedes(current)); } #[test] - fn certified_target_advances_only_head() { + fn certificate_advances_only_head() { let current = execution_header(100, Some(round(1)), digest(1)); let mut tracker = ForkchoiceTracker::new(¤t); - tracker.observe_certificate(round(2), digest(2)); + tracker.observe_finalization(round(2), digest(2)); assert_eq!( - tracker.plan_update(false), + tracker.next_head_update(false), Some(ForkchoiceTargets { head: digest(2), finalized: digest(1), @@ -288,36 +265,36 @@ mod tests { } #[test] - fn roundless_durable_block_keeps_certified_head_and_builds_anchor() { + fn roundless_block_builds_finality_plan_without_moving_certified_head() { let current = execution_header(100, None, digest(1)); let mut tracker = ForkchoiceTracker::new(¤t); let certified = digest(3); - tracker.observe_certificate(round(3), certified); + tracker.observe_finalization(round(3), certified); let block = execution_block(101, None); - let durable = block.digest(); - let BlockForkchoice::Finalize(plan) = tracker.plan_block(&block, false) else { - panic!("a later durable block should require finality work"); - }; + let block_digest = block.digest(); + let plan = tracker + .observe_block(&block) + .expect("a later block should require finality work"); assert_eq!( plan, FinalityPlan { preferred: ForkchoiceTargets { head: certified, - finalized: durable, + finalized: block_digest, }, - anchor: ForkchoiceTargets { - head: durable, - finalized: durable, + block_anchor: ForkchoiceTargets { + head: block_digest, + finalized: block_digest, }, } ); - tracker.note_submitted(plan.anchor); - assert_eq!(tracker.plan_update(false), Some(plan.preferred)); + tracker.note_submitted(plan.block_anchor); + assert_eq!(tracker.next_head_update(false), Some(plan.preferred)); tracker.note_submitted(plan.preferred); - assert_eq!(tracker.plan_update(false), None); + assert_eq!(tracker.next_head_update(false), None); } } diff --git a/crates/consensus/src/follow/executor/mod.rs b/crates/consensus/src/follow/executor/mod.rs index 4456ed8879..39fbe59294 100644 --- a/crates/consensus/src/follow/executor/mod.rs +++ b/crates/consensus/src/follow/executor/mod.rs @@ -1,8 +1,7 @@ //! Execution-layer synchronization for follow mode. //! -//! This is intentionally smaller than the validator executor: it receives -//! already-verified finalized tips, drives forkchoice updates, and advances -//! marshal's floor after execution-layer progress is durable. +//! Verified certificates guide the execution head by round. Marshal's gap-free block stream +//! advances safe and finalized by height. Durable execution progress moves marshal's floor. use std::future::Future; @@ -76,7 +75,7 @@ pub(crate) trait FinalizedBlockProvider: Send + Sync { /// Engine commands issued by the follower executor. pub(crate) trait ExecutionEngine: Send + Sync { - /// Submit a finalized execution payload. + /// Submit a payload for a block delivered after consensus finality. fn new_payload( &self, payload: TempoExecutionData, diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index 6ec958744f..1002126518 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -5,6 +5,7 @@ mod utils; use std::{num::NonZeroU64, time::Duration}; use alloy_primitives::B256; +use alloy_rpc_types_engine::ForkchoiceState; use commonware_consensus::{ Reporter as _, marshal::Update, @@ -18,7 +19,7 @@ use futures::FutureExt as _; use super::{Config, init}; use crate::consensus::Digest; use utils::{ - StubExecutionProvider, StubMarshal, make_block, make_block_at_round, make_prefork_block, + StubExecutionProvider, StubMarshal, make_block, make_block_at_round, make_roundless_block, }; const EPOCH_LENGTH: NonZeroU64 = NonZeroU64::new(10).expect("epoch length is nonzero"); @@ -45,8 +46,16 @@ fn digest(byte: u8) -> Digest { Digest(B256::with_last_byte(byte)) } +fn forkchoice(head: B256, finalized: B256) -> ForkchoiceState { + ForkchoiceState { + head_block_hash: head, + safe_block_hash: finalized, + finalized_block_hash: finalized, + } +} + #[test_traced] -fn block_is_executed_canonicalized_acknowledged_and_advances_floor_to_deep_candidate() { +fn delivered_block_advances_execution_finality_and_marshal_floor() { deterministic::Runner::default().start(|context| async move { let finalized_height = EPOCH_LENGTH.get() * 2; let expected_floor = finalized_height - EPOCH_LENGTH.get() - 1; @@ -94,11 +103,7 @@ fn block_is_executed_canonicalized_acknowledged_and_advances_floor_to_deep_candi assert_eq!(provider.payload_count(), 1); assert_eq!( provider.forkchoices(), - vec![alloy_rpc_types_engine::ForkchoiceState { - head_block_hash: block_hash, - safe_block_hash: block_hash, - finalized_block_hash: block_hash, - }] + vec![forkchoice(block_hash, block_hash)] ); }); } @@ -208,11 +213,11 @@ fn stale_block_with_higher_round_does_not_regress_forkchoice() { } #[test_traced] -fn roundless_prefork_block_advances_finality_by_height() { +fn roundless_block_advances_finality_by_height() { deterministic::Runner::default().start(|context| async move { let current = B256::with_last_byte(100); let provider = StubExecutionProvider::default(); - provider.set_prefork_finalized(100, current); + provider.set_finalized_without_round(100, current); let (actor, mut mailbox) = init( context.child("follower_executor"), @@ -226,22 +231,18 @@ fn roundless_prefork_block_advances_finality_by_height() { ); actor.start(); - let block = make_prefork_block(101, current); + let block = make_roundless_block(101, current); let block_hash = block.block_hash(); let (ack, waiter) = Exact::handle(); assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); waiter .await - .expect("valid pre-TIP block should be acknowledged"); + .expect("valid roundless block should be acknowledged"); assert_eq!(provider.payload_count(), 1); assert_eq!( provider.forkchoices(), - vec![alloy_rpc_types_engine::ForkchoiceState { - head_block_hash: block_hash, - safe_block_hash: block_hash, - finalized_block_hash: block_hash, - }] + vec![forkchoice(block_hash, block_hash)] ); }); } @@ -299,7 +300,7 @@ fn floor_does_not_advance_until_its_execution_block_is_durable() { } #[test_traced] -fn invalid_payload_exits_without_acknowledging_or_canonicalizing() { +fn invalid_payload_exits_before_fcu_or_ack() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); provider.reject_payloads(); @@ -407,15 +408,13 @@ fn tips_are_monotonic_and_coalesced_while_forkchoice_is_in_flight() { wait_until(&context, || provider.forkchoices().len() == 2).await; let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices[0].head_block_hash, first_digest.0); - assert_eq!(forkchoices[1].head_block_hash, highest_digest.0); - assert_eq!(forkchoices[1].safe_block_hash, B256::ZERO); - assert_eq!(forkchoices[1].finalized_block_hash, B256::ZERO); + assert_eq!(forkchoices[0], forkchoice(first_digest.0, B256::ZERO)); + assert_eq!(forkchoices[1], forkchoice(highest_digest.0, B256::ZERO)); }); } #[test_traced] -fn finalization_waits_for_durable_block_before_advancing_finalized() { +fn certificate_advances_head_before_block_advances_finality() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -432,25 +431,20 @@ fn finalization_waits_for_durable_block_before_advancing_finalized() { actor.start(); let block = make_block_at_round(1, B256::ZERO, round(2)); - let finalized = Digest(block.block_hash()); - mailbox.finalization(round(2), finalized); + let block_digest = Digest(block.block_hash()); + mailbox.finalization(round(2), block_digest); wait_until(&context, || provider.forkchoices().len() == 1).await; let forkchoices = provider.forkchoices(); assert_eq!(forkchoices.len(), 1); - assert_eq!(forkchoices[0].head_block_hash, finalized.0); - assert_eq!(forkchoices[0].safe_block_hash, B256::ZERO); - assert_eq!(forkchoices[0].finalized_block_hash, B256::ZERO); + assert_eq!(forkchoices[0], forkchoice(block_digest.0, B256::ZERO)); let (ack, waiter) = Exact::handle(); assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); waiter.await.expect("durable block should be acknowledged"); - wait_until(&context, || provider.forkchoices().len() == 2).await; let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices[1].head_block_hash, finalized.0); - assert_eq!(forkchoices[1].safe_block_hash, finalized.0); - assert_eq!(forkchoices[1].finalized_block_hash, finalized.0); + assert_eq!(forkchoices[1], forkchoice(block_digest.0, block_digest.0)); }); } @@ -476,7 +470,6 @@ fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { mailbox.finalization(round(2), future_head); wait_until(&context, || provider.forkchoices().len() == 1).await; - provider.set_syncing_forkchoice_head(future_head.0); provider.set_forkchoices_syncing(true); let block = make_block_at_round(1, B256::ZERO, round(1)); let finalized = block.block_hash(); @@ -501,22 +494,22 @@ fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { wait_until(&context, || provider.forkchoices().len() == 5).await; - let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices[1].head_block_hash, future_head.0); - assert_eq!(forkchoices[1].safe_block_hash, finalized); - assert_eq!(forkchoices[1].finalized_block_hash, finalized); - - assert_eq!(forkchoices[2], forkchoices[3]); - assert_eq!(forkchoices[3].head_block_hash, finalized); - assert_eq!(forkchoices[3].safe_block_hash, finalized); - assert_eq!(forkchoices[3].finalized_block_hash, finalized); - - assert_eq!(forkchoices[4], forkchoices[1]); + assert_eq!(provider.payload_count(), 1); + assert_eq!( + provider.forkchoices(), + vec![ + forkchoice(future_head.0, B256::ZERO), + forkchoice(future_head.0, finalized), + forkchoice(finalized, finalized), + forkchoice(finalized, finalized), + forkchoice(future_head.0, finalized), + ] + ); }); } #[test_traced] -fn valid_certificate_head_finalizes_delivered_block_in_one_fcu() { +fn valid_preferred_fcu_skips_block_anchor() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -544,20 +537,23 @@ fn valid_certificate_head_finalizes_delivered_block_in_one_fcu() { .await .expect("known certificate head should finalize the delivered block"); - let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices.len(), 2); - assert_eq!(forkchoices[1].head_block_hash, future_head.0); - assert_eq!(forkchoices[1].safe_block_hash, finalized); - assert_eq!(forkchoices[1].finalized_block_hash, finalized); + assert_eq!( + provider.forkchoices(), + vec![ + forkchoice(future_head.0, B256::ZERO), + forkchoice(future_head.0, finalized), + ] + ); }); } #[test_traced] -fn syncing_head_only_forkchoice_does_not_hold_stale_block_acknowledgement() { +fn stale_block_ack_precedes_syncing_head_guidance() { deterministic::Runner::default().start(|context| async move { let current = digest(10); let provider = StubExecutionProvider::default(); provider.set_finalized(100, current.0, round(10)); + provider.set_forkchoices_syncing(true); let release_head_forkchoice = provider.pause_next_forkchoice(); let (actor, mut mailbox) = init( @@ -572,13 +568,7 @@ fn syncing_head_only_forkchoice_does_not_hold_stale_block_acknowledgement() { ); actor.start(); - mailbox.finalization(round(11), digest(11)); - wait_until(&context, || provider.forkchoices().len() == 1).await; - - provider.set_forkchoices_syncing(true); let latest_head = digest(12); - mailbox.finalization(round(12), latest_head); - let stale_block = make_block_at_round(99, digest(98).0, round(9)); let (ack, waiter) = Exact::handle(); assert!( @@ -586,34 +576,35 @@ fn syncing_head_only_forkchoice_does_not_hold_stale_block_acknowledgement() { .report(Update::Block(stale_block.into(), ack)) .accepted() ); + mailbox.finalization(round(12), latest_head); + wait_until(&context, || provider.forkchoices().len() == 1).await; + + waiter + .now_or_never() + .expect("stale block acknowledgement should precede head guidance") + .expect("stale block should be acknowledged"); + provider.set_forkchoices_syncing(false); release_head_forkchoice .send(()) - .expect("the first head FCU should still be waiting"); - wait_until(&context, || provider.forkchoices().len() == 2).await; + .expect("the head FCU should still be waiting"); - let mut waiter = Box::pin(waiter); - tokio::select! { - result = &mut waiter => { - result.expect("head-only FCU should acknowledge the stale block"); - } - _ = context.sleep(Duration::from_millis(100)) => { - panic!("head-only FCU held the stale block acknowledgement"); - } - } + let newer_head = digest(13); + mailbox.finalization(round(13), newer_head); + wait_until(&context, || provider.forkchoices().len() == 2).await; - let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices.len(), 2); - assert_eq!(forkchoices[1].head_block_hash, latest_head.0); - assert_eq!(forkchoices[1].safe_block_hash, current.0); - assert_eq!(forkchoices[1].finalized_block_hash, current.0); + assert_eq!( + provider.forkchoices(), + vec![ + forkchoice(latest_head.0, current.0), + forkchoice(newer_head.0, current.0), + ] + ); }); } -/// An older finalization received while a block FCU is in flight must not -/// become the next forkchoice target. #[test_traced] -fn delayed_finalization_does_not_regress_newer_block_forkchoice() { +fn delayed_certificate_does_not_regress_newer_block_forkchoice() { deterministic::Runner::default().start(|context| async move { let current = Digest(B256::with_last_byte(10)); let provider = StubExecutionProvider::default(); @@ -641,8 +632,8 @@ fn delayed_finalization_does_not_regress_newer_block_forkchoice() { }) .await; - let delayed = Digest(B256::with_last_byte(12)); - mailbox.finalization(round(12), delayed); + let older_certificate = Digest(B256::with_last_byte(12)); + mailbox.finalization(round(12), older_certificate); context.sleep(Duration::from_millis(1)).await; release_block_forkchoice @@ -651,20 +642,20 @@ fn delayed_finalization_does_not_regress_newer_block_forkchoice() { waiter.await.expect("valid payload should be acknowledged"); context.sleep(Duration::from_millis(1)).await; - let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices.len(), 1); - assert_eq!(forkchoices[0].head_block_hash, newest.0); + let expected = forkchoice(newest.0, newest.0); + assert_eq!(provider.forkchoices(), vec![expected]); wait_until(&context, || provider.forkchoices().len() == 2).await; - assert_eq!(provider.forkchoices()[1].head_block_hash, newest.0); + assert_eq!(provider.forkchoices(), vec![expected, expected]); }); } #[test_traced] -fn execution_tip_round_orders_finalizations_after_restart() { +fn startup_header_round_orders_certificate_heads() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); - provider.set_finalized(100, digest(100).0, round(7)); + let current = digest(100); + provider.set_finalized(100, current.0, round(7)); let (actor, mailbox) = init( context.child("follower_executor"), @@ -685,15 +676,16 @@ fn execution_tip_round_orders_finalizations_after_restart() { let newer = digest(102); mailbox.finalization(round(8), newer); wait_until(&context, || !provider.forkchoices().is_empty()).await; - assert_eq!(provider.forkchoices()[0].head_block_hash, newer.0); + assert_eq!(provider.forkchoices(), vec![forkchoice(newer.0, current.0)]); }); } #[test_traced] -fn finalization_supersedes_roundless_prefork_execution_tip() { +fn certificate_supersedes_roundless_startup_head() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); - provider.set_prefork_finalized(100, digest(100).0); + let current = digest(100); + provider.set_finalized_without_round(100, current.0); let (actor, mailbox) = init( context.child("follower_executor"), @@ -710,45 +702,18 @@ fn finalization_supersedes_roundless_prefork_execution_tip() { context.sleep(Duration::from_millis(1)).await; assert!(provider.forkchoices().is_empty()); - let finalized = digest(101); - mailbox.finalization(round(1), finalized); - wait_until(&context, || !provider.forkchoices().is_empty()).await; - assert_eq!(provider.forkchoices()[0].head_block_hash, finalized.0); - }); -} - -#[test_traced] -fn finalization_is_driven_to_from_genesis() { - deterministic::Runner::default().start(|context| async move { - let provider = StubExecutionProvider::default(); - - let (actor, mailbox) = init( - context.child("follower_executor"), - Config { - execution_provider: provider.clone(), - execution_engine: provider.clone(), - marshal: StubMarshal::default(), - epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - fcu_heartbeat_interval: Duration::from_secs(60), - }, - ); - actor.start(); - - let finalized = Digest(B256::with_last_byte(9)); - mailbox.finalization(round(5), finalized); - + let certified_head = digest(101); + mailbox.finalization(round(1), certified_head); wait_until(&context, || !provider.forkchoices().is_empty()).await; - let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices.len(), 1); assert_eq!( - forkchoices[0].head_block_hash, finalized.0, - "nothing is below genesis, so the certificate is driven directly", + provider.forkchoices(), + vec![forkchoice(certified_head.0, current.0)] ); }); } #[test_traced] -fn heartbeat_resubmits_latest_tip_after_interval() { +fn heartbeat_resubmits_current_forkchoice_after_interval() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -774,9 +739,8 @@ fn heartbeat_resubmits_latest_tip_after_interval() { assert_eq!(provider.forkchoices().len(), 1); wait_until(&context, || provider.forkchoices().len() == 2).await; - let forkchoices = provider.forkchoices(); - assert_eq!(forkchoices[0], forkchoices[1]); - assert_eq!(forkchoices[1].head_block_hash, digest.0); + let expected = forkchoice(digest.0, B256::ZERO); + assert_eq!(provider.forkchoices(), vec![expected, expected]); }); } @@ -885,9 +849,9 @@ fn startup_uses_execution_finalized_tip_without_immediate_forkchoice() { assert!(provider.forkchoices().is_empty()); wait_until(&context, || !provider.forkchoices().is_empty()).await; - let forkchoice = provider.forkchoices()[0]; - assert_eq!(forkchoice.head_block_hash, finalized_hash); - assert_eq!(forkchoice.safe_block_hash, finalized_hash); - assert_eq!(forkchoice.finalized_block_hash, finalized_hash); + assert_eq!( + provider.forkchoices(), + vec![forkchoice(finalized_hash, finalized_hash)] + ); }); } diff --git a/crates/consensus/src/follow/executor/test/utils.rs b/crates/consensus/src/follow/executor/test/utils.rs index d031132f09..54224da806 100644 --- a/crates/consensus/src/follow/executor/test/utils.rs +++ b/crates/consensus/src/follow/executor/test/utils.rs @@ -32,7 +32,7 @@ pub(super) fn make_block(height: u64, parent_hash: B256) -> Block { make_block_at_round(height, parent_hash, Round::zero()) } -pub(super) fn make_prefork_block(height: u64, parent_hash: B256) -> Block { +pub(super) fn make_roundless_block(height: u64, parent_hash: B256) -> Block { make_block_with_round(height, parent_hash, None) } @@ -79,7 +79,6 @@ struct StubExecutionProviderInner { reject_payloads: AtomicBool, reject_forkchoices: AtomicBool, sync_forkchoices: AtomicBool, - syncing_forkchoice_head: Mutex>, forkchoice_gate: Mutex>>, } @@ -91,7 +90,7 @@ impl StubExecutionProvider { /// Models a finalized execution header from before TIP-1031, when headers /// had no consensus context and therefore no round. - pub(super) fn set_prefork_finalized(&self, number: u64, hash: B256) { + pub(super) fn set_finalized_without_round(&self, number: u64, hash: B256) { *self.inner.finalized.lock() = BlockNumHash::new(number, hash); *self.inner.finalized_round.lock() = None; } @@ -116,10 +115,6 @@ impl StubExecutionProvider { self.inner.sync_forkchoices.store(syncing, Ordering::SeqCst); } - pub(super) fn set_syncing_forkchoice_head(&self, head: B256) { - *self.inner.syncing_forkchoice_head.lock() = Some(head); - } - pub(super) fn pause_next_forkchoice(&self) -> oneshot::Sender<()> { let (release, gate) = oneshot::channel(); *self.inner.forkchoice_gate.lock() = Some(gate); @@ -196,8 +191,7 @@ impl ExecutionEngine for StubExecutionProvider { self.inner.forkchoices.lock().push(state); let gate = self.inner.forkchoice_gate.lock().take(); let rejected = self.inner.reject_forkchoices.load(Ordering::SeqCst); - let syncing = self.inner.sync_forkchoices.load(Ordering::SeqCst) - || *self.inner.syncing_forkchoice_head.lock() == Some(state.head_block_hash); + let syncing = self.inner.sync_forkchoices.load(Ordering::SeqCst); async move { if let Some(gate) = gate { let _ = gate.await; From 869b60cb2e76f6a33f99a6d14caac197bc24fb03 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 12:14:46 -0400 Subject: [PATCH 06/14] remove syncing loop --- crates/consensus/src/follow/executor/actor.rs | 24 +------------------ crates/consensus/src/follow/executor/fcu.rs | 4 ++-- .../consensus/src/follow/executor/test/mod.rs | 16 +++---------- 3 files changed, 6 insertions(+), 38 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index 817089ac42..f15e47539f 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -25,8 +25,6 @@ use super::{ }; use crate::{consensus::block::Block, utils::OptionFuture}; -const FINALITY_FCU_RETRY_INTERVAL: Duration = Duration::from_secs(1); - pub(crate) struct Actor { context: ContextCell, mailbox: mpsc::UnboundedReceiver, @@ -276,32 +274,12 @@ async fn apply_finality( match submit_forkchoice_update(context, execution_engine, &plan.preferred).await? { ForkchoiceOutcome::Valid => Ok(plan.preferred), ForkchoiceOutcome::Syncing => { - submit_until_valid(context, execution_engine, &plan.block_anchor).await?; + submit_forkchoice_update(context, execution_engine, &plan.block_anchor).await?; Ok(plan.block_anchor) } } } -async fn submit_until_valid( - context: &TContext, - execution_engine: &E, - forkchoice: &ForkchoiceTargets, -) -> eyre::Result<()> { - loop { - let outcome = submit_forkchoice_update(context, execution_engine, forkchoice).await?; - match outcome { - ForkchoiceOutcome::Valid => return Ok(()), - ForkchoiceOutcome::Syncing => { - debug!( - "execution layer is syncing before applying finality; retrying block anchor \ - FCU" - ); - context.sleep(FINALITY_FCU_RETRY_INTERVAL).await; - } - } - } -} - #[instrument( skip_all, fields(block.height = %block.height(), block.digest = %block.digest()), diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs index 4ff350c990..36ee6c07db 100644 --- a/crates/consensus/src/follow/executor/fcu.rs +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -80,8 +80,8 @@ impl ForkchoiceTargets { } } -/// Preferred targets keep the certified head. If Reth reports `SYNCING`, the executor retries the -/// block anchor until `VALID` before it acknowledges the block. +/// Preferred targets keep the certified head. The block anchor provides a fallback when Reth is +/// still syncing the certified head. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(super) struct FinalityPlan { pub(super) preferred: ForkchoiceTargets, diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index 1002126518..16e0fd0231 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -449,7 +449,7 @@ fn certificate_advances_head_before_block_advances_finality() { } #[test_traced] -fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { +fn syncing_certificate_head_submits_block_anchor_once() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); let release_head_forkchoice = provider.pause_next_forkchoice(); @@ -479,20 +479,11 @@ fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { release_head_forkchoice .send(()) .expect("the head FCU should still be waiting"); - wait_until(&context, || provider.forkchoices().len() == 3).await; - - let mut waiter = Box::pin(waiter); - assert!( - waiter.as_mut().now_or_never().is_none(), - "syncing block anchor must hold the acknowledgement" - ); - - provider.set_forkchoices_syncing(false); waiter .await - .expect("valid block anchor should acknowledge the durable block"); + .expect("submitted block anchor should acknowledge the durable block"); - wait_until(&context, || provider.forkchoices().len() == 5).await; + wait_until(&context, || provider.forkchoices().len() == 4).await; assert_eq!(provider.payload_count(), 1); assert_eq!( @@ -501,7 +492,6 @@ fn syncing_certificate_head_falls_back_to_block_anchor_before_acknowledging() { forkchoice(future_head.0, B256::ZERO), forkchoice(future_head.0, finalized), forkchoice(finalized, finalized), - forkchoice(finalized, finalized), forkchoice(future_head.0, finalized), ] ); From 6380505e837bd32d08c6e5de3d987123c628439e Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 12:28:03 -0400 Subject: [PATCH 07/14] finality plan is no longer necessary --- crates/consensus/src/follow/executor/actor.rs | 53 +++----- crates/consensus/src/follow/executor/fcu.rs | 45 ++----- .../consensus/src/follow/executor/test/mod.rs | 114 +----------------- .../src/follow/executor/test/utils.rs | 8 -- 4 files changed, 26 insertions(+), 194 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index f15e47539f..3e74d5c75b 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -20,7 +20,7 @@ use tracing::{Level, debug, error, instrument}; use super::{ Config, ExecutionEngine, FinalizedBlockProvider, Marshal, - fcu::{FinalityPlan, ForkchoiceTargets, ForkchoiceTracker}, + fcu::{ForkchoiceTargets, ForkchoiceTracker}, ingress::Message, }; use crate::{consensus::block::Block, utils::OptionFuture}; @@ -169,9 +169,9 @@ where let execution_engine = self.execution_engine.clone(); let task = if let Some((block, ack)) = self.block_queue.pop_front() { - let finality = self.forkchoice.observe_block(&block); + let forkchoice = self.forkchoice.observe_block(&block); let context = self.context.child("execute_block"); - execute_block(context, execution_engine, block, finality, ack).boxed() + execute_block(context, execution_engine, block, forkchoice, ack).boxed() } else if let Some(forkchoice) = self.forkchoice.next_head_update(heartbeat) { let context = self.context.child("execute_head_update"); execute_head_update(context, execution_engine, forkchoice).boxed() @@ -232,18 +232,11 @@ where type ExecutionTaskResult = eyre::Result>; -enum ForkchoiceOutcome { - Valid, - Syncing, -} - async fn execute_head_update( context: TContext, execution_engine: E, forkchoice: ForkchoiceTargets, ) -> ExecutionTaskResult { - // `SYNCING` is safe because no block acknowledgement depends on head guidance. The heartbeat - // resubmits the targets. submit_forkchoice_update(&context, &execution_engine, &forkchoice).await?; Ok(Some(forkchoice)) } @@ -252,32 +245,17 @@ async fn execute_block( context: TContext, execution_engine: E, block: Block, - finality: Option, + forkchoice: Option, ack: Exact, ) -> ExecutionTaskResult { submit_new_payload(&context, &execution_engine, block).await?; - let submitted = match finality { - Some(plan) => Some(apply_finality(&context, &execution_engine, plan).await?), - None => None, - }; + if let Some(forkchoice) = &forkchoice { + submit_forkchoice_update(&context, &execution_engine, forkchoice).await?; + } ack.acknowledge(); - Ok(submitted) -} - -async fn apply_finality( - context: &TContext, - execution_engine: &E, - plan: FinalityPlan, -) -> eyre::Result { - match submit_forkchoice_update(context, execution_engine, &plan.preferred).await? { - ForkchoiceOutcome::Valid => Ok(plan.preferred), - ForkchoiceOutcome::Syncing => { - submit_forkchoice_update(context, execution_engine, &plan.block_anchor).await?; - Ok(plan.block_anchor) - } - } + Ok(forkchoice) } #[instrument( @@ -323,7 +301,7 @@ async fn submit_forkchoice_update( context: &TContext, execution_engine: &E, forkchoice: &ForkchoiceTargets, -) -> eyre::Result { +) -> eyre::Result<()> { let forkchoice = forkchoice.rpc_state(); let response = execution_engine @@ -334,13 +312,10 @@ async fn submit_forkchoice_update( debug!(payload_status = %response.payload_status, "execution layer reported FCU status"); - if response.payload_status.is_valid() { - return Ok(ForkchoiceOutcome::Valid); - } - - if response.payload_status.is_syncing() { - return Ok(ForkchoiceOutcome::Syncing); - } + ensure!( + !response.is_invalid(), + Report::msg(response.payload_status).wrap_err("execution layer rejected fcu") + ); - Err(Report::msg(response.payload_status).wrap_err("execution layer rejected fcu")) + Ok(()) } diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs index 36ee6c07db..c4fbb690ee 100644 --- a/crates/consensus/src/follow/executor/fcu.rs +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -64,13 +64,6 @@ pub(super) struct ForkchoiceTargets { } impl ForkchoiceTargets { - fn anchored(target: FinalityTarget) -> Self { - Self { - head: target.digest, - finalized: target.digest, - } - } - pub(super) fn rpc_state(self) -> ForkchoiceState { ForkchoiceState { head_block_hash: self.head.0, @@ -80,14 +73,6 @@ impl ForkchoiceTargets { } } -/// Preferred targets keep the certified head. The block anchor provides a fallback when Reth is -/// still syncing the certified head. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) struct FinalityPlan { - pub(super) preferred: ForkchoiceTargets, - pub(super) block_anchor: ForkchoiceTargets, -} - pub(super) struct ForkchoiceTracker { submitted: ForkchoiceTargets, certified_head: Option, @@ -116,7 +101,7 @@ impl ForkchoiceTracker { } } - pub(super) fn observe_block(&mut self, block: &Block) -> Option { + pub(super) fn observe_block(&mut self, block: &Block) -> Option { let header = block.block().sealed_header(); let candidate = FinalityTarget::from_header(header); if !candidate.supersedes(self.finalized) { @@ -130,10 +115,7 @@ impl ForkchoiceTracker { self.observe_finalization(head.round, head.digest); } - Some(FinalityPlan { - preferred: self.desired(), - block_anchor: ForkchoiceTargets::anchored(candidate), - }) + Some(self.desired()) } pub(super) fn next_head_update(&self, heartbeat_due: bool) -> Option { @@ -265,7 +247,7 @@ mod tests { } #[test] - fn roundless_block_builds_finality_plan_without_moving_certified_head() { + fn roundless_block_advances_finality_without_moving_certified_head() { let current = execution_header(100, None, digest(1)); let mut tracker = ForkchoiceTracker::new(¤t); let certified = digest(3); @@ -273,28 +255,19 @@ mod tests { let block = execution_block(101, None); let block_digest = block.digest(); - let plan = tracker + let forkchoice = tracker .observe_block(&block) .expect("a later block should require finality work"); assert_eq!( - plan, - FinalityPlan { - preferred: ForkchoiceTargets { - head: certified, - finalized: block_digest, - }, - block_anchor: ForkchoiceTargets { - head: block_digest, - finalized: block_digest, - }, + forkchoice, + ForkchoiceTargets { + head: certified, + finalized: block_digest, } ); - tracker.note_submitted(plan.block_anchor); - assert_eq!(tracker.next_head_update(false), Some(plan.preferred)); - - tracker.note_submitted(plan.preferred); + tracker.note_submitted(forkchoice); assert_eq!(tracker.next_head_update(false), None); } } diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index 16e0fd0231..5593d671fa 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -4,6 +4,8 @@ mod utils; use std::{num::NonZeroU64, time::Duration}; +use super::{Config, init}; +use crate::consensus::Digest; use alloy_primitives::B256; use alloy_rpc_types_engine::ForkchoiceState; use commonware_consensus::{ @@ -14,10 +16,6 @@ use commonware_consensus::{ use commonware_macros::test_traced; use commonware_runtime::{Clock as _, Runner as _, Supervisor as _, deterministic}; use commonware_utils::{Acknowledgement as _, acknowledgement::Exact}; -use futures::FutureExt as _; - -use super::{Config, init}; -use crate::consensus::Digest; use utils::{ StubExecutionProvider, StubMarshal, make_block, make_block_at_round, make_roundless_block, }; @@ -449,57 +447,7 @@ fn certificate_advances_head_before_block_advances_finality() { } #[test_traced] -fn syncing_certificate_head_submits_block_anchor_once() { - deterministic::Runner::default().start(|context| async move { - let provider = StubExecutionProvider::default(); - let release_head_forkchoice = provider.pause_next_forkchoice(); - - let (actor, mut mailbox) = init( - context.child("follower_executor"), - Config { - execution_provider: provider.clone(), - execution_engine: provider.clone(), - marshal: StubMarshal::default(), - epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - fcu_heartbeat_interval: Duration::from_secs(60), - }, - ); - actor.start(); - - let future_head = digest(9); - mailbox.finalization(round(2), future_head); - wait_until(&context, || provider.forkchoices().len() == 1).await; - - provider.set_forkchoices_syncing(true); - let block = make_block_at_round(1, B256::ZERO, round(1)); - let finalized = block.block_hash(); - let (ack, waiter) = Exact::handle(); - assert!(mailbox.report(Update::Block(block.into(), ack)).accepted()); - - release_head_forkchoice - .send(()) - .expect("the head FCU should still be waiting"); - waiter - .await - .expect("submitted block anchor should acknowledge the durable block"); - - wait_until(&context, || provider.forkchoices().len() == 4).await; - - assert_eq!(provider.payload_count(), 1); - assert_eq!( - provider.forkchoices(), - vec![ - forkchoice(future_head.0, B256::ZERO), - forkchoice(future_head.0, finalized), - forkchoice(finalized, finalized), - forkchoice(future_head.0, finalized), - ] - ); - }); -} - -#[test_traced] -fn valid_preferred_fcu_skips_block_anchor() { +fn certificate_head_is_preserved_when_block_advances_finality() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -537,62 +485,6 @@ fn valid_preferred_fcu_skips_block_anchor() { }); } -#[test_traced] -fn stale_block_ack_precedes_syncing_head_guidance() { - deterministic::Runner::default().start(|context| async move { - let current = digest(10); - let provider = StubExecutionProvider::default(); - provider.set_finalized(100, current.0, round(10)); - provider.set_forkchoices_syncing(true); - let release_head_forkchoice = provider.pause_next_forkchoice(); - - let (actor, mut mailbox) = init( - context.child("follower_executor"), - Config { - execution_provider: provider.clone(), - execution_engine: provider.clone(), - marshal: StubMarshal::default(), - epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), - fcu_heartbeat_interval: Duration::from_secs(60), - }, - ); - actor.start(); - - let latest_head = digest(12); - let stale_block = make_block_at_round(99, digest(98).0, round(9)); - let (ack, waiter) = Exact::handle(); - assert!( - mailbox - .report(Update::Block(stale_block.into(), ack)) - .accepted() - ); - mailbox.finalization(round(12), latest_head); - wait_until(&context, || provider.forkchoices().len() == 1).await; - - waiter - .now_or_never() - .expect("stale block acknowledgement should precede head guidance") - .expect("stale block should be acknowledged"); - - provider.set_forkchoices_syncing(false); - release_head_forkchoice - .send(()) - .expect("the head FCU should still be waiting"); - - let newer_head = digest(13); - mailbox.finalization(round(13), newer_head); - wait_until(&context, || provider.forkchoices().len() == 2).await; - - assert_eq!( - provider.forkchoices(), - vec![ - forkchoice(latest_head.0, current.0), - forkchoice(newer_head.0, current.0), - ] - ); - }); -} - #[test_traced] fn delayed_certificate_does_not_regress_newer_block_forkchoice() { deterministic::Runner::default().start(|context| async move { diff --git a/crates/consensus/src/follow/executor/test/utils.rs b/crates/consensus/src/follow/executor/test/utils.rs index 54224da806..a5c69bc845 100644 --- a/crates/consensus/src/follow/executor/test/utils.rs +++ b/crates/consensus/src/follow/executor/test/utils.rs @@ -78,7 +78,6 @@ struct StubExecutionProviderInner { forkchoices: Mutex>, reject_payloads: AtomicBool, reject_forkchoices: AtomicBool, - sync_forkchoices: AtomicBool, forkchoice_gate: Mutex>>, } @@ -111,10 +110,6 @@ impl StubExecutionProvider { self.inner.reject_forkchoices.store(true, Ordering::SeqCst); } - pub(super) fn set_forkchoices_syncing(&self, syncing: bool) { - self.inner.sync_forkchoices.store(syncing, Ordering::SeqCst); - } - pub(super) fn pause_next_forkchoice(&self) -> oneshot::Sender<()> { let (release, gate) = oneshot::channel(); *self.inner.forkchoice_gate.lock() = Some(gate); @@ -191,7 +186,6 @@ impl ExecutionEngine for StubExecutionProvider { self.inner.forkchoices.lock().push(state); let gate = self.inner.forkchoice_gate.lock().take(); let rejected = self.inner.reject_forkchoices.load(Ordering::SeqCst); - let syncing = self.inner.sync_forkchoices.load(Ordering::SeqCst); async move { if let Some(gate) = gate { let _ = gate.await; @@ -200,8 +194,6 @@ impl ExecutionEngine for StubExecutionProvider { PayloadStatusEnum::Invalid { validation_error: "rejected by test engine".into(), } - } else if syncing { - PayloadStatusEnum::Syncing } else { PayloadStatusEnum::Valid }; From 44a06055f8331d04339e1e087b1a6fc854449d58 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 12:35:06 -0400 Subject: [PATCH 08/14] marshal alias crate change no longer needed --- crates/consensus/src/alias.rs | 34 +--------------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/crates/consensus/src/alias.rs b/crates/consensus/src/alias.rs index 22cffd8fb3..684bb04d17 100644 --- a/crates/consensus/src/alias.rs +++ b/crates/consensus/src/alias.rs @@ -249,10 +249,7 @@ pub(crate) mod marshal { let execution_finalized = execution_finalized_point(execution_node); match archive_range { - Some((floor, tip)) => { - validate_archive_tip(tip, execution_finalized)?; - Ok(FinalizationRange { floor, tip }) - } + Some((floor, tip)) => Ok(FinalizationRange { floor, tip }), None if execution_finalized.0.is_zero() => Ok(FinalizationRange { floor: execution_finalized, // Genesis is not finalized in any round; the zero round @@ -273,35 +270,6 @@ pub(crate) mod marshal { } } - fn validate_archive_tip( - archive_tip: (Round, Height, Digest), - execution_finalized: (Height, Digest), - ) -> eyre::Result<()> { - let (_, archive_height, archive_digest) = archive_tip; - let (execution_height, execution_digest) = execution_finalized; - - ensure!( - archive_height >= execution_height, - "finalized certificate archive tip height `{}` is below execution finalized height \ - `{}`; restore consensus storage that covers execution state or reset execution state", - archive_height, - execution_height, - ); - - if archive_height == execution_height { - ensure!( - archive_digest == execution_digest, - "finalized certificate archive tip digest `{}` does not match execution finalized \ - digest `{}` at height `{}`; restore matching consensus and execution state", - archive_digest, - execution_digest, - archive_height, - ); - } - - Ok(()) - } - async fn finalized_archive_range( archive: &immutable::Archive< TContext, From ca64820d626dcdf22bbbc4a895fe017feefbf916 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 12:38:50 -0400 Subject: [PATCH 09/14] use From for Forkchoice conversion --- crates/consensus/src/follow/executor/actor.rs | 12 +++---- crates/consensus/src/follow/executor/fcu.rs | 32 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index 3e74d5c75b..43d6003f0e 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -20,7 +20,7 @@ use tracing::{Level, debug, error, instrument}; use super::{ Config, ExecutionEngine, FinalizedBlockProvider, Marshal, - fcu::{ForkchoiceTargets, ForkchoiceTracker}, + fcu::{Forkchoice, ForkchoiceTracker}, ingress::Message, }; use crate::{consensus::block::Block, utils::OptionFuture}; @@ -230,12 +230,12 @@ where } } -type ExecutionTaskResult = eyre::Result>; +type ExecutionTaskResult = eyre::Result>; async fn execute_head_update( context: TContext, execution_engine: E, - forkchoice: ForkchoiceTargets, + forkchoice: Forkchoice, ) -> ExecutionTaskResult { submit_forkchoice_update(&context, &execution_engine, &forkchoice).await?; Ok(Some(forkchoice)) @@ -245,7 +245,7 @@ async fn execute_block( context: TContext, execution_engine: E, block: Block, - forkchoice: Option, + forkchoice: Option, ack: Exact, ) -> ExecutionTaskResult { submit_new_payload(&context, &execution_engine, block).await?; @@ -300,9 +300,9 @@ async fn submit_new_payload( async fn submit_forkchoice_update( context: &TContext, execution_engine: &E, - forkchoice: &ForkchoiceTargets, + forkchoice: &Forkchoice, ) -> eyre::Result<()> { - let forkchoice = forkchoice.rpc_state(); + let forkchoice = (*forkchoice).into(); let response = execution_engine .fork_choice_updated(forkchoice, None) diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs index c4fbb690ee..e346e27d4a 100644 --- a/crates/consensus/src/follow/executor/fcu.rs +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -58,23 +58,23 @@ impl FinalityTarget { /// Engine API targets. Safe always follows finalized. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) struct ForkchoiceTargets { +pub(super) struct Forkchoice { pub(super) head: Digest, pub(super) finalized: Digest, } -impl ForkchoiceTargets { - pub(super) fn rpc_state(self) -> ForkchoiceState { - ForkchoiceState { - head_block_hash: self.head.0, - safe_block_hash: self.finalized.0, - finalized_block_hash: self.finalized.0, +impl From for ForkchoiceState { + fn from(forkchoice: Forkchoice) -> Self { + Self { + head_block_hash: forkchoice.head.0, + safe_block_hash: forkchoice.finalized.0, + finalized_block_hash: forkchoice.finalized.0, } } } pub(super) struct ForkchoiceTracker { - submitted: ForkchoiceTargets, + submitted: Forkchoice, certified_head: Option, finalized: FinalityTarget, } @@ -101,7 +101,7 @@ impl ForkchoiceTracker { } } - pub(super) fn observe_block(&mut self, block: &Block) -> Option { + pub(super) fn observe_block(&mut self, block: &Block) -> Option { let header = block.block().sealed_header(); let candidate = FinalityTarget::from_header(header); if !candidate.supersedes(self.finalized) { @@ -118,21 +118,21 @@ impl ForkchoiceTracker { Some(self.desired()) } - pub(super) fn next_head_update(&self, heartbeat_due: bool) -> Option { + pub(super) fn next_head_update(&self, heartbeat_due: bool) -> Option { let desired = self.desired(); (desired != self.submitted || heartbeat_due).then_some(desired) } - pub(super) fn note_submitted(&mut self, submitted: ForkchoiceTargets) { + pub(super) fn note_submitted(&mut self, submitted: Forkchoice) { self.submitted = submitted; } - fn desired(&self) -> ForkchoiceTargets { + fn desired(&self) -> Forkchoice { Self::targets(self.certified_head, self.finalized) } - fn targets(head: Option, finalized: FinalityTarget) -> ForkchoiceTargets { - ForkchoiceTargets { + fn targets(head: Option, finalized: FinalityTarget) -> Forkchoice { + Forkchoice { head: head.map_or(finalized.digest, |target| target.digest), finalized: finalized.digest, } @@ -239,7 +239,7 @@ mod tests { assert_eq!( tracker.next_head_update(false), - Some(ForkchoiceTargets { + Some(Forkchoice { head: digest(2), finalized: digest(1), }) @@ -261,7 +261,7 @@ mod tests { assert_eq!( forkchoice, - ForkchoiceTargets { + Forkchoice { head: certified, finalized: block_digest, } From 8f17efe28578e16b8acc2beb83c844327525ac9c Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 12:45:19 -0400 Subject: [PATCH 10/14] collapse ForkchoiceTracker into just Forkchoice --- crates/consensus/src/follow/executor/actor.rs | 33 +-- crates/consensus/src/follow/executor/fcu.rs | 201 ++++++------------ 2 files changed, 80 insertions(+), 154 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index 43d6003f0e..fd0238a133 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -19,9 +19,7 @@ use tempo_node::TempoExecutionData; use tracing::{Level, debug, error, instrument}; use super::{ - Config, ExecutionEngine, FinalizedBlockProvider, Marshal, - fcu::{Forkchoice, ForkchoiceTracker}, - ingress::Message, + Config, ExecutionEngine, FinalizedBlockProvider, Marshal, fcu::Forkchoice, ingress::Message, }; use crate::{consensus::block::Block, utils::OptionFuture}; @@ -35,7 +33,8 @@ pub(crate) struct Actor { epoch_strategy: FixedEpocher, - forkchoice: ForkchoiceTracker, + last_fcu: Forkchoice, + latest_fcu: Forkchoice, block_queue: VecDeque<(Block, Exact)>, floor_candidate: Option, @@ -69,7 +68,7 @@ where let finalized_header = execution_provider .finalized_header() .expect("failed reading finalized execution header"); - let forkchoice = ForkchoiceTracker::new(&finalized_header); + let forkchoice = Forkchoice::new(&finalized_header); Self { context: ContextCell::new(context), @@ -80,7 +79,8 @@ where execution_provider, execution_engine, - forkchoice, + last_fcu: forkchoice, + latest_fcu: forkchoice, block_queue: VecDeque::new(), floor_candidate: None, execution_task: OptionFuture::none(), @@ -110,7 +110,7 @@ where match result { Ok(submitted_fcu) => { if let Some(submitted_fcu) = submitted_fcu { - self.forkchoice.note_submitted(submitted_fcu); + self.last_fcu = submitted_fcu; } // Floor advancement retries after the next completed execution task. @@ -136,10 +136,10 @@ where if self.floor_candidate.is_none() { self.floor_candidate = Some(height); } - self.forkchoice.observe_finalization(round, digest); + self.latest_fcu.update_head(round, digest); } Message::Finalization { round, digest } => { - self.forkchoice.observe_finalization(round, digest); + self.latest_fcu.update_head(round, digest); } } } @@ -169,10 +169,17 @@ where let execution_engine = self.execution_engine.clone(); let task = if let Some((block, ack)) = self.block_queue.pop_front() { - let forkchoice = self.forkchoice.observe_block(&block); + let forkchoice = self + .latest_fcu + .update_finalized(&block) + .then_some(self.latest_fcu); let context = self.context.child("execute_block"); execute_block(context, execution_engine, block, forkchoice, ack).boxed() - } else if let Some(forkchoice) = self.forkchoice.next_head_update(heartbeat) { + } else if self.latest_fcu.head_digest() != self.last_fcu.head_digest() + || self.latest_fcu.finalized_digest() != self.last_fcu.finalized_digest() + || heartbeat + { + let forkchoice = self.latest_fcu; let context = self.context.child("execute_head_update"); execute_head_update(context, execution_engine, forkchoice).boxed() } else { @@ -293,8 +300,8 @@ async fn submit_new_payload( #[instrument( skip_all, fields( - head.digest = %forkchoice.head, - finalized.digest = %forkchoice.finalized, + head.digest = %forkchoice.head_digest(), + finalized.digest = %forkchoice.finalized_digest(), ) )] async fn submit_forkchoice_update( diff --git a/crates/consensus/src/follow/executor/fcu.rs b/crates/consensus/src/follow/executor/fcu.rs index e346e27d4a..7488a59446 100644 --- a/crates/consensus/src/follow/executor/fcu.rs +++ b/crates/consensus/src/follow/executor/fcu.rs @@ -12,129 +12,63 @@ use crate::consensus::{ block::{Block, round_from_context}, }; +/// Follower forkchoice state. Safe always follows finalized. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -struct CertifiedHead { - round: Round, - digest: Digest, -} - -impl CertifiedHead { - fn from_header(header: &SealedHeader) -> Option { - let round = header.consensus_context.map(round_from_context)?; - Some(Self { - round, - digest: Digest(header.hash()), - }) - } - - const fn from_certificate(round: Round, digest: Digest) -> Self { - Self { round, digest } - } - - fn supersedes(self, current: Self) -> bool { - self.round > current.round - } -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -struct FinalityTarget { - height: Height, - digest: Digest, +pub(super) struct Forkchoice { + head: (Option, Digest), + finalized: (Height, Digest), } -impl FinalityTarget { - fn from_header(header: &SealedHeader) -> Self { +impl Forkchoice { + pub(super) fn new(header: &SealedHeader) -> Self { let tip = header.num_hash(); + let digest = Digest(tip.hash); Self { - height: Height::new(tip.number), - digest: Digest(tip.hash), + head: (header.consensus_context.map(round_from_context), digest), + finalized: (Height::new(tip.number), digest), } } - fn supersedes(self, current: Self) -> bool { - self.height > current.height - } -} - -/// Engine API targets. Safe always follows finalized. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) struct Forkchoice { - pub(super) head: Digest, - pub(super) finalized: Digest, -} - -impl From for ForkchoiceState { - fn from(forkchoice: Forkchoice) -> Self { - Self { - head_block_hash: forkchoice.head.0, - safe_block_hash: forkchoice.finalized.0, - finalized_block_hash: forkchoice.finalized.0, - } + pub(super) const fn head_digest(self) -> Digest { + self.head.1 } -} -pub(super) struct ForkchoiceTracker { - submitted: Forkchoice, - certified_head: Option, - finalized: FinalityTarget, -} - -impl ForkchoiceTracker { - pub(super) fn new(header: &SealedHeader) -> Self { - let certified_head = CertifiedHead::from_header(header); - let finalized = FinalityTarget::from_header(header); - let submitted = Self::targets(certified_head, finalized); - Self { - submitted, - certified_head, - finalized, - } + pub(super) const fn finalized_digest(self) -> Digest { + self.finalized.1 } - pub(super) fn observe_finalization(&mut self, round: Round, digest: Digest) { - let candidate = CertifiedHead::from_certificate(round, digest); - if self - .certified_head - .is_none_or(|current| candidate.supersedes(current)) - { - self.certified_head = Some(candidate); + pub(super) fn update_head(&mut self, round: Round, digest: Digest) { + if self.head.0.is_none_or(|current| round > current) { + self.head = (Some(round), digest); } } - pub(super) fn observe_block(&mut self, block: &Block) -> Option { + pub(super) fn update_finalized(&mut self, block: &Block) -> bool { let header = block.block().sealed_header(); - let candidate = FinalityTarget::from_header(header); - if !candidate.supersedes(self.finalized) { - // Replayed blocks cannot move the certified head, even when their header has a later - // round. - return None; + let tip = header.num_hash(); + let height = Height::new(tip.number); + if height <= self.finalized.0 { + return false; } - self.finalized = candidate; - if let Some(head) = CertifiedHead::from_header(header) { - self.observe_finalization(head.round, head.digest); + let digest = Digest(tip.hash); + self.finalized = (height, digest); + if let Some(round) = header.consensus_context.map(round_from_context) { + self.update_head(round, digest); + } else if self.head.0.is_none() { + self.head.1 = digest; } - Some(self.desired()) - } - - pub(super) fn next_head_update(&self, heartbeat_due: bool) -> Option { - let desired = self.desired(); - (desired != self.submitted || heartbeat_due).then_some(desired) - } - - pub(super) fn note_submitted(&mut self, submitted: Forkchoice) { - self.submitted = submitted; - } - - fn desired(&self) -> Forkchoice { - Self::targets(self.certified_head, self.finalized) + true } +} - fn targets(head: Option, finalized: FinalityTarget) -> Forkchoice { - Forkchoice { - head: head.map_or(finalized.digest, |target| target.digest), - finalized: finalized.digest, +impl From for ForkchoiceState { + fn from(forkchoice: Forkchoice) -> Self { + Self { + head_block_hash: forkchoice.head_digest().0, + safe_block_hash: forkchoice.finalized_digest().0, + finalized_block_hash: forkchoice.finalized_digest().0, } } } @@ -195,79 +129,64 @@ mod tests { #[test] fn later_round_supersedes_earlier_round() { - let newer = CertifiedHead::from_certificate(round(9), digest(1)); - let older = CertifiedHead::from_certificate(round(8), digest(2)); - assert!(newer.supersedes(older)); - assert!(!older.supersedes(newer)); + let mut forkchoice = Forkchoice::new(&execution_header(100, Some(round(8)), digest(1))); + forkchoice.update_head(round(9), digest(2)); + assert_eq!(forkchoice.head, (Some(round(9)), digest(2))); + + forkchoice.update_head(round(8), digest(3)); + assert_eq!(forkchoice.head, (Some(round(9)), digest(2))); } #[test] fn equal_round_does_not_supersede() { - let current = CertifiedHead::from_certificate(round(8), digest(1)); - let conflicting = CertifiedHead::from_certificate(round(8), digest(2)); - assert!(!conflicting.supersedes(current)); + let mut forkchoice = Forkchoice::new(&execution_header(100, Some(round(8)), digest(1))); + forkchoice.update_head(round(8), digest(2)); + assert_eq!(forkchoice.head, (Some(round(8)), digest(1))); } #[test] fn roundless_header_has_no_certified_head() { let header = execution_header(100, None, digest(1)); - assert_eq!(CertifiedHead::from_header(&header), None); + assert_eq!(Forkchoice::new(&header).head, (None, digest(1))); } #[test] fn certified_head_uses_header_round() { let header = execution_header(100, Some(round(2)), digest(1)); - assert_eq!(CertifiedHead::from_header(&header).unwrap().round, round(2)); + assert_eq!(Forkchoice::new(&header).head, (Some(round(2)), digest(1))); } #[test] - fn later_block_height_supersedes_finality_target() { - let current = FinalityTarget::from_header(&execution_header(100, None, digest(1))); - let conflicting = FinalityTarget::from_header(&execution_header(100, None, digest(3))); - let newer = FinalityTarget::from_header(&execution_header(101, None, digest(2))); - assert!(newer.supersedes(current)); - assert!(!current.supersedes(newer)); - assert!(!conflicting.supersedes(current)); + fn later_block_height_advances_finality() { + let mut forkchoice = Forkchoice::new(&execution_header(100, None, digest(1))); + assert!(!forkchoice.update_finalized(&execution_block(100, None))); + assert!(forkchoice.update_finalized(&execution_block(101, None))); + assert_eq!(forkchoice.finalized.0, Height::new(101)); } #[test] fn certificate_advances_only_head() { let current = execution_header(100, Some(round(1)), digest(1)); - let mut tracker = ForkchoiceTracker::new(¤t); + let mut forkchoice = Forkchoice::new(¤t); - tracker.observe_finalization(round(2), digest(2)); + forkchoice.update_head(round(2), digest(2)); - assert_eq!( - tracker.next_head_update(false), - Some(Forkchoice { - head: digest(2), - finalized: digest(1), - }) - ); + assert_eq!(forkchoice.head, (Some(round(2)), digest(2))); + assert_eq!(forkchoice.finalized, (Height::new(100), digest(1))); } #[test] fn roundless_block_advances_finality_without_moving_certified_head() { let current = execution_header(100, None, digest(1)); - let mut tracker = ForkchoiceTracker::new(¤t); + let mut forkchoice = Forkchoice::new(¤t); let certified = digest(3); - tracker.observe_finalization(round(3), certified); + forkchoice.update_head(round(3), certified); let block = execution_block(101, None); let block_digest = block.digest(); - let forkchoice = tracker - .observe_block(&block) - .expect("a later block should require finality work"); - - assert_eq!( - forkchoice, - Forkchoice { - head: certified, - finalized: block_digest, - } - ); + assert!(forkchoice.update_finalized(&block)); - tracker.note_submitted(forkchoice); - assert_eq!(tracker.next_head_update(false), None); + assert_eq!(forkchoice.head_digest(), certified); + assert_eq!(forkchoice.finalized_digest(), block_digest); } } From 1fbfd91af9b8a651a1065b1fb5a65449100142e0 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 12:50:26 -0400 Subject: [PATCH 11/14] revert the rest of the changes that are no longer necessary --- crates/consensus/src/follow/engine.rs | 1 + crates/consensus/src/follow/executor/actor.rs | 116 +++++++++++------- crates/consensus/src/follow/executor/mod.rs | 8 +- .../consensus/src/follow/executor/test/mod.rs | 97 +++++++++++---- .../src/follow/executor/test/utils.rs | 2 +- 5 files changed, 153 insertions(+), 71 deletions(-) diff --git a/crates/consensus/src/follow/engine.rs b/crates/consensus/src/follow/engine.rs index 7891798d81..63ce96a909 100644 --- a/crates/consensus/src/follow/engine.rs +++ b/crates/consensus/src/follow/engine.rs @@ -164,6 +164,7 @@ impl Config { .clone(), marshal: marshal_mailbox.clone(), epoch_strategy: epoch_strategy.clone(), + floor: last_finalized_height, fcu_heartbeat_interval: self.fcu_heartbeat_interval, }, ); diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index fd0238a133..9dd98837eb 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -1,8 +1,13 @@ //! Execution-layer driver for follower nodes. //! -//! This actor imports marshal-delivered blocks, applies finality for advancing heights, and -//! refreshes forkchoice on a timer. It moves marshal's floor behind Reth's finalized state. Reth -//! sync and marshal gap repair fill missing history. +//! This actor sends verified finalized tips to Reth as head and marshal-delivered blocks as safe +//! and finalized forkchoice updates, periodically refreshes that forkchoice with a heartbeat, and +//! advances marshal's floor to one epoch behind Reth's finalized state. +//! +//! Unlike the executor used by validator nodes, it does not build payloads, canonicalize proposal +//! heads, or track blocks proposed by this node. Followers receive complete blocks from their +//! upstream, submit them to Reth as finalized payloads, and rely on Reth's sync machinery plus +//! marshal gap repair to fill history. use std::{collections::VecDeque, time::Duration}; @@ -32,6 +37,7 @@ pub(crate) struct Actor { marshal: M, epoch_strategy: FixedEpocher, + floor: Height, last_fcu: Forkchoice, latest_fcu: Forkchoice, @@ -62,6 +68,7 @@ where execution_engine, marshal, epoch_strategy, + floor, fcu_heartbeat_interval, } = config; @@ -76,6 +83,7 @@ where mailbox, marshal, epoch_strategy, + floor, execution_provider, execution_engine, @@ -108,15 +116,13 @@ where result = &mut self.execution_task => { self.execution_task = OptionFuture::none(); match result { - Ok(submitted_fcu) => { - if let Some(submitted_fcu) = submitted_fcu { - self.last_fcu = submitted_fcu; - } + ExecutionTaskResult::Completed(last_fcu) => { + self.last_fcu = last_fcu; - // Floor advancement retries after the next completed execution task. + // Emits an event on error. let _: Result<_, _> = self.try_advance_floor().await; } - Err(error) => { + ExecutionTaskResult::Fatal(error) => { error!(%error, "execution task failed"); break; } @@ -129,10 +135,9 @@ where self.block_queue.push_back(((*block).clone(), ack)); } + // A Tip update has a persisted finalization, so it can + // start a new floor cycle. Message::Update(Update::Tip(round, height, digest)) => { - // Marshal reports known finalized tips before it completes gap-free - // block delivery. The certificate can guide the head while finalized - // follows delivered blocks. if self.floor_candidate.is_none() { self.floor_candidate = Some(height); } @@ -151,6 +156,11 @@ where } } + fn should_send_forkchoice(&self) -> bool { + self.latest_fcu.head_digest() != self.last_fcu.head_digest() + || self.latest_fcu.finalized_digest() != self.last_fcu.finalized_digest() + } + fn update_fcu_heartbeat_timer(&mut self) { if self.execution_task.is_none() && self.block_queue.is_empty() { if self.fcu_heartbeat_timer.is_none() { @@ -167,26 +177,23 @@ where return; } - let execution_engine = self.execution_engine.clone(); - let task = if let Some((block, ack)) = self.block_queue.pop_front() { + let request = if let Some((block, ack)) = self.block_queue.pop_front() { let forkchoice = self .latest_fcu .update_finalized(&block) .then_some(self.latest_fcu); - let context = self.context.child("execute_block"); - execute_block(context, execution_engine, block, forkchoice, ack).boxed() - } else if self.latest_fcu.head_digest() != self.last_fcu.head_digest() - || self.latest_fcu.finalized_digest() != self.last_fcu.finalized_digest() - || heartbeat - { - let forkchoice = self.latest_fcu; - let context = self.context.child("execute_head_update"); - execute_head_update(context, execution_engine, forkchoice).boxed() + ExecutionRequest::Block(block, forkchoice, ack) + } else if self.should_send_forkchoice() || heartbeat { + ExecutionRequest::Forkchoice(self.latest_fcu) } else { return; }; - self.execution_task.replace(task); + let last_fcu = self.last_fcu; + let context = self.context.child("execute_request"); + let execution_engine = self.execution_engine.clone(); + self.execution_task + .replace(execute_request(context, execution_engine, last_fcu, request).boxed()); } #[instrument(skip_all, err(level = Level::WARN))] @@ -231,38 +238,56 @@ where self.marshal.set_floor(finalization); + self.floor = floor_height; self.floor_candidate = None; Ok(()) } } -type ExecutionTaskResult = eyre::Result>; +enum ExecutionRequest { + Forkchoice(Forkchoice), + Block(Block, Option, Exact), +} -async fn execute_head_update( - context: TContext, - execution_engine: E, - forkchoice: Forkchoice, -) -> ExecutionTaskResult { - submit_forkchoice_update(&context, &execution_engine, &forkchoice).await?; - Ok(Some(forkchoice)) +enum ExecutionTaskResult { + Completed(Forkchoice), + Fatal(Report), } -async fn execute_block( +async fn execute_request( context: TContext, execution_engine: E, - block: Block, - forkchoice: Option, - ack: Exact, + last_fcu: Forkchoice, + request: ExecutionRequest, ) -> ExecutionTaskResult { - submit_new_payload(&context, &execution_engine, block).await?; + match request { + ExecutionRequest::Forkchoice(forkchoice) => { + match submit_forkchoice_update(&context, &execution_engine, &forkchoice).await { + Ok(()) => ExecutionTaskResult::Completed(forkchoice), + Err(error) => ExecutionTaskResult::Fatal(error), + } + } + ExecutionRequest::Block(block, forkchoice, ack) => { + if let Err(error) = submit_new_payload(&context, &execution_engine, block).await { + return ExecutionTaskResult::Fatal(error); + } - if let Some(forkchoice) = &forkchoice { - submit_forkchoice_update(&context, &execution_engine, forkchoice).await?; - } + let last_fcu = if let Some(forkchoice) = forkchoice { + if let Err(error) = + submit_forkchoice_update(&context, &execution_engine, &forkchoice).await + { + return ExecutionTaskResult::Fatal(error); + } + forkchoice + } else { + last_fcu + }; - ack.acknowledge(); - Ok(forkchoice) + ack.acknowledge(); + ExecutionTaskResult::Completed(last_fcu) + } + } } #[instrument( @@ -280,17 +305,16 @@ async fn submit_new_payload( .new_payload(TempoExecutionData { block, block_access_list, - // Marshal delivers blocks after consensus finality checks, so this payload needs no - // validator set. + // can be omitted for finalized blocks validator_set: None, }) .pace(context, Duration::from_millis(20)) .await - .wrap_err("failed sending delivered payload")?; + .wrap_err("failed sending finalized payload")?; ensure!( payload_status.is_valid() || payload_status.is_syncing(), - "payload status of delivered block was neither valid nor syncing: \ + "payload status of finalized block was neither valid nor syncing: \ `{payload_status}`" ); diff --git a/crates/consensus/src/follow/executor/mod.rs b/crates/consensus/src/follow/executor/mod.rs index 39fbe59294..b1203abcfe 100644 --- a/crates/consensus/src/follow/executor/mod.rs +++ b/crates/consensus/src/follow/executor/mod.rs @@ -1,7 +1,8 @@ //! Execution-layer synchronization for follow mode. //! -//! Verified certificates guide the execution head by round. Marshal's gap-free block stream -//! advances safe and finalized by height. Durable execution progress moves marshal's floor. +//! This is intentionally smaller than the validator executor: it receives +//! already-verified finalized tips, drives forkchoice updates, and advances +//! marshal's floor after execution-layer progress is durable. use std::future::Future; @@ -46,6 +47,7 @@ pub(crate) struct Config { pub(crate) execution_engine: E, pub(crate) marshal: M, pub(crate) epoch_strategy: FixedEpocher, + pub(crate) floor: Height, pub(crate) fcu_heartbeat_interval: std::time::Duration, } @@ -75,7 +77,7 @@ pub(crate) trait FinalizedBlockProvider: Send + Sync { /// Engine commands issued by the follower executor. pub(crate) trait ExecutionEngine: Send + Sync { - /// Submit a payload for a block delivered after consensus finality. + /// Submit a finalized execution payload. fn new_payload( &self, payload: TempoExecutionData, diff --git a/crates/consensus/src/follow/executor/test/mod.rs b/crates/consensus/src/follow/executor/test/mod.rs index 5593d671fa..6da2fcb8ef 100644 --- a/crates/consensus/src/follow/executor/test/mod.rs +++ b/crates/consensus/src/follow/executor/test/mod.rs @@ -4,8 +4,6 @@ mod utils; use std::{num::NonZeroU64, time::Duration}; -use super::{Config, init}; -use crate::consensus::Digest; use alloy_primitives::B256; use alloy_rpc_types_engine::ForkchoiceState; use commonware_consensus::{ @@ -16,6 +14,9 @@ use commonware_consensus::{ use commonware_macros::test_traced; use commonware_runtime::{Clock as _, Runner as _, Supervisor as _, deterministic}; use commonware_utils::{Acknowledgement as _, acknowledgement::Exact}; + +use super::{Config, init}; +use crate::consensus::Digest; use utils::{ StubExecutionProvider, StubMarshal, make_block, make_block_at_round, make_roundless_block, }; @@ -53,7 +54,7 @@ fn forkchoice(head: B256, finalized: B256) -> ForkchoiceState { } #[test_traced] -fn delivered_block_advances_execution_finality_and_marshal_floor() { +fn block_is_executed_canonicalized_acknowledged_and_advances_floor_to_deep_candidate() { deterministic::Runner::default().start(|context| async move { let finalized_height = EPOCH_LENGTH.get() * 2; let expected_floor = finalized_height - EPOCH_LENGTH.get() - 1; @@ -71,6 +72,7 @@ fn delivered_block_advances_execution_finality_and_marshal_floor() { execution_engine: provider.clone(), marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -101,7 +103,11 @@ fn delivered_block_advances_execution_finality_and_marshal_floor() { assert_eq!(provider.payload_count(), 1); assert_eq!( provider.forkchoices(), - vec![forkchoice(block_hash, block_hash)] + vec![alloy_rpc_types_engine::ForkchoiceState { + head_block_hash: block_hash, + safe_block_hash: block_hash, + finalized_block_hash: block_hash, + }] ); }); } @@ -121,6 +127,7 @@ fn floor_candidate_uses_execution_depth_and_next_tip_starts_new_cycle() { execution_engine: provider.clone(), marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -181,7 +188,7 @@ fn floor_candidate_uses_execution_depth_and_next_tip_starts_new_cycle() { } #[test_traced] -fn stale_block_with_higher_round_does_not_regress_forkchoice() { +fn block_at_or_below_finalized_tip_does_not_regress_forkchoice() { deterministic::Runner::default().start(|context| async move { let finalized_height = EPOCH_LENGTH.get(); let provider = StubExecutionProvider::default(); @@ -194,6 +201,7 @@ fn stale_block_with_higher_round_does_not_regress_forkchoice() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -215,7 +223,7 @@ fn roundless_block_advances_finality_by_height() { deterministic::Runner::default().start(|context| async move { let current = B256::with_last_byte(100); let provider = StubExecutionProvider::default(); - provider.set_finalized_without_round(100, current); + provider.set_prefork_finalized(100, current); let (actor, mut mailbox) = init( context.child("follower_executor"), @@ -224,6 +232,7 @@ fn roundless_block_advances_finality_by_height() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -261,6 +270,7 @@ fn floor_does_not_advance_until_its_execution_block_is_durable() { execution_engine: provider, marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -298,7 +308,7 @@ fn floor_does_not_advance_until_its_execution_block_is_durable() { } #[test_traced] -fn invalid_payload_exits_before_fcu_or_ack() { +fn invalid_payload_exits_without_acknowledging_or_canonicalizing() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); provider.reject_payloads(); @@ -310,6 +320,7 @@ fn invalid_payload_exits_before_fcu_or_ack() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -343,6 +354,7 @@ fn forkchoice_failure_exits_without_acknowledging_block() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -375,6 +387,7 @@ fn tips_are_monotonic_and_coalesced_while_forkchoice_is_in_flight() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -411,8 +424,9 @@ fn tips_are_monotonic_and_coalesced_while_forkchoice_is_in_flight() { }); } +// A finalization can advance forkchoice before execution receives its block. #[test_traced] -fn certificate_advances_head_before_block_advances_finality() { +fn finalization_drives_forkchoice_by_round() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -423,6 +437,7 @@ fn certificate_advances_head_before_block_advances_finality() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -458,6 +473,7 @@ fn certificate_head_is_preserved_when_block_advances_finality() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -485,8 +501,10 @@ fn certificate_head_is_preserved_when_block_advances_finality() { }); } +/// An older finalization received while a block FCU is in flight must not +/// become the next forkchoice target. #[test_traced] -fn delayed_certificate_does_not_regress_newer_block_forkchoice() { +fn delayed_finalization_does_not_regress_newer_block_forkchoice() { deterministic::Runner::default().start(|context| async move { let current = Digest(B256::with_last_byte(10)); let provider = StubExecutionProvider::default(); @@ -500,6 +518,7 @@ fn delayed_certificate_does_not_regress_newer_block_forkchoice() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); @@ -514,8 +533,8 @@ fn delayed_certificate_does_not_regress_newer_block_forkchoice() { }) .await; - let older_certificate = Digest(B256::with_last_byte(12)); - mailbox.finalization(round(12), older_certificate); + let delayed = Digest(B256::with_last_byte(12)); + mailbox.finalization(round(12), delayed); context.sleep(Duration::from_millis(1)).await; release_block_forkchoice @@ -533,7 +552,7 @@ fn delayed_certificate_does_not_regress_newer_block_forkchoice() { } #[test_traced] -fn startup_header_round_orders_certificate_heads() { +fn execution_tip_round_orders_finalizations_after_restart() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); let current = digest(100); @@ -546,6 +565,7 @@ fn startup_header_round_orders_certificate_heads() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -563,11 +583,11 @@ fn startup_header_round_orders_certificate_heads() { } #[test_traced] -fn certificate_supersedes_roundless_startup_head() { +fn finalization_supersedes_roundless_prefork_execution_tip() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); let current = digest(100); - provider.set_finalized_without_round(100, current.0); + provider.set_prefork_finalized(100, current.0); let (actor, mailbox) = init( context.child("follower_executor"), @@ -576,6 +596,7 @@ fn certificate_supersedes_roundless_startup_head() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -595,7 +616,36 @@ fn certificate_supersedes_roundless_startup_head() { } #[test_traced] -fn heartbeat_resubmits_current_forkchoice_after_interval() { +fn finalization_is_driven_to_from_genesis() { + deterministic::Runner::default().start(|context| async move { + let provider = StubExecutionProvider::default(); + + let (actor, mailbox) = init( + context.child("follower_executor"), + Config { + execution_provider: provider.clone(), + execution_engine: provider.clone(), + marshal: StubMarshal::default(), + epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), + fcu_heartbeat_interval: Duration::from_secs(60), + }, + ); + actor.start(); + + let finalized = Digest(B256::with_last_byte(9)); + mailbox.finalization(round(5), finalized); + + wait_until(&context, || !provider.forkchoices().is_empty()).await; + assert_eq!( + provider.forkchoices(), + vec![forkchoice(finalized.0, B256::ZERO)] + ); + }); +} + +#[test_traced] +fn heartbeat_resubmits_latest_tip_after_interval() { deterministic::Runner::default().start(|context| async move { let provider = StubExecutionProvider::default(); @@ -606,6 +656,7 @@ fn heartbeat_resubmits_current_forkchoice_after_interval() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); @@ -621,8 +672,9 @@ fn heartbeat_resubmits_current_forkchoice_after_interval() { assert_eq!(provider.forkchoices().len(), 1); wait_until(&context, || provider.forkchoices().len() == 2).await; - let expected = forkchoice(digest.0, B256::ZERO); - assert_eq!(provider.forkchoices(), vec![expected, expected]); + let forkchoices = provider.forkchoices(); + assert_eq!(forkchoices[0], forkchoices[1]); + assert_eq!(forkchoices[1].head_block_hash, digest.0); }); } @@ -639,6 +691,7 @@ fn heartbeat_waits_for_in_flight_execution() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); @@ -680,6 +733,7 @@ fn durable_block_read_failure_does_not_exit_actor() { execution_engine: provider.clone(), marshal: marshal.clone(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: Duration::from_secs(60), }, ); @@ -721,6 +775,7 @@ fn startup_uses_execution_finalized_tip_without_immediate_forkchoice() { execution_engine: provider.clone(), marshal: StubMarshal::default(), epoch_strategy: FixedEpocher::new(EPOCH_LENGTH), + floor: Height::zero(), fcu_heartbeat_interval: HEARTBEAT_INTERVAL, }, ); @@ -731,9 +786,9 @@ fn startup_uses_execution_finalized_tip_without_immediate_forkchoice() { assert!(provider.forkchoices().is_empty()); wait_until(&context, || !provider.forkchoices().is_empty()).await; - assert_eq!( - provider.forkchoices(), - vec![forkchoice(finalized_hash, finalized_hash)] - ); + let forkchoice = provider.forkchoices()[0]; + assert_eq!(forkchoice.head_block_hash, finalized_hash); + assert_eq!(forkchoice.safe_block_hash, finalized_hash); + assert_eq!(forkchoice.finalized_block_hash, finalized_hash); }); } diff --git a/crates/consensus/src/follow/executor/test/utils.rs b/crates/consensus/src/follow/executor/test/utils.rs index a5c69bc845..d2d1b4b6c5 100644 --- a/crates/consensus/src/follow/executor/test/utils.rs +++ b/crates/consensus/src/follow/executor/test/utils.rs @@ -89,7 +89,7 @@ impl StubExecutionProvider { /// Models a finalized execution header from before TIP-1031, when headers /// had no consensus context and therefore no round. - pub(super) fn set_finalized_without_round(&self, number: u64, hash: B256) { + pub(super) fn set_prefork_finalized(&self, number: u64, hash: B256) { *self.inner.finalized.lock() = BlockNumHash::new(number, hash); *self.inner.finalized_round.lock() = None; } From 1e366f8a4e5608cdecef9d05d429f919e35b0544 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 14:49:37 -0400 Subject: [PATCH 12/14] latest_fcu -> pending_fcu --- crates/consensus/src/follow/executor/actor.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index 9dd98837eb..c2b0b55299 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -40,7 +40,7 @@ pub(crate) struct Actor { floor: Height, last_fcu: Forkchoice, - latest_fcu: Forkchoice, + pending_fcu: Forkchoice, block_queue: VecDeque<(Block, Exact)>, floor_candidate: Option, @@ -88,7 +88,7 @@ where execution_engine, last_fcu: forkchoice, - latest_fcu: forkchoice, + pending_fcu: forkchoice, block_queue: VecDeque::new(), floor_candidate: None, execution_task: OptionFuture::none(), @@ -141,10 +141,10 @@ where if self.floor_candidate.is_none() { self.floor_candidate = Some(height); } - self.latest_fcu.update_head(round, digest); + self.pending_fcu.update_head(round, digest); } Message::Finalization { round, digest } => { - self.latest_fcu.update_head(round, digest); + self.pending_fcu.update_head(round, digest); } } } @@ -157,8 +157,8 @@ where } fn should_send_forkchoice(&self) -> bool { - self.latest_fcu.head_digest() != self.last_fcu.head_digest() - || self.latest_fcu.finalized_digest() != self.last_fcu.finalized_digest() + self.pending_fcu.head_digest() != self.last_fcu.head_digest() + || self.pending_fcu.finalized_digest() != self.last_fcu.finalized_digest() } fn update_fcu_heartbeat_timer(&mut self) { @@ -179,12 +179,12 @@ where let request = if let Some((block, ack)) = self.block_queue.pop_front() { let forkchoice = self - .latest_fcu + .pending_fcu .update_finalized(&block) - .then_some(self.latest_fcu); + .then_some(self.pending_fcu); ExecutionRequest::Block(block, forkchoice, ack) } else if self.should_send_forkchoice() || heartbeat { - ExecutionRequest::Forkchoice(self.latest_fcu) + ExecutionRequest::Forkchoice(self.pending_fcu) } else { return; }; From 8c46984c9fe31db402becb9bfb84f836e50faee7 Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 14:59:35 -0400 Subject: [PATCH 13/14] comment on last/pending fcu --- crates/consensus/src/follow/executor/actor.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index c2b0b55299..e1737482d7 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -39,7 +39,9 @@ pub(crate) struct Actor { epoch_strategy: FixedEpocher, floor: Height, + // The last completed FCU last_fcu: Forkchoice, + // Next FCU to be delivered when it supersed the last pending_fcu: Forkchoice, block_queue: VecDeque<(Block, Exact)>, From 99cc949c8084047c3560a77c757d2d1a8f6d9a8f Mon Sep 17 00:00:00 2001 From: Hamdi Allam Date: Fri, 28 Aug 2026 15:28:33 -0400 Subject: [PATCH 14/14] typo --- crates/consensus/src/follow/executor/actor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/consensus/src/follow/executor/actor.rs b/crates/consensus/src/follow/executor/actor.rs index e1737482d7..5262b19167 100644 --- a/crates/consensus/src/follow/executor/actor.rs +++ b/crates/consensus/src/follow/executor/actor.rs @@ -41,7 +41,7 @@ pub(crate) struct Actor { // The last completed FCU last_fcu: Forkchoice, - // Next FCU to be delivered when it supersed the last + // Next FCU to be delivered when it superseded the last pending_fcu: Forkchoice, block_queue: VecDeque<(Block, Exact)>,