fix: vote for the justified checkpoint when the target walk lands behind it

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<AttestationData, RejectionReason> {
+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<AttestationData, Re
         };
     }
 
-    if source.slot.0 > 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<u64, Bytes32>) {
+        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),
+            }
+        );
+    }
+}

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<AttestationData, RejectionReason> {
+    pub fn attestation_data(&self, slot: Slot) -> AttestationData {
         attestation_data(&self.store, slot)
     }
 

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(())
     }
 

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.