diff --git a/crates/verity-chain/src/fork_choice/duties.rs b/crates/verity-chain/src/fork_choice/duties.rs index 2698385..5dba028 100644 --- a/crates/verity-chain/src/fork_choice/duties.rs +++ b/crates/verity-chain/src/fork_choice/duties.rs @@ -5,13 +5,13 @@ //! at the unsigned [`AttestationData`]. //! //! Transcribed from leanSpec `src/lean_spec/spec/forks/lstar/validator_duties.py`, read at -//! commit `0588c2d215a955a516378677a92db2a5666802f3`. +//! commit `0b7d33ec`, plus leanSpec PR #1207 (issue #1206): the vote's target is raised to +//! the source when the walk lands behind it, which every lean client already did. use verity_types::config::JUSTIFICATION_LOOKBACK_SLOTS; use verity_types::primitives::ZERO_HASH; use verity_types::{AttestationData, Bytes32, Checkpoint, Slot}; -use crate::error::RejectionReason; use crate::fork_choice::store::Store; use crate::justification::is_justifiable_after; @@ -75,13 +75,15 @@ pub fn attestation_target(store: &Store) -> Checkpoint { /// head stands in for it, which at that point is the anchor at slot 0 — the block the /// checkpoint means. /// -/// # Errors -/// -/// [`RejectionReason::SourceAfterTarget`] when the source ends up ahead of the target. -/// leanSpec asserts here; Runtime Shell code must not panic, so the impossible vote is -/// returned as a rejection instead of being cast. +/// The target walk is bounded by the safe target and the finalized checkpoint, never by the +/// justified one, so on a sparse chain the justifiability walk can land behind the source +/// (Verity issue #44 saw the head chain 62 → 59 → 51 → 50 → 49 → 45 → 42 with finalized +/// 42 and justified 49: three steps reach 50, neither 50 nor 49 is justifiable after 42, +/// and the walk stops at 45). A vote with its source after its target is one no peer admits, +/// so the target is raised to the justified checkpoint instead: it is on the head chain, it +/// is justifiable, and the vote keeps its weight in fork choice rather than being dropped. #[must_use = "this produces the vote; signing it is the validator client's job"] -pub fn attestation_data(store: &Store, slot: Slot) -> Result { +pub fn attestation_data(store: &Store, slot: Slot) -> AttestationData { let head = Checkpoint { root: store.head, slot: slot_of(store, store.head).unwrap_or(store.latest_finalized.slot), @@ -99,16 +101,18 @@ pub fn attestation_data(store: &Store, slot: Slot) -> Result target.slot.0 { - return Err(RejectionReason::SourceAfterTarget); - } + let target = if source.slot.0 > target.slot.0 { + source + } else { + target + }; - Ok(AttestationData { + AttestationData { slot, head, target, source, - }) + } } /// The slot of a known block, or `None` where the root is not in the local view. @@ -126,3 +130,105 @@ fn parent_of(store: &Store, root: Bytes32) -> Bytes32 { .get(&root) .map_or(root, |block| block.parent_root) } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use verity_types::{Block, Bytes32, Checkpoint, Slot}; + + use crate::fork_choice::store::Store; + use crate::fork_choice::testing::anchored_on_genesis; + use crate::merkle::hash_tree_root; + + use super::{attestation_data, attestation_target}; + + /// The head chain seen in the three-node devnet run behind issue #44, laid over a genesis + /// store without state transitions: the target walk reads only slots and parent links. + /// + /// Finalized at 42, safe target at 42, and the head state's justified where the caller + /// says. Returns the store and the root of each slot. + fn devnet_chain(justified_slot: u64) -> (Store, HashMap) { + let (mut store, genesis) = anchored_on_genesis(3); + let anchor_root = store.head; + let mut roots = HashMap::new(); + let mut parent_root = anchor_root; + for slot in [42, 45, 49, 50, 51, 59, 62] { + let block = Block { + slot: Slot(slot), + parent_root, + state_root: store.blocks[&anchor_root].state_root, + ..Block::default() + }; + parent_root = hash_tree_root(&block); + store.blocks.insert(parent_root, block); + roots.insert(slot, parent_root); + } + + let finalized = Checkpoint { + root: roots[&42], + slot: Slot(42), + }; + let justified = Checkpoint { + root: roots[&justified_slot], + slot: Slot(justified_slot), + }; + let mut head_state = genesis; + head_state.latest_justified = justified; + head_state.latest_finalized = finalized; + + store.head = parent_root; + store.safe_target = roots[&42]; + store.latest_finalized = finalized; + store.latest_justified = justified; + store.states.insert(parent_root, head_state); + (store, roots) + } + + #[test] + fn should_raise_the_target_to_the_source_when_the_walk_lands_behind_it() { + // Three steps from 62 reach 50; neither 50 nor 49 is justifiable after 42 (deltas 8 + // and 7), so the walk stops at 45 while the head's justified sits at 49. + let (store, roots) = devnet_chain(49); + let justified = Checkpoint { + root: roots[&49], + slot: Slot(49), + }; + + assert_eq!( + attestation_target(&store), + Checkpoint { + root: roots[&45], + slot: Slot(45), + } + ); + + let data = attestation_data(&store, Slot(63)); + + assert_eq!(data.source, justified); + assert_eq!(data.target, justified); + assert_eq!( + data.head, + Checkpoint { + root: roots[&62], + slot: Slot(62), + } + ); + } + + #[test] + fn should_keep_the_target_when_it_sits_at_or_after_the_source() { + let (store, roots) = devnet_chain(45); + + let data = attestation_data(&store, Slot(63)); + + assert_eq!(data.source.slot, Slot(45)); + assert_eq!( + data.target, + Checkpoint { + root: roots[&45], + slot: Slot(45), + } + ); + } +} diff --git a/crates/verity-chain/src/view.rs b/crates/verity-chain/src/view.rs index c426924..a56add5 100644 --- a/crates/verity-chain/src/view.rs +++ b/crates/verity-chain/src/view.rs @@ -31,7 +31,6 @@ use verity_types::{ Validators, }; -use crate::error::RejectionReason; use crate::fork_choice::duties::{attestation_data, attestation_target}; use crate::fork_choice::store::{AttestationSignatureEntry, Store}; use crate::fork_choice::weights::block_weights; @@ -204,12 +203,10 @@ impl ChainView { /// The vote a validator should cast at `slot`. /// - /// # Errors - /// - /// [`RejectionReason::SourceAfterTarget`] when the head's justified checkpoint sits ahead - /// of the target the walk selected, which is a vote no peer would admit. + /// The target never sits behind the source: when the walk lands behind the head's + /// justified checkpoint, that checkpoint is the target (see [`attestation_data`]). #[must_use = "this produces the vote; signing it is the validator's job"] - pub fn attestation_data(&self, slot: Slot) -> Result { + pub fn attestation_data(&self, slot: Slot) -> AttestationData { attestation_data(&self.store, slot) } diff --git a/crates/verity-validator/src/duties.rs b/crates/verity-validator/src/duties.rs index 87c0500..5673a1b 100644 --- a/crates/verity-validator/src/duties.rs +++ b/crates/verity-validator/src/duties.rs @@ -288,9 +288,7 @@ impl DutyService { return Ok(()); } - // The vote is produced before the slot is marked, so a view that cannot yet answer - // leaves the slot open for the next interval to retry. - let data = view.attestation_data(slot)?; + let data = view.attestation_data(slot); self.attested.insert(slot); self.attested .retain(|attested| attested.0 + ATTESTED_SLOT_RETENTION > slot.0); @@ -314,7 +312,12 @@ impl DutyService { } } - tracing::debug!(slot = slot.0, target = data.target.slot.0, "attested"); + tracing::debug!( + slot = slot.0, + source = data.source.slot.0, + target = data.target.slot.0, + "attested" + ); Ok(()) } diff --git a/crates/verity-validator/src/error.rs b/crates/verity-validator/src/error.rs index 98840a0..3dbfa8b 100644 --- a/crates/verity-validator/src/error.rs +++ b/crates/verity-validator/src/error.rs @@ -25,7 +25,7 @@ pub enum DutyError { Signing(SignatureError), /// Aggregating or merging proofs failed. Aggregation(AggregationError), - /// The block or vote this node would have produced is one no peer would accept. + /// The block this node would have produced is one no peer would accept. Rejected(RejectionReason), /// The chain view holds no post-state for its own head, so no duty can be resolved /// against a registry. Transient, and only around startup.