diff --git a/src/binary_agreement/binary_agreement.rs b/src/binary_agreement/binary_agreement.rs index 49150e4..a142f11 100644 --- a/src/binary_agreement/binary_agreement.rs +++ b/src/binary_agreement/binary_agreement.rs @@ -1,6 +1,10 @@ use std::collections::BTreeMap; +use std::fmt::Debug; use std::sync::Arc; +use bincode; +use serde::Serialize; + use super::bool_multimap::BoolMultimap; use super::bool_set::BoolSet; use super::sbv_broadcast::{self, SbvBroadcast}; @@ -39,10 +43,13 @@ impl From for CoinState { pub struct BinaryAgreement { /// Shared network information. netinfo: Arc>, - /// Session ID, e.g, the Honey Badger algorithm epoch. - session_id: u64, - /// The ID of the proposer of the value for this Binary Agreement instance. - proposer_id: N, + // We store the session ID twice: once in its serialized form, to create the `ThresholdSign` + // nonces, and once boxed, for debug output. + /// Session identifier, to prevent replaying messages in other instances, + /// e.g, the Honey Badger algorithm epoch plus the proposer ID. + session_id: Box, + /// The serialized session identifier. + ser_session_id: Vec, /// Binary Agreement algorithm epoch. epoch: u32, /// This epoch's Synchronized Binary Value Broadcast instance. @@ -96,18 +103,16 @@ impl DistAlgorithm for BinaryAgreement { } impl BinaryAgreement { - /// Creates a new `BinaryAgreement` instance. The `session_id` and `proposer_id` are used to - /// uniquely identify this instance: its messages cannot be replayed in an instance with - /// different values. - // TODO: Use a generic type argument for that instead of something `Subset`-specific. - pub fn new(netinfo: Arc>, session_id: u64, proposer_id: N) -> Result { - if !netinfo.is_node_validator(&proposer_id) { - return Err(Error::UnknownProposer); - } + /// Creates a new `BinaryAgreement` instance with the given session identifier, to prevent + /// replaying messages in other instances. + pub fn new(netinfo: Arc>, session_id: T) -> Result + where + T: Serialize + Sync + Send + Debug + 'static, + { Ok(BinaryAgreement { netinfo: netinfo.clone(), - session_id, - proposer_id, + ser_session_id: bincode::serialize(&session_id)?, + session_id: Box::new(session_id), epoch: 0, sbv_broadcast: SbvBroadcast::new(netinfo), received_conf: BTreeMap::new(), @@ -127,7 +132,7 @@ impl BinaryAgreement { } // Set the initial estimated value to the input value. self.estimated = Some(input); - debug!("{:?}/{:?} Input {}", self.our_id(), self.proposer_id, input); + debug!("{:?}/{:?} Input {}", self.our_id(), self.session_id, input); let sbvb_step = self.sbv_broadcast.handle_input(input)?; self.handle_sbvb_step(sbvb_step) } @@ -316,20 +321,19 @@ impl BinaryAgreement { /// Creates the initial coin state for the current epoch, i.e. sets it to the predetermined /// value, or initializes a `ThresholdSign` instance. - fn coin_state(&self) -> CoinState { - match self.epoch % 3 { + fn coin_state(&self) -> Result> { + Ok(match self.epoch % 3 { 0 => CoinState::Decided(true), 1 => CoinState::Decided(false), _ => { let nonce = Nonce::new( self.netinfo.invocation_id().as_ref(), - self.session_id, - self.netinfo.node_index(&self.proposer_id).unwrap(), + &self.ser_session_id, self.epoch, - ); + )?; CoinState::InProgress(Box::new(ThresholdSign::new(self.netinfo.clone(), nonce))) } - } + }) } /// Decides on a value and broadcasts a `Term` message with that value. @@ -345,7 +349,7 @@ impl BinaryAgreement { debug!( "{:?}/{:?} (is_validator: {}) decision: {}", self.netinfo.our_id(), - self.proposer_id, + self.session_id, self.netinfo.is_validator(), b ); @@ -387,11 +391,11 @@ impl BinaryAgreement { } self.conf_values = None; self.epoch += 1; - self.coin_state = self.coin_state(); + self.coin_state = self.coin_state()?; debug!( "{:?} BinaryAgreement instance {:?} started epoch {}, {} terminated", self.netinfo.our_id(), - self.proposer_id, + self.session_id, self.epoch, self.received_conf.len(), ); diff --git a/src/binary_agreement/mod.rs b/src/binary_agreement/mod.rs index 403552b..353bd0d 100644 --- a/src/binary_agreement/mod.rs +++ b/src/binary_agreement/mod.rs @@ -68,6 +68,10 @@ mod bool_multimap; pub mod bool_set; mod sbv_broadcast; +use std::io; + +use bincode; +use byteorder::{BigEndian, WriteBytesExt}; use rand; use self::bool_set::BoolSet; @@ -82,8 +86,23 @@ pub enum Error { HandleThresholdSign(threshold_sign::Error), #[fail(display = "Error invoking the common coin: {}", _0)] InvokeCoin(threshold_sign::Error), - #[fail(display = "Unknown proposer")] - UnknownProposer, + // Strings because `io` and `bincode` errors lack `Eq` and `Clone`. + #[fail(display = "Error writing epoch for nonce: {}", _0)] + Io(String), + #[fail(display = "Error serializing session ID for nonce: {}", _0)] + Serialize(String), +} + +impl From for Error { + fn from(err: io::Error) -> Error { + Error::Io(format!("{:?}", err)) + } +} + +impl From for Error { + fn from(err: bincode::Error) -> Error { + Error::Io(format!("{:?}", err)) + } } /// An Binary Agreement result. @@ -151,14 +170,13 @@ struct Nonce(Vec); impl Nonce { pub fn new( invocation_id: &[u8], - session_id: u64, - proposer_id: usize, + session_id: &[u8], binary_agreement_epoch: u32, - ) -> Self { - Nonce(Vec::from(format!( - "Nonce for Honey Badger {:?}@{}:{}:{}", - invocation_id, session_id, binary_agreement_epoch, proposer_id - ))) + ) -> Result { + let mut vec = invocation_id.to_vec(); + vec.write_u32::(binary_agreement_epoch)?; + vec.extend(session_id); + Ok(Nonce(vec)) } } diff --git a/src/network_info.rs b/src/network_info.rs index ff9dc37..bb5e946 100644 --- a/src/network_info.rs +++ b/src/network_info.rs @@ -116,7 +116,8 @@ impl NetworkInfo { &self.public_keys } - /// The index of a node in a canonical numbering of all nodes. + /// The index of a node in a canonical numbering of all nodes. This is the index where the + /// node appears in `all_ids`. pub fn node_index(&self, id: &N) -> Option { self.node_indices.get(id).cloned() } diff --git a/src/subset.rs b/src/subset.rs index 64ddd80..2c37607 100644 --- a/src/subset.rs +++ b/src/subset.rs @@ -131,11 +131,13 @@ impl Subset { // Create all Binary Agreement instances. let mut ba_instances: BTreeMap> = BTreeMap::new(); - for proposer_id in netinfo.all_ids() { + for (proposer_idx, proposer_id) in netinfo.all_ids().enumerate() { ba_instances.insert( proposer_id.clone(), - BinaryAgreement::new(netinfo.clone(), session_id, proposer_id.clone()) - .map_err(Error::NewBinaryAgreement)?, + BinaryAgreement::new( + netinfo.clone(), + BaSessionId(session_id, proposer_idx as u32), + ).map_err(Error::NewBinaryAgreement)?, ); } @@ -366,3 +368,9 @@ impl Subset { } } } + +/// A session identifier for a `BinaryAgreement` instance run as a `Subset` sub-algorithm. It +/// consists of the `Subset` instance's own session ID, and the index of the proposer whose +/// contribution this `BinaryAgreement` is about. +#[derive(Debug, Serialize)] +struct BaSessionId(u64, u32); diff --git a/tests/binary_agreement.rs b/tests/binary_agreement.rs index aa5e1db..d9d8e0c 100644 --- a/tests/binary_agreement.rs +++ b/tests/binary_agreement.rs @@ -85,7 +85,7 @@ where ); let adversary = |_| new_adversary(num_good_nodes, num_faulty_nodes); let new_ba = |netinfo: Arc>| { - BinaryAgreement::new(netinfo, 0, NodeId(0)).expect("Binary Agreement instance") + BinaryAgreement::new(netinfo, 0).expect("Binary Agreement instance") }; let network = TestNetwork::new(num_good_nodes, num_faulty_nodes, adversary, new_ba); test_binary_agreement(network, input);