From 1ee1765ba11e2924c8acf6ab10cb962c0937425b Mon Sep 17 00:00:00 2001 From: adust09 Date: Sat, 12 Sep 2026 01:25:07 +0900 Subject: [PATCH] fix: vote for the justified checkpoint when the target walk lands behind it The attestation duty returned SourceAfterTarget, and the validator cast nothing, whenever the target walk selected a block behind the head's justified checkpoint. The walk is bounded by the safe target and the finalized checkpoint, never by the justified one, so a sparse chain reaches this: the three-node devnet5 run behind #42 had the head chain 62 -> 59 -> 51 -> 50 -> 49 -> 45 -> 42 with finalized 42 and justified 49, and the validator's weight was missing from slots 16 and 64. Every other lean client raises the target to the justified checkpoint here (ethlambda, ream, zeam, gean, lantern), and leanSpec PR #1207 (issue #1206) makes the spec say the same. attestation_data now does so and is infallible: the duty path had no other way to fail. The attested log names the source next to the target so a raised vote is visible. DutyError::Rejected stays for block production, which still returns rejections. Closes #44 Co-Authored-By: Claude Fable 5.1 --- crates/verity-chain/src/fork_choice/duties.rs | 132 ++++++++++++++++-- crates/verity-chain/src/view.rs | 9 +- crates/verity-validator/src/duties.rs | 11 +- crates/verity-validator/src/error.rs | 2 +- 4 files changed, 130 insertions(+), 24 deletions(-) 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.