Merge pull request #279 from poanetwork/afck-no-dkg-fault

Log more DKG-related faults, check faults in tests.
This commit is contained in:
Vladimir Komendantskiy 2018-10-23 10:27:26 +01:00 committed by GitHub
commit 13308906aa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 104 additions and 55 deletions

View File

@ -133,11 +133,17 @@ where
/// Proposes a contribution in the current epoch.
pub fn propose(&mut self, contrib: C) -> Result<Step<C, N>> {
let key_gen_messages = self
.key_gen_msg_buffer
.iter()
.filter(|kg_msg| kg_msg.epoch() == self.start_epoch)
.cloned()
.collect();
let step = self
.honey_badger
.handle_input(InternalContrib {
contrib,
key_gen_messages: self.key_gen_msg_buffer.clone(),
key_gen_messages,
votes: self.vote_counter.pending_votes().cloned().collect(),
}).map_err(ErrorKind::ProposeHoneyBadger)?;
self.process_output(step)
@ -228,19 +234,11 @@ where
}
};
// If the joining node is correct, it will send at most (N + 1)² + 1 key generation
// messages.
if Some(sender_id) == kgs.change.candidate() {
let n = self.netinfo.num_nodes() + 1;
if kgs.candidate_msg_count > n * n {
info!(
"Too many key gen messages from candidate {:?}: {:?}.",
sender_id, kg_msg
);
let fault_kind = FaultKind::TooManyCandidateKeyGenMessages;
return Ok(Fault::new(sender_id.clone(), fault_kind).into());
}
kgs.candidate_msg_count += 1;
// If the sender is correct, it will send at most _N + 1_ key generation messages:
// one `Part`, and for each validator an `Ack`. _N_ is the node number _after_ the change.
if kgs.count_messages(sender_id) > kgs.key_gen.num_nodes() + 1 {
let fault_kind = FaultKind::TooManyKeyGenMessages;
return Ok(Fault::new(sender_id.clone(), fault_kind).into());
}
let tx = SignedKeyGenMsg(self.start_epoch, sender_id.clone(), kg_msg, sig);
@ -273,23 +271,18 @@ where
self.key_gen_msg_buffer
.retain(|skgm| !key_gen_messages.contains(skgm));
for SignedKeyGenMsg(epoch, s_id, kg_msg, sig) in key_gen_messages {
if epoch < self.start_epoch {
info!("Obsolete key generation message: {:?}.", kg_msg);
continue;
}
if !self.verify_signature(&s_id, &sig, &kg_msg)? {
info!(
"Invalid signature in {:?}'s batch from {:?} for: {:?}.",
id, s_id, kg_msg
);
if epoch != self.start_epoch {
let fault_kind = FaultKind::InvalidKeyGenMessageEpoch;
step.fault_log.append(id.clone(), fault_kind);
} else if !self.verify_signature(&s_id, &sig, &kg_msg)? {
let fault_kind = FaultKind::InvalidKeyGenMessageSignature;
step.fault_log.append(id.clone(), fault_kind);
continue;
} else {
step.extend(match kg_msg {
KeyGenMessage::Part(part) => self.handle_part(&s_id, part)?,
KeyGenMessage::Ack(ack) => self.handle_ack(&s_id, ack)?.into(),
});
}
step.extend(match kg_msg {
KeyGenMessage::Part(part) => self.handle_part(&s_id, part)?,
KeyGenMessage::Ack(ack) => self.handle_ack(&s_id, ack)?.into(),
});
}
}
@ -358,8 +351,7 @@ where
self.start_epoch = epoch;
self.key_gen_msg_buffer.retain(|kg_msg| kg_msg.0 >= epoch);
let netinfo = Arc::new(self.netinfo.clone());
let counter = VoteCounter::new(netinfo.clone(), epoch);
mem::replace(&mut self.vote_counter, counter);
self.vote_counter = VoteCounter::new(netinfo.clone(), epoch);
self.honey_badger = HoneyBadger::builder(netinfo)
.max_future_epochs(self.max_future_epochs)
.rng(self.rng.sub_rng())
@ -371,8 +363,9 @@ where
let outcome = if let Some(kgs) = self.key_gen_state.as_mut() {
kgs.key_gen.handle_part(&mut self.rng, &sender_id, part)
} else {
// No key generation ongoing. Return early.
return Ok(Step::default());
// No key generation ongoing.
let fault_kind = FaultKind::UnexpectedKeyGenPart;
return Ok(Fault::new(sender_id.clone(), fault_kind).into());
};
match outcome {
@ -387,7 +380,9 @@ where
if let Some(kgs) = self.key_gen_state.as_mut() {
Ok(kgs.key_gen.handle_ack(sender_id, ack))
} else {
Ok(FaultLog::new())
// No key generation ongoing.
let fault_kind = FaultKind::UnexpectedKeyGenAck;
return Ok(Fault::new(sender_id.clone(), fault_kind).into());
}
}

View File

@ -150,9 +150,9 @@ struct KeyGenState<N> {
key_gen: SyncKeyGen<N>,
/// The change for which key generation is performed.
change: Change<N>,
/// The number of key generation messages received from the candidate. At most _N² + 1_ are
/// The number of key generation messages received from each peer. At most _N + 1_ are
/// accepted.
candidate_msg_count: usize,
msg_count: BTreeMap<N, usize>,
}
impl<N: NodeIdT> KeyGenState<N> {
@ -160,7 +160,7 @@ impl<N: NodeIdT> KeyGenState<N> {
KeyGenState {
key_gen,
change,
candidate_msg_count: 0,
msg_count: BTreeMap::new(),
}
}
@ -178,6 +178,13 @@ impl<N: NodeIdT> KeyGenState<N> {
Change::Add(_, _) | Change::Remove(_) => None,
}
}
/// Increments the message count for the given node, and returns the new count.
fn count_messages(&mut self, node_id: &N) -> usize {
let count = self.msg_count.entry(node_id.clone()).or_insert(0);
*count += 1;
*count
}
}
/// The contribution for the internal `HoneyBadger` instance: this includes a user-defined
@ -195,3 +202,10 @@ struct InternalContrib<C, N> {
/// A signed internal message.
#[derive(Eq, PartialEq, Debug, Serialize, Deserialize, Hash, Clone)]
struct SignedKeyGenMsg<N>(u64, N, KeyGenMessage, Signature);
impl<N> SignedKeyGenMsg<N> {
/// Returns the start epoch of the ongoing key generation.
fn epoch(&self) -> u64 {
self.0
}
}

View File

@ -12,8 +12,6 @@ pub enum AckMessageFault {
NodeCount,
#[fail(display = "Sender does not exist")]
SenderExist,
#[fail(display = "Duplicate ack")]
DuplicateAck,
#[fail(display = "Value decryption failed")]
ValueDecryption,
#[fail(display = "Value deserialization failed")]
@ -44,14 +42,19 @@ pub enum FaultKind {
/// `HoneyBadger` could not deserialize bytes (i.e. a serialized Batch)
/// from a given proposer into a vector of transactions.
BatchDeserializationFailed,
/// `DynamicHoneyBadger` received a key generation message with an invalid
/// signature.
/// `DynamicHoneyBadger` received a key generation message with an invalid signature.
InvalidKeyGenMessageSignature,
/// `DynamicHoneyBadger` received a key generation message with an invalid epoch.
InvalidKeyGenMessageEpoch,
/// `DynamicHoneyBadger` received a key generation message when there was no key generation in
/// progress.
UnexpectedKeyGenMessage,
/// `DynamicHoneyBadger` received more key generation messages from the candidate than expected.
TooManyCandidateKeyGenMessages,
/// `DynamicHoneyBadger` received a signed `Ack` when no key generation in progress.
UnexpectedKeyGenAck,
/// `DynamicHoneyBadger` received a signed `Part` when no key generation in progress.
UnexpectedKeyGenPart,
/// `DynamicHoneyBadger` received more key generation messages from the peer than expected.
TooManyKeyGenMessages,
/// `DynamicHoneyBadger` received a message (Accept, Propose, or Change)
/// with an invalid signature.
IncorrectPayloadSignature,
@ -59,6 +62,8 @@ pub enum FaultKind {
AckMessage(AckMessageFault),
/// `DynamicHoneyBadger`/`SyncKeyGen` received an invalid Part message.
InvalidPartMessage,
/// `DynamicHoneyBadger`/`SyncKeyGen` received multiple Part messages from a node.
MultiplePartMessages,
/// `DynamicHoneyBadger` received a change vote with an invalid signature.
InvalidVoteSignature,
/// A validator committed an invalid vote in `DynamicHoneyBadger`.

View File

@ -137,6 +137,7 @@ extern crate tiny_keccak;
pub extern crate threshold_crypto as crypto;
mod fault_log;
mod messaging;
mod network_info;
mod traits;
@ -145,7 +146,6 @@ pub mod binary_agreement;
pub mod broadcast;
pub mod coin;
pub mod dynamic_honey_badger;
pub mod fault_log;
pub mod honey_badger;
pub mod queueing_honey_badger;
pub mod subset;
@ -155,6 +155,7 @@ pub mod transaction_queue;
pub mod util;
pub use crypto::pairing;
pub use fault_log::{AckMessageFault, Fault, FaultKind, FaultLog};
pub use messaging::{SourcedMessage, Target, TargetedMessage};
pub use network_info::NetworkInfo;
pub use traits::{Contribution, DistAlgorithm, Message, NodeIdT, Step};

View File

@ -226,7 +226,7 @@ impl Debug for Ack {
}
/// The information needed to track a single proposer's secret sharing process.
#[derive(Debug)]
#[derive(Debug, PartialEq, Eq)]
struct ProposalState {
/// The proposer's commitment.
commit: BivarCommitment,
@ -275,7 +275,7 @@ pub struct SyncKeyGen<N> {
our_idx: Option<u64>,
/// Our secret key.
sec_key: SecretKey,
/// The public keys of all nodes, by node index.
/// The public keys of all nodes, by node ID.
pub_keys: BTreeMap<N, PublicKey>,
/// Proposed bivariate polynomials.
parts: BTreeMap<u64, ProposalState>,
@ -340,11 +340,16 @@ impl<N: NodeIdT> SyncKeyGen<N> {
sender_id: &N,
Part(commit, rows): Part,
) -> Option<PartOutcome<N>> {
let sender_idx = self.node_index(sender_id)?;
let sender_idx = self.node_index(sender_id)?; // TODO: Return an error.
let opt_commit_row = self.our_idx.map(|idx| commit.row(idx + 1));
match self.parts.entry(sender_idx) {
Entry::Occupied(_) => {
debug!("Received multiple parts from node {:?}.", sender_id);
Entry::Occupied(entry) => {
if *entry.get() != ProposalState::new(commit) {
debug!("Received multiple parts from node {:?}.", sender_id);
let fault_log =
FaultLog::init(sender_id.clone(), FaultKind::MultiplePartMessages);
return Some(PartOutcome::Invalid(fault_log));
}
return None;
}
Entry::Vacant(entry) => {
@ -456,6 +461,11 @@ impl<N: NodeIdT> SyncKeyGen<N> {
Ok(netinfo)
}
/// Returns the number of nodes participating in the key generation.
pub fn num_nodes(&self) -> usize {
self.pub_keys.len()
}
/// Handles an `Ack` message or returns an error string.
fn handle_ack_or_err(
&mut self,
@ -470,7 +480,7 @@ impl<N: NodeIdT> SyncKeyGen<N> {
.get_mut(&proposer_idx)
.ok_or_else(|| Fault::SenderExist)?;
if !part.acks.insert(sender_idx) {
return Err(Fault::DuplicateAck);
return Ok(());
}
let our_idx = match self.our_idx {
Some(our_idx) => our_idx,

View File

@ -7,7 +7,7 @@ use crypto::SecretKeyShare;
use rand::{self, Rng};
use hbbft::dynamic_honey_badger::Batch;
use hbbft::{Contribution, DistAlgorithm, NetworkInfo, Step, Target, TargetedMessage};
use hbbft::{Contribution, DistAlgorithm, Fault, NetworkInfo, Step, Target, TargetedMessage};
/// A node identifier. In the tests, nodes are simply numbered.
#[derive(Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Clone, Copy, Serialize, Deserialize, Rand)]
@ -25,6 +25,8 @@ pub struct TestNode<D: DistAlgorithm> {
outputs: Vec<D::Output>,
/// Outgoing messages to be sent to other nodes.
messages: VecDeque<TargetedMessage<D::Message, D::NodeId>>,
/// Collected fault logs.
faults: Vec<Fault<D::NodeId>>,
}
impl<D: DistAlgorithm> TestNode<D> {
@ -44,6 +46,7 @@ impl<D: DistAlgorithm> TestNode<D> {
let step = self.algo.handle_input(input).expect("input");
self.outputs.extend(step.output);
self.messages.extend(step.messages);
self.faults.extend(step.fault_log.0);
}
/// Returns the internal algorithm's instance.
@ -60,6 +63,7 @@ impl<D: DistAlgorithm> TestNode<D> {
queue: VecDeque::new(),
outputs: step.output.into_iter().collect(),
messages: step.messages,
faults: step.fault_log.0,
}
}
@ -73,9 +77,10 @@ impl<D: DistAlgorithm> TestNode<D> {
.expect("handling message");
self.outputs.extend(step.output);
self.messages.extend(step.messages);
self.faults.extend(step.fault_log.0);
}
/// Checks whether the node has messages to process
/// Checks whether the node has messages to process.
fn is_idle(&self) -> bool {
self.queue.is_empty()
}
@ -480,6 +485,8 @@ where
}
while !self.observer.queue.is_empty() {
self.observer.handle_message();
let faults: Vec<_> = self.observer.faults.drain(..).collect();
self.check_faults(faults);
}
}
@ -501,7 +508,7 @@ where
let id = self.adversary.pick_node(&self.nodes);
// The node handles the incoming message and creates new outgoing ones to be dispatched.
let msgs: Vec<_> = {
let (msgs, faults): (Vec<_>, Vec<_>) = {
let mut node = self.nodes.get_mut(&id).unwrap();
// Ensure the adversary is playing fair by selecting a node that will result in actual
@ -513,8 +520,12 @@ where
);
node.handle_message();
node.messages.drain(..).collect()
(
node.messages.drain(..).collect(),
node.faults.drain(..).collect(),
)
};
self.check_faults(faults);
self.dispatch_messages(id, msgs);
id
@ -522,11 +533,15 @@ where
/// Inputs a value in node `id`.
pub fn input(&mut self, id: NodeId, value: D::Input) {
let msgs: Vec<_> = {
let (msgs, faults): (Vec<_>, Vec<_>) = {
let mut node = self.nodes.get_mut(&id).expect("input instance");
node.handle_input(value);
node.messages.drain(..).collect()
(
node.messages.drain(..).collect(),
node.faults.drain(..).collect(),
)
};
self.check_faults(faults);
self.dispatch_messages(id, msgs);
}
@ -541,6 +556,15 @@ where
self.input(id, value.clone());
}
}
/// Verifies that no correct node is reported as faulty.
fn check_faults<I: IntoIterator<Item = Fault<D::NodeId>>>(&self, faults: I) {
for fault in faults {
if self.nodes.contains_key(&fault.node_id) {
panic!("Unexpected fault: {:?}", fault);
}
}
}
}
impl<A: Adversary<D>, C, D> TestNetwork<A, D>