From b963ee6fdc150da0428e791acfe6282b77bcde29 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 13 Aug 2018 14:53:08 +0200 Subject: [PATCH 01/11] equivocation detection --- src/lib.rs | 14 ++++++++-- src/round.rs | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 src/round.rs diff --git a/src/lib.rs b/src/lib.rs index c028e5e3..ed9244d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,14 +24,12 @@ use std::fmt; /// A prevote for a block and its ancestors. pub struct Prevote { - round: u64, target: H, weight: usize, } /// A precommit for a block and its ancestors. pub struct Precommit { - round: u64, target: H, weight: usize, } @@ -65,3 +63,15 @@ pub trait Chain { /// If the block is not a descendent of `base`, returns an error. fn ancestry(&self, base: H, block: H) -> Result, Error>; } + +/// An equivocation (double-vote) in a given round. +pub struct Equivocation { + /// The round number equivocated in. + pub round_number: u64, + /// The identity of the equivocator. + pub identity: Id, + /// The first vote in the equivocation. + pub first: (V, S), + /// The second vote in the equivocation. + pub second: (V, S), +} diff --git a/src/round.rs b/src/round.rs new file mode 100644 index 00000000..8ae2f6ac --- /dev/null +++ b/src/round.rs @@ -0,0 +1,75 @@ +// Copyright 2018 Parity Technologies (UK) Ltd. +// This file is part of finality-afg. + +// finality-afg is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// finality-afg is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with finality-afg. If not, see . + +//! Logic for a single round of AfG. + +use vote_graph::VoteGraph; + +use std::collections::hash_map::{HashMap, Entry}; +use std::hash::Hash; +use std::ops::AddAssign; + +use super::Equivocation; + +#[derive(Hash, Eq, PartialEq)] +struct Address; + +#[derive(Debug, Clone)] +struct VoteCount { + prevote: usize, + precommit: usize, +} + +impl AddAssign for VoteCount { + fn add_assign(&mut self, rhs: VoteCount) { + *self.prevote += rhs.prevote; + *self.precommit += rhs.precommit; + } +} + +struct VoteTracker { + votes: HashMap, + current_weight: usize, +} + +impl VoteTracker { + fn new() -> Self { + VoteTracker { + votes: HashMap::new(); + current_weight: 0, + } + } + + // track a vote. if the vote is an equivocation, returns a proof-of-equivocation and + // otherwise notes the current amount of weight on the tracked vote-set + fn add_vote(&mut self, id: Id, vote: Vote, signature: Signature, weight: usize) + -> Result<(), Equivocation> + { + match self.votes.entry(id) { + Entry::Vacant(mut vacant) => { + vacant.insert((vote, signature)); + } + } + } +} + +/// Stores data for a round. +pub struct Round { + graph: VoteGraph, + threshold_weight: usize, + total_weight: usize, + current_weight: usize, +} From c9d922c57c503664859b0558a2b54c98e39d3019 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 13 Aug 2018 15:05:16 +0200 Subject: [PATCH 02/11] update hackmd link --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index ed9244d1..b7b75124 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,7 +16,7 @@ //! Finality gadget for blockchains. //! -//! https://hackmd.io/svMTltnGQsSR1GCjRKOPbw +//! https://hackmd.io/iA4XazxWRJ21LqMxwPSEZg?view mod vote_graph; From 8d81c471c441ac77e307322e31fb6d0b8b52409d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 14 Aug 2018 12:54:36 +0200 Subject: [PATCH 03/11] round counter skeleton --- src/lib.rs | 3 +++ src/round.rs | 68 +++++++++++++++++++++++++++++++++++++++-------- src/vote_graph.rs | 3 ++- 3 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b7b75124..bde9ce46 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,17 +18,20 @@ //! //! https://hackmd.io/iA4XazxWRJ21LqMxwPSEZg?view +mod round; mod vote_graph; use std::fmt; /// A prevote for a block and its ancestors. +#[derive(Clone, PartialEq, Eq)] pub struct Prevote { target: H, weight: usize, } /// A precommit for a block and its ancestors. +#[derive(Clone, PartialEq, Eq)] pub struct Precommit { target: H, weight: usize, diff --git a/src/round.rs b/src/round.rs index 8ae2f6ac..1d761d69 100644 --- a/src/round.rs +++ b/src/round.rs @@ -22,12 +22,12 @@ use std::collections::hash_map::{HashMap, Entry}; use std::hash::Hash; use std::ops::AddAssign; -use super::Equivocation; +use super::{Equivocation, Prevote, Precommit}; #[derive(Hash, Eq, PartialEq)] struct Address; -#[derive(Debug, Clone)] +#[derive(Default, Debug, Clone)] struct VoteCount { prevote: usize, precommit: usize, @@ -35,8 +35,8 @@ struct VoteCount { impl AddAssign for VoteCount { fn add_assign(&mut self, rhs: VoteCount) { - *self.prevote += rhs.prevote; - *self.precommit += rhs.precommit; + self.prevote += rhs.prevote; + self.precommit += rhs.precommit; } } @@ -45,31 +45,77 @@ struct VoteTracker { current_weight: usize, } -impl VoteTracker { +impl VoteTracker { fn new() -> Self { VoteTracker { - votes: HashMap::new(); + votes: HashMap::new(), current_weight: 0, } } // track a vote. if the vote is an equivocation, returns a proof-of-equivocation and - // otherwise notes the current amount of weight on the tracked vote-set + // otherwise notes the current amount of weight on the tracked vote-set. + // + // since this struct doesn't track the round number of votes, that must be set + // by the caller. fn add_vote(&mut self, id: Id, vote: Vote, signature: Signature, weight: usize) -> Result<(), Equivocation> { - match self.votes.entry(id) { + match self.votes.entry(id.clone()) { Entry::Vacant(mut vacant) => { vacant.insert((vote, signature)); } + Entry::Occupied(mut occupied) => { + if occupied.get().0 != vote { + return Err(Equivocation { + round_number: 0, + identity: id, + first: occupied.get().clone(), + second: (vote, signature), + }) + } + } } + + Ok(()) } } +/// Parameters for starting a round. +pub struct RoundParams { + /// The round number for votes. + pub round_number: usize, + /// The amount of byzantine-faulty voting weight that exists (assumed) in the + /// system. + pub faulty_weight: usize, + /// The amount of total voting weight in the system + pub total_weight: usize, + /// The base block to build on. + pub base: (H, usize), +} + /// Stores data for a round. -pub struct Round { +pub struct Round { graph: VoteGraph, - threshold_weight: usize, + prevote: VoteTracker, Signature>, + precommit: VoteTracker, Signature>, + round_number: usize, + faulty_weight: usize, total_weight: usize, - current_weight: usize, +} + +impl Round { + /// Create a new round accumulator for given round number and with given weight. + /// Not guaranteed to work correctly unless total_weight more than 3x larger than faulty_weight + pub fn new(round_params: RoundParams) -> Self { + let (base_hash, base_number) = round_params.base; + Round { + round_number: round_params.round_number, + faulty_weight: round_params.faulty_weight, + total_weight: round_params.total_weight, + graph: VoteGraph::new(base_hash, base_number), + prevote: VoteTracker::new(), + precommit: VoteTracker::new(), + } + } } diff --git a/src/vote_graph.rs b/src/vote_graph.rs index 8084d7d8..84402bec 100644 --- a/src/vote_graph.rs +++ b/src/vote_graph.rs @@ -68,7 +68,8 @@ impl VoteGraph where H: Hash + Eq + Clone + Ord, V: ::std::ops::AddAssign + Default + Clone, { - fn new(base_hash: H, base_number: usize) -> Self { + /// Create a new `VoteGraph` with base node as given. + pub fn new(base_hash: H, base_number: usize) -> Self { let mut entries = HashMap::new(); entries.insert(base_hash.clone(), Entry { number: base_number, From 053194c31fd6d11680063023d31182f6e7c8527f Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 14 Aug 2018 18:27:17 +0200 Subject: [PATCH 04/11] walking backwards from GHOST ancestor --- src/round.rs | 9 +++ src/vote_graph.rs | 144 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 3 deletions(-) diff --git a/src/round.rs b/src/round.rs index 1d761d69..349ce0ba 100644 --- a/src/round.rs +++ b/src/round.rs @@ -102,6 +102,7 @@ pub struct Round { round_number: usize, faulty_weight: usize, total_weight: usize, + prevote_ghost: Option<(H, usize)>, } impl Round { @@ -116,6 +117,14 @@ impl R graph: VoteGraph::new(base_hash, base_number), prevote: VoteTracker::new(), precommit: VoteTracker::new(), + prevote_ghost: None, } } + + /// Fetch the "round-estimate": the best block which might have been finalized + /// in this round. + pub fn estimate(&self) -> (H, usize) { + let remaining_commit_votes = self.total_weight - self.precommit.current_weight; + unimplemented!() + } } diff --git a/src/vote_graph.rs b/src/vote_graph.rs index 84402bec..cd2f04c4 100644 --- a/src/vote_graph.rs +++ b/src/vote_graph.rs @@ -21,6 +21,7 @@ use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::hash::Hash; +use std::ops::AddAssign; use super::{Chain, Error}; @@ -54,6 +55,14 @@ impl Entry { fn ancestor_node(&self) -> Option { self.ancestors.last().map(|x| x.clone()) } + + // the number of the ancestor block. + fn ancestor_number(&self) -> Option { + match self.ancestors.len() { + 0 => None, + len => Some(self.number - len), + } + } } /// Maintains a DAG of blocks in the chain which have votes attached to them, @@ -66,7 +75,7 @@ pub struct VoteGraph { impl VoteGraph where H: Hash + Eq + Clone + Ord, - V: ::std::ops::AddAssign + Default + Clone, + V: AddAssign + Default + Clone, { /// Create a new `VoteGraph` with base node as given. pub fn new(base_hash: H, base_number: usize) -> Self { @@ -117,6 +126,136 @@ impl VoteGraph where Ok(()) } + /// Find the highest block which is either an ancestor of or equal to the given, which fulfills a + /// condition. + pub fn find_ancestor<'a, F>(&'a self, mut hash: H, mut number: usize, condition: F) -> Option<(H, usize)> + where F: Fn(&V) -> bool + { + let entries = &self.entries; + let get_node = |hash: &_| -> &'a _ { + entries.get(hash) + .expect("node either base or referenced by other in graph; qed") + }; + + enum Walk { + // continue to ancestor node with given hash. + Continue(Option), + // stay on same node-set but our new block hash is recorded. + Stay(H), + } + + // frontier: the set of those vote-nodes which converge to a vote node + // containing our block. + enum FrontierKind<'a, H: 'a, V: 'a> { + Single(&'a Entry), + // with many, the `usize` is an index into the section where + // the nodes contain our block in their direct ancestry. + Many(Vec<&'a Entry>, usize, V), + } + + impl<'a, H: 'a + Hash + PartialEq + Clone, V: 'a + AddAssign + Clone> FrontierKind<'a, H, V> { + fn cumulative_vote(&self) -> &V { + match *self { + FrontierKind::Single(ref e) => &e.cumulative_vote, + FrontierKind::Many(_, _, ref v) => v, + } + } + + // updates the cumulative vote, based on the given number and + // the assumption that we're walking backwards in steps of 1. + // returns an ancestor hash if we have exhausted these edges. + fn step_backwards(&mut self, to: usize) -> Walk { + match *self { + FrontierKind::Single(ref entry) => Walk::Continue(entry.ancestor_node()), + FrontierKind::Many(ref mut entries, ref mut idx, ref mut v) => { + // we can skip to next node if all entries contained our last visited block. + let n_entries = entries.len(); + let can_skip = n_entries == *idx; + let block_is_ancestor = entries[0].ancestor_number().map_or(true, |x| x == to); + if can_skip || block_is_ancestor { + return Walk::Continue(entries[0].ancestor_node()); + } + + let canonical = entries[0].ancestor_block(to) + .expect("walking back by steps of 1; passed ancestor check already; qed") + .clone(); + + for i in *idx..n_entries { + if entries[i].in_direct_ancestry(&canonical, to) + .expect("all nodes in frontier have same ancestor; qed") + { + entries.swap(i, *idx); + (*v) += entries[i].cumulative_vote.clone(); + *idx += 1; + } + } + + if n_entries == *idx { + Walk::Continue(entries[0].ancestor_node()) + } else { + Walk::Stay(canonical) + } + } + } + } + } + + // `frontier` is a vector of nodes which converge to the same ancestor as our block. + // `idx` is an index into the vector where the section of nodes not containing the + // block are. can be one past the end if all frontier-nodes contain the block in ancestry. + let mut frontier = match self.find_containing_nodes(hash.clone(), number) { + None => FrontierKind::Single(get_node(&hash)), + Some(containing) => { + if containing.is_empty() { return None } + let ancestor_hash = get_node(&containing[0]).ancestor_node() + .expect("block is not base because we found non-direct containing nodes. \ + all non-base nodes have an ancestor node; qed"); + + let ancestor = get_node(&ancestor_hash); + let mut descendents: Vec<_> = ancestor.descendents.iter().map(get_node).collect(); + + // sort the descendents, putting those which contain our block + // at the front and others in the back. + descendents.sort_by_key(|x| x.in_direct_ancestry(&hash, number) + .map_or(1u8, |in_direct| if in_direct { 0 } else { 1 })); + + if descendents.len() == 1 { + FrontierKind::Single(descendents[0]) + } else { + let n_containing = containing.len(); + let cumulative_vote = descendents.iter() + .take(n_containing) + .map(|x| x.cumulative_vote.clone()) + .fold(Default::default(), |mut a, c| { a += c; a }); + + FrontierKind::Many(descendents, n_containing, cumulative_vote) + } + } + }; + + loop { + if condition(frontier.cumulative_vote()) { + return Some((hash.clone(), number)) + } + + number = number - 1; + match frontier.step_backwards(number) { + Walk::Continue(None) => break, // reached the base, no luck. + Walk::Continue(Some(ancestor_node)) => { + let node = get_node(&ancestor_node); + number = node.number; + hash = ancestor_node; + frontier = FrontierKind::Single(node); + } + Walk::Stay(ancestor_block_hash) => { + hash = ancestor_block_hash; + } + } + } + + None + } + /// Find the best GHOST descendent of the given block. /// Pass a closure used to evaluate the cumulative vote value. /// @@ -138,10 +277,9 @@ impl VoteGraph where let mut node_key = current_best .and_then(|(hash, number)| match self.find_containing_nodes(hash.clone(), number) { None => Some(hash), - Some(ref x) if !x.is_empty() => Some(x[0].clone()), + Some(ref x) if !x.is_empty() => get_node(&x[0]).ancestor_node(), Some(_) => None, }) - .and_then(|hash| get_node(&hash).ancestor_node()) .unwrap_or_else(|| self.base.clone()); let mut active_node = get_node(&node_key); From 804a010683bd76801bd7d6d76aab39813d10fee2 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 15 Aug 2018 16:57:35 +0200 Subject: [PATCH 05/11] better find-ancestor implementation --- src/vote_graph.rs | 214 +++++++++++++++++++--------------------------- 1 file changed, 88 insertions(+), 126 deletions(-) diff --git a/src/vote_graph.rs b/src/vote_graph.rs index cd2f04c4..9ea6b4e9 100644 --- a/src/vote_graph.rs +++ b/src/vote_graph.rs @@ -55,13 +55,22 @@ impl Entry { fn ancestor_node(&self) -> Option { self.ancestors.last().map(|x| x.clone()) } +} - // the number of the ancestor block. - fn ancestor_number(&self) -> Option { - match self.ancestors.len() { - 0 => None, - len => Some(self.number - len), - } +// a subchain of blocks by hash. +struct Subchain { + hashes: Vec, // forward order. + best_number: usize, +} + +impl Subchain { + fn blocks_reverse<'a>(&'a self) -> impl Iterator + 'a { + let best = self.best_number; + self.hashes.iter().rev().cloned().enumerate().map(move |(i, x)| (x, best - i)) + } + + fn best(&self) -> Option<(H, usize)> { + self.hashes.last().map(|x| (x.clone(), self.best_number)) } } @@ -128,7 +137,7 @@ impl VoteGraph where /// Find the highest block which is either an ancestor of or equal to the given, which fulfills a /// condition. - pub fn find_ancestor<'a, F>(&'a self, mut hash: H, mut number: usize, condition: F) -> Option<(H, usize)> + pub fn find_ancestor<'a, F>(&'a self, hash: H, number: usize, condition: F) -> Option<(H, usize)> where F: Fn(&V) -> bool { let entries = &self.entries; @@ -137,123 +146,54 @@ impl VoteGraph where .expect("node either base or referenced by other in graph; qed") }; - enum Walk { - // continue to ancestor node with given hash. - Continue(Option), - // stay on same node-set but our new block hash is recorded. - Stay(H), - } - - // frontier: the set of those vote-nodes which converge to a vote node - // containing our block. - enum FrontierKind<'a, H: 'a, V: 'a> { - Single(&'a Entry), - // with many, the `usize` is an index into the section where - // the nodes contain our block in their direct ancestry. - Many(Vec<&'a Entry>, usize, V), - } - - impl<'a, H: 'a + Hash + PartialEq + Clone, V: 'a + AddAssign + Clone> FrontierKind<'a, H, V> { - fn cumulative_vote(&self) -> &V { - match *self { - FrontierKind::Single(ref e) => &e.cumulative_vote, - FrontierKind::Many(_, _, ref v) => v, + // we store two nodes with an edge between them that is the canonical + // chain. + // the `node_key` always points to the ancestor node, and the `canonical_node` + // points to the higher node. + let (mut node_key, mut canonical_node) = match self.find_containing_nodes(hash.clone(), number) { + None => { + let node = get_node(&hash); + if condition(&node.cumulative_vote) { + return Some((hash, number)) } - } - // updates the cumulative vote, based on the given number and - // the assumption that we're walking backwards in steps of 1. - // returns an ancestor hash if we have exhausted these edges. - fn step_backwards(&mut self, to: usize) -> Walk { - match *self { - FrontierKind::Single(ref entry) => Walk::Continue(entry.ancestor_node()), - FrontierKind::Many(ref mut entries, ref mut idx, ref mut v) => { - // we can skip to next node if all entries contained our last visited block. - let n_entries = entries.len(); - let can_skip = n_entries == *idx; - let block_is_ancestor = entries[0].ancestor_number().map_or(true, |x| x == to); - if can_skip || block_is_ancestor { - return Walk::Continue(entries[0].ancestor_node()); - } - - let canonical = entries[0].ancestor_block(to) - .expect("walking back by steps of 1; passed ancestor check already; qed") - .clone(); - - for i in *idx..n_entries { - if entries[i].in_direct_ancestry(&canonical, to) - .expect("all nodes in frontier have same ancestor; qed") - { - entries.swap(i, *idx); - (*v) += entries[i].cumulative_vote.clone(); - *idx += 1; - } - } - - if n_entries == *idx { - Walk::Continue(entries[0].ancestor_node()) - } else { - Walk::Stay(canonical) - } - } - } + (node.ancestor_node()?, node) } - } + Some(ref x) if !x.is_empty() => { + let node = get_node(&x[0]); + let key = node.ancestor_node() + .expect("node containing block in ancestry has ancestor node; qed"); - // `frontier` is a vector of nodes which converge to the same ancestor as our block. - // `idx` is an index into the vector where the section of nodes not containing the - // block are. can be one past the end if all frontier-nodes contain the block in ancestry. - let mut frontier = match self.find_containing_nodes(hash.clone(), number) { - None => FrontierKind::Single(get_node(&hash)), - Some(containing) => { - if containing.is_empty() { return None } - let ancestor_hash = get_node(&containing[0]).ancestor_node() - .expect("block is not base because we found non-direct containing nodes. \ - all non-base nodes have an ancestor node; qed"); - - let ancestor = get_node(&ancestor_hash); - let mut descendents: Vec<_> = ancestor.descendents.iter().map(get_node).collect(); - - // sort the descendents, putting those which contain our block - // at the front and others in the back. - descendents.sort_by_key(|x| x.in_direct_ancestry(&hash, number) - .map_or(1u8, |in_direct| if in_direct { 0 } else { 1 })); - - if descendents.len() == 1 { - FrontierKind::Single(descendents[0]) - } else { - let n_containing = containing.len(); - let cumulative_vote = descendents.iter() - .take(n_containing) - .map(|x| x.cumulative_vote.clone()) - .fold(Default::default(), |mut a, c| { a += c; a }); - - FrontierKind::Many(descendents, n_containing, cumulative_vote) - } + (key, node) } + Some(_) => return None, }; - loop { - if condition(frontier.cumulative_vote()) { - return Some((hash.clone(), number)) - } + let initial_node_key = node_key.clone(); - number = number - 1; - match frontier.step_backwards(number) { - Walk::Continue(None) => break, // reached the base, no luck. - Walk::Continue(Some(ancestor_node)) => { - let node = get_node(&ancestor_node); - number = node.number; - hash = ancestor_node; - frontier = FrontierKind::Single(node); - } - Walk::Stay(ancestor_block_hash) => { - hash = ancestor_block_hash; - } - } + // search backwards until we find the first vote-node that + // meets the condition. + let mut active_node = get_node(&node_key); + while !condition(&active_node.cumulative_vote) { + node_key = match active_node.ancestor_node() { + Some(n) => n, + None => return None, + }; + + canonical_node = active_node; + active_node = get_node(&node_key); } - None + // find the GHOST merge-point after the active_node. + // constrain it to be within the canonical chain. + let good_subchain = self.ghost_find_merge_point(node_key, active_node, condition); + + // TODO: binding is required for some reason. + let x = good_subchain.blocks_reverse().find(|&(ref good_hash, good_number)| + canonical_node.in_direct_ancestry(good_hash, good_number).unwrap_or(false) + ); + + x } /// Find the best GHOST descendent of the given block. @@ -307,7 +247,7 @@ impl VoteGraph where // its descendents comprise frontier of vote-nodes which individually don't have enough votes // to pass the threshold but some subset of them join either at `active_node`'s block or at some // descendent block of it, giving that block sufficient votes. - Some(self.ghost_find_merge_point(node_key, active_node, condition)) + self.ghost_find_merge_point(node_key, active_node, condition).best() } // given a key, node pair (which must correspond), assuming this node fulfills the condition, @@ -315,10 +255,10 @@ impl VoteGraph where // node itself. fn ghost_find_merge_point<'a, F>( &'a self, - mut node_key: H, - mut active_node: &'a Entry, + node_key: H, + active_node: &'a Entry, condition: F, - ) -> (H, usize) + ) -> Subchain where F: Fn(&V) -> bool { let mut descendent_nodes: Vec<_> = active_node.descendents.iter() @@ -326,8 +266,9 @@ impl VoteGraph where .collect(); let base_number = active_node.number; - let (mut best_hash, mut best_number) = (node_key, active_node.number); + let mut best_number = active_node.number; let mut descendent_blocks = Vec::with_capacity(descendent_nodes.len()); + let mut hashes = vec![node_key]; // TODO: for long ranges of blocks this could get inefficient for offset in 1usize.. { @@ -339,6 +280,7 @@ impl VoteGraph where descendent_blocks[idx].1 += d_node.cumulative_vote.clone(); if condition(&descendent_blocks[idx].1) { new_best = Some(d_block.clone()); + break } } Err(idx) => descendent_blocks.insert(idx, ( @@ -351,19 +293,23 @@ impl VoteGraph where match new_best { Some(new_best) => { - best_hash = new_best; best_number += 1; + + descendent_blocks.clear(); + descendent_nodes.retain( + |n| n.in_direct_ancestry(&new_best, best_number).unwrap_or(false) + ); + + hashes.push(new_best); } None => break, } - - descendent_blocks.clear(); - descendent_nodes.retain( - |n| n.in_direct_ancestry(&best_hash, best_number).unwrap_or(false) - ); } - (best_hash, best_number) + Subchain { + hashes, + best_number, + } } // attempts to find the containing node keys for the given hash and number. @@ -656,4 +602,20 @@ mod tests { assert_eq!(tracker.find_ghost(Some(("C", 4)), |&x| x >= 250), Some(("F", 7))); assert_eq!(tracker.find_ghost(Some(("B", 3)), |&x| x >= 250), Some(("F", 7))); } + + #[test] + fn walk_back_from_block_in_edge_fork_below() { + let mut chain = DummyChain::new(); + let mut tracker = VoteGraph::new(GENESIS_HASH, 1); + + chain.push_blocks(GENESIS_HASH, &["A", "B", "C"]); + chain.push_blocks("C", &["D1", "E1", "F1", "G1", "H1", "I1"]); + chain.push_blocks("C", &["D2", "E2", "F2", "G2", "H2", "I2"]); + + tracker.insert("B", 3, 10usize, &chain).unwrap(); + tracker.insert("F1", 7, 5usize, &chain).unwrap(); + tracker.insert("G2", 8, 5usize, &chain).unwrap(); + + assert_eq!(tracker.find_ancestor("G2", 8, |&x| x > 5).unwrap(), ("C", 4)); + } } From c40e8407a1fc4e040aeca3203fa22669169fb972 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 15 Aug 2018 17:19:02 +0200 Subject: [PATCH 06/11] refine tests for vote graph --- src/vote_graph.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/src/vote_graph.rs b/src/vote_graph.rs index 9ea6b4e9..694b1143 100644 --- a/src/vote_graph.rs +++ b/src/vote_graph.rs @@ -473,6 +473,10 @@ mod tests { parent = descendent; } } + + fn number(&self, hash: &'static str) -> usize { + self.inner.get(hash).unwrap().number + } } impl Chain<&'static str> for DummyChain { @@ -616,6 +620,78 @@ mod tests { tracker.insert("F1", 7, 5usize, &chain).unwrap(); tracker.insert("G2", 8, 5usize, &chain).unwrap(); - assert_eq!(tracker.find_ancestor("G2", 8, |&x| x > 5).unwrap(), ("C", 4)); + let test_cases = &[ + "D1", + "D2", + "E1", + "E2", + "F1", + "F2", + "G2", + ]; + + for block in test_cases { + let number = chain.number(block); + assert_eq!(tracker.find_ancestor(block, number, |&x| x > 5).unwrap(), ("C", 4)); + } + } + + #[test] + fn walk_back_from_fork_block_node_below() { + let mut chain = DummyChain::new(); + let mut tracker = VoteGraph::new(GENESIS_HASH, 1); + + chain.push_blocks(GENESIS_HASH, &["A", "B", "C", "D"]); + chain.push_blocks("D", &["E1", "F1", "G1", "H1", "I1"]); + chain.push_blocks("D", &["E2", "F2", "G2", "H2", "I2"]); + + tracker.insert("B", 3, 10usize, &chain).unwrap(); + tracker.insert("F1", 7, 5usize, &chain).unwrap(); + tracker.insert("G2", 8, 5usize, &chain).unwrap(); + + assert_eq!(tracker.find_ancestor("G2", 8, |&x| x > 5).unwrap(), ("D", 5)); + let test_cases = &[ + "E1", + "E2", + "F1", + "F2", + "G2", + ]; + + for block in test_cases { + let number = chain.number(block); + assert_eq!(tracker.find_ancestor(block, number, |&x| x > 5).unwrap(), ("D", 5)); + } + } + + #[test] + fn walk_back_at_node() { + let mut chain = DummyChain::new(); + let mut tracker = VoteGraph::new(GENESIS_HASH, 1); + + chain.push_blocks(GENESIS_HASH, &["A", "B", "C"]); + chain.push_blocks("C", &["D1", "E1", "F1", "G1", "H1", "I1"]); + chain.push_blocks("C", &["D2", "E2", "F2"]); + + tracker.insert("C", 4, 10usize, &chain).unwrap(); + tracker.insert("F1", 7, 5usize, &chain).unwrap(); + tracker.insert("F2", 7, 5usize, &chain).unwrap(); + tracker.insert("I1", 10, 1usize, &chain).unwrap(); + + let test_cases = &[ + "C", + "D1", + "D2", + "E1", + "E2", + "F1", + "F2", + "I1", + ]; + + for block in test_cases { + let number = chain.number(block); + assert_eq!(tracker.find_ancestor(block, number, |&x| x >= 20).unwrap(), ("C", 4)); + } } } From d76f54190a8f4f409fa8d82e4deb33bf50b6858f Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 23 Aug 2018 16:16:39 +0200 Subject: [PATCH 07/11] round accumulator. compute estimate and whether rounds are completable. --- src/lib.rs | 8 +-- src/round.rs | 173 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 162 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bde9ce46..ed3a2c98 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,15 +26,15 @@ use std::fmt; /// A prevote for a block and its ancestors. #[derive(Clone, PartialEq, Eq)] pub struct Prevote { - target: H, - weight: usize, + target_hash: H, + target_number: u32, } /// A precommit for a block and its ancestors. #[derive(Clone, PartialEq, Eq)] pub struct Precommit { - target: H, - weight: usize, + target_hash: H, + target_number: u32, } #[derive(Clone, PartialEq, Debug)] diff --git a/src/round.rs b/src/round.rs index 349ce0ba..bfc74865 100644 --- a/src/round.rs +++ b/src/round.rs @@ -22,7 +22,7 @@ use std::collections::hash_map::{HashMap, Entry}; use std::hash::Hash; use std::ops::AddAssign; -use super::{Equivocation, Prevote, Precommit}; +use super::{Equivocation, Prevote, Precommit, Chain}; #[derive(Hash, Eq, PartialEq)] struct Address; @@ -82,49 +82,192 @@ impl VoteTracker { +pub struct RoundParams { /// The round number for votes. - pub round_number: usize, - /// The amount of byzantine-faulty voting weight that exists (assumed) in the - /// system. - pub faulty_weight: usize, - /// The amount of total voting weight in the system - pub total_weight: usize, + pub round_number: u64, + /// Actors and weights in the round. + pub voters: HashMap, /// The base block to build on. pub base: (H, usize), } +pub enum Error { + PrevoteEquivocation(Equivocation, S>), + PrecommitEquivocation(Equivocation, S>), + Chain(::Error), +} + /// Stores data for a round. pub struct Round { graph: VoteGraph, prevote: VoteTracker, Signature>, precommit: VoteTracker, Signature>, - round_number: usize, + round_number: u64, + voters: HashMap, faulty_weight: usize, total_weight: usize, prevote_ghost: Option<(H, usize)>, + estimate: Option<(H, usize)>, + completable: bool, } impl Round { /// Create a new round accumulator for given round number and with given weight. /// Not guaranteed to work correctly unless total_weight more than 3x larger than faulty_weight - pub fn new(round_params: RoundParams) -> Self { + pub fn new(round_params: RoundParams) -> Self { let (base_hash, base_number) = round_params.base; + let total_weight: usize = round_params.voters.values().cloned().sum(); + let faulty_weight = total_weight.saturating_sub(1) / 3; + Round { round_number: round_params.round_number, - faulty_weight: round_params.faulty_weight, - total_weight: round_params.total_weight, + faulty_weight: faulty_weight, + total_weight: total_weight, + voters: round_params.voters, graph: VoteGraph::new(base_hash, base_number), prevote: VoteTracker::new(), precommit: VoteTracker::new(), prevote_ghost: None, + estimate: None, + completable: false, + } + } + + /// Import a prevote. Has no effect on internal state if an equivocation. + pub fn import_prevote>( + &mut self, + chain: &C, + vote: Prevote, + signer: Id, + signature: Signature, + ) -> Result<(), Error> { + let weight = match self.voters.get(&signer) { + Some(weight) => *weight, + None => return Ok(()), + }; + + self.prevote.add_vote(signer, vote.clone(), signature, weight) + .map_err(|mut e| { e.round_number = self.round_number; e }) + .map_err(Error::PrevoteEquivocation)?; + + let vc = VoteCount { + prevote: weight, + precommit: 0, + }; + + self.graph.insert(vote.target_hash, vote.target_number as usize, vc, chain) + .map_err(Error::Chain)?; + + // update prevote-GHOST + let threshold = self.threshold(); + if self.prevote.current_weight >= threshold { + self.prevote_ghost = self.graph.find_ghost(self.prevote_ghost.take(), |v| v.prevote >= threshold); } + + self.update_estimate(); + Ok(()) + } + + /// Import a prevote. Has no effect on internal state if an equivocation. + pub fn import_precommit>( + &mut self, + chain: &C, + vote: Precommit, + signer: Id, + signature: Signature, + ) -> Result<(), Error> { + let weight = match self.voters.get(&signer) { + Some(weight) => *weight, + None => return Ok(()), + }; + + self.precommit.add_vote(signer, vote.clone(), signature, weight) + .map_err(|mut e| { e.round_number = self.round_number; e }) + .map_err(Error::PrecommitEquivocation)?; + + let vc = VoteCount { + prevote: 0, + precommit: weight, + }; + + self.graph.insert(vote.target_hash, vote.target_number as usize, vc, chain) + .map_err(Error::Chain)?; + + self.update_estimate(); + Ok(()) + } + + // update the round-estimate and whether the round is completable. + fn update_estimate(&mut self) { + let threshold = self.threshold(); + if self.prevote.current_weight < threshold { return } + + let remaining_commit_votes = self.total_weight - self.precommit.current_weight; + let (g_hash, g_num) = match self.prevote_ghost.clone() { + None => return, + Some(x) => x, + }; + + self.estimate = self.graph.find_ancestor( + g_hash.clone(), + g_num, + |vote| vote.precommit + remaining_commit_votes >= threshold, + ); + + self.completable = self.estimate.clone().map_or(false, |(b_hash, b_num)| { + b_hash != g_hash || { + // round-estimate is the same as the prevote-ghost. + // this round is still completable if no further blocks + // could have commit-supermajority. + let remaining_commit_votes = self.total_weight - self.precommit.current_weight; + let threshold = self.threshold(); + + self.graph.find_ghost(Some((b_hash, b_num)), |count| { + count.prevote >= threshold && count.precommit + remaining_commit_votes >=threshold + }).map_or(true, |x| x.0 == g_hash) + } + }) } /// Fetch the "round-estimate": the best block which might have been finalized /// in this round. - pub fn estimate(&self) -> (H, usize) { - let remaining_commit_votes = self.total_weight - self.precommit.current_weight; - unimplemented!() + /// + /// Returns `None` when new new blocks could have been finalized in this round, + /// according to our estimate. + pub fn estimate(&self) -> Option<&(H, usize)> { + self.estimate.as_ref() + } + + /// Returns `true` when the round is completable. + /// + /// This is the case when the round-estimate is an ancestor of the prevote-ghost head, + /// or when they are the same block _and_ none of its children could possibly have + /// enough precommits. + pub fn completable(&self) -> bool { + self.completable + } + + // Threshold number of weight for supermajority. + pub fn threshold(&self) -> usize { + threshold(self.total_weight, self.faulty_weight) + } +} + +fn threshold(total_weight: usize, faulty_weight: usize) -> usize { + let mut double_supermajority = total_weight + faulty_weight + 1; + double_supermajority += double_supermajority & 1; + double_supermajority / 2 +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn threshold_is_right() { + assert_eq!(threshold(10, 3), 7); + assert_eq!(threshold(100, 33), 67); + assert_eq!(threshold(101, 33), 68); + assert_eq!(threshold(102, 33), 68); } } From 4f5bced1884518b6eae8c3dcb17311645e944ee6 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 23 Aug 2018 17:36:53 +0200 Subject: [PATCH 08/11] initial estimate logic (subtly wrong) --- src/lib.rs | 20 ++++++++++-- src/round.rs | 53 ++++++++++++++++++++++++++++++-- src/testing.rs | 77 +++++++++++++++++++++++++++++++++++++++++++++++ src/vote_graph.rs | 58 +---------------------------------- 4 files changed, 146 insertions(+), 62 deletions(-) create mode 100644 src/testing.rs diff --git a/src/lib.rs b/src/lib.rs index ed3a2c98..e0eebecb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,22 +21,37 @@ mod round; mod vote_graph; +#[cfg(test)] +mod testing; + use std::fmt; /// A prevote for a block and its ancestors. -#[derive(Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Prevote { target_hash: H, target_number: u32, } +impl Prevote { + pub fn new(target_hash: H, target_number: u32) -> Self { + Prevote { target_hash, target_number } + } +} + /// A precommit for a block and its ancestors. -#[derive(Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Precommit { target_hash: H, target_number: u32, } +impl Precommit { + pub fn new(target_hash: H, target_number: u32) -> Self { + Precommit { target_hash, target_number } + } +} + #[derive(Clone, PartialEq, Debug)] pub enum Error { BlockNotInSubtree, @@ -68,6 +83,7 @@ pub trait Chain { } /// An equivocation (double-vote) in a given round. +#[derive(Debug, Clone, PartialEq)] pub struct Equivocation { /// The round number equivocated in. pub round_number: u64, diff --git a/src/round.rs b/src/round.rs index bfc74865..26f197ff 100644 --- a/src/round.rs +++ b/src/round.rs @@ -77,6 +77,8 @@ impl VoteTracker { pub base: (H, usize), } +#[derive(Debug)] pub enum Error { PrevoteEquivocation(Equivocation, S>), PrecommitEquivocation(Equivocation, S>), @@ -222,9 +225,9 @@ impl R let remaining_commit_votes = self.total_weight - self.precommit.current_weight; let threshold = self.threshold(); - self.graph.find_ghost(Some((b_hash, b_num)), |count| { - count.prevote >= threshold && count.precommit + remaining_commit_votes >=threshold - }).map_or(true, |x| x.0 == g_hash) + self.graph.find_ghost(Some((b_hash, b_num)), |count| + count.precommit + remaining_commit_votes >= threshold + ).map_or(true, |x| x.0 == g_hash) } }) } @@ -262,6 +265,18 @@ fn threshold(total_weight: usize, faulty_weight: usize) -> usize { #[cfg(test)] mod tests { use super::*; + use testing::{GENESIS_HASH, DummyChain}; + + fn voters() -> HashMap<&'static str, usize> { + [ + ("Alice", 5), + ("Bob", 7), + ("Eve", 3), + ].iter().cloned().collect() + } + + #[derive(PartialEq, Eq, Hash, Clone, Debug)] + struct Signature(&'static str); #[test] fn threshold_is_right() { @@ -270,4 +285,36 @@ mod tests { assert_eq!(threshold(101, 33), 68); assert_eq!(threshold(102, 33), 68); } + + #[test] + fn estimate_is_valid() { + let mut chain = DummyChain::new(); + chain.push_blocks(GENESIS_HASH, &["A", "B", "C", "D", "E", "F"]); + chain.push_blocks("E", &["EA", "EB", "EC", "ED"]); + chain.push_blocks("F", &["FA", "FB", "FC"]); + + let mut round = Round::new(RoundParams { + round_number: 1, + voters: voters(), + base: ("C", 4), + }); + + round.import_prevote( + &chain, + Prevote::new("FC", 10), + "Alice", + Signature("Alice"), + ).unwrap(); + + round.import_prevote( + &chain, + Prevote::new("ED", 10), + "Bob", + Signature("Bob"), + ).unwrap(); + + assert_eq!(round.prevote_ghost, Some(("E", 6))); + assert_eq!(round.estimate(), Some(&("E", 6))); + assert!(!round.completable()); + } } diff --git a/src/testing.rs b/src/testing.rs new file mode 100644 index 00000000..b44580a3 --- /dev/null +++ b/src/testing.rs @@ -0,0 +1,77 @@ +// Copyright 2018 Parity Technologies (UK) Ltd. +// This file is part of finality-afg. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with finality-afg. If not, see . + +//! Helpers for testing + +use std::collections::HashMap; +use super::{Chain, Error}; + +pub const GENESIS_HASH: &str = "genesis"; +const NULL_HASH: &str = "NULL"; + +struct BlockRecord { + number: usize, + parent: &'static str, +} + +pub struct DummyChain { + inner: HashMap<&'static str, BlockRecord>, +} + +impl DummyChain { + pub fn new() -> Self { + let mut inner = HashMap::new(); + inner.insert(GENESIS_HASH, BlockRecord { number: 1, parent: NULL_HASH }); + + DummyChain { inner } + } + + pub fn push_blocks(&mut self, mut parent: &'static str, blocks: &[&'static str]) { + let base_number = self.inner.get(parent).unwrap().number + 1; + for (i, descendent) in blocks.iter().enumerate() { + self.inner.insert(descendent, BlockRecord { + number: base_number + i, + parent, + }); + + parent = descendent; + } + } + + pub fn number(&self, hash: &'static str) -> usize { + self.inner.get(hash).unwrap().number + } +} + +impl Chain<&'static str> for DummyChain { + fn ancestry(&self, base: &'static str, mut block: &'static str) -> Result, Error> { + let mut ancestry = Vec::new(); + + loop { + match self.inner.get(block) { + None => return Err(Error::BlockNotInSubtree), + Some(record) => { block = record.parent; } + } + + ancestry.push(block); + + if block == NULL_HASH { return Err(Error::BlockNotInSubtree) } + if block == base { break } + } + + Ok(ancestry) + } +} diff --git a/src/vote_graph.rs b/src/vote_graph.rs index 694b1143..4888a86a 100644 --- a/src/vote_graph.rs +++ b/src/vote_graph.rs @@ -441,63 +441,7 @@ impl VoteGraph where #[cfg(test)] mod tests { use super::*; - - const GENESIS_HASH: &str = "genesis"; - const NULL_HASH: &str = "NULL"; - - struct BlockRecord { - number: usize, - parent: &'static str, - } - - struct DummyChain { - inner: HashMap<&'static str, BlockRecord>, - } - - impl DummyChain { - fn new() -> Self { - let mut inner = HashMap::new(); - inner.insert(GENESIS_HASH, BlockRecord { number: 1, parent: NULL_HASH }); - - DummyChain { inner } - } - - fn push_blocks(&mut self, mut parent: &'static str, blocks: &[&'static str]) { - let base_number = self.inner.get(parent).unwrap().number + 1; - for (i, descendent) in blocks.iter().enumerate() { - self.inner.insert(descendent, BlockRecord { - number: base_number + i, - parent, - }); - - parent = descendent; - } - } - - fn number(&self, hash: &'static str) -> usize { - self.inner.get(hash).unwrap().number - } - } - - impl Chain<&'static str> for DummyChain { - fn ancestry(&self, base: &'static str, mut block: &'static str) -> Result, Error> { - let mut ancestry = Vec::new(); - - loop { - match self.inner.get(block) { - None => return Err(Error::BlockNotInSubtree), - Some(record) => { block = record.parent; } - } - - ancestry.push(block); - - if block == NULL_HASH { return Err(Error::BlockNotInSubtree) } - if block == base { break } - } - - Ok(ancestry) - } - } + use testing::{GENESIS_HASH, DummyChain}; #[test] fn graph_fork_not_at_node() { From d5520da3c537292e80b86161ca8996abed3f08fe Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 23 Aug 2018 18:15:42 +0200 Subject: [PATCH 09/11] constrain GHOST results to be on same chain as "current" --- src/round.rs | 13 ++++++++++--- src/vote_graph.rs | 45 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/round.rs b/src/round.rs index 26f197ff..196bafc5 100644 --- a/src/round.rs +++ b/src/round.rs @@ -225,9 +225,16 @@ impl R let remaining_commit_votes = self.total_weight - self.precommit.current_weight; let threshold = self.threshold(); - self.graph.find_ghost(Some((b_hash, b_num)), |count| - count.precommit + remaining_commit_votes >= threshold - ).map_or(true, |x| x.0 == g_hash) + // when the remaining votes are at least the threshold, + // we can always have commit-supermajority. + // + // once it's below that level, we only need to consider already + // blocks referenced in the graph, because no new leaf nodes + // could ever have enough commits. + remaining_commit_votes < threshold && + self.graph.find_ghost(Some((b_hash, b_num)), |count| + count.precommit + remaining_commit_votes >= threshold + ).map_or(true, |x| x == (g_hash, g_num)) } }) } diff --git a/src/vote_graph.rs b/src/vote_graph.rs index 4888a86a..4d208b5d 100644 --- a/src/vote_graph.rs +++ b/src/vote_graph.rs @@ -69,6 +69,11 @@ impl Subchain { self.hashes.iter().rev().cloned().enumerate().map(move |(i, x)| (x, best - i)) } + fn block_at(&self, number: usize) -> Option<&H> { + let rev_off = self.best_number.checked_sub(number)?; + self.hashes.len().checked_sub(rev_off + 1).map(|i| &self.hashes[i]) + } + fn best(&self) -> Option<(H, usize)> { self.hashes.last().map(|x| (x.clone(), self.best_number)) } @@ -186,7 +191,7 @@ impl VoteGraph where // find the GHOST merge-point after the active_node. // constrain it to be within the canonical chain. - let good_subchain = self.ghost_find_merge_point(node_key, active_node, condition); + let good_subchain = self.ghost_find_merge_point(node_key, active_node, None, condition); // TODO: binding is required for some reason. let x = good_subchain.blocks_reverse().find(|&(ref good_hash, good_number)| @@ -214,13 +219,19 @@ impl VoteGraph where .expect("node either base or referenced by other in graph; qed") }; - let mut node_key = current_best + let (mut node_key, mut force_constrain) = current_best + .clone() .and_then(|(hash, number)| match self.find_containing_nodes(hash.clone(), number) { - None => Some(hash), - Some(ref x) if !x.is_empty() => get_node(&x[0]).ancestor_node(), + None => Some((hash, false)), + Some(ref x) if !x.is_empty() => { + let ancestor = get_node(&x[0]).ancestor_node() + .expect("node containing non-node in history always has ancestor; qed"); + + Some((ancestor, true)) + } Some(_) => None, }) - .unwrap_or_else(|| self.base.clone()); + .unwrap_or_else(|| (self.base.clone(), false)); let mut active_node = get_node(&node_key); @@ -231,11 +242,22 @@ impl VoteGraph where let next_descendent = active_node.descendents .iter() .map(|d| (d.clone(), get_node(d))) + .filter(|&(ref d_key, ref node)| { + // take only descendents with our block in the ancestry. + if let (true, Some(&(ref h, n))) = (force_constrain, current_best.as_ref()) { + node.in_direct_ancestry(h, n).unwrap_or(false) + } else { + true + } + }) .filter(|&(_, ref node)| condition(&node.cumulative_vote)) .next(); match next_descendent { Some((key, node)) => { + // once we've made at least one hop, we don't need to constrain + // ancestry anymore. + force_constrain = false; node_key = key; active_node = node; } @@ -247,7 +269,12 @@ impl VoteGraph where // its descendents comprise frontier of vote-nodes which individually don't have enough votes // to pass the threshold but some subset of them join either at `active_node`'s block or at some // descendent block of it, giving that block sufficient votes. - self.ghost_find_merge_point(node_key, active_node, condition).best() + self.ghost_find_merge_point( + node_key, + active_node, + if force_constrain { current_best } else { None }, + condition, + ).best() } // given a key, node pair (which must correspond), assuming this node fulfills the condition, @@ -257,12 +284,18 @@ impl VoteGraph where &'a self, node_key: H, active_node: &'a Entry, + force_constrain: Option<(H, usize)>, condition: F, ) -> Subchain where F: Fn(&V) -> bool { let mut descendent_nodes: Vec<_> = active_node.descendents.iter() .map(|h| self.entries.get(h).expect("descendents always present in node storage; qed")) + .filter(|n| if let Some((ref h, num)) = force_constrain { + n.in_direct_ancestry(h, num).unwrap_or(false) + } else { + true + }) .collect(); let base_number = active_node.number; From b27883429a3364042db6e16b5e641721b5e68b52 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 24 Aug 2018 19:22:27 +0200 Subject: [PATCH 10/11] fix grumbles and introduce_branch --- src/round.rs | 54 +++++++++++++++++++++++++++++++---------- src/vote_graph.rs | 61 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/src/round.rs b/src/round.rs index 196bafc5..ebe7b6ce 100644 --- a/src/round.rs +++ b/src/round.rs @@ -61,15 +61,15 @@ impl VoteTracker Result<(), Equivocation> { - match self.votes.entry(id.clone()) { + match self.votes.entry(id) { Entry::Vacant(mut vacant) => { vacant.insert((vote, signature)); } - Entry::Occupied(mut occupied) => { + Entry::Occupied(occupied) => { if occupied.get().0 != vote { return Err(Equivocation { round_number: 0, - identity: id, + identity: occupied.key().clone(), first: occupied.get().clone(), second: (vote, signature), }) @@ -102,19 +102,24 @@ pub enum Error { /// Stores data for a round. pub struct Round { - graph: VoteGraph, - prevote: VoteTracker, Signature>, - precommit: VoteTracker, Signature>, + graph: VoteGraph, // DAG of blocks which have been voted on. + prevote: VoteTracker, Signature>, // tracks prevotes that have been counted + precommit: VoteTracker, Signature>, // tracks precommits round_number: u64, voters: HashMap, faulty_weight: usize, total_weight: usize, - prevote_ghost: Option<(H, usize)>, - estimate: Option<(H, usize)>, - completable: bool, + prevote_ghost: Option<(H, usize)>, // current memoized prevote-GHOST block + finalized: Option<(H, usize)>, // best finalized block in this round. + estimate: Option<(H, usize)>, // current memoized round-estimate + completable: bool, // whether the round is completable } -impl Round { +impl Round where + Id: Hash + Clone + Eq, + H: Hash + Clone + Eq + Ord + ::std::fmt::Debug, + Signature: Eq + Clone, +{ /// Create a new round accumulator for given round number and with given weight. /// Not guaranteed to work correctly unless total_weight more than 3x larger than faulty_weight pub fn new(round_params: RoundParams) -> Self { @@ -131,12 +136,14 @@ impl R prevote: VoteTracker::new(), precommit: VoteTracker::new(), prevote_ghost: None, + finalized: None, estimate: None, completable: false, } } - /// Import a prevote. Has no effect on internal state if an equivocation. + /// Import a prevote. Has no effect on internal state if an equivocation or if the + /// signer is not a known voter. pub fn import_prevote>( &mut self, chain: &C, @@ -171,7 +178,8 @@ impl R Ok(()) } - /// Import a prevote. Has no effect on internal state if an equivocation. + /// Import a prevote. Has no effect on internal state if an equivocation or if + /// the signer is not a known voter. pub fn import_precommit>( &mut self, chain: &C, @@ -196,6 +204,12 @@ impl R self.graph.insert(vote.target_hash, vote.target_number as usize, vc, chain) .map_err(Error::Chain)?; + // anything finalized? + let threshold = self.threshold(); + if self.precommit.current_weight >= threshold { + self.finalized = self.graph.find_ghost(self.finalized.take(), |v| v.precommit >= threshold); + } + self.update_estimate(); Ok(()) } @@ -248,6 +262,11 @@ impl R self.estimate.as_ref() } + /// Fetch the most recently finalized block. + pub fn finalized(&self) -> Option<&(H, usize)> { + self.finalized.as_ref() + } + /// Returns `true` when the round is completable. /// /// This is the case when the round-estimate is an ancestor of the prevote-ghost head, @@ -323,5 +342,16 @@ mod tests { assert_eq!(round.prevote_ghost, Some(("E", 6))); assert_eq!(round.estimate(), Some(&("E", 6))); assert!(!round.completable()); + + round.import_prevote( + &chain, + Prevote::new("F", 7), + "Eve", + Signature("Eve"), + ).unwrap(); + + assert_eq!(round.prevote_ghost, Some(("E", 6))); + assert_eq!(round.estimate(), Some(&("E", 6))); } + } diff --git a/src/vote_graph.rs b/src/vote_graph.rs index 4d208b5d..c94b4ba5 100644 --- a/src/vote_graph.rs +++ b/src/vote_graph.rs @@ -88,8 +88,8 @@ pub struct VoteGraph { } impl VoteGraph where - H: Hash + Eq + Clone + Ord, - V: AddAssign + Default + Clone, + H: Hash + Eq + Clone + Ord + Debug, + V: AddAssign + Default + Clone + Debug, { /// Create a new `VoteGraph` with base node as given. pub fn new(base_hash: H, base_number: usize) -> Self { @@ -242,7 +242,7 @@ impl VoteGraph where let next_descendent = active_node.descendents .iter() .map(|d| (d.clone(), get_node(d))) - .filter(|&(ref d_key, ref node)| { + .filter(|&(_, ref node)| { // take only descendents with our block in the ancestry. if let (true, Some(&(ref h, n))) = (force_constrain, current_best.as_ref()) { node.in_direct_ancestry(h, n).unwrap_or(false) @@ -413,13 +413,18 @@ impl VoteGraph where { let offset = entry.number.checked_sub(ancestor_number) .expect("this function only invoked with direct ancestors; qed"); + let prev_ancestor = entry.ancestor_node(); let new_ancestors = entry.ancestors.drain(offset..); - let new_entry = maybe_entry.get_or_insert_with(move || Entry { - number: ancestor_number, - ancestors: new_ancestors.collect(), - descendents: vec![], - cumulative_vote: V::default(), + let &mut (ref mut new_entry, _) = maybe_entry.get_or_insert_with(move || { + let new_entry = Entry { + number: ancestor_number, + ancestors: new_ancestors.collect(), + descendents: vec![], + cumulative_vote: V::default(), + }; + + (new_entry, prev_ancestor) }); new_entry.descendents.push(descendent); @@ -429,7 +434,15 @@ impl VoteGraph where maybe_entry }); - if let Some(new_entry) = produced_entry { + if let Some((new_entry, prev_ancestor)) = produced_entry { + if let Some(prev_ancestor) = prev_ancestor { + let mut prev_ancestor_node = self.entries.get_mut(&prev_ancestor) + .expect("Prior ancestor is referenced from a node; qed"); + + prev_ancestor_node.descendents.retain(|h| !new_entry.descendents.contains(&h)); + prev_ancestor_node.descendents.push(ancestor_hash.clone()); + } + assert!( self.entries.insert(ancestor_hash, new_entry).is_none(), "thus function is only invoked when there is no entry for the ancestor already; qed", @@ -584,6 +597,36 @@ mod tests { assert_eq!(tracker.find_ghost(Some(("B", 3)), |&x| x >= 250), Some(("F", 7))); } + #[test] + fn ghost_introduce_branch() { + let mut chain = DummyChain::new(); + let mut tracker = VoteGraph::new(GENESIS_HASH, 1); + + chain.push_blocks(GENESIS_HASH, &["A", "B", "C", "D", "E", "F"]); + chain.push_blocks("E", &["EA", "EB", "EC", "ED"]); + chain.push_blocks("F", &["FA", "FB", "FC"]); + + tracker.insert("FC", 10, 5usize, &chain).unwrap(); + tracker.insert("ED", 10, 7, &chain).unwrap(); + + assert_eq!(tracker.find_ghost(None, |&x| x >= 10), Some(("E", 6))); + + assert_eq!(tracker.entries.get(GENESIS_HASH).unwrap().descendents, vec!["FC", "ED"]); + + // introduce a branch in the middle. + tracker.insert("E", 6, 3, &chain).unwrap(); + + assert_eq!(tracker.entries.get(GENESIS_HASH).unwrap().descendents, vec!["E"]); + let descendents = &tracker.entries.get("E").unwrap().descendents; + assert_eq!(descendents.len(), 2); + assert!(descendents.contains(&"ED")); + assert!(descendents.contains(&"FC")); + + assert_eq!(tracker.find_ghost(None, |&x| x >= 10), Some(("E", 6))); + assert_eq!(tracker.find_ghost(Some(("C", 4)), |&x| x >= 10), Some(("E", 6))); + assert_eq!(tracker.find_ghost(Some(("E", 6)), |&x| x >= 10), Some(("E", 6))); + } + #[test] fn walk_back_from_block_in_edge_fork_below() { let mut chain = DummyChain::new(); From 11c469e5b0d767ccea87faa17d4fb45995942332 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 24 Aug 2018 19:25:33 +0200 Subject: [PATCH 11/11] add a finalization test --- src/round.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/round.rs b/src/round.rs index ebe7b6ce..91cc6969 100644 --- a/src/round.rs +++ b/src/round.rs @@ -354,4 +354,42 @@ mod tests { assert_eq!(round.estimate(), Some(&("E", 6))); } + #[test] + fn finalization() { + let mut chain = DummyChain::new(); + chain.push_blocks(GENESIS_HASH, &["A", "B", "C", "D", "E", "F"]); + chain.push_blocks("E", &["EA", "EB", "EC", "ED"]); + chain.push_blocks("F", &["FA", "FB", "FC"]); + + let mut round = Round::new(RoundParams { + round_number: 1, + voters: voters(), + base: ("C", 4), + }); + + round.import_precommit( + &chain, + Precommit::new("FC", 10), + "Alice", + Signature("Alice"), + ).unwrap(); + + round.import_precommit( + &chain, + Precommit::new("ED", 10), + "Bob", + Signature("Bob"), + ).unwrap(); + + assert_eq!(round.finalized, Some(("E", 6))); + + round.import_precommit( + &chain, + Precommit::new("EA", 7), + "Eve", + Signature("Eve"), + ).unwrap(); + + assert_eq!(round.finalized, Some(("EA", 7))); + } }