From b744feceec1f0ee9edb7983c3fa3046d7c282ddd Mon Sep 17 00:00:00 2001 From: LucienSong Date: Thu, 28 May 2026 04:08:53 +0800 Subject: [PATCH 1/4] fix(consensus): harden attestation and view-change signing payloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements white-paper §1568-1596 signing payload requirements: Attestation (WP §1568-1582): - Extended signing payload from 40 bytes (block_hash||block_number) to 112 bytes: SHELL_ATTEST_V1\0 || chain_id(8) || epoch(8) || parent_hash(32) || block_hash(32) || block_number(8) || round(8) - Added chain_id, parent_hash, round fields to Attestation struct - Added ATTESTATION_EPOCH_BLOCKS = 1000 constant - chain_id binding prevents cross-network replay attacks - parent_hash binding prevents cross-fork replay attacks ViewChangeMessage (WP §1585-1596): - Extended signing payload from 16 bytes (view||block_number) to 56 bytes: SHELL_VIEWCHG_V1 || chain_id(8) || block_number(8) || view(8) || highest_qc_hash(32) - Added chain_id, highest_qc_hash fields to ViewChangeMessage struct - ViewChangeState gains chain_id field; record_view_change rejects messages from foreign chains Witness pruning (WP §storage): - proof_replacement_grace: Full 0→128, Light 0→128 (MIN_AMENDMENT_DEPTH) - Prevents witness deletion before amendments can be confirmed All 220 consensus + node tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/consensus/src/finality.rs | 202 ++++++++++++++++++--------- crates/consensus/src/view_change.rs | 69 +++++++-- crates/consensus/src/wpoa.rs | 10 +- crates/node/src/node/event_loop.rs | 14 +- crates/node/src/node/mod.rs | 4 +- crates/node/src/node/p2p_handlers.rs | 23 ++- crates/node/src/pruning.rs | 16 +-- 7 files changed, 245 insertions(+), 93 deletions(-) diff --git a/crates/consensus/src/finality.rs b/crates/consensus/src/finality.rs index f77b3a6..303c762 100644 --- a/crates/consensus/src/finality.rs +++ b/crates/consensus/src/finality.rs @@ -11,44 +11,100 @@ use std::collections::{HashMap, HashSet}; /// `ShellHash → HashSet
` mapping, bounded at this many unique block hashes. const MAX_PENDING_ATTESTATION_BLOCKS: usize = 512; +/// Number of blocks per attestation epoch. Used to derive the epoch field +/// in the attestation signing payload (WP §attestation binding). +pub const ATTESTATION_EPOCH_BLOCKS: u64 = 1000; + +/// Domain tag for the attestation signing payload. +/// Prevents cross-protocol message reuse. +const ATTEST_DOMAIN: &[u8; 16] = b"SHELL_ATTEST_V1\0"; + /// An attestation is a validator's signed confirmation that they accept a block. /// Validators broadcast attestations after importing a valid block. /// When attesting weight exceeds 2/3 of total validator weight, the block becomes finalized. +/// +/// Signing payload (WP §1568-1582): +/// SHELL_ATTEST_V1\0 || chain_id(8 BE) || epoch(8 BE) || parent_hash(32) +/// || block_hash(32) || block_number(8 BE) || round(8 BE) #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Attestation { + /// Chain ID — binds the attestation to a specific network. + pub chain_id: u64, + /// Hash of the parent block — prevents cross-fork replays. + pub parent_hash: ShellHash, /// Hash of the attested block. pub block_hash: ShellHash, /// Number of the attested block. pub block_number: u64, /// Address of the attesting validator. pub validator: Address, - /// PQ signature over (block_hash || block_number) by the validator. + /// Consensus round (view) in which this block was produced. + /// Always 0 for standard PoA; set to the wPoA view after a view-change. + pub round: u64, + /// PQ signature over the signing payload. pub signature: Vec, } impl Attestation { /// Create a new attestation. + #[allow(clippy::too_many_arguments)] pub fn new( + chain_id: u64, + parent_hash: ShellHash, block_hash: ShellHash, block_number: u64, validator: Address, + round: u64, signature: Vec, ) -> Self { Self { + chain_id, + parent_hash, block_hash, block_number, validator, + round, signature, } } - /// The message that must be signed: block_hash ++ block_number (big-endian). - pub fn signing_message(block_hash: &ShellHash, block_number: u64) -> Vec { - let mut msg = Vec::with_capacity(40); + /// Derive the epoch from a block number. + pub fn epoch_of(block_number: u64) -> u64 { + block_number / ATTESTATION_EPOCH_BLOCKS + } + + /// The canonical signing payload (WP §1568-1582): + /// SHELL_ATTEST_V1\0 || chain_id(8 BE) || epoch(8 BE) || parent_hash(32) + /// || block_hash(32) || block_number(8 BE) || round(8 BE) + pub fn signing_message( + chain_id: u64, + parent_hash: &ShellHash, + block_hash: &ShellHash, + block_number: u64, + round: u64, + ) -> Vec { + let epoch = Self::epoch_of(block_number); + let mut msg = Vec::with_capacity(112); + msg.extend_from_slice(ATTEST_DOMAIN); + msg.extend_from_slice(&chain_id.to_be_bytes()); + msg.extend_from_slice(&epoch.to_be_bytes()); + msg.extend_from_slice(parent_hash.as_bytes()); msg.extend_from_slice(block_hash.as_bytes()); msg.extend_from_slice(&block_number.to_be_bytes()); + msg.extend_from_slice(&round.to_be_bytes()); msg } + + /// Reconstruct the signing message from this attestation's own fields. + pub fn own_signing_message(&self) -> Vec { + Self::signing_message( + self.chain_id, + &self.parent_hash, + &self.block_hash, + self.block_number, + self.round, + ) + } } /// Tracks finality state: which blocks have been finalized and pending attestations. @@ -257,7 +313,7 @@ impl FinalityState { let messages: Vec> = attestations .iter() - .map(|att| Attestation::signing_message(&att.block_hash, att.block_number)) + .map(|att| att.own_signing_message()) .collect(); let sigs: Vec = attestations @@ -353,6 +409,12 @@ mod tests { Address::from(bytes) } + /// Test helper: build an Attestation with zero values for chain_id, parent_hash, + /// and round. Tests that only verify quorum logic (not signature binding) use this. + fn make_att(block_hash: ShellHash, block_number: u64, validator: Address, sig: Vec) -> Attestation { + Attestation::new(0, ShellHash::ZERO, block_hash, block_number, validator, 0, sig) + } + fn strict_quorum_weight(total_weight: u64) -> u64 { if total_weight == 0 { return 0; @@ -364,7 +426,7 @@ mod tests { fn test_attestation_new() { let hash = make_hash(1); let addr = make_addr(1); - let att = Attestation::new(hash, 10, addr, vec![1, 2, 3]); + let att = make_att(hash, 10, addr, vec![1, 2, 3]); assert_eq!(att.block_hash, hash); assert_eq!(att.block_number, 10); assert_eq!(att.validator, addr); @@ -374,10 +436,21 @@ mod tests { #[test] fn test_signing_message() { let hash = make_hash(42); - let msg = Attestation::signing_message(&hash, 100); - assert_eq!(msg.len(), 40); // 32 bytes hash + 8 bytes number - assert_eq!(msg[0], 42); - assert_eq!(&msg[32..], &100u64.to_be_bytes()); + let parent = ShellHash::ZERO; + let chain_id: u64 = 1337; + let block_number: u64 = 100; + let round: u64 = 0; + let epoch = Attestation::epoch_of(block_number); + let msg = Attestation::signing_message(chain_id, &parent, &hash, block_number, round); + // Domain (16) + chain_id (8) + epoch (8) + parent_hash (32) + block_hash (32) + height (8) + round (8) + assert_eq!(msg.len(), 112); + assert_eq!(&msg[..16], b"SHELL_ATTEST_V1\0"); + assert_eq!(&msg[16..24], &chain_id.to_be_bytes()); + assert_eq!(&msg[24..32], &epoch.to_be_bytes()); + assert_eq!(&msg[32..64], parent.as_bytes()); + assert_eq!(&msg[64..96], hash.as_bytes()); + assert_eq!(&msg[96..104], &block_number.to_be_bytes()); + assert_eq!(&msg[104..112], &round.to_be_bytes()); } #[test] @@ -397,8 +470,8 @@ mod tests { let mut state = FinalityState::new(); let hash = make_hash(1); let addr = make_addr(1); - let att1 = Attestation::new(hash, 10, addr, vec![1]); - let att2 = Attestation::new(hash, 10, addr, vec![2]); + let att1 = make_att(hash, 10, addr, vec![1]); + let att2 = make_att(hash, 10, addr, vec![2]); assert!(state.record_attestation(att1)); assert!(!state.record_attestation(att2)); // duplicate validator @@ -411,7 +484,7 @@ mod tests { let hash = make_hash(1); // 1 of 3 validators - state.record_attestation(Attestation::new(hash, 10, make_addr(1), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(1), vec![])); assert!(!state.check_finality_weighted(&hash, 10, 3)); assert_eq!(state.last_finalized_number(), 0); } @@ -422,9 +495,9 @@ mod tests { let hash = make_hash(1); // 3 of 3 validators is the minimum strict supermajority for uniform weights. - state.record_attestation(Attestation::new(hash, 10, make_addr(1), vec![])); - state.record_attestation(Attestation::new(hash, 10, make_addr(2), vec![])); - state.record_attestation(Attestation::new(hash, 10, make_addr(3), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(1), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(2), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(3), vec![])); assert!(state.check_finality_weighted(&hash, 10, 3)); assert_eq!(state.last_finalized_number(), 10); assert_eq!(state.last_finalized_hash(), &hash); @@ -435,8 +508,8 @@ mod tests { let mut state = FinalityState::new(); let hash = make_hash(9); - state.record_attestation_weighted(Attestation::new(hash, 10, make_addr(1), vec![]), 2); - state.record_attestation_weighted(Attestation::new(hash, 10, make_addr(2), vec![]), 2); + state.record_attestation_weighted(make_att(hash, 10, make_addr(1), vec![]), 2); + state.record_attestation_weighted(make_att(hash, 10, make_addr(2), vec![]), 2); assert_eq!(state.attested_weight(&hash), 4); assert!(!state.check_finality_weighted(&hash, 10, 6)); } @@ -446,8 +519,8 @@ mod tests { let mut state = FinalityState::new(); let hash = make_hash(10); - state.record_attestation_weighted(Attestation::new(hash, 10, make_addr(1), vec![]), 4); - state.record_attestation_weighted(Attestation::new(hash, 10, make_addr(2), vec![]), 1); + state.record_attestation_weighted(make_att(hash, 10, make_addr(1), vec![]), 4); + state.record_attestation_weighted(make_att(hash, 10, make_addr(2), vec![]), 1); assert_eq!(state.attestation_count(&hash), 2); assert_eq!(state.attested_weight(&hash), 5); assert!(state.check_finality_weighted(&hash, 10, 6)); @@ -458,7 +531,7 @@ mod tests { let mut state = FinalityState::new(); let hash = make_hash(11); - state.record_attestation_weighted(Attestation::new(hash, 1, make_addr(1), vec![]), 1); + state.record_attestation_weighted(make_att(hash, 1, make_addr(1), vec![]), 1); assert!(state.check_finality_weighted(&hash, 1, 1)); assert_eq!(state.last_finalized_hash(), &hash); } @@ -469,9 +542,9 @@ mod tests { let hash = make_hash(1); // Even with weighted quorum, block 10 < finalized 20 → no update - state.record_attestation(Attestation::new(hash, 10, make_addr(1), vec![])); - state.record_attestation(Attestation::new(hash, 10, make_addr(2), vec![])); - state.record_attestation(Attestation::new(hash, 10, make_addr(3), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(1), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(2), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(3), vec![])); assert!(!state.check_finality_weighted(&hash, 10, 3)); assert_eq!(state.last_finalized_number(), 20); } @@ -483,7 +556,7 @@ mod tests { let addr = make_addr(1); assert!(!state.has_attested(&hash, &addr)); - state.record_attestation(Attestation::new(hash, 10, addr, vec![])); + state.record_attestation(make_att(hash, 10, addr, vec![])); assert!(state.has_attested(&hash, &addr)); } @@ -494,7 +567,7 @@ mod tests { let hash2 = make_hash(2); let validator = make_addr(1); - state.record_attestation(Attestation::new(hash1, 10, validator, vec![])); + state.record_attestation(make_att(hash1, 10, validator, vec![])); // Same validator, same height, different hash → equivocation let conflict = state.detect_equivocation(&hash2, 10, &validator); @@ -508,7 +581,7 @@ mod tests { let hash2 = make_hash(2); let validator = make_addr(1); - state.record_attestation(Attestation::new(hash1, 10, validator, vec![])); + state.record_attestation(make_att(hash1, 10, validator, vec![])); // Different height → not equivocation let conflict = state.detect_equivocation(&hash2, 11, &validator); @@ -521,12 +594,12 @@ mod tests { let hash1 = make_hash(1); let hash2 = make_hash(2); - state.record_attestation(Attestation::new(hash1, 5, make_addr(1), vec![])); - state.record_attestation(Attestation::new(hash2, 15, make_addr(1), vec![])); + state.record_attestation(make_att(hash1, 5, make_addr(1), vec![])); + state.record_attestation(make_att(hash2, 15, make_addr(1), vec![])); // Finalize at block 15 → prune block 5 attestations - state.record_attestation(Attestation::new(hash2, 15, make_addr(2), vec![])); - state.record_attestation(Attestation::new(hash2, 15, make_addr(3), vec![])); + state.record_attestation(make_att(hash2, 15, make_addr(2), vec![])); + state.record_attestation(make_att(hash2, 15, make_addr(3), vec![])); assert!(state.check_finality_weighted(&hash2, 15, 3)); assert_eq!(state.attestation_count(&hash1), 0); // pruned @@ -541,11 +614,11 @@ mod tests { // 7 validators, BFT quorum = ceil(14/3) = 5 for i in 0..4 { - state.record_attestation(Attestation::new(hash, 10, make_addr(i), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(i), vec![])); } assert!(!state.check_finality_weighted(&hash, 10, 7)); // 4 < 5 - state.record_attestation(Attestation::new(hash, 10, make_addr(4), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(4), vec![])); assert!(state.check_finality_weighted(&hash, 10, 7)); // 5 >= 5 } @@ -568,7 +641,7 @@ mod tests { // Add one weight less than quorum → should NOT finalize. for i in 0..(quorum - 1) { - state.record_attestation(Attestation::new(hash, 100, make_addr(i as u8), vec![])); + state.record_attestation(make_att(hash, 100, make_addr(i as u8), vec![])); } assert!( !state.check_finality_weighted(&hash, 100, total), @@ -578,7 +651,7 @@ mod tests { let mut state2 = FinalityState::new(); for i in 0..quorum { - state2.record_attestation(Attestation::new(hash, 100, make_addr(i as u8), vec![])); + state2.record_attestation(make_att(hash, 100, make_addr(i as u8), vec![])); } assert!( state2.check_finality_weighted(&hash, 100, total), @@ -594,7 +667,7 @@ mod tests { // 5 of 10 validators → quorum is 6 for i in 0..5 { - state.record_attestation(Attestation::new(hash, 50, make_addr(i), vec![])); + state.record_attestation(make_att(hash, 50, make_addr(i), vec![])); } assert!(!state.check_finality_weighted(&hash, 50, 10)); assert_eq!( @@ -611,7 +684,7 @@ mod tests { // Round 1: finalize block 10 let hash10 = make_hash(10); for i in 0..3 { - state.record_attestation(Attestation::new(hash10, 10, make_addr(i), vec![])); + state.record_attestation(make_att(hash10, 10, make_addr(i), vec![])); } assert!(state.check_finality_weighted(&hash10, 10, 4)); // quorum = 3 for N=4 assert_eq!(state.last_finalized_number(), 10); @@ -619,7 +692,7 @@ mod tests { // Round 2: finalize block 20 let hash20 = make_hash(20); for i in 0..3 { - state.record_attestation(Attestation::new(hash20, 20, make_addr(100 + i), vec![])); + state.record_attestation(make_att(hash20, 20, make_addr(100 + i), vec![])); } assert!(state.check_finality_weighted(&hash20, 20, 4)); assert_eq!(state.last_finalized_number(), 20); @@ -628,7 +701,7 @@ mod tests { // Round 3: finalize block 30 let hash30 = make_hash(30); for i in 0..3 { - state.record_attestation(Attestation::new(hash30, 30, make_addr(200 + i), vec![])); + state.record_attestation(make_att(hash30, 30, make_addr(200 + i), vec![])); } assert!(state.check_finality_weighted(&hash30, 30, 4)); assert_eq!(state.last_finalized_number(), 30); @@ -645,12 +718,12 @@ mod tests { // Add 66 attestations → not enough for i in 0..66u8 { - state.record_attestation(Attestation::new(hash, 500, make_addr(i), vec![])); + state.record_attestation(make_att(hash, 500, make_addr(i), vec![])); } assert!(!state.check_finality_weighted(&hash, 500, total)); // Add 1 more → exactly 67 → quorum - state.record_attestation(Attestation::new(hash, 500, make_addr(66), vec![])); + state.record_attestation(make_att(hash, 500, make_addr(66), vec![])); assert!(state.check_finality_weighted(&hash, 500, total)); assert_eq!(state.last_finalized_number(), 500); } @@ -662,7 +735,7 @@ mod tests { // Finalize block 20 first let hash20 = make_hash(20); for i in 0..3 { - state.record_attestation(Attestation::new(hash20, 20, make_addr(i), vec![])); + state.record_attestation(make_att(hash20, 20, make_addr(i), vec![])); } assert!(state.check_finality_weighted(&hash20, 20, 4)); assert_eq!(state.last_finalized_number(), 20); @@ -670,7 +743,7 @@ mod tests { // Try to finalize block 15 (lower) — should fail let hash15 = make_hash(15); for i in 10..13 { - state.record_attestation(Attestation::new(hash15, 15, make_addr(i), vec![])); + state.record_attestation(make_att(hash15, 15, make_addr(i), vec![])); } assert!(!state.check_finality_weighted(&hash15, 15, 4)); assert_eq!( @@ -682,7 +755,7 @@ mod tests { // Finalize block 25 (higher) — should succeed let hash25 = make_hash(25); for i in 20..23 { - state.record_attestation(Attestation::new(hash25, 25, make_addr(i), vec![])); + state.record_attestation(make_att(hash25, 25, make_addr(i), vec![])); } assert!(state.check_finality_weighted(&hash25, 25, 4)); assert_eq!(state.last_finalized_number(), 25); @@ -696,13 +769,13 @@ mod tests { let hash_future = make_hash(3); // Attestation at height 5 - state.record_attestation(Attestation::new(hash_low, 5, make_addr(1), vec![])); + state.record_attestation(make_att(hash_low, 5, make_addr(1), vec![])); // Attestation at height 10 - state.record_attestation(Attestation::new(hash_high, 10, make_addr(2), vec![])); - state.record_attestation(Attestation::new(hash_high, 10, make_addr(3), vec![])); - state.record_attestation(Attestation::new(hash_high, 10, make_addr(4), vec![])); + state.record_attestation(make_att(hash_high, 10, make_addr(2), vec![])); + state.record_attestation(make_att(hash_high, 10, make_addr(3), vec![])); + state.record_attestation(make_att(hash_high, 10, make_addr(4), vec![])); // Attestation at height 20 - state.record_attestation(Attestation::new(hash_future, 20, make_addr(5), vec![])); + state.record_attestation(make_att(hash_future, 20, make_addr(5), vec![])); // Finalize at height 10 → prune heights <= 10 assert!(state.check_finality_weighted(&hash_high, 10, 3)); @@ -723,17 +796,17 @@ mod tests { let hash1 = make_hash(1); let hash2 = make_hash(2); - state.record_attestation(Attestation::new(hash1, 10, make_addr(1), vec![])); + state.record_attestation(make_att(hash1, 10, make_addr(1), vec![])); assert_eq!(state.total_pending_attestations(), 1); - state.record_attestation(Attestation::new(hash1, 10, make_addr(2), vec![])); + state.record_attestation(make_att(hash1, 10, make_addr(2), vec![])); assert_eq!(state.total_pending_attestations(), 2); - state.record_attestation(Attestation::new(hash2, 11, make_addr(3), vec![])); + state.record_attestation(make_att(hash2, 11, make_addr(3), vec![])); assert_eq!(state.total_pending_attestations(), 3); // Duplicate should not increase count - state.record_attestation(Attestation::new(hash1, 10, make_addr(1), vec![])); + state.record_attestation(make_att(hash1, 10, make_addr(1), vec![])); assert_eq!(state.total_pending_attestations(), 3); } @@ -752,7 +825,7 @@ mod tests { let hash = make_hash(1); let validator = make_addr(1); - state.record_attestation(Attestation::new(hash, 10, validator, vec![])); + state.record_attestation(make_att(hash, 10, validator, vec![])); // Same hash, same validator — not equivocation (just a duplicate) let conflict = state.detect_equivocation(&hash, 10, &validator); @@ -766,9 +839,9 @@ mod tests { let hash_b = make_hash(2); // Different validators attest to different blocks at height 10. - state.record_attestation_weighted(Attestation::new(hash_a, 10, make_addr(1), vec![]), 4); - state.record_attestation_weighted(Attestation::new(hash_a, 10, make_addr(2), vec![]), 1); - state.record_attestation_weighted(Attestation::new(hash_b, 10, make_addr(3), vec![]), 1); + state.record_attestation_weighted(make_att(hash_a, 10, make_addr(1), vec![]), 4); + state.record_attestation_weighted(make_att(hash_a, 10, make_addr(2), vec![]), 1); + state.record_attestation_weighted(make_att(hash_b, 10, make_addr(3), vec![]), 1); assert_eq!(state.attestation_count(&hash_a), 2); assert_eq!(state.attested_weight(&hash_a), 5); @@ -785,17 +858,17 @@ mod tests { let hash = make_hash(1); // 1 of 10 validators - state.record_attestation(Attestation::new(hash, 10, make_addr(1), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(1), vec![])); assert!(!state.check_finality_weighted(&hash, 10, 10)); // BFT quorum = 7 // 6 of 10 validators for i in 2..=6 { - state.record_attestation(Attestation::new(hash, 10, make_addr(i), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(i), vec![])); } assert!(!state.check_finality_weighted(&hash, 10, 10)); // still only 6 < 7 // 7 of 10 validators → exactly quorum - state.record_attestation(Attestation::new(hash, 10, make_addr(7), vec![])); + state.record_attestation(make_att(hash, 10, make_addr(7), vec![])); assert!(state.check_finality_weighted(&hash, 10, 10)); } @@ -811,7 +884,10 @@ mod tests { let block_number: u64 = 100; // Sign the attestation message with a real Dilithium key. - let msg = Attestation::signing_message(&block_hash, block_number); + let chain_id: u64 = 0; + let parent_hash = ShellHash::ZERO; + let round: u64 = 0; + let msg = Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number, round); let sig = signer.sign(&msg).expect("signing must succeed"); assert!(!sig.data.is_empty(), "signature must not be empty"); @@ -824,7 +900,7 @@ mod tests { // Record the attestation with the real signature. let attestation = - Attestation::new(block_hash, block_number, validator_addr, sig.data.clone()); + make_att(block_hash, block_number, validator_addr, sig.data.clone()); let mut state = FinalityState::new(); assert!(state.record_attestation(attestation)); assert_eq!(state.attestation_count(&block_hash), 1); @@ -841,7 +917,7 @@ mod tests { assert!(stored_valid, "stored attestation signature must verify"); // Verify a tampered message does not pass. - let wrong_msg = Attestation::signing_message(&block_hash, block_number + 1); + let wrong_msg = Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number + 1, round); let wrong_valid = verifier.verify(&pubkey, &wrong_msg, &stored_sig).unwrap(); assert!(!wrong_valid, "signature must not verify for wrong message"); } diff --git a/crates/consensus/src/view_change.rs b/crates/consensus/src/view_change.rs index 2b5600d..0e25c5e 100644 --- a/crates/consensus/src/view_change.rs +++ b/crates/consensus/src/view_change.rs @@ -2,34 +2,73 @@ use std::collections::HashMap; use std::time::{SystemTime, UNIX_EPOCH}; use serde::{Deserialize, Serialize}; -use shell_primitives::Address; +use shell_primitives::{Address, ShellHash}; pub const VIEW_CHANGE_TIMEOUT_MS: u64 = 10_000; +/// Domain tag for the view-change signing payload. +const VIEWCHG_DOMAIN: &[u8; 16] = b"SHELL_VIEWCHG_V1"; + +/// A validator's request to advance the consensus view (rotate the proposer). +/// +/// Signing payload (WP §1585-1596): +/// SHELL_VIEWCHG_V1 || chain_id(8 BE) || block_number(8 BE) || view(8 BE) || highest_qc_hash(32) #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ViewChangeMessage { - pub view: u64, + /// Chain ID — binds the message to a specific network. + pub chain_id: u64, + /// Block number at which the view change is requested. pub block_number: u64, + /// Requested view number. + pub view: u64, + /// Hash of the highest QC seen by this validator (last finalized block hash). + pub highest_qc_hash: ShellHash, + /// Address of the validator requesting the view change. pub validator: Address, + /// PQ signature over the signing payload. pub signature: Vec, } impl ViewChangeMessage { - pub fn new(view: u64, block_number: u64, validator: Address, signature: Vec) -> Self { + pub fn new( + chain_id: u64, + block_number: u64, + view: u64, + highest_qc_hash: ShellHash, + validator: Address, + signature: Vec, + ) -> Self { Self { - view, + chain_id, block_number, + view, + highest_qc_hash, validator, signature, } } - pub fn signing_message(view: u64, block_number: u64) -> Vec { - let mut msg = Vec::with_capacity(16); - msg.extend_from_slice(&view.to_be_bytes()); + /// The canonical signing payload (WP §1585-1596): + /// SHELL_VIEWCHG_V1 || chain_id(8 BE) || block_number(8 BE) || view(8 BE) || highest_qc_hash(32) + pub fn signing_message( + chain_id: u64, + block_number: u64, + view: u64, + highest_qc_hash: &ShellHash, + ) -> Vec { + let mut msg = Vec::with_capacity(56); + msg.extend_from_slice(VIEWCHG_DOMAIN); + msg.extend_from_slice(&chain_id.to_be_bytes()); msg.extend_from_slice(&block_number.to_be_bytes()); + msg.extend_from_slice(&view.to_be_bytes()); + msg.extend_from_slice(highest_qc_hash.as_bytes()); msg } + + /// Reconstruct the signing message from this message's own fields. + pub fn own_signing_message(&self) -> Vec { + Self::signing_message(self.chain_id, self.block_number, self.view, &self.highest_qc_hash) + } } #[derive(Debug, Clone)] @@ -40,6 +79,9 @@ pub struct ViewChangeState { quorum_weight: u64, validator_weights: HashMap, pending_view_change_weights: HashMap, + /// Chain ID used to reject cross-chain view-change messages. + /// Zero means unconfigured (no chain-ID check). + chain_id: u64, } impl ViewChangeState { @@ -51,13 +93,22 @@ impl ViewChangeState { quorum_weight: 1, validator_weights: HashMap::new(), pending_view_change_weights: HashMap::new(), + chain_id: 0, } } + /// Set the chain ID to enforce on incoming view-change messages. + pub fn set_chain_id(&mut self, chain_id: u64) { + self.chain_id = chain_id; + } + pub fn record_view_change(&mut self, msg: ViewChangeMessage) -> bool { if msg.view != self.current_view { return false; } + if self.chain_id != 0 && msg.chain_id != self.chain_id { + return false; + } let validator_weight = self .validator_weights @@ -158,8 +209,8 @@ mod tests { let mut state = ViewChangeState::new(); state.configure_quorum(HashMap::from([(addr(1), 1), (addr(2), 1), (addr(3), 1)]), 3); - let first = ViewChangeMessage::new(0, 7, addr(1), vec![1]); - let second = ViewChangeMessage::new(0, 7, addr(2), vec![2]); + let first = ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(1), vec![1]); + let second = ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(2), vec![2]); assert!(!state.record_view_change(first)); assert!(state.record_view_change(second)); diff --git a/crates/consensus/src/wpoa.rs b/crates/consensus/src/wpoa.rs index 7da1810..27fea9a 100644 --- a/crates/consensus/src/wpoa.rs +++ b/crates/consensus/src/wpoa.rs @@ -18,7 +18,7 @@ use std::sync::{Arc, Mutex}; use async_trait::async_trait; use shell_core::{Block, BlockHeader}; use shell_crypto::{PQSignature, Signer, Verifier}; -use shell_primitives::Address; +use shell_primitives::{Address, ShellHash}; use crate::poa::PoaEngine; use crate::validator::{ValidatorSet, ValidatorSetConfig}; @@ -566,8 +566,8 @@ mod tests { fn view_change_quorum_advances_view() { let mut e = engine(vec![addr(1), addr(2), addr(3)], vec![1, 1, 1]); - assert!(!e.handle_view_change_message(ViewChangeMessage::new(0, 7, addr(1), vec![1]), 3,)); - assert!(e.handle_view_change_message(ViewChangeMessage::new(0, 7, addr(2), vec![2]), 3,)); + assert!(!e.handle_view_change_message(ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(1), vec![1]), 3,)); + assert!(e.handle_view_change_message(ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(2), vec![2]), 3,)); assert_eq!(e.current_view(), 1); assert_eq!(e.proposer_for_block(0), addr(2)); } @@ -576,8 +576,8 @@ mod tests { fn note_block_progress_resets_view_change_state() { let mut e = engine(vec![addr(1), addr(2), addr(3)], vec![1, 1, 1]); - assert!(!e.handle_view_change_message(ViewChangeMessage::new(0, 9, addr(1), vec![1]), 3,)); - assert!(e.handle_view_change_message(ViewChangeMessage::new(0, 9, addr(2), vec![2]), 3,)); + assert!(!e.handle_view_change_message(ViewChangeMessage::new(0, 9, 0, ShellHash::ZERO, addr(1), vec![1]), 3,)); + assert!(e.handle_view_change_message(ViewChangeMessage::new(0, 9, 0, ShellHash::ZERO, addr(2), vec![2]), 3,)); assert_eq!(e.current_view(), 1); e.note_block_progress(42); diff --git a/crates/node/src/node/event_loop.rs b/crates/node/src/node/event_loop.rs index 79bd91d..f6635a1 100644 --- a/crates/node/src/node/event_loop.rs +++ b/crates/node/src/node/event_loop.rs @@ -716,13 +716,21 @@ impl Node { .expect("validated block producer has proposer address"); let view = self.consensus.read().current_view(); let block_number = self.head_number() + 1; - let signing_message = - ViewChangeMessage::signing_message(view, block_number); + let chain_id = self.config.chain_id; + let highest_qc_hash = self.finality.read().last_finalized_hash().clone(); + let signing_message = ViewChangeMessage::signing_message( + chain_id, + block_number, + view, + &highest_qc_hash, + ); match signer.sign(&signing_message) { Ok(signature) => { let msg = ViewChangeMessage::new( - view, + chain_id, block_number, + view, + highest_qc_hash, validator, signature.data, ); diff --git a/crates/node/src/node/mod.rs b/crates/node/src/node/mod.rs index a9a1ec4..a20e3c6 100644 --- a/crates/node/src/node/mod.rs +++ b/crates/node/src/node/mod.rs @@ -6010,8 +6010,10 @@ mod tests { fn wpoa_network_message_wpoa_view_change_serde() { let voter = Address::from([0xef; 32]); let msg = NetworkMessage::WPoaViewChange(Box::new(ViewChangeMessage::new( - 3, + 0, 10, + 3, + ShellHash::ZERO, voter, vec![9, 9, 9], ))); diff --git a/crates/node/src/node/p2p_handlers.rs b/crates/node/src/node/p2p_handlers.rs index c64dc72..81a334f 100644 --- a/crates/node/src/node/p2p_handlers.rs +++ b/crates/node/src/node/p2p_handlers.rs @@ -60,8 +60,8 @@ impl Node { NodeError::Startup(format!("unknown attestation validator: {:?}", validator)) })?; - // Verify the attestation signature. - let msg = Attestation::signing_message(&block_hash, block_number); + // Verify the attestation signature using the payload that was signed. + let msg = attestation.own_signing_message(); let sig_type = shell_crypto::infer_signature_type_from_address(pubkey, &validator) .ok_or_else(|| { NodeError::Startup(format!( @@ -151,15 +151,30 @@ impl Node { ) -> Result { let proposer_addr = self.config.proposer_address.ok_or(NodeError::NotProposer)?; - let msg = Attestation::signing_message(&block_hash, block_number); + // Look up the parent hash so the signing payload binds to the specific fork. + let parent_hash = self + .chain_store + .get_header_by_hash(&block_hash) + .ok() + .flatten() + .map(|h| h.parent_hash) + .unwrap_or(ShellHash::ZERO); + + let chain_id = self.config.chain_id; + // round = 0 for standard PoA; wPoA round is embedded per-block in Phase 2. + let round: u64 = 0; + let msg = Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number, round); let sig = signer .sign(&msg) .map_err(|e| NodeError::Startup(format!("failed to sign attestation: {e}")))?; Ok(Attestation::new( + chain_id, + parent_hash, block_hash, block_number, proposer_addr, + round, sig.data, )) } @@ -624,7 +639,7 @@ impl Node { )) })?; - let signing_message = ViewChangeMessage::signing_message(msg.view, msg.block_number); + let signing_message = msg.own_signing_message(); let sig_type = shell_crypto::infer_signature_type_from_address(pubkey, &msg.validator) .ok_or_else(|| { NodeError::Startup(format!( diff --git a/crates/node/src/pruning.rs b/crates/node/src/pruning.rs index c86fadb..e97287a 100644 --- a/crates/node/src/pruning.rs +++ b/crates/node/src/pruning.rs @@ -28,8 +28,8 @@ use shell_storage::{ /// | Profile (WP name) | body_retention | witness_retention | proof_replacement_grace | keep_recent | /// |-------------------------|---------------|------------------|------------------------|-------------| /// | Archive | 0 (forever) | 0 (forever) | u64::MAX (never) | 0 (forever) | -/// | Full | 0 (forever) | 128 | 0 (immediate) | 0 (forever) | -/// | Pruned / Rolling / Light| 4 096 | 64 | 0 (immediate) | 4 096 | +/// | Full | 0 (forever) | 128 | 128 (MIN_AMENDMENT_DEPTH) | 0 (forever) | +/// | Pruned / Rolling / Light| 4 096 | 64 | 128 (MIN_AMENDMENT_DEPTH) | 4 096 | #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "lowercase")] pub enum StorageProfile { @@ -78,10 +78,10 @@ impl StorageProfile { match self { // Archive: keep everything forever; never delete witness even after STARK proof. Self::Archive => (0, 0, 0, u64::MAX), - // Full: keep TX forever; delete witness when STARK proof arrives. - Self::Full => (0, DEFAULT_WITNESS_RETENTION, 0, 0), - // Light: rolling 4 096-block window (~2.3 h at 2 s/block). - Self::Light => (4_096, 64, 4_096, 0), + // Full: keep TX forever; grace = MIN_AMENDMENT_DEPTH (WP §storage). + Self::Full => (0, DEFAULT_WITNESS_RETENTION, 0, 128), + // Light: rolling 4 096-block window (~2.3 h at 2 s/block); grace = MIN_AMENDMENT_DEPTH. + Self::Light => (4_096, 64, 4_096, 128), } } @@ -544,7 +544,7 @@ mod tests { assert_eq!(body, 0, "full: body_retention must be 0"); assert!(witness > 0, "full: witness_retention must be non-zero"); assert_eq!(keep, 0, "full: keep_recent must be 0"); - assert_eq!(grace, 0, "full: grace must be 0"); + assert_eq!(grace, 128, "full: grace must be MIN_AMENDMENT_DEPTH (128)"); } #[test] @@ -553,7 +553,7 @@ mod tests { assert!(body > 0, "light: body_retention must be non-zero"); assert!(witness > 0, "light: witness_retention must be non-zero"); assert!(keep > 0, "light: keep_recent must be non-zero"); - assert_eq!(grace, 0, "light: grace must be 0"); + assert_eq!(grace, 128, "light: grace must be MIN_AMENDMENT_DEPTH (128)"); } #[test] From 9781ecbe11f709644eaf3198d4ae34462f1dfa16 Mon Sep 17 00:00:00 2001 From: LucienSong Date: Thu, 28 May 2026 14:58:31 +0800 Subject: [PATCH 2/4] fix(consensus): wire chain_id into WPoaConfig and ViewChangeState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add chain_id field to WPoaConfig (defaults to 0 for backwards compat) - Call view_change_state.set_chain_id() in WPoaEngine::new() when chain_id != 0 - Add #[serde(default)] to Attestation.chain_id/parent_hash/round and ViewChangeMessage.chain_id/highest_qc_hash for rolling-upgrade safety - Fix ViewChangeMessage signing_message capacity: 56 → 72 bytes - Add signing_message_has_correct_layout_and_length unit test - create_attestation() now returns Err on missing header instead of using ZERO - handle_attestation() and handle_wpoa_view_change() reject cross-chain messages - Add MIN_AMENDMENT_DEPTH constant; fix pruning doc table column order Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/consensus/src/finality.rs | 38 +++++++++++++++++++---- crates/consensus/src/view_change.rs | 45 ++++++++++++++++++++++++++-- crates/consensus/src/wpoa.rs | 32 ++++++++++++++++---- crates/network/src/message.rs | 7 ++++- crates/node/src/node/p2p_handlers.rs | 30 +++++++++++++++---- crates/node/src/pruning.rs | 31 +++++++++++++------ 6 files changed, 155 insertions(+), 28 deletions(-) diff --git a/crates/consensus/src/finality.rs b/crates/consensus/src/finality.rs index 303c762..6b42c0a 100644 --- a/crates/consensus/src/finality.rs +++ b/crates/consensus/src/finality.rs @@ -26,11 +26,17 @@ const ATTEST_DOMAIN: &[u8; 16] = b"SHELL_ATTEST_V1\0"; /// Signing payload (WP §1568-1582): /// SHELL_ATTEST_V1\0 || chain_id(8 BE) || epoch(8 BE) || parent_hash(32) /// || block_hash(32) || block_number(8 BE) || round(8 BE) +/// +/// `chain_id`, `parent_hash`, and `round` are tagged `#[serde(default)]` so that +/// messages produced by pre-Phase-1 nodes still deserialise without panicking; +/// their signatures will fail verification against the new payload. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Attestation { /// Chain ID — binds the attestation to a specific network. + #[serde(default)] pub chain_id: u64, /// Hash of the parent block — prevents cross-fork replays. + #[serde(default)] pub parent_hash: ShellHash, /// Hash of the attested block. pub block_hash: ShellHash, @@ -40,6 +46,7 @@ pub struct Attestation { pub validator: Address, /// Consensus round (view) in which this block was produced. /// Always 0 for standard PoA; set to the wPoA view after a view-change. + #[serde(default)] pub round: u64, /// PQ signature over the signing payload. pub signature: Vec, @@ -411,8 +418,21 @@ mod tests { /// Test helper: build an Attestation with zero values for chain_id, parent_hash, /// and round. Tests that only verify quorum logic (not signature binding) use this. - fn make_att(block_hash: ShellHash, block_number: u64, validator: Address, sig: Vec) -> Attestation { - Attestation::new(0, ShellHash::ZERO, block_hash, block_number, validator, 0, sig) + fn make_att( + block_hash: ShellHash, + block_number: u64, + validator: Address, + sig: Vec, + ) -> Attestation { + Attestation::new( + 0, + ShellHash::ZERO, + block_hash, + block_number, + validator, + 0, + sig, + ) } fn strict_quorum_weight(total_weight: u64) -> u64 { @@ -887,7 +907,8 @@ mod tests { let chain_id: u64 = 0; let parent_hash = ShellHash::ZERO; let round: u64 = 0; - let msg = Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number, round); + let msg = + Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number, round); let sig = signer.sign(&msg).expect("signing must succeed"); assert!(!sig.data.is_empty(), "signature must not be empty"); @@ -899,8 +920,7 @@ mod tests { assert!(valid, "real Dilithium signature must verify"); // Record the attestation with the real signature. - let attestation = - make_att(block_hash, block_number, validator_addr, sig.data.clone()); + let attestation = make_att(block_hash, block_number, validator_addr, sig.data.clone()); let mut state = FinalityState::new(); assert!(state.record_attestation(attestation)); assert_eq!(state.attestation_count(&block_hash), 1); @@ -917,7 +937,13 @@ mod tests { assert!(stored_valid, "stored attestation signature must verify"); // Verify a tampered message does not pass. - let wrong_msg = Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number + 1, round); + let wrong_msg = Attestation::signing_message( + chain_id, + &parent_hash, + &block_hash, + block_number + 1, + round, + ); let wrong_valid = verifier.verify(&pubkey, &wrong_msg, &stored_sig).unwrap(); assert!(!wrong_valid, "signature must not verify for wrong message"); } diff --git a/crates/consensus/src/view_change.rs b/crates/consensus/src/view_change.rs index 0e25c5e..7259a21 100644 --- a/crates/consensus/src/view_change.rs +++ b/crates/consensus/src/view_change.rs @@ -13,15 +13,21 @@ const VIEWCHG_DOMAIN: &[u8; 16] = b"SHELL_VIEWCHG_V1"; /// /// Signing payload (WP §1585-1596): /// SHELL_VIEWCHG_V1 || chain_id(8 BE) || block_number(8 BE) || view(8 BE) || highest_qc_hash(32) +/// +/// `chain_id` and `highest_qc_hash` are tagged `#[serde(default)]` so that messages +/// produced by pre-Phase-1 nodes (missing these fields) still deserialise without +/// panicking; their signatures will fail verification against the new payload. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ViewChangeMessage { /// Chain ID — binds the message to a specific network. + #[serde(default)] pub chain_id: u64, /// Block number at which the view change is requested. pub block_number: u64, /// Requested view number. pub view: u64, /// Hash of the highest QC seen by this validator (last finalized block hash). + #[serde(default)] pub highest_qc_hash: ShellHash, /// Address of the validator requesting the view change. pub validator: Address, @@ -50,13 +56,15 @@ impl ViewChangeMessage { /// The canonical signing payload (WP §1585-1596): /// SHELL_VIEWCHG_V1 || chain_id(8 BE) || block_number(8 BE) || view(8 BE) || highest_qc_hash(32) + /// + /// Total: 16 + 8 + 8 + 8 + 32 = 72 bytes. pub fn signing_message( chain_id: u64, block_number: u64, view: u64, highest_qc_hash: &ShellHash, ) -> Vec { - let mut msg = Vec::with_capacity(56); + let mut msg = Vec::with_capacity(72); msg.extend_from_slice(VIEWCHG_DOMAIN); msg.extend_from_slice(&chain_id.to_be_bytes()); msg.extend_from_slice(&block_number.to_be_bytes()); @@ -67,7 +75,12 @@ impl ViewChangeMessage { /// Reconstruct the signing message from this message's own fields. pub fn own_signing_message(&self) -> Vec { - Self::signing_message(self.chain_id, self.block_number, self.view, &self.highest_qc_hash) + Self::signing_message( + self.chain_id, + self.block_number, + self.view, + &self.highest_qc_hash, + ) } } @@ -233,4 +246,32 @@ mod tests { assert_eq!(state.advance_view(), 2); assert_eq!(state.advance_view(), 3); } + + #[test] + fn signing_message_has_correct_layout_and_length() { + let chain_id: u64 = 42; + let block_number: u64 = 1_000; + let view: u64 = 3; + let qc_hash = ShellHash::from([0xab; 32]); + + let msg = ViewChangeMessage::signing_message(chain_id, block_number, view, &qc_hash); + + // Total length: 16 (domain) + 8 + 8 + 8 + 32 = 72 bytes + assert_eq!(msg.len(), 72); + + // Domain tag at [0..16] + assert_eq!(&msg[0..16], b"SHELL_VIEWCHG_V1"); + + // chain_id at [16..24] big-endian + assert_eq!(&msg[16..24], &42u64.to_be_bytes()); + + // block_number at [24..32] big-endian + assert_eq!(&msg[24..32], &1_000u64.to_be_bytes()); + + // view at [32..40] big-endian + assert_eq!(&msg[32..40], &3u64.to_be_bytes()); + + // highest_qc_hash at [40..72] + assert_eq!(&msg[40..72], &[0xab; 32]); + } } diff --git a/crates/consensus/src/wpoa.rs b/crates/consensus/src/wpoa.rs index 27fea9a..62fcae1 100644 --- a/crates/consensus/src/wpoa.rs +++ b/crates/consensus/src/wpoa.rs @@ -37,6 +37,9 @@ pub struct WPoaConfig { pub weights: Vec, /// Validator set lifecycle parameters. pub validator_set_config: ValidatorSetConfig, + /// Chain ID used to reject cross-chain view-change messages. + /// 0 means unconfigured (no chain-ID enforcement in `ViewChangeState`). + pub chain_id: u64, } impl WPoaConfig { @@ -47,6 +50,7 @@ impl WPoaConfig { poa, weights: vec![1u64; n], validator_set_config: ValidatorSetConfig::default(), + chain_id: 0, } } @@ -59,6 +63,7 @@ impl WPoaConfig { weights, poa, validator_set_config: ValidatorSetConfig::default(), + chain_id: 0, } } } @@ -93,12 +98,17 @@ impl WPoaEngine { let validator_set = ValidatorSet::from_genesis(entries, config.validator_set_config.clone()); + let mut view_change_state = ViewChangeState::new(); + if config.chain_id != 0 { + view_change_state.set_chain_id(config.chain_id); + } + Self { inner: PoaEngine::new(poa), validator_set, validator_set_config: config.validator_set_config, slash_weights: HashMap::new(), - view_change_state: Mutex::new(ViewChangeState::new()), + view_change_state: Mutex::new(view_change_state), signer: None, } } @@ -566,8 +576,14 @@ mod tests { fn view_change_quorum_advances_view() { let mut e = engine(vec![addr(1), addr(2), addr(3)], vec![1, 1, 1]); - assert!(!e.handle_view_change_message(ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(1), vec![1]), 3,)); - assert!(e.handle_view_change_message(ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(2), vec![2]), 3,)); + assert!(!e.handle_view_change_message( + ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(1), vec![1]), + 3, + )); + assert!(e.handle_view_change_message( + ViewChangeMessage::new(0, 7, 0, ShellHash::ZERO, addr(2), vec![2]), + 3, + )); assert_eq!(e.current_view(), 1); assert_eq!(e.proposer_for_block(0), addr(2)); } @@ -576,8 +592,14 @@ mod tests { fn note_block_progress_resets_view_change_state() { let mut e = engine(vec![addr(1), addr(2), addr(3)], vec![1, 1, 1]); - assert!(!e.handle_view_change_message(ViewChangeMessage::new(0, 9, 0, ShellHash::ZERO, addr(1), vec![1]), 3,)); - assert!(e.handle_view_change_message(ViewChangeMessage::new(0, 9, 0, ShellHash::ZERO, addr(2), vec![2]), 3,)); + assert!(!e.handle_view_change_message( + ViewChangeMessage::new(0, 9, 0, ShellHash::ZERO, addr(1), vec![1]), + 3, + )); + assert!(e.handle_view_change_message( + ViewChangeMessage::new(0, 9, 0, ShellHash::ZERO, addr(2), vec![2]), + 3, + )); assert_eq!(e.current_view(), 1); e.note_block_progress(42); diff --git a/crates/network/src/message.rs b/crates/network/src/message.rs index 6869373..a8e6a1f 100644 --- a/crates/network/src/message.rs +++ b/crates/network/src/message.rs @@ -409,9 +409,12 @@ mod tests { #[test] fn serde_roundtrip_new_attestation() { let attestation = Attestation { + chain_id: 1, + parent_hash: ShellHash::ZERO, block_hash: ShellHash::default(), block_number: 99, validator: Address::from_public_key(b"validator-key", 0), + round: 0, signature: vec![1, 2, 3, 4], }; let msg = NetworkMessage::NewAttestation(Box::new(attestation)); @@ -518,8 +521,10 @@ mod tests { #[test] fn serde_roundtrip_wpoa_view_change() { let msg = NetworkMessage::WPoaViewChange(Box::new(ViewChangeMessage::new( - 3, + 1, 42, + 3, + ShellHash::ZERO, Address::from_public_key(b"voter-key", 0), vec![1, 2, 3], ))); diff --git a/crates/node/src/node/p2p_handlers.rs b/crates/node/src/node/p2p_handlers.rs index 81a334f..8030d32 100644 --- a/crates/node/src/node/p2p_handlers.rs +++ b/crates/node/src/node/p2p_handlers.rs @@ -54,6 +54,14 @@ impl Node { } } + // Reject cross-network attestations: chain_id must match our own. + if attestation.chain_id != self.config.chain_id { + return Err(NodeError::Startup(format!( + "attestation chain_id {} does not match local chain_id {}", + attestation.chain_id, self.config.chain_id + ))); + } + // Verify the attesting validator is a known authority. let known = self.known_authorities.read(); let pubkey = known.get(&validator).ok_or_else(|| { @@ -152,18 +160,22 @@ impl Node { let proposer_addr = self.config.proposer_address.ok_or(NodeError::NotProposer)?; // Look up the parent hash so the signing payload binds to the specific fork. + // Return an error if the header is missing — signing with ZERO parent_hash would + // produce an invalid payload that misses the intended fork-binding guarantee. let parent_hash = self .chain_store .get_header_by_hash(&block_hash) - .ok() - .flatten() - .map(|h| h.parent_hash) - .unwrap_or(ShellHash::ZERO); + .map_err(|e| NodeError::Startup(format!("failed to look up header for attestation parent_hash: {e}")))? + .ok_or_else(|| NodeError::Startup(format!( + "header not found for block {block_hash} — cannot create attestation with correct parent_hash" + )))? + .parent_hash; let chain_id = self.config.chain_id; // round = 0 for standard PoA; wPoA round is embedded per-block in Phase 2. let round: u64 = 0; - let msg = Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number, round); + let msg = + Attestation::signing_message(chain_id, &parent_hash, &block_hash, block_number, round); let sig = signer .sign(&msg) .map_err(|e| NodeError::Startup(format!("failed to sign attestation: {e}")))?; @@ -631,6 +643,14 @@ impl Node { ))); } + // Reject cross-chain view-change injection. + if msg.chain_id != self.config.chain_id { + return Err(NodeError::Startup(format!( + "view-change chain_id {} does not match local chain_id {}", + msg.chain_id, self.config.chain_id + ))); + } + let known = self.known_authorities.read(); let pubkey = known.get(&msg.validator).ok_or_else(|| { NodeError::Startup(format!( diff --git a/crates/node/src/pruning.rs b/crates/node/src/pruning.rs index e97287a..4b90614 100644 --- a/crates/node/src/pruning.rs +++ b/crates/node/src/pruning.rs @@ -16,6 +16,13 @@ use shell_storage::{ DEFAULT_WITNESS_RETENTION, }; +/// Grace period (in blocks) before a replaced witness bundle may be deleted. +/// Equals `MIN_AMENDMENT_DEPTH` — the minimum depth at which an on-chain amendment +/// that replaces a witness bundle can be considered confirmed. +/// Using a fixed 0 grace period would delete witness bundles before amendments +/// at those heights are confirmed, breaking proof retrieval. +pub const MIN_AMENDMENT_DEPTH: u64 = 128; + /// High-level node storage classification. /// /// Each profile maps to a concrete set of pruning parameters. The `--storage-profile` @@ -25,11 +32,11 @@ use shell_storage::{ /// White-paper canonical names are `Archive`, `Full`, and `Pruned (Rolling)`. /// `Light` is an accepted alias for `Pruned` (backwards compatible). /// -/// | Profile (WP name) | body_retention | witness_retention | proof_replacement_grace | keep_recent | -/// |-------------------------|---------------|------------------|------------------------|-------------| -/// | Archive | 0 (forever) | 0 (forever) | u64::MAX (never) | 0 (forever) | -/// | Full | 0 (forever) | 128 | 128 (MIN_AMENDMENT_DEPTH) | 0 (forever) | -/// | Pruned / Rolling / Light| 4 096 | 64 | 128 (MIN_AMENDMENT_DEPTH) | 4 096 | +/// | Profile (WP name) | body_retention | witness_retention | keep_recent | proof_replacement_grace | +/// |-------------------------|----------------|-------------------|-------------|----------------------------------| +/// | Archive | 0 (forever) | 0 (forever) | 0 (forever) | u64::MAX (never delete) | +/// | Full | 0 (forever) | 128 | 0 (forever) | MIN_AMENDMENT_DEPTH (128) | +/// | Pruned / Rolling / Light| 4 096 | 64 | 4 096 | MIN_AMENDMENT_DEPTH (128) | #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "lowercase")] pub enum StorageProfile { @@ -79,9 +86,9 @@ impl StorageProfile { // Archive: keep everything forever; never delete witness even after STARK proof. Self::Archive => (0, 0, 0, u64::MAX), // Full: keep TX forever; grace = MIN_AMENDMENT_DEPTH (WP §storage). - Self::Full => (0, DEFAULT_WITNESS_RETENTION, 0, 128), + Self::Full => (0, DEFAULT_WITNESS_RETENTION, 0, MIN_AMENDMENT_DEPTH), // Light: rolling 4 096-block window (~2.3 h at 2 s/block); grace = MIN_AMENDMENT_DEPTH. - Self::Light => (4_096, 64, 4_096, 128), + Self::Light => (4_096, 64, 4_096, MIN_AMENDMENT_DEPTH), } } @@ -544,7 +551,10 @@ mod tests { assert_eq!(body, 0, "full: body_retention must be 0"); assert!(witness > 0, "full: witness_retention must be non-zero"); assert_eq!(keep, 0, "full: keep_recent must be 0"); - assert_eq!(grace, 128, "full: grace must be MIN_AMENDMENT_DEPTH (128)"); + assert_eq!( + grace, MIN_AMENDMENT_DEPTH, + "full: grace must be MIN_AMENDMENT_DEPTH (128)" + ); } #[test] @@ -553,7 +563,10 @@ mod tests { assert!(body > 0, "light: body_retention must be non-zero"); assert!(witness > 0, "light: witness_retention must be non-zero"); assert!(keep > 0, "light: keep_recent must be non-zero"); - assert_eq!(grace, 128, "light: grace must be MIN_AMENDMENT_DEPTH (128)"); + assert_eq!( + grace, MIN_AMENDMENT_DEPTH, + "light: grace must be MIN_AMENDMENT_DEPTH (128)" + ); } #[test] From 0795f055b8ec52297f1b798ec9092841ba7383f5 Mon Sep 17 00:00:00 2001 From: LucienSong Date: Thu, 28 May 2026 17:55:13 +0800 Subject: [PATCH 3/4] fix(consensus): move ShellHash import into test module to fix unused-import clippy error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/consensus/src/wpoa.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/consensus/src/wpoa.rs b/crates/consensus/src/wpoa.rs index 62fcae1..41aac4c 100644 --- a/crates/consensus/src/wpoa.rs +++ b/crates/consensus/src/wpoa.rs @@ -18,7 +18,7 @@ use std::sync::{Arc, Mutex}; use async_trait::async_trait; use shell_core::{Block, BlockHeader}; use shell_crypto::{PQSignature, Signer, Verifier}; -use shell_primitives::{Address, ShellHash}; +use shell_primitives::Address; use crate::poa::PoaEngine; use crate::validator::{ValidatorSet, ValidatorSetConfig}; @@ -419,6 +419,7 @@ mod tests { use super::*; use crate::{poa::PoaConfig, VIEW_CHANGE_TIMEOUT_MS}; use shell_crypto::{PQSignature, SignatureType}; + use shell_primitives::ShellHash; fn addr(n: u8) -> Address { Address::from([n; 20]) From 0da7e6a26d2a52a5cdf5923a87f423b1180aac51 Mon Sep 17 00:00:00 2001 From: LucienSong Date: Thu, 28 May 2026 18:09:46 +0800 Subject: [PATCH 4/4] fix(node): dereference Copy ShellHash instead of clone() in event_loop Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/node/src/node/event_loop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/node/src/node/event_loop.rs b/crates/node/src/node/event_loop.rs index f6635a1..1cdf110 100644 --- a/crates/node/src/node/event_loop.rs +++ b/crates/node/src/node/event_loop.rs @@ -717,7 +717,7 @@ impl Node { let view = self.consensus.read().current_view(); let block_number = self.head_number() + 1; let chain_id = self.config.chain_id; - let highest_qc_hash = self.finality.read().last_finalized_hash().clone(); + let highest_qc_hash = *self.finality.read().last_finalized_hash(); let signing_message = ViewChangeMessage::signing_message( chain_id, block_number,