diff --git a/README.md b/README.md index 1500610..2e977c8 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,6 @@ +[![Build Status](https://travis-ci.com/poanetwork/hbbft.svg?branch=master)](https://travis-ci.com/poanetwork/hbbft) +[![Gitter](https://badges.gitter.im/poanetwork/hbbft.svg)](https://gitter.im/poanetwork/hbbft?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge) + # About An implementation of the paper @@ -42,4 +45,3 @@ Once you have verified that the `protoc` binary is in your `$PATH`, you can build `hbbft` using cargo: $ cargo build [--release] - diff --git a/src/broadcast.rs b/src/broadcast.rs index 5bfa274..46a5fbf 100644 --- a/src/broadcast.rs +++ b/src/broadcast.rs @@ -94,7 +94,7 @@ impl Broadcast { message, }) => { if let Message::Broadcast(b) = message { - self.on_remote_message(uid, &b, tx) + self.on_remote_message(uid, b, tx) } else { Err(Error::UnexpectedMessage) } @@ -107,7 +107,7 @@ impl Broadcast { fn on_remote_message( &self, uid: messaging::NodeUid, - message: &BroadcastMessage, + message: BroadcastMessage, tx: &Sender, ) -> Result { let (output, messages) = self.handle_broadcast_message(&uid, message)?; @@ -207,22 +207,17 @@ impl Broadcast { // Split the value into chunks/shards, encode them with erasure codes. // Assemble a Merkle tree from data and parity shards. Take all proofs // from this tree and send them, each to its own node. - self.send_shards(value) - .map_err(Error::from) - .map(|(proof, remote_messages)| { - // Record the first proof as if it were sent by the node to - // itself. - let h = proof.root_hash.clone(); - if proof.validate(h.as_slice()) { - // Save the leaf value for reconstructing the tree later. - state.leaf_values[index_of_proof(&proof)] = - Some(proof.value.clone().into_boxed_slice()); - state.leaf_values_num += 1; - state.root_hash = Some(h); - } + self.send_shards(value).map(|(proof, remote_messages)| { + // Record the first proof as if it were sent by the node to itself. + let h = proof.root_hash.clone(); + // Save the leaf value for reconstructing the tree later. + state.leaf_values[index_of_proof(&proof)] = + Some(proof.value.clone().into_boxed_slice()); + state.leaf_values_num += 1; + state.root_hash = Some(h); - remote_messages - }) + remote_messages + }) } pub fn our_id(&self) -> &NodeUid { @@ -247,8 +242,7 @@ impl Broadcast { ); // Insert the length of `v` so it can be decoded without the padding. let payload_len = value.len() as u8; - value.insert(0, payload_len); // TODO: Handle messages larger than 255 - // bytes. + value.insert(0, payload_len); // TODO: Handle messages larger than 255 bytes. let value_len = value.len(); // Size of a Merkle tree leaf value, in bytes. let shard_len = if value_len % data_shard_num > 0 { @@ -265,17 +259,12 @@ impl Broadcast { // Divide the vector into chunks/shards. let shards_iter = value.chunks_mut(shard_len); // Convert the iterator over slices into a vector of slices. - let mut shards: Vec<&mut [u8]> = Vec::new(); - for s in shards_iter { - shards.push(s); - } + let mut shards: Vec<&mut [u8]> = shards_iter.collect(); debug!("Shards before encoding: {:?}", shards); // Construct the parity chunks/shards - self.coding - .encode(shards.as_mut_slice()) - .map_err(Error::from)?; + self.coding.encode(&mut shards)?; debug!("Shards: {:?}", shards); @@ -290,19 +279,21 @@ impl Broadcast { let mut outgoing = VecDeque::new(); // Send each proof to a node. + // TODO: This generates the wrong proof if a leaf occurs more than once. Consider using the + // `merkle_light` crate instead. for (leaf_value, uid) in mtree.iter().zip(self.all_uids.clone()) { - let proof = mtree.gen_proof(leaf_value.to_vec()); - if let Some(proof) = proof { - if uid == self.our_id { - // The proof is addressed to this node. - result = Ok(proof); - } else { - // Rest of the proofs are sent to remote nodes. - outgoing.push_back(TargetedBroadcastMessage { - target: BroadcastTarget::Node(uid), - message: BroadcastMessage::Value(proof), - }); - } + let proof = mtree + .gen_proof(leaf_value.to_vec()) + .ok_or(Error::ProofConstructionFailed)?; + if uid == self.our_id { + // The proof is addressed to this node. + result = Ok(proof); + } else { + // Rest of the proofs are sent to remote nodes. + outgoing.push_back(TargetedBroadcastMessage { + target: BroadcastTarget::Node(uid), + message: BroadcastMessage::Value(proof), + }); } } @@ -313,13 +304,13 @@ impl Broadcast { pub fn handle_broadcast_message( &self, sender_id: &NodeUid, - message: &BroadcastMessage, + message: BroadcastMessage, ) -> Result<(Option, MessageQueue), Error> { let state = self.state.write().unwrap(); match message { BroadcastMessage::Value(p) => self.handle_value(sender_id, p, state), BroadcastMessage::Echo(p) => self.handle_echo(p, state), - BroadcastMessage::Ready(ref hash) => self.handle_ready(hash, state), + BroadcastMessage::Ready(hash) => self.handle_ready(hash, state), } } @@ -327,7 +318,7 @@ impl Broadcast { fn handle_value( &self, sender_id: &NodeUid, - p: &Proof, + p: Proof, mut state: RwLockWriteGuard, ) -> Result<(Option, MessageQueue), Error> { if *sender_id != self.proposer_id { @@ -343,13 +334,12 @@ impl Broadcast { ); } - if let Some(ref h) = state.root_hash.clone() { - if p.validate(h.as_slice()) { - // Save the leaf value for reconstructing the tree - // later. - state.leaf_values[index_of_proof(&p)] = Some(p.value.clone().into_boxed_slice()); - state.leaf_values_num += 1; - } + if state.root_hash.as_ref().map_or(false, |h| p.validate(h)) { + // TODO: Should messages failing this be echoed at all? + // Save the leaf value for reconstructing the tree later. + let idx = index_of_proof(&p); + state.leaf_values[idx] = Some(p.value.clone().into_boxed_slice()); + state.leaf_values_num += 1; } // Enqueue a broadcast of an echo of this proof. @@ -364,7 +354,7 @@ impl Broadcast { /// Handles a received echo and verifies the proof it contains. fn handle_echo( &self, - p: &Proof, + p: Proof, mut state: RwLockWriteGuard, ) -> Result<(Option, MessageQueue), Error> { if state.root_hash.is_none() { @@ -389,9 +379,9 @@ impl Broadcast { } state.echo_num += 1; - // Save the leaf value for reconstructing the - // tree later. - state.leaf_values[index_of_proof(&p)] = Some(p.value.clone().into_boxed_slice()); + // Save the leaf value for reconstructing the tree later. + let idx = index_of_proof(&p); + state.leaf_values[idx] = Some(p.value.into_boxed_slice()); state.leaf_values_num += 1; // Upon receiving 2f + 1 matching READY(h) @@ -401,6 +391,7 @@ impl Broadcast { return Ok((None, VecDeque::new())); } + // TODO: Only decode once. Don't repeat for every ECHO message. let value = decode_from_shards( &mut state.leaf_values, &self.coding, @@ -408,9 +399,8 @@ impl Broadcast { &h, )?; - if state.ready_to_decode - && state.leaf_values_num >= self.num_nodes - 2 * self.num_faulty_nodes - { + if state.ready_to_decode && !state.has_output { + state.has_output = true; return Ok((Some(value), VecDeque::new())); } @@ -422,19 +412,20 @@ impl Broadcast { state.ready_sent = true; let msg = TargetedBroadcastMessage { target: BroadcastTarget::All, - message: BroadcastMessage::Ready(h.to_owned()), + message: BroadcastMessage::Ready(h.clone()), }; - let (output, ready_msgs) = self.handle_ready(&h, state)?; + let (output, ready_msgs) = self.handle_ready(h, state)?; Ok((output, iter::once(msg).chain(ready_msgs).collect())) } fn handle_ready( &self, - hash: &[u8], + hash: Vec, mut state: RwLockWriteGuard, ) -> Result<(Option, MessageQueue), Error> { // Update the number Ready has been received with this hash. - *state.readys.entry(hash.to_vec()).or_insert(1) += 1; + // TODO: Don't accept multiple ready messages from the same node. + *state.readys.entry(hash).or_insert(1) += 1; // Check that the root hash matches. let h = if let Some(h) = state.root_hash.clone() { @@ -862,7 +853,7 @@ where } fn decode_from_shards( - leaf_values: &mut Vec>>, + leaf_values: &mut [Option>], coding: &ReedSolomon, data_shard_num: usize, root_hash: &[u8], @@ -870,19 +861,16 @@ fn decode_from_shards( where T: Clone + Debug + Hashable + Send + Sync + From> + Into>, { - // Try to interpolate the Merkle tree using the Reed-Solomon erasure coding - // scheme. - coding.reconstruct_shards(leaf_values.as_mut_slice())?; + // Try to interpolate the Merkle tree using the Reed-Solomon erasure coding scheme. + coding.reconstruct_shards(leaf_values)?; // Recompute the Merkle tree root. - // + // Collect shards for tree construction. - let mut shards: Vec = Vec::new(); - for l in leaf_values.iter() { - if let Some(ref v) = *l { - shards.push(v.to_vec()); - } - } + let shards: Vec = leaf_values + .iter() + .filter_map(|l| l.as_ref().map(|v| v.to_vec())) + .collect(); // Construct the Merkle tree. let mtree = MerkleTree::from_vec(&::ring::digest::SHA256, shards); // If the root hash of the reconstructed tree does not match the one @@ -959,6 +947,8 @@ fn index_of_path(mut path: Vec) -> usize { } /// Computes the Merkle tree leaf index of a value in a given proof. -fn index_of_proof(p: &Proof) -> usize { +// TODO: This currently only works if the number of leaves is a power of two. With the +// `merkle_light` crate, it might not even be needed, though. +pub fn index_of_proof(p: &Proof) -> usize { index_of_path(path_of_lemma(&p.lemma)) } diff --git a/src/common_subset.rs b/src/common_subset.rs index 96a8926..7b58217 100644 --- a/src/common_subset.rs +++ b/src/common_subset.rs @@ -120,7 +120,7 @@ impl CommonSubset { let input_result = { if let Some(broadcast_instance) = self.broadcast_instances.get(&uid) { broadcast_instance - .handle_broadcast_message(&uid, &bmessage) + .handle_broadcast_message(&uid, bmessage) .map(|(value, queue)| { instance_result = value; queue.into_iter().map(Output::Broadcast).collect() diff --git a/src/proto/mod.rs b/src/proto/mod.rs index f4254cb..90d7491 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -53,9 +53,10 @@ impl<'a, T: Send + Sync + fmt::Debug> fmt::Debug for HexProof<'a, T> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!( f, - "Proof {{ algorithm: {:?}, root_hash: {:?}, lemma: .., value: {:?} }}", + "Proof {{ algorithm: {:?}, root_hash: {:?}, lemma for leaf #{}, value: {:?} }}", self.0.algorithm, HexBytes(&self.0.root_hash), + ::broadcast::index_of_proof(self.0), self.0.value ) } @@ -75,7 +76,7 @@ impl fmt::Debug for BroadcastMessage { #[derive(Clone, Debug, PartialEq)] pub enum AgreementMessage { BVal(bool), - Aux(bool) + Aux(bool), } impl Message { @@ -198,7 +199,7 @@ impl AgreementMessage { let mut p = AgreementProto::new(); match self { AgreementMessage::BVal(b) => p.set_bval(b), - AgreementMessage::Aux(b) => p.set_aux(b) + AgreementMessage::Aux(b) => p.set_aux(b), } p } @@ -208,11 +209,9 @@ impl AgreementMessage { pub fn from_proto(mp: AgreementProto) -> Option { if mp.has_bval() { Some(AgreementMessage::BVal(mp.get_bval())) - } - else if mp.has_aux() { + } else if mp.has_aux() { Some(AgreementMessage::Aux(mp.get_aux())) - } - else { + } else { None } } diff --git a/tests/broadcast.rs b/tests/broadcast.rs index 92d31fa..e529027 100644 --- a/tests/broadcast.rs +++ b/tests/broadcast.rs @@ -50,7 +50,7 @@ impl TestNode { let (from_id, msg) = self.queue.pop_front().expect("message not found"); debug!("Handling {:?} -> {:?}: {:?}", from_id, self.id, msg); let (output, msgs) = self.broadcast - .handle_broadcast_message(&from_id, &msg) + .handle_broadcast_message(&from_id, msg) .expect("handling message"); if let Some(output) = output.clone() { self.outputs.push(output); @@ -213,7 +213,6 @@ impl TestNetwork { where Q: IntoIterator> + fmt::Debug, { - debug!("Sending: {:?}", msgs); for msg in msgs { match msg { TargetedBroadcastMessage { @@ -270,12 +269,11 @@ impl TestNetwork { } /// Broadcasts a value from node 0 and expects all good nodes to receive it. -fn test_broadcast(mut network: TestNetwork) { +fn test_broadcast(mut network: TestNetwork, proposed_value: &[u8]) { // TODO: This returns an error in all but the first test. let _ = simple_logger::init_with_level(log::Level::Debug); - // Make node 0 propose a value. - let proposed_value = b"Foo"; + // Make node 0 propose the value. network.propose_value(NodeId(0), proposed_value.to_vec()); // Handle messages in random order until all nodes have output the proposed value. @@ -289,16 +287,34 @@ fn test_broadcast(mut network: TestNetwork) { } } +// TODO: Unignore once equal shards don't cause problems anymore. +#[test] +#[ignore] +fn test_8_broadcast_equal_leaves() { + let adversary = SilentAdversary::new(MessageScheduler::Random); + // Space is ASCII character 32. So 32 spaces will create shards that are all equal, even if the + // length of the value is inserted. + test_broadcast(TestNetwork::new(8, 0, adversary), &vec![b' '; 32]); +} + +// TODO: Unignore once node numbers are supported that are not powers of two. +#[test] +#[ignore] +fn test_13_broadcast_nodes_random_delivery() { + let adversary = SilentAdversary::new(MessageScheduler::Random); + test_broadcast(TestNetwork::new(13, 0, adversary), b"Foo"); +} + #[test] fn test_11_5_broadcast_nodes_random_delivery() { let adversary = SilentAdversary::new(MessageScheduler::Random); - test_broadcast(TestNetwork::new(11, 5, adversary)); + test_broadcast(TestNetwork::new(11, 5, adversary), b"Foo"); } #[test] fn test_11_5_broadcast_nodes_first_delivery() { let adversary = SilentAdversary::new(MessageScheduler::First); - test_broadcast(TestNetwork::new(11, 5, adversary)); + test_broadcast(TestNetwork::new(11, 5, adversary), b"Foo"); } #[test] @@ -306,7 +322,7 @@ fn test_11_5_broadcast_nodes_random_delivery_adv_propose() { let good_nodes: BTreeSet = (0..11).map(NodeId).collect(); let adv_nodes: BTreeSet = (11..16).map(NodeId).collect(); let adversary = ProposeAdversary::new(MessageScheduler::Random, good_nodes, adv_nodes); - test_broadcast(TestNetwork::new(11, 5, adversary)); + test_broadcast(TestNetwork::new(11, 5, adversary), b"Foo"); } #[test] @@ -314,5 +330,5 @@ fn test_11_5_broadcast_nodes_first_delivery_adv_propose() { let good_nodes: BTreeSet = (0..11).map(NodeId).collect(); let adv_nodes: BTreeSet = (11..16).map(NodeId).collect(); let adversary = ProposeAdversary::new(MessageScheduler::First, good_nodes, adv_nodes); - test_broadcast(TestNetwork::new(11, 5, adversary)); + test_broadcast(TestNetwork::new(11, 5, adversary), b"Foo"); }