Skip to content

Commit d737044

Browse files
authored
fix(service): add timestamp to events, update config setup, refactor proposal type (#4)
* feat(event): add timestamp into default service * Enhance ConsensusEvent enum by adding PartialEq and Eq traits for improved comparison capabilities. * Implement get_proposal_config method and enhance ConsensusConfig with new configuration methods - Added `get_proposal_config` method in `service.rs` to retrieve the consensus configuration for a specific proposal. - Introduced new methods in `ConsensusConfig` for setting timeout, threshold, and liveness criteria, improving configurability. - Updated tests to ensure that proposal configurations correctly preserve timeout overrides and validate dynamic max rounds logic. * Refactor Proposal to use Vec<u8> for payload instead of String.
1 parent a44a58f commit d737044

12 files changed

+166
-77
lines changed

src/protos/messages/v1/consensus.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ package consensus.v1;
44
// Proposal represents a consensus proposal that needs voting
55
message Proposal {
66
string name = 10; // Proposal name
7-
string payload = 11; // Payload with the proposal data
7+
bytes payload = 11; // Payload with the proposal data
88
uint32 proposal_id = 12; // Unique identifier of the proposal
99
bytes proposal_owner = 13; // Public key of the creator
1010
repeated Vote votes = 14; // Vote list in the proposal

src/service.rs

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
session::{ConsensusConfig, ConsensusSession, ConsensusState},
1111
storage::{ConsensusStorage, InMemoryConsensusStorage},
1212
types::{ConsensusEvent, SessionTransition},
13-
utils::{calculate_consensus_result, has_sufficient_votes},
13+
utils::{calculate_consensus_result, current_timestamp, has_sufficient_votes},
1414
};
1515
/// The main service that handles proposals, votes, and consensus.
1616
///
@@ -147,6 +147,16 @@ where
147147
Ok(Some(result))
148148
}
149149

150+
/// Get the resolved per-proposal consensus configuration for an existing session.
151+
pub async fn get_proposal_config(
152+
&self,
153+
scope: &Scope,
154+
proposal_id: u32,
155+
) -> Result<ConsensusConfig, ConsensusError> {
156+
let session = self.get_session(scope, proposal_id).await?;
157+
Ok(session.config)
158+
}
159+
150160
/// Get all proposals that have reached consensus, along with their results.
151161
///
152162
/// Returns a map from proposal ID to result (`true` for YES, `false` for NO).
@@ -270,6 +280,9 @@ where
270280
proposal: Option<&Proposal>,
271281
) -> Result<ConsensusConfig, ConsensusError> {
272282
// 1. If explicit config override exists, use it as base
283+
// NOTE: if a per-proposal override is provided, we should not stomp its timeout
284+
// from the proposal's expiration fields (the caller explicitly chose it).
285+
let has_explicit_override = proposal_override.is_some();
273286
let base_config = if let Some(override_config) = proposal_override {
274287
override_config
275288
} else if let Some(scope_config) = self.storage.get_scope_config(scope).await? {
@@ -280,8 +293,11 @@ where
280293

281294
// 2. Apply proposal field overrides if proposal is provided
282295
if let Some(prop) = proposal {
283-
// Calculate timeout from expiration_timestamp (absolute timestamp) - timestamp (creation time)
284-
let timeout_seconds = if prop.expiration_timestamp > prop.timestamp {
296+
// Calculate timeout from expiration_timestamp (absolute timestamp) - timestamp (creation time),
297+
// unless an explicit override was supplied.
298+
let timeout_seconds = if has_explicit_override {
299+
base_config.consensus_timeout()
300+
} else if prop.expiration_timestamp > prop.timestamp {
285301
Duration::from_secs(prop.expiration_timestamp - prop.timestamp)
286302
} else {
287303
base_config.consensus_timeout()
@@ -342,12 +358,19 @@ where
342358
ConsensusEvent::ConsensusReached {
343359
proposal_id,
344360
result: consensus_result,
361+
timestamp: current_timestamp()?,
345362
},
346363
);
347364
Ok(consensus_result)
348365
}
349366
None => {
350-
self.emit_event(scope, ConsensusEvent::ConsensusFailed { proposal_id });
367+
self.emit_event(
368+
scope,
369+
ConsensusEvent::ConsensusFailed {
370+
proposal_id,
371+
timestamp: current_timestamp()?,
372+
},
373+
);
351374
Err(ConsensusError::InsufficientVotesAtTimeout)
352375
}
353376
}
@@ -423,6 +446,7 @@ where
423446
ConsensusEvent::ConsensusReached {
424447
proposal_id,
425448
result,
449+
timestamp: current_timestamp().unwrap_or(0),
426450
},
427451
);
428452
}

src/session.rs

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,24 @@ impl ConsensusConfig {
6666
ConsensusConfig::from(NetworkType::Gossipsub)
6767
}
6868

69-
pub fn set_up_rounds(&mut self, max_rounds: u32) -> Result<(), ConsensusError> {
70-
if max_rounds == 0 {
71-
return Err(ConsensusError::InvalidMaxRounds);
72-
}
73-
self.max_rounds = max_rounds;
74-
Ok(())
69+
/// Set consensus timeout (validated) and return the updated config.
70+
pub fn with_timeout(mut self, consensus_timeout: Duration) -> Result<Self, ConsensusError> {
71+
crate::utils::validate_timeout(consensus_timeout)?;
72+
self.consensus_timeout = consensus_timeout;
73+
Ok(self)
74+
}
75+
76+
/// Set consensus threshold (validated) and return the updated config.
77+
pub fn with_threshold(mut self, consensus_threshold: f64) -> Result<Self, ConsensusError> {
78+
crate::utils::validate_threshold(consensus_threshold)?;
79+
self.consensus_threshold = consensus_threshold;
80+
Ok(self)
81+
}
82+
83+
/// Set liveness criteria and return the updated config.
84+
pub fn with_liveness_criteria(mut self, liveness_criteria: bool) -> Self {
85+
self.liveness_criteria = liveness_criteria;
86+
self
7587
}
7688

7789
/// Create a new ConsensusConfig with the given values.
@@ -381,8 +393,7 @@ mod tests {
381393
.unwrap();
382394

383395
let proposal = request.into_proposal().unwrap();
384-
let mut config = ConsensusConfig::gossipsub();
385-
config.set_up_rounds(2).unwrap();
396+
let config = ConsensusConfig::gossipsub();
386397
let mut session = ConsensusSession::new(proposal, config);
387398

388399
// Round 1 -> Round 2 (first vote)
@@ -409,12 +420,14 @@ mod tests {
409420

410421
#[tokio::test]
411422
async fn enforce_max_rounds_p2p() {
412-
// P2P: max_rounds = 2 means maximum 2 votes
413-
// Round 1 = 0 votes, Round 2 = 1 vote, Round 3 = 2 votes
414-
// So max_rounds = 2 allows up to round 3 (2 votes)
423+
// P2P defaults: max_rounds = 0 triggers dynamic calculation based on expected voters.
424+
// For threshold=2/3 and expected_voters=5, max_round_limit = ceil(2n/3) = 4 votes.
425+
// Round 1 = 0 votes, Round 2 = 1 vote, ... Round 5 = 4 votes.
415426
let signer1 = PrivateKeySigner::random();
416427
let signer2 = PrivateKeySigner::random();
417428
let signer3 = PrivateKeySigner::random();
429+
let signer4 = PrivateKeySigner::random();
430+
let signer5 = PrivateKeySigner::random();
418431

419432
let request = CreateProposalRequest::new(
420433
"Test".into(),
@@ -427,8 +440,7 @@ mod tests {
427440
.unwrap();
428441

429442
let proposal = request.into_proposal().unwrap();
430-
let mut config = ConsensusConfig::p2p();
431-
config.set_up_rounds(2).unwrap();
443+
let config = ConsensusConfig::p2p();
432444
let mut session = ConsensusSession::new(proposal, config);
433445

434446
// Round 1 -> Round 2 (first vote, 1 vote total)
@@ -443,9 +455,21 @@ mod tests {
443455
assert_eq!(session.proposal.round, 3);
444456
assert_eq!(session.votes.len(), 2);
445457

446-
// Third vote would be round 4 (3 votes total), which exceeds max_rounds = 2
458+
// Round 3 -> Round 4 (third vote, 3 votes total) - should succeed
447459
let vote3 = build_vote(&session.proposal, true, signer3).await.unwrap();
448-
let err = session.add_vote(vote3).unwrap_err();
460+
session.add_vote(vote3).unwrap();
461+
assert_eq!(session.proposal.round, 4);
462+
assert_eq!(session.votes.len(), 3);
463+
464+
// Round 4 -> Round 5 (fourth vote, 4 votes total) - should succeed (dynamic limit = 4)
465+
let vote4 = build_vote(&session.proposal, true, signer4).await.unwrap();
466+
session.add_vote(vote4).unwrap();
467+
assert_eq!(session.proposal.round, 5);
468+
assert_eq!(session.votes.len(), 4);
469+
470+
// Fifth vote would exceed dynamic max_round_limit (=4 votes)
471+
let vote5 = build_vote(&session.proposal, true, signer5).await.unwrap();
472+
let err = session.add_vote(vote5).unwrap_err();
449473
assert!(matches!(err, ConsensusError::MaxRoundsExceeded));
450474
}
451475
}

src/types.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@ use crate::{
66
utils::{current_timestamp, generate_id, validate_expected_voters_count, validate_timeout},
77
};
88

9-
#[derive(Debug, Clone)]
9+
#[derive(Debug, Clone, PartialEq, Eq)]
1010
pub enum ConsensusEvent {
1111
/// Consensus was reached! The proposal has a final result (yes or no).
12-
ConsensusReached { proposal_id: u32, result: bool },
12+
ConsensusReached {
13+
proposal_id: u32,
14+
result: bool,
15+
timestamp: u64,
16+
},
1317
/// Consensus failed - not enough votes were collected before the timeout.
14-
ConsensusFailed { proposal_id: u32 },
18+
ConsensusFailed { proposal_id: u32, timestamp: u64 },
1519
}
1620

1721
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -27,7 +31,7 @@ pub struct CreateProposalRequest {
2731
/// A short name for the proposal (e.g., "Upgrade to v2").
2832
pub name: String,
2933
/// Additional details about what's being voted on.
30-
pub payload: String,
34+
pub payload: Vec<u8>,
3135
/// The address (public key bytes) of whoever created this proposal.
3236
pub proposal_owner: Vec<u8>,
3337
/// How many people are expected to vote (used to calculate consensus threshold).
@@ -42,7 +46,7 @@ impl CreateProposalRequest {
4246
/// Create a new proposal request with validation.
4347
pub fn new(
4448
name: String,
45-
payload: String,
49+
payload: Vec<u8>,
4650
proposal_owner: Vec<u8>,
4751
expected_voters_count: u32,
4852
expiration_timestamp: u64,

tests/concurrency_tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use hashgraph_like_consensus::{
1010

1111
const SCOPE: &str = "concurrency_scope";
1212
const PROPOSAL_NAME: &str = "Concurrency Test";
13-
const PROPOSAL_PAYLOAD: &str = "";
13+
const PROPOSAL_PAYLOAD: Vec<u8> = vec![];
1414

1515
const EXPIRATION: u64 = 120;
1616
const EXPIRATION_WAIT_TIME: u64 = 100;
@@ -39,7 +39,7 @@ async fn test_concurrent_vote_casting() {
3939
&scope,
4040
CreateProposalRequest::new(
4141
PROPOSAL_NAME.to_string(),
42-
PROPOSAL_PAYLOAD.to_string(),
42+
PROPOSAL_PAYLOAD,
4343
owner_bytes(&proposal_owner),
4444
EXPECTED_VOTERS_COUNT_10,
4545
EXPIRATION,
@@ -98,7 +98,7 @@ async fn test_concurrent_proposal_operations() {
9898
&scope_clone,
9999
CreateProposalRequest::new(
100100
format!("Proposal {i}"),
101-
PROPOSAL_PAYLOAD.to_string(),
101+
PROPOSAL_PAYLOAD,
102102
owner_bytes(&proposal_owner),
103103
EXPECTED_VOTERS_COUNT_3,
104104
EXPIRATION,
@@ -136,7 +136,7 @@ async fn test_concurrent_duplicate_vote_rejection() {
136136
&scope,
137137
CreateProposalRequest::new(
138138
PROPOSAL_NAME.to_string(),
139-
PROPOSAL_PAYLOAD.to_string(),
139+
PROPOSAL_PAYLOAD,
140140
owner_bytes(&proposal_owner),
141141
EXPECTED_VOTERS_COUNT_3,
142142
EXPIRATION,

0 commit comments

Comments
 (0)