Skip to content

Commit ef1a566

Browse files
romanzacseemenkina
andauthored
test: Improve coverage (#10)
* test: improve coverage session scope_config storage_stream * test: cover gaps security correctness critical * test: overflow during additions id-collapse safety * fix: handle large number edge cases in ID generation, timestamps, and round limits (#12) - Updated README to correct the link for the Hashgraph-like Consensus Protocol RFC. - Enhanced session handling by enforcing maximum vote count based on expected voters, ensuring robust error handling for overflow scenarios. --------- Co-authored-by: Ekaterina Broslavskaya <seemenkina@gmail.com>
1 parent afc0d4f commit ef1a566

File tree

9 files changed

+831
-14
lines changed

9 files changed

+831
-14
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
/target
22
/.vscode
33
/.idea
4+
/.claude
5+
/docs
6+
CLAUDE.md

README.md

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Perfect for group governance, voting systems, or any scenario where you need dis
1717
- **Event-driven** - Subscribe to consensus outcomes via a broadcast event bus
1818
- **Cryptographic integrity** - Votes are signed with secp256k1 and chained in a hashgraph structure
1919

20-
Based on the [Hashgraph-like Consensus Protocol RFC](https://github.com/vacp2p/rfc-index/blob/main/vac/raw/consensus-hashgraphlike.md).
20+
Based on the [Hashgraph-like Consensus Protocol RFC](https://lip.logos.co/ift-ts/raw/consensus-hashgraphlike.html).
2121

2222
## Installation
2323

@@ -88,10 +88,10 @@ Scope (group / channel)
8888

8989
### Network Types
9090

91-
| Type | Rounds | Behavior |
92-
|------|--------|----------|
93-
| **Gossipsub** (default) | Fixed 2 rounds | Round 1 = proposal broadcast, Round 2 = all votes |
94-
| **P2P** | Dynamic `ceil(2n/3)` | Each vote advances the round by one |
91+
| Type | Rounds | Behavior |
92+
| ----------------------- | -------------------- | ------------------------------------------------- |
93+
| **Gossipsub** (default) | Fixed 2 rounds | Round 1 = proposal broadcast, Round 2 = all votes |
94+
| **P2P** | Dynamic `ceil(2n/3)` | Each vote advances the round by one |
9595

9696
## API Reference
9797

@@ -270,12 +270,12 @@ pub trait ConsensusEventBus<Scope> {
270270

271271
The `utils` module provides low-level helpers:
272272

273-
| Function | Description |
274-
|----------|-------------|
275-
| `validate_proposal()` | Validate a proposal and its votes |
276-
| `validate_vote()` | Verify a vote's signature and structure |
277-
| `validate_vote_chain()` | Ensure parent/received hash chains are correct |
278-
| `has_sufficient_votes()` | Quick threshold check (count-based) |
273+
| Function | Description |
274+
| ------------------------------ | ------------------------------------------------------------------------ |
275+
| `validate_proposal()` | Validate a proposal and its votes |
276+
| `validate_vote()` | Verify a vote's signature and structure |
277+
| `validate_vote_chain()` | Ensure parent/received hash chains are correct |
278+
| `has_sufficient_votes()` | Quick threshold check (count-based) |
279279
| `calculate_consensus_result()` | Determine result from collected votes using threshold and liveness rules |
280280

281281
## Building

src/session.rs

Lines changed: 184 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,13 @@ impl ConsensusSession {
264264
}
265265
}
266266

267+
// Each distinct voter can vote at most once, so the batch size
268+
// is bounded by expected_voters_count (u32). Reject early if violated.
269+
if votes.len() > self.proposal.expected_voters_count as usize {
270+
self.state = ConsensusState::Failed;
271+
return Err(ConsensusError::MaxRoundsExceeded);
272+
}
273+
267274
validate_vote_chain(&votes)?;
268275
for vote in &votes {
269276
validate_vote(vote, expiration_timestamp, creation_time)?;
@@ -287,6 +294,12 @@ impl ConsensusSession {
287294
/// - For P2P: Calculates `(current_round - 1) + vote_count`.
288295
/// - For Gossipsub: Moves to Round 2 if `vote_count > 0`.
289296
fn check_round_limit(&mut self, vote_count: usize) -> Result<(), ConsensusError> {
297+
// vote_count cannot exceed expected_voters_count (u32); reject if it does
298+
if vote_count > self.proposal.expected_voters_count as usize {
299+
self.state = ConsensusState::Failed;
300+
return Err(ConsensusError::MaxRoundsExceeded);
301+
}
302+
290303
// Determine the value to compare against the limit based on configuration
291304
let projected_value = if self.config.use_gossipsub_rounds {
292305
// Gossipsub Logic:
@@ -303,6 +316,7 @@ impl ConsensusSession {
303316
// RFC Section 2.5.3: Round increments per vote.
304317
// Current existing votes = round - 1.
305318
// Projected total = Existing votes + New votes.
319+
// vote_count is bounded by expected_voters_count (u32), safe to cast
306320
let current_votes = self.proposal.round.saturating_sub(1);
307321
current_votes.saturating_add(vote_count as u32)
308322
};
@@ -336,6 +350,7 @@ impl ConsensusSession {
336350
} else {
337351
// RFC Section 2.5.3: P2P
338352
// Round increments for every vote added.
353+
// vote_count is bounded by expected_voters_count (u32), safe to cast
339354
self.proposal.round = self.proposal.round.saturating_add(vote_count as u32);
340355
}
341356
}
@@ -381,11 +396,13 @@ impl ConsensusSession {
381396

382397
#[cfg(test)]
383398
mod tests {
399+
use std::time::Duration;
400+
384401
use alloy::signers::local::PrivateKeySigner;
385402

386403
use crate::{
387404
error::ConsensusError,
388-
session::{ConsensusConfig, ConsensusSession},
405+
session::{ConsensusConfig, ConsensusSession, ConsensusState},
389406
types::CreateProposalRequest,
390407
utils::build_vote,
391408
};
@@ -489,4 +506,170 @@ mod tests {
489506
let err = session.add_vote(vote5).unwrap_err();
490507
assert!(matches!(err, ConsensusError::MaxRoundsExceeded));
491508
}
509+
510+
#[test]
511+
fn consensus_config_builder_and_getters_cover_edges() {
512+
let cfg = ConsensusConfig::gossipsub()
513+
.with_threshold(0.75)
514+
.unwrap()
515+
.with_timeout(Duration::from_secs(42))
516+
.unwrap()
517+
.with_liveness_criteria(false);
518+
519+
assert_eq!(cfg.consensus_threshold(), 0.75);
520+
assert_eq!(cfg.consensus_timeout(), Duration::from_secs(42));
521+
assert!(!cfg.liveness_criteria());
522+
523+
let err = ConsensusConfig::gossipsub()
524+
.with_threshold(1.1)
525+
.unwrap_err();
526+
assert!(matches!(err, ConsensusError::InvalidConsensusThreshold));
527+
528+
let err = ConsensusConfig::gossipsub()
529+
.with_timeout(Duration::from_secs(0))
530+
.unwrap_err();
531+
assert!(matches!(err, ConsensusError::InvalidTimeout));
532+
533+
// Covers max_round_limit branch when P2P-like mode uses explicit max_rounds (non-zero).
534+
let explicit = ConsensusConfig::new(2.0 / 3.0, Duration::from_secs(60), 7, false, true);
535+
assert_eq!(explicit.max_round_limit(100), 7);
536+
}
537+
538+
#[tokio::test]
539+
async fn add_vote_rejects_non_active_and_reports_reached_when_finalized() {
540+
let signer = PrivateKeySigner::random();
541+
let request = CreateProposalRequest::new(
542+
"Test".into(),
543+
"".into(),
544+
signer.address().as_slice().to_vec(),
545+
3,
546+
60,
547+
true,
548+
)
549+
.unwrap();
550+
let proposal = request.into_proposal().unwrap();
551+
552+
// Failed sessions reject new votes.
553+
let mut failed_session =
554+
ConsensusSession::new(proposal.clone(), ConsensusConfig::gossipsub());
555+
failed_session.state = ConsensusState::Failed;
556+
let vote = build_vote(&failed_session.proposal, true, signer.clone())
557+
.await
558+
.unwrap();
559+
let err = failed_session.add_vote(vote).unwrap_err();
560+
assert!(matches!(err, ConsensusError::SessionNotActive));
561+
562+
// Finalized sessions return existing transition/result.
563+
let mut finalized_session = ConsensusSession::new(proposal, ConsensusConfig::gossipsub());
564+
finalized_session.state = ConsensusState::ConsensusReached(true);
565+
let vote = build_vote(&finalized_session.proposal, true, signer)
566+
.await
567+
.unwrap();
568+
let transition = finalized_session.add_vote(vote).unwrap();
569+
assert!(matches!(
570+
transition,
571+
crate::types::SessionTransition::ConsensusReached(true)
572+
));
573+
}
574+
575+
#[tokio::test]
576+
async fn initialize_with_votes_non_active_duplicate_and_zero_votes_paths() {
577+
let signer = PrivateKeySigner::random();
578+
let request = CreateProposalRequest::new(
579+
"Test".into(),
580+
"".into(),
581+
signer.address().as_slice().to_vec(),
582+
4,
583+
60,
584+
true,
585+
)
586+
.unwrap();
587+
let proposal = request.into_proposal().unwrap();
588+
589+
// Non-active sessions reject initialization.
590+
let mut inactive = ConsensusSession::new(proposal.clone(), ConsensusConfig::gossipsub());
591+
inactive.state = ConsensusState::Failed;
592+
let err = inactive
593+
.initialize_with_votes(vec![], proposal.expiration_timestamp, proposal.timestamp)
594+
.unwrap_err();
595+
assert!(matches!(err, ConsensusError::SessionNotActive));
596+
597+
// Duplicate owners are rejected before chain/signature checks.
598+
let mut dup_session = ConsensusSession::new(proposal.clone(), ConsensusConfig::gossipsub());
599+
let vote1 = build_vote(&dup_session.proposal, true, signer.clone())
600+
.await
601+
.unwrap();
602+
let vote2 = build_vote(&dup_session.proposal, false, signer)
603+
.await
604+
.unwrap();
605+
let err = dup_session
606+
.initialize_with_votes(
607+
vec![vote1, vote2],
608+
proposal.expiration_timestamp,
609+
proposal.timestamp,
610+
)
611+
.unwrap_err();
612+
assert!(matches!(err, ConsensusError::DuplicateVote));
613+
614+
// Explicitly exercise gossipsub projected round branch where vote_count == 0.
615+
let mut zero_votes = ConsensusSession::new(proposal, ConsensusConfig::gossipsub());
616+
zero_votes.check_round_limit(0).unwrap();
617+
}
618+
619+
#[test]
620+
fn p2p_round_limit_should_reject_effectively_huge_vote_count() {
621+
if usize::BITS <= 32 {
622+
return;
623+
}
624+
625+
let signer = PrivateKeySigner::random();
626+
let request = CreateProposalRequest::new(
627+
"TruncationTest".into(),
628+
vec![],
629+
signer.address().as_slice().to_vec(),
630+
1,
631+
60,
632+
true,
633+
)
634+
.unwrap();
635+
636+
let proposal = request.into_proposal().unwrap();
637+
let mut session = ConsensusSession::new(proposal, ConsensusConfig::p2p());
638+
639+
let wrapped_vote_count = (u32::MAX as usize) + 1;
640+
641+
// Desired behavior: an effectively huge batch must be rejected by round-limit checks.
642+
let result = session.check_round_limit(wrapped_vote_count);
643+
assert!(
644+
result.is_err(),
645+
"effectively huge vote_count should not pass round-limit checks"
646+
);
647+
}
648+
649+
#[test]
650+
fn p2p_update_round_should_advance_for_max_u32_vote_count() {
651+
let signer = PrivateKeySigner::random();
652+
let request = CreateProposalRequest::new(
653+
"RoundUpdateMax".into(),
654+
vec![],
655+
signer.address().as_slice().to_vec(),
656+
u32::MAX,
657+
60,
658+
true,
659+
)
660+
.unwrap();
661+
662+
let proposal = request.into_proposal().unwrap();
663+
let mut session = ConsensusSession::new(proposal, ConsensusConfig::p2p());
664+
let starting_round = session.proposal.round;
665+
666+
// vote_count at the u32 boundary should still advance the round via saturating_add
667+
session.update_round(u32::MAX as usize);
668+
669+
assert!(
670+
session.proposal.round > starting_round,
671+
"round should advance when max u32 vote_count is applied"
672+
);
673+
assert_eq!(session.proposal.round, u32::MAX);
674+
}
492675
}

src/types.rs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,37 @@ impl CreateProposalRequest {
9494
expected_voters_count: self.expected_voters_count,
9595
round: 1,
9696
timestamp: now,
97-
expiration_timestamp: now + self.expiration_timestamp,
97+
expiration_timestamp: now.saturating_add(self.expiration_timestamp),
9898
liveness_criteria_yes: self.liveness_criteria_yes,
9999
})
100100
}
101101
}
102+
103+
#[cfg(test)]
104+
mod tests {
105+
use super::CreateProposalRequest;
106+
107+
#[test]
108+
fn into_proposal_should_not_overflow_expiration_timestamp() {
109+
let request = CreateProposalRequest::new(
110+
"overflow-check".to_string(),
111+
vec![],
112+
vec![1u8; 20],
113+
1,
114+
u64::MAX,
115+
true,
116+
)
117+
.expect("request should be valid");
118+
119+
// Desired behavior: proposal creation should not panic on overflow-prone input,
120+
// and expiration should never be earlier than creation timestamp.
121+
let proposal = request
122+
.into_proposal()
123+
.expect("proposal creation should handle large expiration safely");
124+
125+
assert!(
126+
proposal.expiration_timestamp >= proposal.timestamp,
127+
"expiration must not overflow below creation timestamp"
128+
);
129+
}
130+
}

src/utils.rs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,18 @@ use crate::{
2020

2121
const SIGNATURE_LENGTH: usize = 65;
2222

23+
/// Fold a 128-bit value into 32 bits via XOR so every bit contributes.
24+
fn fold_u128_to_u32(n: u128) -> u32 {
25+
((n >> 96) as u32) ^ ((n >> 64) as u32) ^ ((n >> 32) as u32) ^ (n as u32)
26+
}
27+
2328
/// Generate a unique 32-bit ID from a UUID.
29+
///
30+
/// Uses XOR folding so all 128 bits of the UUID contribute to the result,
31+
/// avoiding collisions from simple truncation.
2432
pub fn generate_id() -> u32 {
2533
let uuid = Uuid::new_v4();
26-
uuid.as_u128() as u32
34+
fold_u128_to_u32(uuid.as_u128())
2735
}
2836

2937
/// Compute the hash of a vote for signing and validation.
@@ -377,3 +385,32 @@ pub fn has_sufficient_votes(
377385
let required_votes = calculate_required_votes(expected_voters, consensus_threshold);
378386
total_votes >= required_votes
379387
}
388+
389+
#[cfg(test)]
390+
mod tests {
391+
use uuid::Uuid;
392+
393+
use super::fold_u128_to_u32;
394+
395+
#[test]
396+
fn id_generation_should_not_collapse_distinct_128bit_values() {
397+
let low = 0xDEADBEEFu32;
398+
let high_a = 0x00000001u128;
399+
let high_b = 0xABCDEF01u128;
400+
401+
let value_a = (high_a << 32) | (low as u128);
402+
let value_b = (high_b << 32) | (low as u128);
403+
404+
let uuid_a = Uuid::from_u128(value_a);
405+
let uuid_b = Uuid::from_u128(value_b);
406+
407+
let id_a = fold_u128_to_u32(uuid_a.as_u128());
408+
let id_b = fold_u128_to_u32(uuid_b.as_u128());
409+
410+
// Desired behavior: distinct 128-bit values should remain distinct after conversion.
411+
assert_ne!(
412+
id_a, id_b,
413+
"distinct 128-bit values should not collapse to the same 32-bit id"
414+
);
415+
}
416+
}

0 commit comments

Comments
 (0)