diff --git a/crates/consensus/Cargo.toml b/crates/consensus/Cargo.toml index 3848020..249c412 100644 --- a/crates/consensus/Cargo.toml +++ b/crates/consensus/Cargo.toml @@ -26,6 +26,7 @@ malachite = [ [dependencies] anyhow = { workspace = true } async-trait = { workspace = true } +thiserror = { workspace = true } tracing = { workspace = true } hex = { workspace = true } diff --git a/crates/consensus/src/config.rs b/crates/consensus/src/config.rs index 652c48b..a1bae01 100644 --- a/crates/consensus/src/config.rs +++ b/crates/consensus/src/config.rs @@ -47,3 +47,36 @@ impl ConsensusConfig { &self.chain_id } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_new_with_defaults() { + let config = ConsensusConfig::new("test-chain"); + assert_eq!(config.chain_id(), "test-chain"); + assert_eq!(config.propose_timeout, Duration::from_secs(1)); + assert_eq!(config.prevote_timeout, Duration::from_secs(1)); + assert_eq!(config.precommit_timeout, Duration::from_secs(1)); + } + + #[test] + fn test_builder_pattern() { + let config = ConsensusConfig::new("my-chain") + .with_propose_timeout(Duration::from_millis(500)) + .with_prevote_timeout(Duration::from_millis(300)) + .with_precommit_timeout(Duration::from_millis(200)); + + assert_eq!(config.chain_id(), "my-chain"); + assert_eq!(config.propose_timeout, Duration::from_millis(500)); + assert_eq!(config.prevote_timeout, Duration::from_millis(300)); + assert_eq!(config.precommit_timeout, Duration::from_millis(200)); + } + + #[test] + fn test_chain_id_from_string() { + let config = ConsensusConfig::new(String::from("dynamic-chain")); + assert_eq!(config.chain_id(), "dynamic-chain"); + } +} diff --git a/crates/consensus/src/context.rs b/crates/consensus/src/context.rs index 141fbd9..1d07d5a 100644 --- a/crates/consensus/src/context.rs +++ b/crates/consensus/src/context.rs @@ -3,6 +3,7 @@ use informalsystems_malachitebft_core_types::{ }; use crate::config::ConsensusConfig; +use crate::error::ConsensusError; use crate::proposal::{CutProposal, CutProposalPart}; use crate::signing::Ed25519SigningScheme; use crate::types::{ConsensusHeight, ConsensusValue}; @@ -34,17 +35,36 @@ pub struct CipherBftContext { } impl CipherBftContext { - /// Create a new context. - pub fn new( + /// Create a new context with validation. + /// + /// # Errors + /// Returns `ConsensusError::EmptyValidatorSet` if the validator set is empty. + pub fn try_new( config: ConsensusConfig, validator_set: ConsensusValidatorSet, initial_height: ConsensusHeight, - ) -> Self { - Self { + ) -> Result { + if validator_set.is_empty() { + return Err(ConsensusError::EmptyValidatorSet); + } + Ok(Self { config, validator_set, initial_height, - } + }) + } + + /// Create a new context. + /// + /// # Panics + /// Panics if the validator set is empty. Use `try_new` for fallible construction. + pub fn new( + config: ConsensusConfig, + validator_set: ConsensusValidatorSet, + initial_height: ConsensusHeight, + ) -> Self { + Self::try_new(config, validator_set, initial_height) + .expect("validator set must not be empty") } /// Access the initial height. diff --git a/crates/consensus/src/engine.rs b/crates/consensus/src/engine.rs index 613fc82..2543f32 100644 --- a/crates/consensus/src/engine.rs +++ b/crates/consensus/src/engine.rs @@ -17,11 +17,11 @@ use anyhow::Result; use informalsystems_malachitebft_config::{ - ConsensusConfig as EngineConsensusConfig, ValuePayload as EngineValuePayload, + ConsensusConfig as EngineConsensusConfig, TimeoutConfig, ValuePayload as EngineValuePayload, }; use informalsystems_malachitebft_core_consensus::Params as ConsensusParams; -use informalsystems_malachitebft_core_types::ValuePayload; use informalsystems_malachitebft_core_driver::ThresholdParams; +use informalsystems_malachitebft_core_types::ValuePayload; use informalsystems_malachitebft_app::types::core::SigningProvider; use informalsystems_malachitebft_engine::consensus::{Consensus, ConsensusRef}; use informalsystems_malachitebft_engine::host::HostRef; @@ -35,6 +35,7 @@ use tracing::info_span; use crate::config::ConsensusConfig; use crate::context::CipherBftContext; +use crate::error::ConsensusError; use crate::types::ConsensusHeight; use crate::validator_set::{ConsensusAddress, ConsensusValidator, ConsensusValidatorSet}; @@ -56,6 +57,9 @@ use crate::validator_set::{ConsensusAddress, ConsensusValidator, ConsensusValida /// * `validators` - List of validators with Ed25519 public keys and voting power /// * `initial_height` - Starting height for consensus (defaults to 1 if None) /// +/// # Errors +/// Returns `ConsensusError::EmptyValidatorSet` if the validator list is empty. +/// /// # Example /// ```rust,ignore /// use cipherbft_consensus::{create_context, ConsensusValidator}; @@ -69,18 +73,18 @@ use crate::validator_set::{ConsensusAddress, ConsensusValidator, ConsensusValida /// 100, // voting power /// ), /// ]; -/// let ctx = create_context("my-chain", validators, None); +/// let ctx = create_context("my-chain", validators, None)?; /// ``` pub fn create_context( chain_id: impl Into, validators: Vec, initial_height: Option, -) -> CipherBftContext { +) -> Result { let config = ConsensusConfig::new(chain_id); let validator_set = ConsensusValidatorSet::new(validators); let initial_height = initial_height.unwrap_or_else(|| ConsensusHeight::from(1)); - CipherBftContext::new(config, validator_set, initial_height) + CipherBftContext::try_new(config, validator_set, initial_height) } /// Create Malachite consensus params using our Context components. @@ -107,6 +111,24 @@ pub fn default_engine_config_single_part() -> EngineConsensusConfig { cfg } +/// Create engine config with timeouts from `ConsensusConfig`. +/// +/// This wires the CipherBFT consensus timeouts into the Malachite engine config. +pub fn create_engine_config(config: &ConsensusConfig) -> EngineConsensusConfig { + let timeout_config = TimeoutConfig { + propose_timeout: config.propose_timeout, + prevote_timeout: config.prevote_timeout, + precommit_timeout: config.precommit_timeout, + ..Default::default() + }; + + EngineConsensusConfig { + value_payload: EngineValuePayload::ProposalOnly, + timeouts: timeout_config, + ..Default::default() + } +} + /// Bundles all actor handles returned after spawning. pub struct EngineHandles { pub node: NodeRef, diff --git a/crates/consensus/src/error.rs b/crates/consensus/src/error.rs new file mode 100644 index 0000000..4b848e4 --- /dev/null +++ b/crates/consensus/src/error.rs @@ -0,0 +1,46 @@ +//! Consensus layer error types + +use thiserror::Error; + +/// Errors that can occur during consensus operations. +#[derive(Debug, Error, Clone, PartialEq, Eq)] +pub enum ConsensusError { + /// Validator set cannot be empty for consensus operations. + #[error("validator set cannot be empty")] + EmptyValidatorSet, + + /// Invalid configuration provided. + #[error("invalid configuration: {0}")] + InvalidConfig(String), +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_empty_validator_set_error_display() { + let err = ConsensusError::EmptyValidatorSet; + assert_eq!(err.to_string(), "validator set cannot be empty"); + } + + #[test] + fn test_invalid_config_error_display() { + let err = ConsensusError::InvalidConfig("bad timeout".to_string()); + assert_eq!(err.to_string(), "invalid configuration: bad timeout"); + } + + #[test] + fn test_error_equality() { + let err1 = ConsensusError::EmptyValidatorSet; + let err2 = ConsensusError::EmptyValidatorSet; + assert_eq!(err1, err2); + + let err3 = ConsensusError::InvalidConfig("a".to_string()); + let err4 = ConsensusError::InvalidConfig("a".to_string()); + assert_eq!(err3, err4); + + let err5 = ConsensusError::InvalidConfig("b".to_string()); + assert_ne!(err3, err5); + } +} diff --git a/crates/consensus/src/lib.rs b/crates/consensus/src/lib.rs index ab1e99f..cc9aa44 100644 --- a/crates/consensus/src/lib.rs +++ b/crates/consensus/src/lib.rs @@ -5,8 +5,11 @@ //! existing builds remain unaffected while we wire up the consensus layer. pub mod config; +pub mod error; pub mod types; +pub use error::ConsensusError; + #[cfg(feature = "malachite")] pub mod context; #[cfg(feature = "malachite")] @@ -35,6 +38,6 @@ pub use validator_set::ConsensusValidatorSet; pub use vote::ConsensusVote; #[cfg(feature = "malachite")] pub use engine::{ - create_context, default_consensus_params, default_engine_config_single_part, EngineHandles, - MalachiteEngineBuilder, + create_context, create_engine_config, default_consensus_params, + default_engine_config_single_part, EngineHandles, MalachiteEngineBuilder, }; diff --git a/crates/consensus/src/proposal.rs b/crates/consensus/src/proposal.rs index e1c32e5..dbe494e 100644 --- a/crates/consensus/src/proposal.rs +++ b/crates/consensus/src/proposal.rs @@ -94,6 +94,8 @@ impl MalachiteProposalPart for CutProposalPart { impl PartialEq for CutProposalPart { fn eq(&self, other: &Self) -> bool { self.cut.hash() == other.cut.hash() + && self.first == other.first + && self.last == other.last } } diff --git a/crates/consensus/src/types.rs b/crates/consensus/src/types.rs index af242c9..b6c0dc8 100644 --- a/crates/consensus/src/types.rs +++ b/crates/consensus/src/types.rs @@ -133,3 +133,87 @@ pub use malachite_impls::ConsensusRound; #[cfg(not(feature = "malachite"))] /// Placeholder round type when Malachite is disabled. pub type ConsensusRound = i64; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_consensus_height_next() { + let h = ConsensusHeight(5); + assert_eq!(h.next(), ConsensusHeight(6)); + } + + #[test] + fn test_consensus_height_from_u64() { + let h: ConsensusHeight = 42u64.into(); + assert_eq!(h.0, 42); + } + + #[test] + fn test_consensus_height_into_u64() { + let h = ConsensusHeight(100); + let n: u64 = h.into(); + assert_eq!(n, 100); + } + + #[test] + fn test_consensus_height_ordering() { + let h1 = ConsensusHeight(1); + let h2 = ConsensusHeight(2); + assert!(h1 < h2); + assert!(h2 > h1); + assert_eq!(h1, ConsensusHeight(1)); + } + + #[test] + fn test_consensus_height_display() { + let h = ConsensusHeight(123); + assert_eq!(format!("{}", h), "123"); + } + + #[test] + fn test_consensus_value_id_display() { + let hash = Hash::compute(b"test"); + let id = ConsensusValueId(hash); + // Display should delegate to Hash's Display + assert!(!format!("{}", id).is_empty()); + } + + #[test] + fn test_consensus_value_equality_by_hash() { + let cut1 = Cut::new(1); + let cut2 = Cut::new(1); + let cut3 = Cut::new(2); + + let v1 = ConsensusValue(cut1); + let v2 = ConsensusValue(cut2); + let v3 = ConsensusValue(cut3); + + // Same height empty cuts should have same hash + assert_eq!(v1, v2); + // Different height should differ + assert_ne!(v1, v3); + } + + #[test] + fn test_consensus_value_ordering() { + let cut1 = Cut::new(1); + let cut2 = Cut::new(2); + + let v1 = ConsensusValue(cut1); + let v2 = ConsensusValue(cut2); + + // Ordering should be deterministic by hash + assert!(v1.cmp(&v2) != Ordering::Equal || v1 == v2); + } + + #[test] + fn test_consensus_value_into_cut() { + let cut = Cut::new(42); + let height = cut.height; + let value = ConsensusValue(cut); + let recovered = value.into_cut(); + assert_eq!(recovered.height, height); + } +} diff --git a/crates/consensus/src/validator_set.rs b/crates/consensus/src/validator_set.rs index a197087..62bb5b8 100644 --- a/crates/consensus/src/validator_set.rs +++ b/crates/consensus/src/validator_set.rs @@ -82,6 +82,11 @@ impl ConsensusValidatorSet { pub fn len(&self) -> usize { self.validators.len() } + + /// Check if the validator set is empty. + pub fn is_empty(&self) -> bool { + self.validators.is_empty() + } } #[cfg(feature = "malachite")]