From 9ada9881493bd199b75aaac1b4d1fa9119ba952d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Fri, 23 Dec 2022 23:19:09 +0100 Subject: [PATCH 01/14] init --- finality-aleph/src/lib.rs | 1 + finality-aleph/src/sync/forest/graph.rs | 126 +++++++++++++++++++ finality-aleph/src/sync/forest/mod.rs | 159 ++++++++++++++++++++++++ finality-aleph/src/sync/mod.rs | 1 + 4 files changed, 287 insertions(+) create mode 100644 finality-aleph/src/sync/forest/graph.rs create mode 100644 finality-aleph/src/sync/forest/mod.rs create mode 100644 finality-aleph/src/sync/mod.rs diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs index 4793919d65..565017cd95 100644 --- a/finality-aleph/src/lib.rs +++ b/finality-aleph/src/lib.rs @@ -44,6 +44,7 @@ mod nodes; mod party; mod session; mod session_map; +mod sync; #[cfg(test)] pub mod testing; diff --git a/finality-aleph/src/sync/forest/graph.rs b/finality-aleph/src/sync/forest/graph.rs new file mode 100644 index 0000000000..f149bf5bf0 --- /dev/null +++ b/finality-aleph/src/sync/forest/graph.rs @@ -0,0 +1,126 @@ +use std::{ + collections::{HashMap, HashSet}, + iter::Iterator, +}; + +pub trait Key: Clone + std::cmp::Eq + std::hash::Hash {} +impl Key for T {} + +struct Vertex { + value: V, + parent: Option, + children: HashSet, +} + +pub enum Error { + KeyAlreadyExists, + MissingKey, + MissingChildKey, + MissingParentKey, + ParentAlreadySet, + CriticalBug, +} + +pub struct Forest { + vertices: HashMap>, + root: K, + root_children: HashSet, +} + +impl Forest { + pub fn new(root: K) -> Self { + Self { + vertices: HashMap::new(), + root, + root_children: HashSet::new(), + } + } + + pub fn contains_key(&self, key: &K) -> bool { + self.vertices.contains_key(key) || &self.root == key + } + + pub fn insert(&mut self, key: K, value: V, parent: Option) -> Result<(), Error> { + if self.contains_key(&key) { + return Err(Error::KeyAlreadyExists); + } + if let Some(parent) = parent.clone() { + if !self.contains_key(&parent) { + return Err(Error::MissingParentKey); + } + if self.root == parent { + self.root_children.insert(key.clone()); + } else { + self.vertices + .get_mut(&parent) + .ok_or(Error::CriticalBug)? + .children + .insert(key.clone()); + } + } + self.vertices.insert( + key, + Vertex { + value, + parent, + children: HashSet::new(), + }, + ); + Ok(()) + } + + pub fn get(&self, key: &K) -> Option<&V> { + self.vertices.get(key).map(|x| &x.value) + } + + pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { + self.vertices.get_mut(key).map(|x| &mut x.value) + } + + pub fn get_root(&self) -> &K { + &self.root + } + + pub fn get_parent_key(&mut self, key: &K) -> Option<&K> { + self.vertices.get(key).map_or(None, |x| x.parent.as_ref()) + } + + pub fn set_parent(&mut self, child: K, parent: K) -> Result<(), Error> { + // child must not be the root + if !self.vertices.contains_key(&child) { + return Err(Error::MissingChildKey); + } + if !self.contains_key(&parent) { + return Err(Error::MissingParentKey); + } + let mut v_child = self.vertices.get_mut(&child).ok_or(Error::CriticalBug)?; + if v_child.parent.is_some() { + return Err(Error::ParentAlreadySet); + } + v_child.parent = Some(parent.clone()); + + let children = if self.root == parent { + &mut self.root_children + } else { + &mut self + .vertices + .get_mut(&parent) + .ok_or(Error::CriticalBug)? + .children + }; + if children.contains(&child) { + return Err(Error::CriticalBug); + } + children.insert(child); + + Ok(()) + } + + pub fn prune(&mut self, key: K) -> Result, Error> { + // cannot prune the root + if !self.vertices.contains_key(&key) { + return Err(Error::MissingKey); + } + Ok(vec![]) + } +} diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs new file mode 100644 index 0000000000..5d13ee73f4 --- /dev/null +++ b/finality-aleph/src/sync/forest/mod.rs @@ -0,0 +1,159 @@ +use std::collections::{HashMap, HashSet}; + +pub mod graph; +use graph::{Error, Forest as Graph}; + +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] +pub struct Hash; +#[derive(Clone, std::cmp::PartialOrd, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] +pub struct Number; +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] +pub struct HashNumber { + hash: Hash, + number: Number, +} +pub struct Header; +#[derive(std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] +pub struct PeerID; + +impl Header { + fn parent(&self) -> HashNumber { + HashNumber { + hash: Hash, + number: Number, + } + } +} + +// struct Body; +pub struct Justification; + +enum ForestJustification { + Justification(Justification), + Empty, +} + +struct Vertex { + header: Option
, + justification: Option, + important: bool, + know_most: HashSet, +} + +pub enum Request { + Header(HashNumber), + Block(HashNumber), + JustificationsBelow(HashNumber), +} + +pub struct Forest { + graph: Graph, + compost_bin: HashSet, +} + +impl Forest { + pub fn new(highest_justified: HashNumber) -> Self { + Self { + graph: Graph::new(highest_justified), + compost_bin: HashSet::new(), + } + } + + fn minimal_number(&self) -> &Number { + &self.graph.get_root().number + } + + fn filter_compost_bin(&mut self) { + let minimal = self.minimal_number().clone(); + self.compost_bin = self + .compost_bin + .drain() + .filter(|x| x.number > minimal) + .collect(); + } + + fn is_relevant(&self, hashnumber: &HashNumber) -> bool { + &hashnumber.number > self.minimal_number() && !self.compost_bin.contains(hashnumber) + } + + fn is_important(&self, hashnumber: &HashNumber) -> Option { + self.graph.get(hashnumber).map(|x| x.important) + } + + // We want to prune a specific vertex whenever we know that it is irrelevant + // for extending the blockchain. When we do this, we remove it from the forest + // and put its hash+number into the compost bin if it is above the highest + // justified block. Then we prune all its descendants. + // This action never returns any requests. + pub fn prune(&mut self, vertex: HashNumber) {} + + pub fn insert_hashnumber( + &mut self, + hashnumber: HashNumber, + sender: Option, + important: bool, + ) -> Result, Error> { + #[derive(std::cmp::PartialEq)] + enum PreviousState { + Nonexistent, + Unimportant, + Important, + } + let previous_state: PreviousState; + if !self.is_relevant(&hashnumber) { + return Ok(None); + } + match self.graph.get_mut(&hashnumber) { + Some(mut vertex) => { + if vertex.justification.is_some() { + return Ok(None); + } + previous_state = if vertex.important { + PreviousState::Important + } else { + PreviousState::Unimportant + }; + if let Some(sender) = sender { + vertex.know_most.insert(sender); + } + if important { + vertex.important = true; + let mut parent = hashnumber.clone(); + loop { + parent = match self.graph.get_parent_key(&parent) { + Some(k) => k.clone(), + None => break, + }; + vertex = match self.graph.get_mut(&parent) { + Some(v) => v, + None => break, + }; + vertex.important = true; + } + } + } + None => { + previous_state = PreviousState::Nonexistent; + let mut know_most = HashSet::new(); + if let Some(sender) = sender { + know_most.insert(sender); + } + let vertex = Vertex { + header: None, + justification: None, + important, + know_most, + }; + self.graph.insert(hashnumber.clone(), vertex, None)?; + } + } + + if important && previous_state != PreviousState::Important { + return Ok(Some(Request::Block(hashnumber))); + } + if !important && previous_state == PreviousState::Nonexistent { + return Ok(Some(Request::Header(hashnumber))); + } + Ok(None) + } +} diff --git a/finality-aleph/src/sync/mod.rs b/finality-aleph/src/sync/mod.rs new file mode 100644 index 0000000000..5daed48291 --- /dev/null +++ b/finality-aleph/src/sync/mod.rs @@ -0,0 +1 @@ +pub mod forest; From d8171217a87676304f5b211700b69ebf772d83c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Tue, 27 Dec 2022 20:18:47 +0100 Subject: [PATCH 02/14] the forest grows --- finality-aleph/src/sync/forest/graph.rs | 64 +++- finality-aleph/src/sync/forest/mod.rs | 384 +++++++++++++++++------- 2 files changed, 332 insertions(+), 116 deletions(-) diff --git a/finality-aleph/src/sync/forest/graph.rs b/finality-aleph/src/sync/forest/graph.rs index f149bf5bf0..f8da976015 100644 --- a/finality-aleph/src/sync/forest/graph.rs +++ b/finality-aleph/src/sync/forest/graph.rs @@ -81,10 +81,41 @@ impl Forest { &self.root } - pub fn get_parent_key(&mut self, key: &K) -> Option<&K> { + pub fn get_parent(&mut self, key: &K) -> Result, Error> { + match self.get_parent_key(&key) { + Some(parent_key) => { + if parent_key == self.get_root() { + return Ok(None); + }; + match self.get_mut(&parent_key.clone()) { + Some(v) => Ok(Some(v)), + None => Err(Error::CriticalBug), + } + } + None => Ok(None), + } + } + + pub fn get_parent_key(&self, key: &K) -> Option<&K> { self.vertices.get(key).map_or(None, |x| x.parent.as_ref()) } + pub fn get_children_keys(&self, key: &K) -> Option<&HashSet> { + if &self.root == key { + Some(&self.root_children) + } else { + self.vertices.get_mut(&key).map(|v| &v.children) + } + } + + pub fn get_mut_children_keys(&mut self, key: &K) -> Option<&mut HashSet> { + if &self.root == key { + Some(&mut self.root_children) + } else { + self.vertices.get_mut(&key).map(|v| &mut v.children) + } + } + pub fn set_parent(&mut self, child: K, parent: K) -> Result<(), Error> { // child must not be the root if !self.vertices.contains_key(&child) { @@ -99,15 +130,9 @@ impl Forest { } v_child.parent = Some(parent.clone()); - let children = if self.root == parent { - &mut self.root_children - } else { - &mut self - .vertices - .get_mut(&parent) - .ok_or(Error::CriticalBug)? - .children - }; + let children = self + .get_mut_children_keys(&parent) + .ok_or(Error::CriticalBug)?; if children.contains(&child) { return Err(Error::CriticalBug); } @@ -116,11 +141,26 @@ impl Forest { Ok(()) } - pub fn prune(&mut self, key: K) -> Result, Error> { + /// TODO + pub fn prune(&mut self, key: K) -> Result, Error> { // cannot prune the root if !self.vertices.contains_key(&key) { return Err(Error::MissingKey); } - Ok(vec![]) + // TODO + Ok(HashSet::new()) + } + + /// TODO + /// check if connected + /// prune branches + /// returns: trunk, pruned + pub fn cut_trunk(&mut self, key: K) -> Result<(Vec<(K, V)>, HashSet), Error> { + // must cut something + if !self.vertices.contains_key(&key) { + return Err(Error::MissingKey); + } + // TODO + Ok((vec![], HashSet::new())) } } diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 5d13ee73f4..0e1c2c1361 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -1,49 +1,82 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; pub mod graph; use graph::{Error, Forest as Graph}; #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] pub struct Hash; -#[derive(Clone, std::cmp::PartialOrd, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] -pub struct Number; + #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] pub struct HashNumber { hash: Hash, - number: Number, + number: u32, } -pub struct Header; -#[derive(std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] + +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] pub struct PeerID; -impl Header { - fn parent(&self) -> HashNumber { - HashNumber { - hash: Hash, - number: Number, +#[derive(Clone)] +pub struct Justification; + +struct Vertex { + body_imported: bool, + justification: Option, + know_most: HashSet, + required: bool, +} + +impl Vertex { + fn new(holder: Option, required: bool) -> Self { + let know_most = match holder { + Some(peer_id) => HashSet::from([peer_id]), + None => HashSet::new(), + }; + Vertex { + body_imported: false, + justification: None, + know_most, + required, } } -} -// struct Body; -pub struct Justification; + fn add_holder(&mut self, holder: Option) { + if let Some(peer_id) = holder { + self.know_most.insert(peer_id); + }; + } +} -enum ForestJustification { - Justification(Justification), +#[derive(std::cmp::PartialEq, std::cmp::Eq)] +enum State { + Unknown, Empty, + EmptyRequired, + Header, + HeaderRequired, + Body, + JustifiedHeader, + Full, + HopelessFork, + BelowMinimal, } -struct Vertex { - header: Option
, - justification: Option, - important: bool, - know_most: HashSet, +pub enum RequestType { + Header, + Body, + JustificationsBelow, } -pub enum Request { - Header(HashNumber), - Block(HashNumber), - JustificationsBelow(HashNumber), +/// TODO: RETHINK +impl From for Option { + fn from(state: State) -> Self { + use State::*; + match state { + Unknown | HopelessFork | BelowMinimal => None, + Empty => Some(RequestType::Header), + EmptyRequired | HeaderRequired | Header => Some(RequestType::Body), + Body | JustifiedHeader | Full => Some(RequestType::JustificationsBelow), + } + } } pub struct Forest { @@ -59,101 +92,244 @@ impl Forest { } } - fn minimal_number(&self) -> &Number { - &self.graph.get_root().number + fn minimal_number(&self) -> u32 { + self.graph.get_root().number } - fn filter_compost_bin(&mut self) { - let minimal = self.minimal_number().clone(); - self.compost_bin = self - .compost_bin - .drain() - .filter(|x| x.number > minimal) - .collect(); - } - - fn is_relevant(&self, hashnumber: &HashNumber) -> bool { - &hashnumber.number > self.minimal_number() && !self.compost_bin.contains(hashnumber) + fn state(&self, hashnumber: &HashNumber) -> State { + // Check if below the current lower bound. + if hashnumber.number <= self.minimal_number() { + return State::BelowMinimal; + } + // Check if it's a hopeless fork. + if self.compost_bin.contains(hashnumber) { + return State::HopelessFork; + } + // Check if we know it. + let vertex = match self.graph.get(hashnumber) { + Some(v) => v, + None => return State::Unknown, + }; + // Check if we don't know the parent, thus we haven't received the header. + if self.graph.get_parent_key(hashnumber).is_none() { + return match vertex.required { + true => State::EmptyRequired, + false => State::Empty, + }; + }; + // Check the content: body and justification. + match (&vertex.justification, vertex.body_imported) { + (Some(_), true) => State::Full, + (Some(_), false) => State::JustifiedHeader, + (None, true) => State::Body, + (None, false) => match vertex.required { + true => State::HeaderRequired, + false => State::Header, + }, + } } - fn is_important(&self, hashnumber: &HashNumber) -> Option { - self.graph.get(hashnumber).map(|x| x.important) + /// Bumps flag `required` of the vertex and all its ancestors. + fn bump_required(&mut self, hashnumber: &HashNumber) -> Result, Error> { + let mut modified = HashSet::new(); + let mut hashnumber = hashnumber.clone(); + let mut old_state = self.state(&hashnumber); + let mut vertex = match self.graph.get_mut(&hashnumber) { + Some(v) => v, + None => return Err(Error::MissingKey), + }; + loop { + if vertex.required { + break; + }; + vertex.required = true; + if old_state != self.state(&hashnumber) { + modified.insert(hashnumber.clone()); + }; + hashnumber = match self.graph.get_parent_key(&hashnumber) { + Some(k) => { + if k == self.graph.get_root() { + break; + }; + k.clone() + } + None => break, + }; + old_state = self.state(&hashnumber); + vertex = match self.graph.get_mut(&hashnumber) { + Some(v) => v, + None => return Err(Error::CriticalBug), + }; + } + Ok(modified) } - // We want to prune a specific vertex whenever we know that it is irrelevant - // for extending the blockchain. When we do this, we remove it from the forest - // and put its hash+number into the compost bin if it is above the highest - // justified block. Then we prune all its descendants. - // This action never returns any requests. - pub fn prune(&mut self, vertex: HashNumber) {} - - pub fn insert_hashnumber( + pub fn update_hashnumber( &mut self, hashnumber: HashNumber, - sender: Option, - important: bool, - ) -> Result, Error> { - #[derive(std::cmp::PartialEq)] - enum PreviousState { - Nonexistent, - Unimportant, - Important, - } - let previous_state: PreviousState; - if !self.is_relevant(&hashnumber) { - return Ok(None); - } - match self.graph.get_mut(&hashnumber) { - Some(mut vertex) => { - if vertex.justification.is_some() { - return Ok(None); - } - previous_state = if vertex.important { - PreviousState::Important - } else { - PreviousState::Unimportant + holder: Option, + bump_required: bool, + ) -> Result, Error> { + use State::*; + match self.state(&hashnumber) { + // skip if the vertex is irrelevant, or we have a justification, + // thus the information about the holder is unrelated, and the vertex + // is required "by default" + HopelessFork | BelowMinimal | JustifiedHeader | Full => Ok(HashSet::new()), + // create the vertex if unknown to us + Unknown => { + self.graph + .insert(hashnumber.clone(), Vertex::new(holder, bump_required), None)?; + Ok(HashSet::from([hashnumber])) + } + // update the vertex content + Empty | EmptyRequired | Header | HeaderRequired | Body => { + // add holder + if let Some(peer_id) = holder { + match self.graph.get_mut(&hashnumber) { + Some(vertex) => vertex.know_most.insert(peer_id), + // we know the vertex + None => return Err(Error::CriticalBug), + }; }; - if let Some(sender) = sender { - vertex.know_most.insert(sender); + // bump required - all ancestors + match bump_required { + true => self.bump_required(&hashnumber), + false => Ok(HashSet::new()), } - if important { - vertex.important = true; - let mut parent = hashnumber.clone(); - loop { - parent = match self.graph.get_parent_key(&parent) { - Some(k) => k.clone(), - None => break, - }; - vertex = match self.graph.get_mut(&parent) { - Some(v) => v, - None => break, - }; - vertex.important = true; + } + } + } + + pub fn update_header( + &mut self, + hashnumber: HashNumber, + parent_hashnumber: HashNumber, + holder: Option, + bump_required: bool, + ) -> Result, Error> { + use State::*; + let mut modified = + self.update_hashnumber(hashnumber.clone(), holder.clone(), bump_required)?; + modified.extend(match self.state(&hashnumber) { + // skip if the vertex is irrelevant, or we have a justification, + // thus the information about the holder is unrelated, and the vertex + // is required "by default" + HopelessFork | BelowMinimal | JustifiedHeader | Full => HashSet::new(), + // we've just updated the hashnumber + Unknown => return Err(Error::CriticalBug), + // we already have the header + Header | HeaderRequired | Body => { + self.graph + .get_mut(&hashnumber) + .ok_or(Error::CriticalBug)? + .add_holder(holder); + HashSet::new() + } + // this is the first time we got the header, thus the parent is not set + Empty | EmptyRequired => { + let mut modified = self.update_hashnumber( + parent_hashnumber.clone(), + holder.clone(), + bump_required, + )?; + // modify hashnumber vertex - add parent (we've already called `update_hashnumber`, + // therefore we don't need to use `holder` and `bump_required` here + self.graph + .set_parent(hashnumber.clone(), parent_hashnumber.clone())?; + modified.insert(hashnumber.clone()); + match self.state(&parent_hashnumber) { + Unknown => return Err(Error::CriticalBug), + HopelessFork | BelowMinimal => { + self.compost_bin.extend(self.graph.prune(hashnumber)?); } - } + Empty | EmptyRequired | Header | HeaderRequired | Body | JustifiedHeader + | Full => (), + }; + modified } - None => { - previous_state = PreviousState::Nonexistent; - let mut know_most = HashSet::new(); - if let Some(sender) = sender { - know_most.insert(sender); + }); + Ok(modified) + } + + pub fn update_header_and_justification( + &mut self, + hashnumber: HashNumber, + parent_hashnumber: HashNumber, + justification: Justification, + holder: Option, + ) -> Result, Error> { + use State::*; + let mut modified = + self.update_header(hashnumber.clone(), parent_hashnumber, holder.clone(), false)?; + modified.extend(match self.state(&hashnumber) { + // skip if the vertex is irrelevant + BelowMinimal => HashSet::new(), + // we've just updated the hashnumber, added header, and justified vertex cannot be a HopelessFork + Unknown | Empty | EmptyRequired | HopelessFork => return Err(Error::CriticalBug), + // we already have the justification + JustifiedHeader | Full => { + self.graph + .get_mut(&hashnumber) + .ok_or(Error::CriticalBug)? + .add_holder(holder); + HashSet::new() + } + // this is the first time we got the justification + Header | HeaderRequired | Body => { + let vertex = self.graph.get_mut(&hashnumber).ok_or(Error::CriticalBug)?; + vertex.know_most = HashSet::new(); + vertex.add_holder(holder); + vertex.justification = Some(justification); + HashSet::from([hashnumber]) + } + }); + Ok(modified) + } + + fn find_trunk_top(&self) -> Result { + let mut top = self.graph.get_root().clone(); + 'outer: loop { + for child in self + .graph + .get_children_keys(&top) + .ok_or(Error::CriticalBug)? + .iter() + { + if self.state(child) == State::Full { + top = child.clone(); + continue 'outer; } - let vertex = Vertex { - header: None, - justification: None, - important, - know_most, - }; - self.graph.insert(hashnumber.clone(), vertex, None)?; } + break; } + Ok(top) + } - if important && previous_state != PreviousState::Important { - return Ok(Some(Request::Block(hashnumber))); - } - if !important && previous_state == PreviousState::Nonexistent { - return Ok(Some(Request::Header(hashnumber))); + pub fn finalize(&mut self) -> Result>, Error> { + let top = self.find_trunk_top()?; + if &top == self.graph.get_root() { + return Ok(None); } - Ok(None) + let (trunk, pruned) = self.graph.cut_trunk(top)?; + self.compost_bin.extend(pruned); + let minimal_number = self.minimal_number(); + self.compost_bin = self + .compost_bin + .drain() + .filter(|x| x.number > minimal_number) + .collect(); + Ok(Some( + trunk + .into_iter() + .map( + |(hashnumber, vertex)| -> Result<(HashNumber, Justification), Error> { + Ok((hashnumber, vertex.justification.ok_or(Error::CriticalBug)?)) + }, + ) + .collect::, Error>>()?, + )) } + + // pub fn state_summary ... } From 27a342dce5d41879ef1f40208a3d0097b4b87804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Mon, 2 Jan 2023 17:33:11 +0100 Subject: [PATCH 03/14] Vertex --- finality-aleph/src/sync/forest/mod.rs | 99 ++++++++-------- finality-aleph/src/sync/forest/vertex.rs | 140 +++++++++++++++++++++++ 2 files changed, 190 insertions(+), 49 deletions(-) create mode 100644 finality-aleph/src/sync/forest/vertex.rs diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 0e1c2c1361..2a7796335f 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -1,7 +1,5 @@ use std::collections::HashSet; - -pub mod graph; -use graph::{Error, Forest as Graph}; +use log::error; #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] pub struct Hash; @@ -12,52 +10,39 @@ pub struct HashNumber { number: u32, } -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] -pub struct PeerID; - -#[derive(Clone)] -pub struct Justification; +pub struct Header; -struct Vertex { - body_imported: bool, - justification: Option, - know_most: HashSet, - required: bool, +impl Header { + pub fn parent_hashnumber(&self) -> HashNumber { + HashNumber{ hash: Hash, number: 0 } + } } -impl Vertex { - fn new(holder: Option, required: bool) -> Self { - let know_most = match holder { - Some(peer_id) => HashSet::from([peer_id]), - None => HashSet::new(), - }; - Vertex { - body_imported: false, - justification: None, - know_most, - required, - } - } +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +pub struct Justification; - fn add_holder(&mut self, holder: Option) { - if let Some(peer_id) = holder { - self.know_most.insert(peer_id); - }; +impl Justification { + pub fn header(&self) -> Header { + Header } } -#[derive(std::cmp::PartialEq, std::cmp::Eq)] -enum State { - Unknown, - Empty, - EmptyRequired, - Header, - HeaderRequired, - Body, - JustifiedHeader, - Full, - HopelessFork, - BelowMinimal, +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] +pub struct PeerID; + +// TODO - merge graph with forest +pub mod graph; +pub mod vertex; + +use graph::{Error, Forest as Graph}; +use vertex::{Vertex, Error as VertexError, Importance, Content}; + +pub enum VertexState { + Unknown, + HopelessFork, + BelowMinimal, + HighestFinalized, + Candidate(Content, Importance), } pub enum RequestType { @@ -67,14 +52,30 @@ pub enum RequestType { } /// TODO: RETHINK -impl From for Option { - fn from(state: State) -> Self { - use State::*; +impl From for Option { + fn from(state: VertexState) -> Self { + use VertexState::*; + use Content::*; + use Importance::*; + use RequestType::{Header as RHeader, Body, JustificationsBelow}; match state { - Unknown | HopelessFork | BelowMinimal => None, - Empty => Some(RequestType::Header), - EmptyRequired | HeaderRequired | Header => Some(RequestType::Body), - Body | JustifiedHeader | Full => Some(RequestType::JustificationsBelow), + Unknown | HopelessFork | BelowMinimal | HighestFinalized => None, + Candidate(Empty, Auxiliary) => Some(RHeader), + Candidate(Empty, TopRequired) => Some(Body), + Candidate(Empty, Required) => Some(Body), + Candidate(Empty, Imported) => { + error!(target: "aleph-sync", "Forbidden state combination: (Empty, Imported), interpreting as (Header, Imported)"); + Some(JustificationsBelow) + }, + Candidate(Header, Auxiliary) => None, + Candidate(Header, TopRequired) => Some(Body), + Candidate(Header, Required) => Some(Body), + Candidate(Header, Imported) => Some(JustificationsBelow), + Candidate(Justification, Auxiliary) => { + error!(target: "aleph-sync", "Forbidden state combination: (Justification, Auxiliary), interpreting as (Justification, _)"); + Some(JustificationsBelow) + }, + Candidate(Justification, _) => Some(JustificationsBelow), } } } diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs new file mode 100644 index 0000000000..04c0bb5b86 --- /dev/null +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -0,0 +1,140 @@ +use std::collections::HashSet; + +use super::{Justification, PeerID, HashNumber, Header}; + +pub enum Error { + ContentCorrupted, + InvalidTransition, + InvalidHeader, + InvalidJustification, +} + +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +pub enum Importance { + Auxiliary, + TopRequired, + Required, + Imported, +} + +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +pub enum Content { + Empty, + Header, + Justification, +} + +pub struct Vertex { + know_most: HashSet, + importance: Importance, + parent: Option, + justification: Option, +} + +impl Vertex { + pub fn new(holder: Option) -> Self { + let know_most = match holder { + Some(peer_id) => HashSet::from([peer_id]), + None => HashSet::new(), + }; + Vertex { + know_most, + importance: Importance::Auxiliary, + parent: None, + justification: None, + } + } + + fn content(&self) -> Result { + match (self.parent, self.justification) { + (Some(_), Some(_)) => Ok(Content::Justification), + (Some(_), None) => Ok(Content::Header), + (None, Some(_)) => Err(Error::ContentCorrupted), + (None, None) => Ok(Content::Empty), + } + } + + pub fn state(&self) -> Result<(Content, Importance), Error> { + Ok((self.content()?, self.importance.clone())) + } + + pub fn know_most(&self) -> HashSet { + self.know_most + } + + pub fn parent(&self) -> Option { + self.parent + } + + pub fn justification(self) -> Option { + self.justification + } + + pub fn add_holder(&mut self, holder: Option) { + if let Some(peer_id) = holder { + self.know_most.insert(peer_id); + }; + } + + // STATE CHANGING METHODS + + pub fn bump_required(&mut self, is_top: bool) -> Result { + use Importance::*; + match (self.importance, is_top) { + (Auxiliary, true) => { + self.importance = Importance::TopRequired; + Ok(true) + }, + (Auxiliary, false) => { + self.importance = Importance::Required; + Ok(true) + }, + (TopRequired, false) => { + self.importance = Importance::Required; + Ok(true) + }, + (Required, true) => Err(Error::InvalidTransition), + _ => Ok(false), + } + } + + pub fn insert_header(&mut self, header: Header) -> Result { + match self.parent { + Some(hashnumber) => { + if hashnumber != header.parent_hashnumber() { + return Err(Error::InvalidHeader); + }; + Ok(false) + }, + None => { + self.parent = Some(header.parent_hashnumber()); + Ok(true) + }, + } + } + + pub fn insert_body(&mut self, header: Header) -> Result { + Ok(self.insert_header(header)? || match self.importance { + Importance::Imported => false, + _ => { + self.importance = Importance::Imported; + true + }, + }) + } + + pub fn insert_justification(&mut self, justification: Justification) -> Result { + Ok(self.insert_header(justification.header())? || match self.justification { + Some(current_justification) => { + if justification != current_justification { + return Err(Error::InvalidJustification); + }; + false + }, + None => { + self.justification = Some(justification); + true + }, + }) + } +} From d602c911314be60b9f9b125ea7b80e7732e35289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Tue, 3 Jan 2023 20:59:01 +0100 Subject: [PATCH 04/14] first version --- finality-aleph/src/sync/forest/graph.rs | 166 ------- finality-aleph/src/sync/forest/mod.rs | 598 +++++++++++++---------- finality-aleph/src/sync/forest/vertex.rs | 219 ++++++--- finality-aleph/src/sync/mod.rs | 3 + 4 files changed, 472 insertions(+), 514 deletions(-) diff --git a/finality-aleph/src/sync/forest/graph.rs b/finality-aleph/src/sync/forest/graph.rs index f8da976015..e69de29bb2 100644 --- a/finality-aleph/src/sync/forest/graph.rs +++ b/finality-aleph/src/sync/forest/graph.rs @@ -1,166 +0,0 @@ -use std::{ - collections::{HashMap, HashSet}, - iter::Iterator, -}; - -pub trait Key: Clone + std::cmp::Eq + std::hash::Hash {} -impl Key for T {} - -struct Vertex { - value: V, - parent: Option, - children: HashSet, -} - -pub enum Error { - KeyAlreadyExists, - MissingKey, - MissingChildKey, - MissingParentKey, - ParentAlreadySet, - CriticalBug, -} - -pub struct Forest { - vertices: HashMap>, - root: K, - root_children: HashSet, -} - -impl Forest { - pub fn new(root: K) -> Self { - Self { - vertices: HashMap::new(), - root, - root_children: HashSet::new(), - } - } - - pub fn contains_key(&self, key: &K) -> bool { - self.vertices.contains_key(key) || &self.root == key - } - - pub fn insert(&mut self, key: K, value: V, parent: Option) -> Result<(), Error> { - if self.contains_key(&key) { - return Err(Error::KeyAlreadyExists); - } - if let Some(parent) = parent.clone() { - if !self.contains_key(&parent) { - return Err(Error::MissingParentKey); - } - if self.root == parent { - self.root_children.insert(key.clone()); - } else { - self.vertices - .get_mut(&parent) - .ok_or(Error::CriticalBug)? - .children - .insert(key.clone()); - } - } - self.vertices.insert( - key, - Vertex { - value, - parent, - children: HashSet::new(), - }, - ); - Ok(()) - } - - pub fn get(&self, key: &K) -> Option<&V> { - self.vertices.get(key).map(|x| &x.value) - } - - pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { - self.vertices.get_mut(key).map(|x| &mut x.value) - } - - pub fn get_root(&self) -> &K { - &self.root - } - - pub fn get_parent(&mut self, key: &K) -> Result, Error> { - match self.get_parent_key(&key) { - Some(parent_key) => { - if parent_key == self.get_root() { - return Ok(None); - }; - match self.get_mut(&parent_key.clone()) { - Some(v) => Ok(Some(v)), - None => Err(Error::CriticalBug), - } - } - None => Ok(None), - } - } - - pub fn get_parent_key(&self, key: &K) -> Option<&K> { - self.vertices.get(key).map_or(None, |x| x.parent.as_ref()) - } - - pub fn get_children_keys(&self, key: &K) -> Option<&HashSet> { - if &self.root == key { - Some(&self.root_children) - } else { - self.vertices.get_mut(&key).map(|v| &v.children) - } - } - - pub fn get_mut_children_keys(&mut self, key: &K) -> Option<&mut HashSet> { - if &self.root == key { - Some(&mut self.root_children) - } else { - self.vertices.get_mut(&key).map(|v| &mut v.children) - } - } - - pub fn set_parent(&mut self, child: K, parent: K) -> Result<(), Error> { - // child must not be the root - if !self.vertices.contains_key(&child) { - return Err(Error::MissingChildKey); - } - if !self.contains_key(&parent) { - return Err(Error::MissingParentKey); - } - let mut v_child = self.vertices.get_mut(&child).ok_or(Error::CriticalBug)?; - if v_child.parent.is_some() { - return Err(Error::ParentAlreadySet); - } - v_child.parent = Some(parent.clone()); - - let children = self - .get_mut_children_keys(&parent) - .ok_or(Error::CriticalBug)?; - if children.contains(&child) { - return Err(Error::CriticalBug); - } - children.insert(child); - - Ok(()) - } - - /// TODO - pub fn prune(&mut self, key: K) -> Result, Error> { - // cannot prune the root - if !self.vertices.contains_key(&key) { - return Err(Error::MissingKey); - } - // TODO - Ok(HashSet::new()) - } - - /// TODO - /// check if connected - /// prune branches - /// returns: trunk, pruned - pub fn cut_trunk(&mut self, key: K) -> Result<(Vec<(K, V)>, HashSet), Error> { - // must cut something - if !self.vertices.contains_key(&key) { - return Err(Error::MissingKey); - } - // TODO - Ok((vec![], HashSet::new())) - } -} diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 2a7796335f..1362995abe 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -1,48 +1,19 @@ -use std::collections::HashSet; -use log::error; +use std::collections::{HashMap, HashSet}; -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] -pub struct Hash; +use vertex::{Error as VertexError, Importance, TransitionSummary, Vertex}; -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] -pub struct HashNumber { - hash: Hash, - number: u32, -} - -pub struct Header; - -impl Header { - pub fn parent_hashnumber(&self) -> HashNumber { - HashNumber{ hash: Hash, number: 0 } - } -} - -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -pub struct Justification; +use super::{BlockIdentifier, Header, Justification, PeerID}; -impl Justification { - pub fn header(&self) -> Header { - Header - } -} - -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq, std::hash::Hash)] -pub struct PeerID; +mod vertex; -// TODO - merge graph with forest -pub mod graph; -pub mod vertex; +type BlockIdFor = <::Header as Header>::Identifier; -use graph::{Error, Forest as Graph}; -use vertex::{Vertex, Error as VertexError, Importance, Content}; - -pub enum VertexState { - Unknown, - HopelessFork, - BelowMinimal, - HighestFinalized, - Candidate(Content, Importance), +pub enum VertexState<'a, I: PeerID, J: Justification> { + Unknown, + HopelessFork, + BelowMinimal, + HighestFinalized, + Candidate(&'a mut Vertex), } pub enum RequestType { @@ -51,286 +22,371 @@ pub enum RequestType { JustificationsBelow, } -/// TODO: RETHINK -impl From for Option { - fn from(state: VertexState) -> Self { - use VertexState::*; - use Content::*; - use Importance::*; - use RequestType::{Header as RHeader, Body, JustificationsBelow}; - match state { - Unknown | HopelessFork | BelowMinimal | HighestFinalized => None, - Candidate(Empty, Auxiliary) => Some(RHeader), - Candidate(Empty, TopRequired) => Some(Body), - Candidate(Empty, Required) => Some(Body), - Candidate(Empty, Imported) => { - error!(target: "aleph-sync", "Forbidden state combination: (Empty, Imported), interpreting as (Header, Imported)"); - Some(JustificationsBelow) - }, - Candidate(Header, Auxiliary) => None, - Candidate(Header, TopRequired) => Some(Body), - Candidate(Header, Required) => Some(Body), - Candidate(Header, Imported) => Some(JustificationsBelow), - Candidate(Justification, Auxiliary) => { - error!(target: "aleph-sync", "Forbidden state combination: (Justification, Auxiliary), interpreting as (Justification, _)"); - Some(JustificationsBelow) - }, - Candidate(Justification, _) => Some(JustificationsBelow), - } +// /// TODO: RETHINK +// impl From for Option { +// fn from(state: VertexState) -> Self { +// use VertexState::*; +// use Content::*; +// use Importance::*; +// use RequestType::{Header as RHeader, Body, JustificationsBelow}; +// match state { +// Unknown | HopelessFork | BelowMinimal | HighestFinalized => None, +// Candidate(Empty, Auxiliary) => Some(RHeader), +// Candidate(Empty, TopRequired) => Some(Body), +// Candidate(Empty, Required) => Some(Body), +// Candidate(Empty, Imported) => { +// error!(target: "aleph-sync", "Forbidden state combination: (Empty, Imported), interpreting as (Header, Imported)"); +// Some(JustificationsBelow) +// }, +// Candidate(Header, Auxiliary) => None, +// Candidate(Header, TopRequired) => Some(Body), +// Candidate(Header, Required) => Some(Body), +// Candidate(Header, Imported) => Some(JustificationsBelow), +// Candidate(Justification, Auxiliary) => { +// error!(target: "aleph-sync", "Forbidden state combination: (Justification, Auxiliary), interpreting as (Justification, _)"); +// Some(JustificationsBelow) +// }, +// Candidate(Justification, _) => Some(JustificationsBelow), +// } +// } +// } + +pub enum Error { + Vertex(VertexError), + MissingParent, + MissingVertex, + MissingChildrenHashSet, + MissingJustification, + CriticalBug, + JustificationPruned, + HeaderMissingParentID, +} + +impl From for Error { + fn from(err: VertexError) -> Self { + Self::Vertex(err) } } -pub struct Forest { - graph: Graph, - compost_bin: HashSet, +pub struct Forest { + vertices: HashMap, Vertex>, + children: HashMap, HashSet>>, + root_id: BlockIdFor, + compost_bin: HashSet>, } -impl Forest { - pub fn new(highest_justified: HashNumber) -> Self { +impl Forest { + pub fn new(highest_justified: BlockIdFor) -> Self { Self { - graph: Graph::new(highest_justified), + vertices: HashMap::new(), + children: HashMap::from([(highest_justified.clone(), HashSet::new())]), + root_id: highest_justified, compost_bin: HashSet::new(), } } fn minimal_number(&self) -> u32 { - self.graph.get_root().number + self.root_id.number() } - fn state(&self, hashnumber: &HashNumber) -> State { - // Check if below the current lower bound. - if hashnumber.number <= self.minimal_number() { - return State::BelowMinimal; - } - // Check if it's a hopeless fork. - if self.compost_bin.contains(hashnumber) { - return State::HopelessFork; - } - // Check if we know it. - let vertex = match self.graph.get(hashnumber) { - Some(v) => v, - None => return State::Unknown, - }; - // Check if we don't know the parent, thus we haven't received the header. - if self.graph.get_parent_key(hashnumber).is_none() { - return match vertex.required { - true => State::EmptyRequired, - false => State::Empty, - }; - }; - // Check the content: body and justification. - match (&vertex.justification, vertex.body_imported) { - (Some(_), true) => State::Full, - (Some(_), false) => State::JustifiedHeader, - (None, true) => State::Body, - (None, false) => match vertex.required { - true => State::HeaderRequired, - false => State::Header, - }, + fn get_mut(&mut self, id: &BlockIdFor) -> Result, Error> { + use VertexState::*; + if id == &self.root_id { + Ok(HighestFinalized) + } else if id.number() <= self.minimal_number() { + Ok(BelowMinimal) + } else if self.compost_bin.contains(id) { + Ok(HopelessFork) + } else { + match self.vertices.get_mut(id) { + Some(vertex) => Ok(Candidate(vertex)), + None => Ok(Unknown), + } } } - /// Bumps flag `required` of the vertex and all its ancestors. - fn bump_required(&mut self, hashnumber: &HashNumber) -> Result, Error> { + fn add_holder(&mut self, id: BlockIdFor, holder: Option) -> Result { + Ok(match self.get_mut(&id)? { + VertexState::Candidate(vertex) => vertex.add_holder(holder), + _ => false, + }) + } + + fn add_justification_holder( + &mut self, + id: BlockIdFor, + holder: Option, + ) -> Result { + Ok(match self.get_mut(&id)? { + VertexState::Candidate(vertex) => vertex.add_justification_holder(holder), + _ => false, + }) + } + + fn insert_vertex(&mut self, id: BlockIdFor) -> Result, Error> { + use VertexState::*; + Ok(match self.get_mut(&id)? { + Unknown => { + let (vertex, summary) = Vertex::new(); + if self.vertices.insert(id.clone(), vertex).is_some() + || self.children.insert(id, HashSet::new()).is_some() + { + return Err(Error::CriticalBug); + } + Some(summary) + } + _ => None, + }) + } + + fn remove_vertex(&mut self, id: &BlockIdFor) -> Result, Error> { + self.children + .remove(id) + .ok_or(Error::MissingChildrenHashSet)?; + Ok(self + .vertices + .remove(id) + .ok_or(Error::MissingVertex)? + .justification()) + } + + fn try_bump_required_recursive( + &mut self, + id: &BlockIdFor, + ) -> Result>, Error> { + use VertexState::{Candidate, HighestFinalized}; let mut modified = HashSet::new(); - let mut hashnumber = hashnumber.clone(); - let mut old_state = self.state(&hashnumber); - let mut vertex = match self.graph.get_mut(&hashnumber) { - Some(v) => v, - None => return Err(Error::MissingKey), - }; - loop { - if vertex.required { - break; - }; - vertex.required = true; - if old_state != self.state(&hashnumber) { - modified.insert(hashnumber.clone()); - }; - hashnumber = match self.graph.get_parent_key(&hashnumber) { - Some(k) => { - if k == self.graph.get_root() { - break; - }; - k.clone() + let mut guard = id.number() as i64 - self.minimal_number() as i64; + if let Candidate(mut vertex) = self.get_mut(id)? { + // if condition is false, then it's already required + // we proceed nevertheless, because we might've just set the parent + if vertex.try_set_top_required()?.is_some() { + modified.insert(id.clone()); + } + let mut id: BlockIdFor; + loop { + // check if has parent + id = match vertex.parent() { + Some(id) => id.clone(), + None => break, + }; + // check if we reached the root + vertex = match self.get_mut(&id)? { + Candidate(vertex) => vertex, + HighestFinalized => break, + _ => return Err(Error::CriticalBug), + }; + // check if already required + match vertex.try_set_required()? { + Some(_) => modified.insert(id.clone()), + None => break, + }; + // avoid infinite loop + guard -= 1; + if guard < 0 { + return Err(Error::CriticalBug); } - None => break, - }; - old_state = self.state(&hashnumber); - vertex = match self.graph.get_mut(&hashnumber) { - Some(v) => v, - None => return Err(Error::CriticalBug), - }; + } } Ok(modified) } - pub fn update_hashnumber( - &mut self, - hashnumber: HashNumber, - holder: Option, - bump_required: bool, - ) -> Result, Error> { - use State::*; - match self.state(&hashnumber) { - // skip if the vertex is irrelevant, or we have a justification, - // thus the information about the holder is unrelated, and the vertex - // is required "by default" - HopelessFork | BelowMinimal | JustifiedHeader | Full => Ok(HashSet::new()), - // create the vertex if unknown to us - Unknown => { - self.graph - .insert(hashnumber.clone(), Vertex::new(holder, bump_required), None)?; - Ok(HashSet::from([hashnumber])) + fn descendants(&self, id: &BlockIdFor) -> Result>, Error> { + let mut result = HashSet::new(); + let mut current = HashSet::from([id.clone()]); + while !current.is_empty() { + let mut next_current = HashSet::new(); + for current_id in current.into_iter() { + next_current.extend( + self.children + .get(¤t_id) + .ok_or(Error::MissingChildrenHashSet)? + .clone(), + ); + } + current = next_current; + result.extend(current.clone()); + } + Ok(result) + } + + fn prune(&mut self, id: &BlockIdFor) -> Result>, Error> { + let mut to_be_pruned = self.descendants(id)?; + to_be_pruned.insert(id.clone()); + for id in to_be_pruned.iter() { + if self.remove_vertex(id)?.is_some() { + return Err(Error::JustificationPruned); } - // update the vertex content - Empty | EmptyRequired | Header | HeaderRequired | Body => { - // add holder - if let Some(peer_id) = holder { - match self.graph.get_mut(&hashnumber) { - Some(vertex) => vertex.know_most.insert(peer_id), - // we know the vertex - None => return Err(Error::CriticalBug), + } + Ok(to_be_pruned) + } + + fn process_transition( + &mut self, + id: BlockIdFor, + summary: Option, + modified: &mut HashSet>, + ) -> Result<(), Error> { + use Importance::*; + use VertexState::*; + if let Some(summary) = summary { + modified.insert(id.clone()); + if summary.gained_parent { + if let Candidate(vertex) = self.get_mut(&id)? { + let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; + match self.get_mut(&parent_id)? { + Unknown => return Err(Error::MissingParent), + HighestFinalized | Candidate(_) => self + .children + .get_mut(&parent_id) + .ok_or(Error::MissingChildrenHashSet)? + .insert(id.clone()), + HopelessFork | BelowMinimal => { + modified.extend(self.prune(&id)?); + return Ok(()); + } }; }; - // bump required - all ancestors - match bump_required { - true => self.bump_required(&hashnumber), - false => Ok(HashSet::new()), + if let Candidate(vertex) = self.get_mut(&id)? { + let (_, importance) = vertex.state()?; + match importance { + Required | TopRequired => { + modified.extend(self.try_bump_required_recursive(&id)?) + } + Auxiliary | Imported => (), + } } } } + Ok(()) + } + + pub fn set_required(&mut self, id: &BlockIdFor) -> Result>, Error> { + self.try_bump_required_recursive(id) + } + + pub fn update_block_identifier( + &mut self, + id: BlockIdFor, + holder: Option, + ) -> Result>, Error> { + let mut modified = HashSet::new(); + let summary = self.insert_vertex(id.clone())?; + self.process_transition(id.clone(), summary, &mut modified)?; + self.add_holder(id, holder)?; + Ok(modified) } pub fn update_header( &mut self, - hashnumber: HashNumber, - parent_hashnumber: HashNumber, - holder: Option, - bump_required: bool, - ) -> Result, Error> { - use State::*; - let mut modified = - self.update_hashnumber(hashnumber.clone(), holder.clone(), bump_required)?; - modified.extend(match self.state(&hashnumber) { - // skip if the vertex is irrelevant, or we have a justification, - // thus the information about the holder is unrelated, and the vertex - // is required "by default" - HopelessFork | BelowMinimal | JustifiedHeader | Full => HashSet::new(), - // we've just updated the hashnumber - Unknown => return Err(Error::CriticalBug), - // we already have the header - Header | HeaderRequired | Body => { - self.graph - .get_mut(&hashnumber) - .ok_or(Error::CriticalBug)? - .add_holder(holder); - HashSet::new() - } - // this is the first time we got the header, thus the parent is not set - Empty | EmptyRequired => { - let mut modified = self.update_hashnumber( - parent_hashnumber.clone(), - holder.clone(), - bump_required, - )?; - // modify hashnumber vertex - add parent (we've already called `update_hashnumber`, - // therefore we don't need to use `holder` and `bump_required` here - self.graph - .set_parent(hashnumber.clone(), parent_hashnumber.clone())?; - modified.insert(hashnumber.clone()); - match self.state(&parent_hashnumber) { - Unknown => return Err(Error::CriticalBug), - HopelessFork | BelowMinimal => { - self.compost_bin.extend(self.graph.prune(hashnumber)?); - } - Empty | EmptyRequired | Header | HeaderRequired | Body | JustifiedHeader - | Full => (), - }; - modified - } - }); + header: &J::Header, + holder: Option, + ) -> Result>, Error> { + use VertexState::Candidate; + let id = header.id(); + let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; + let mut modified = self.update_block_identifier(parent_id, holder.clone())?; + modified.extend(self.update_block_identifier(id.clone(), holder)?); + if let Candidate(vertex) = self.get_mut(&id)? { + let summary = vertex.try_insert_header(header)?; + self.process_transition(id, summary, &mut modified)?; + } Ok(modified) } - pub fn update_header_and_justification( + pub fn update_body( &mut self, - hashnumber: HashNumber, - parent_hashnumber: HashNumber, - justification: Justification, - holder: Option, - ) -> Result, Error> { - use State::*; - let mut modified = - self.update_header(hashnumber.clone(), parent_hashnumber, holder.clone(), false)?; - modified.extend(match self.state(&hashnumber) { - // skip if the vertex is irrelevant - BelowMinimal => HashSet::new(), - // we've just updated the hashnumber, added header, and justified vertex cannot be a HopelessFork - Unknown | Empty | EmptyRequired | HopelessFork => return Err(Error::CriticalBug), - // we already have the justification - JustifiedHeader | Full => { - self.graph - .get_mut(&hashnumber) - .ok_or(Error::CriticalBug)? - .add_holder(holder); - HashSet::new() - } - // this is the first time we got the justification - Header | HeaderRequired | Body => { - let vertex = self.graph.get_mut(&hashnumber).ok_or(Error::CriticalBug)?; - vertex.know_most = HashSet::new(); - vertex.add_holder(holder); - vertex.justification = Some(justification); - HashSet::from([hashnumber]) - } - }); + header: &J::Header, + holder: Option, + ) -> Result>, Error> { + use VertexState::Candidate; + let id = header.id(); + let mut modified = self.update_header(header, holder)?; + if let Candidate(vertex) = self.get_mut(&id)? { + let summary = vertex.try_insert_body(header)?; + self.process_transition(id, summary, &mut modified)?; + } + Ok(modified) + } + + pub fn update_justification( + &mut self, + justification: J, + holder: Option, + ) -> Result>, Error> { + use VertexState::Candidate; + let header = justification.header(); + let id = header.id(); + let mut modified = self.update_header(header, holder.clone())?; + if let Candidate(vertex) = self.get_mut(&id)? { + let summary = vertex.try_insert_justification(justification)?; + vertex.add_justification_holder(holder); + self.process_transition(id, summary, &mut modified)?; + } Ok(modified) } - fn find_trunk_top(&self) -> Result { - let mut top = self.graph.get_root().clone(); - 'outer: loop { - for child in self - .graph - .get_children_keys(&top) - .ok_or(Error::CriticalBug)? - .iter() - { - if self.state(child) == State::Full { - top = child.clone(); - continue 'outer; + fn find_full_child(&mut self, id: &BlockIdFor) -> Result>, Error> { + let children = self.children.get(id).ok_or(Error::MissingChildrenHashSet)?; + for child_id in children.clone().iter() { + if let VertexState::Candidate(vertex) = self.get_mut(child_id)? { + if vertex.is_full()? { + return Ok(Some(child_id.clone())); } } - break; } - Ok(top) + Ok(None) } - pub fn finalize(&mut self) -> Result>, Error> { - let top = self.find_trunk_top()?; - if &top == self.graph.get_root() { - return Ok(None); + fn find_trunk(&mut self) -> Result>, Error> { + let mut trunk = vec![]; + let mut id = self.root_id.clone(); + while let Some(child_id) = self.find_full_child(&id)? { + trunk.push(child_id.clone()); + id = child_id; } - let (trunk, pruned) = self.graph.cut_trunk(top)?; - self.compost_bin.extend(pruned); + Ok(trunk) + } + + #[allow(clippy::type_complexity)] + pub fn finalize(&mut self) -> Result, J)>>, Error> { + let trunk = self.find_trunk()?; + let new_root_id = match trunk.last() { + Some(last) => last.clone(), + None => return Ok(None), + }; + // pruned branches don't have to be connected to the trunk! + let to_be_pruned: HashSet> = self + .vertices + .keys() + .filter(|x| x.number() <= new_root_id.number()) + .cloned() + .collect(); + for id in to_be_pruned.difference(&HashSet::from_iter(trunk.iter().cloned())) { + if self.vertices.contains_key(id) { + self.prune(id)?; + } + } + let new_root_children = self + .children + .get(&new_root_id) + .ok_or(Error::MissingChildrenHashSet)? + .clone(); + let mut finalized = vec![]; + for id in trunk.into_iter() { + match self.remove_vertex(&id)? { + Some(justification) => finalized.push((id, justification)), + None => return Err(Error::MissingJustification), + }; + } + self.root_id = new_root_id.clone(); + self.children.insert(new_root_id, new_root_children); let minimal_number = self.minimal_number(); self.compost_bin = self .compost_bin .drain() - .filter(|x| x.number > minimal_number) + .filter(|x| x.number() > minimal_number) .collect(); - Ok(Some( - trunk - .into_iter() - .map( - |(hashnumber, vertex)| -> Result<(HashNumber, Justification), Error> { - Ok((hashnumber, vertex.justification.ok_or(Error::CriticalBug)?)) - }, - ) - .collect::, Error>>()?, - )) + Ok(Some(finalized)) } - - // pub fn state_summary ... } diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 04c0bb5b86..2cd1518773 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -1,12 +1,11 @@ -use std::collections::HashSet; +use std::{collections::HashSet, marker::PhantomData}; -use super::{Justification, PeerID, HashNumber, Header}; +use super::{Header, Justification, PeerID}; pub enum Error { ContentCorrupted, - InvalidTransition, InvalidHeader, - InvalidJustification, + HeaderMissingParentID, } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] @@ -24,29 +23,82 @@ pub enum Content { Justification, } -pub struct Vertex { - know_most: HashSet, - importance: Importance, - parent: Option, - justification: Option, +type BlockIdFor = <::Header as Header>::Identifier; + +pub struct TransitionSummary { + pub gained_parent: bool, + pub gained_justification: bool, } -impl Vertex { - pub fn new(holder: Option) -> Self { - let know_most = match holder { - Some(peer_id) => HashSet::from([peer_id]), - None => HashSet::new(), - }; - Vertex { - know_most, - importance: Importance::Auxiliary, - parent: None, - justification: None, +impl TransitionSummary { + fn just_created() -> Self { + Self { + gained_parent: false, + gained_justification: false, } } +} + +struct TransitionSummaryMaker { + content_before: Content, + importance_before: Importance, + phantom: PhantomData<(I, J)>, +} + +impl TransitionSummaryMaker { + fn new(vertex_before: &Vertex) -> Result { + Ok(Self { + content_before: vertex_before.content()?, + importance_before: vertex_before.importance.clone(), + phantom: PhantomData, + }) + } + + fn make(self, vertex_after: &Vertex) -> Result, Error> { + use Content::*; + let (content_after, importance_after) = vertex_after.state()?; + Ok( + if self.content_before == content_after && self.importance_before == importance_after { + None + } else { + let (gained_parent, gained_justification) = + match (self.content_before, content_after) { + (Empty, Header) => (true, false), + (Empty, Justification) => (true, true), + (Header, Justification) => (false, true), + _ => (false, false), + }; + Some(TransitionSummary { + gained_parent, + gained_justification, + }) + }, + ) + } +} + +pub struct Vertex { + know_most: HashSet, + importance: Importance, + parent: Option>, + justification: Option, +} + +impl Vertex { + pub fn new() -> (Self, TransitionSummary) { + ( + Self { + know_most: HashSet::new(), + importance: Importance::Auxiliary, + parent: None, + justification: None, + }, + TransitionSummary::just_created(), + ) + } fn content(&self) -> Result { - match (self.parent, self.justification) { + match (&self.parent, &self.justification) { (Some(_), Some(_)) => Ok(Content::Justification), (Some(_), None) => Ok(Content::Header), (None, Some(_)) => Err(Error::ContentCorrupted), @@ -58,83 +110,96 @@ impl Vertex { Ok((self.content()?, self.importance.clone())) } - pub fn know_most(&self) -> HashSet { - self.know_most + pub fn is_full(&self) -> Result { + Ok(self.state()? == (Content::Justification, Importance::Imported)) + } + + pub fn know_most(&self) -> &HashSet { + &self.know_most } - pub fn parent(&self) -> Option { - self.parent + pub fn parent(&self) -> &Option> { + &self.parent } - pub fn justification(self) -> Option { + pub fn justification(self) -> Option { self.justification } - pub fn add_holder(&mut self, holder: Option) { - if let Some(peer_id) = holder { - self.know_most.insert(peer_id); - }; + pub fn add_holder(&mut self, holder: Option) -> bool { + match (self.content(), holder) { + (Ok(Content::Empty), Some(peer_id)) | (Ok(Content::Header), Some(peer_id)) => { + self.know_most.insert(peer_id) + } + _ => false, + } + } + + pub fn add_justification_holder(&mut self, holder: Option) -> bool { + match holder { + Some(peer_id) => self.know_most.insert(peer_id), + None => false, + } } // STATE CHANGING METHODS - pub fn bump_required(&mut self, is_top: bool) -> Result { + pub fn try_set_top_required(&mut self) -> Result, Error> { use Importance::*; - match (self.importance, is_top) { - (Auxiliary, true) => { - self.importance = Importance::TopRequired; - Ok(true) - }, - (Auxiliary, false) => { - self.importance = Importance::Required; - Ok(true) - }, - (TopRequired, false) => { - self.importance = Importance::Required; - Ok(true) - }, - (Required, true) => Err(Error::InvalidTransition), - _ => Ok(false), + let summary_maker = TransitionSummaryMaker::new(&*self)?; + if self.importance == Auxiliary { + self.importance = TopRequired; } + summary_maker.make(&*self) } - pub fn insert_header(&mut self, header: Header) -> Result { - match self.parent { - Some(hashnumber) => { - if hashnumber != header.parent_hashnumber() { + pub fn try_set_required(&mut self) -> Result, Error> { + use Importance::*; + let summary_maker = TransitionSummaryMaker::new(&*self)?; + match self.importance { + Auxiliary | TopRequired => self.importance = Required, + _ => (), + }; + summary_maker.make(&*self) + } + + pub fn try_insert_header( + &mut self, + header: &J::Header, + ) -> Result, Error> { + let summary_maker = TransitionSummaryMaker::new(&*self)?; + let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; + match &self.parent { + Some(id) => { + if id != &parent_id { return Err(Error::InvalidHeader); }; - Ok(false) - }, - None => { - self.parent = Some(header.parent_hashnumber()); - Ok(true) - }, + } + None => self.parent = Some(parent_id), } + summary_maker.make(&*self) } - pub fn insert_body(&mut self, header: Header) -> Result { - Ok(self.insert_header(header)? || match self.importance { - Importance::Imported => false, - _ => { - self.importance = Importance::Imported; - true - }, - }) + pub fn try_insert_body( + &mut self, + header: &J::Header, + ) -> Result, Error> { + let summary_maker = TransitionSummaryMaker::new(&*self)?; + self.try_insert_header(header)?; + self.importance = Importance::Imported; + summary_maker.make(&*self) } - pub fn insert_justification(&mut self, justification: Justification) -> Result { - Ok(self.insert_header(justification.header())? || match self.justification { - Some(current_justification) => { - if justification != current_justification { - return Err(Error::InvalidJustification); - }; - false - }, - None => { - self.justification = Some(justification); - true - }, - }) + pub fn try_insert_justification( + &mut self, + justification: J, + ) -> Result, Error> { + let summary_maker = TransitionSummaryMaker::new(&*self)?; + self.try_insert_header(justification.header())?; + if self.justification.is_none() { + self.justification = Some(justification); + self.know_most.clear(); + } + summary_maker.make(&*self) } } diff --git a/finality-aleph/src/sync/mod.rs b/finality-aleph/src/sync/mod.rs index 2f3c292c1f..d4d61caad7 100644 --- a/finality-aleph/src/sync/mod.rs +++ b/finality-aleph/src/sync/mod.rs @@ -10,6 +10,9 @@ mod ticker; const LOG_TARGET: &str = "aleph-block-sync"; +/// The identifier of a connected peer. +pub trait PeerID: Clone + Hash + Eq {} + /// The identifier of a block, the least amount of knowledge we can have about a block. pub trait BlockIdentifier: Clone + Hash + Debug + Eq { /// The block number, useful when reasoning about hopeless forks. From 803d328a2575086cd7c5d1e835f4657685173999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Tue, 3 Jan 2023 20:59:34 +0100 Subject: [PATCH 05/14] remove empty file --- finality-aleph/src/sync/forest/graph.rs | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 finality-aleph/src/sync/forest/graph.rs diff --git a/finality-aleph/src/sync/forest/graph.rs b/finality-aleph/src/sync/forest/graph.rs deleted file mode 100644 index e69de29bb2..0000000000 From effaadf3297065100c0adb074dbf6672b9b061fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Tue, 3 Jan 2023 21:13:53 +0100 Subject: [PATCH 06/14] add parent check to update_body --- finality-aleph/src/sync/forest/mod.rs | 15 ++++++++++++++- finality-aleph/src/sync/forest/vertex.rs | 4 ++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 1362995abe..2e917c024b 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -60,6 +60,7 @@ pub enum Error { CriticalBug, JustificationPruned, HeaderMissingParentID, + ParentNotImported, } impl From for Error { @@ -299,9 +300,21 @@ impl Forest { header: &J::Header, holder: Option, ) -> Result>, Error> { - use VertexState::Candidate; + use VertexState::*; let id = header.id(); let mut modified = self.update_header(header, holder)?; + if let Candidate(vertex) = self.get_mut(&id)? { + let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; + match self.get_mut(&parent_id)? { + Unknown | HopelessFork | BelowMinimal => return Err(Error::MissingVertex), + HighestFinalized => (), + Candidate(parent_vertex) => { + if !parent_vertex.is_imported() { + return Err(Error::ParentNotImported); + } + } + }; + } if let Candidate(vertex) = self.get_mut(&id)? { let summary = vertex.try_insert_body(header)?; self.process_transition(id, summary, &mut modified)?; diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 2cd1518773..9ccb9e9caa 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -114,6 +114,10 @@ impl Vertex { Ok(self.state()? == (Content::Justification, Importance::Imported)) } + pub fn is_imported(&self) -> bool { + self.importance == Importance::Imported + } + pub fn know_most(&self) -> &HashSet { &self.know_most } From b19b6cebfc3844b599f12a1c21aa5d57fd7d7cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Wed, 4 Jan 2023 11:02:01 +0100 Subject: [PATCH 07/14] polishing --- finality-aleph/src/sync/forest/mod.rs | 278 ++++++++++------------- finality-aleph/src/sync/forest/vertex.rs | 65 +++--- 2 files changed, 154 insertions(+), 189 deletions(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 2e917c024b..9ca9c237b4 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -56,11 +56,13 @@ pub enum Error { MissingParent, MissingVertex, MissingChildrenHashSet, - MissingJustification, - CriticalBug, - JustificationPruned, + TrunkMissingJustification, + RootPruned, + UnknownIDPresent, + ShouldBePruned, + InfiniteLoop, HeaderMissingParentID, - ParentNotImported, + ParentShouldBeImported, } impl From for Error { @@ -86,139 +88,54 @@ impl Forest { } } - fn minimal_number(&self) -> u32 { - self.root_id.number() - } - - fn get_mut(&mut self, id: &BlockIdFor) -> Result, Error> { + fn get_mut(&mut self, id: &BlockIdFor) -> VertexState { use VertexState::*; if id == &self.root_id { - Ok(HighestFinalized) - } else if id.number() <= self.minimal_number() { - Ok(BelowMinimal) + HighestFinalized + } else if id.number() <= self.root_id.number() { + BelowMinimal } else if self.compost_bin.contains(id) { - Ok(HopelessFork) + HopelessFork } else { match self.vertices.get_mut(id) { - Some(vertex) => Ok(Candidate(vertex)), - None => Ok(Unknown), + Some(vertex) => Candidate(vertex), + None => Unknown, } } } - fn add_holder(&mut self, id: BlockIdFor, holder: Option) -> Result { - Ok(match self.get_mut(&id)? { - VertexState::Candidate(vertex) => vertex.add_holder(holder), - _ => false, - }) - } - - fn add_justification_holder( - &mut self, - id: BlockIdFor, - holder: Option, - ) -> Result { - Ok(match self.get_mut(&id)? { - VertexState::Candidate(vertex) => vertex.add_justification_holder(holder), - _ => false, - }) - } - - fn insert_vertex(&mut self, id: BlockIdFor) -> Result, Error> { - use VertexState::*; - Ok(match self.get_mut(&id)? { - Unknown => { - let (vertex, summary) = Vertex::new(); - if self.vertices.insert(id.clone(), vertex).is_some() - || self.children.insert(id, HashSet::new()).is_some() - { - return Err(Error::CriticalBug); - } - Some(summary) - } - _ => None, - }) - } - - fn remove_vertex(&mut self, id: &BlockIdFor) -> Result, Error> { - self.children - .remove(id) - .ok_or(Error::MissingChildrenHashSet)?; - Ok(self - .vertices - .remove(id) - .ok_or(Error::MissingVertex)? - .justification()) - } - - fn try_bump_required_recursive( - &mut self, - id: &BlockIdFor, - ) -> Result>, Error> { - use VertexState::{Candidate, HighestFinalized}; - let mut modified = HashSet::new(); - let mut guard = id.number() as i64 - self.minimal_number() as i64; - if let Candidate(mut vertex) = self.get_mut(id)? { - // if condition is false, then it's already required - // we proceed nevertheless, because we might've just set the parent - if vertex.try_set_top_required()?.is_some() { - modified.insert(id.clone()); - } - let mut id: BlockIdFor; - loop { - // check if has parent - id = match vertex.parent() { - Some(id) => id.clone(), - None => break, - }; - // check if we reached the root - vertex = match self.get_mut(&id)? { - Candidate(vertex) => vertex, - HighestFinalized => break, - _ => return Err(Error::CriticalBug), - }; - // check if already required - match vertex.try_set_required()? { - Some(_) => modified.insert(id.clone()), - None => break, - }; - // avoid infinite loop - guard -= 1; - if guard < 0 { - return Err(Error::CriticalBug); - } - } + fn prune(&mut self, id: BlockIdFor) -> Result>, Error> { + if let VertexState::HighestFinalized = self.get_mut(&id) { + return Err(Error::RootPruned); } - Ok(modified) - } - - fn descendants(&self, id: &BlockIdFor) -> Result>, Error> { - let mut result = HashSet::new(); - let mut current = HashSet::from([id.clone()]); + let mut to_be_pruned: HashSet> = HashSet::new(); + let mut current = HashSet::from([id]); + let mut guard = self.vertices.len() as i64; while !current.is_empty() { + to_be_pruned.extend(current.clone()); let mut next_current = HashSet::new(); - for current_id in current.into_iter() { + for current_id in current.iter() { next_current.extend( self.children - .get(¤t_id) + .get(current_id) .ok_or(Error::MissingChildrenHashSet)? .clone(), ); } current = next_current; - result.extend(current.clone()); + // avoid infinite loop + if guard < 0 { + return Err(Error::InfiniteLoop); + } + guard -= 1; } - Ok(result) - } - - fn prune(&mut self, id: &BlockIdFor) -> Result>, Error> { - let mut to_be_pruned = self.descendants(id)?; - to_be_pruned.insert(id.clone()); for id in to_be_pruned.iter() { - if self.remove_vertex(id)?.is_some() { - return Err(Error::JustificationPruned); - } + self.children + .remove(id) + .ok_or(Error::MissingChildrenHashSet)?; + self.vertices.remove(id).ok_or(Error::MissingVertex)?; } + self.compost_bin.extend(to_be_pruned.clone()); Ok(to_be_pruned) } @@ -233,9 +150,9 @@ impl Forest { if let Some(summary) = summary { modified.insert(id.clone()); if summary.gained_parent { - if let Candidate(vertex) = self.get_mut(&id)? { + if let Candidate(vertex) = self.get_mut(&id) { let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; - match self.get_mut(&parent_id)? { + match self.get_mut(&parent_id) { Unknown => return Err(Error::MissingParent), HighestFinalized | Candidate(_) => self .children @@ -243,17 +160,15 @@ impl Forest { .ok_or(Error::MissingChildrenHashSet)? .insert(id.clone()), HopelessFork | BelowMinimal => { - modified.extend(self.prune(&id)?); + modified.extend(self.prune(id)?); return Ok(()); } }; }; - if let Candidate(vertex) = self.get_mut(&id)? { + if let Candidate(vertex) = self.get_mut(&id) { let (_, importance) = vertex.state()?; match importance { - Required | TopRequired => { - modified.extend(self.try_bump_required_recursive(&id)?) - } + Required | TopRequired => modified.extend(self.set_required(&id)?), Auxiliary | Imported => (), } } @@ -263,7 +178,41 @@ impl Forest { } pub fn set_required(&mut self, id: &BlockIdFor) -> Result>, Error> { - self.try_bump_required_recursive(id) + use VertexState::{Candidate, HighestFinalized}; + let mut modified = HashSet::new(); + let mut guard = id.number() as i64 - self.root_id.number() as i64; + if let Candidate(mut vertex) = self.get_mut(id) { + // if condition is false, then it's already required + // we proceed nevertheless, because we might've just set the parent + if vertex.try_set_top_required()?.is_some() { + modified.insert(id.clone()); + } + let mut id: BlockIdFor; + loop { + // check if has parent + id = match vertex.parent() { + Some(id) => id.clone(), + None => break, + }; + // check if we reached the root + vertex = match self.get_mut(&id) { + Candidate(vertex) => vertex, + HighestFinalized => break, + _ => return Err(Error::ShouldBePruned), + }; + // check if already required + match vertex.try_set_required()? { + Some(_) => modified.insert(id.clone()), + None => break, + }; + // avoid infinite loop + guard -= 1; + if guard < 0 { + return Err(Error::InfiniteLoop); + } + } + } + Ok(modified) } pub fn update_block_identifier( @@ -272,9 +221,20 @@ impl Forest { holder: Option, ) -> Result>, Error> { let mut modified = HashSet::new(); - let summary = self.insert_vertex(id.clone())?; - self.process_transition(id.clone(), summary, &mut modified)?; - self.add_holder(id, holder)?; + // insert vertex + let summary = match self.get_mut(&id) { + VertexState::Unknown => { + let (vertex, summary) = Vertex::new(holder); + if self.vertices.insert(id.clone(), vertex).is_some() + || self.children.insert(id.clone(), HashSet::new()).is_some() + { + return Err(Error::UnknownIDPresent); + } + Some(summary) + } + _ => None, + }; + self.process_transition(id, summary, &mut modified)?; Ok(modified) } @@ -287,9 +247,9 @@ impl Forest { let id = header.id(); let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; let mut modified = self.update_block_identifier(parent_id, holder.clone())?; - modified.extend(self.update_block_identifier(id.clone(), holder)?); - if let Candidate(vertex) = self.get_mut(&id)? { - let summary = vertex.try_insert_header(header)?; + modified.extend(self.update_block_identifier(id.clone(), holder.clone())?); + if let Candidate(vertex) = self.get_mut(&id) { + let summary = vertex.try_insert_header(header, holder)?; self.process_transition(id, summary, &mut modified)?; } Ok(modified) @@ -302,21 +262,21 @@ impl Forest { ) -> Result>, Error> { use VertexState::*; let id = header.id(); - let mut modified = self.update_header(header, holder)?; - if let Candidate(vertex) = self.get_mut(&id)? { + let mut modified = self.update_header(header, holder.clone())?; + if let Candidate(vertex) = self.get_mut(&id) { let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; - match self.get_mut(&parent_id)? { + match self.get_mut(&parent_id) { Unknown | HopelessFork | BelowMinimal => return Err(Error::MissingVertex), HighestFinalized => (), Candidate(parent_vertex) => { if !parent_vertex.is_imported() { - return Err(Error::ParentNotImported); + return Err(Error::ParentShouldBeImported); } } }; } - if let Candidate(vertex) = self.get_mut(&id)? { - let summary = vertex.try_insert_body(header)?; + if let Candidate(vertex) = self.get_mut(&id) { + let summary = vertex.try_insert_body(header, holder)?; self.process_transition(id, summary, &mut modified)?; } Ok(modified) @@ -331,18 +291,20 @@ impl Forest { let header = justification.header(); let id = header.id(); let mut modified = self.update_header(header, holder.clone())?; - if let Candidate(vertex) = self.get_mut(&id)? { - let summary = vertex.try_insert_justification(justification)?; - vertex.add_justification_holder(holder); + if let Candidate(vertex) = self.get_mut(&id) { + let summary = vertex.try_insert_justification(justification, holder)?; self.process_transition(id, summary, &mut modified)?; } Ok(modified) } - fn find_full_child(&mut self, id: &BlockIdFor) -> Result>, Error> { + fn find_next_trunk_vertex( + &mut self, + id: &BlockIdFor, + ) -> Result>, Error> { let children = self.children.get(id).ok_or(Error::MissingChildrenHashSet)?; for child_id in children.clone().iter() { - if let VertexState::Candidate(vertex) = self.get_mut(child_id)? { + if let VertexState::Candidate(vertex) = self.get_mut(child_id) { if vertex.is_full()? { return Ok(Some(child_id.clone())); } @@ -351,19 +313,16 @@ impl Forest { Ok(None) } - fn find_trunk(&mut self) -> Result>, Error> { + #[allow(clippy::type_complexity)] + pub fn finalize(&mut self) -> Result, J)>>, Error> { + // find trunk let mut trunk = vec![]; let mut id = self.root_id.clone(); - while let Some(child_id) = self.find_full_child(&id)? { + while let Some(child_id) = self.find_next_trunk_vertex(&id)? { trunk.push(child_id.clone()); id = child_id; } - Ok(trunk) - } - - #[allow(clippy::type_complexity)] - pub fn finalize(&mut self) -> Result, J)>>, Error> { - let trunk = self.find_trunk()?; + // new root let new_root_id = match trunk.last() { Some(last) => last.clone(), None => return Ok(None), @@ -377,29 +336,42 @@ impl Forest { .collect(); for id in to_be_pruned.difference(&HashSet::from_iter(trunk.iter().cloned())) { if self.vertices.contains_key(id) { - self.prune(id)?; + self.prune(id.clone())?; } } + // keep new root children, as we'll remove the new root in a moment let new_root_children = self .children .get(&new_root_id) .ok_or(Error::MissingChildrenHashSet)? .clone(); + // remove trunk let mut finalized = vec![]; for id in trunk.into_iter() { - match self.remove_vertex(&id)? { + self.children + .remove(&id) + .ok_or(Error::MissingChildrenHashSet)?; + match self + .vertices + .remove(&id) + .ok_or(Error::MissingVertex)? + .justification() + { Some(justification) => finalized.push((id, justification)), - None => return Err(Error::MissingJustification), + None => return Err(Error::TrunkMissingJustification), }; } + // set new root self.root_id = new_root_id.clone(); self.children.insert(new_root_id, new_root_children); - let minimal_number = self.minimal_number(); + // filter compost bin self.compost_bin = self .compost_bin .drain() - .filter(|x| x.number() > minimal_number) + .filter(|x| x.number() > self.root_id.number()) .collect(); Ok(Some(finalized)) } + + // pub fn sth_request(...) {} } diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 9ccb9e9caa..8d27dc47ef 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -27,14 +27,12 @@ type BlockIdFor = <::Header as Header>::Identifier; pub struct TransitionSummary { pub gained_parent: bool, - pub gained_justification: bool, } impl TransitionSummary { fn just_created() -> Self { Self { gained_parent: false, - gained_justification: false, } } } @@ -61,17 +59,11 @@ impl TransitionSummaryMaker { if self.content_before == content_after && self.importance_before == importance_after { None } else { - let (gained_parent, gained_justification) = - match (self.content_before, content_after) { - (Empty, Header) => (true, false), - (Empty, Justification) => (true, true), - (Header, Justification) => (false, true), - _ => (false, false), - }; - Some(TransitionSummary { - gained_parent, - gained_justification, - }) + let gained_parent = matches!( + (self.content_before, content_after), + (Empty, Header | Justification) + ); + Some(TransitionSummary { gained_parent }) }, ) } @@ -85,16 +77,15 @@ pub struct Vertex { } impl Vertex { - pub fn new() -> (Self, TransitionSummary) { - ( - Self { - know_most: HashSet::new(), - importance: Importance::Auxiliary, - parent: None, - justification: None, - }, - TransitionSummary::just_created(), - ) + pub fn new(holder: Option) -> (Self, TransitionSummary) { + let mut vertex = Self { + know_most: HashSet::new(), + importance: Importance::Auxiliary, + parent: None, + justification: None, + }; + vertex.add_holder(holder); + (vertex, TransitionSummary::just_created()) } fn content(&self) -> Result { @@ -130,20 +121,13 @@ impl Vertex { self.justification } - pub fn add_holder(&mut self, holder: Option) -> bool { + pub fn add_holder(&mut self, holder: Option) { match (self.content(), holder) { (Ok(Content::Empty), Some(peer_id)) | (Ok(Content::Header), Some(peer_id)) => { - self.know_most.insert(peer_id) + self.know_most.insert(peer_id); } - _ => false, - } - } - - pub fn add_justification_holder(&mut self, holder: Option) -> bool { - match holder { - Some(peer_id) => self.know_most.insert(peer_id), - None => false, - } + _ => (), + }; } // STATE CHANGING METHODS @@ -170,6 +154,7 @@ impl Vertex { pub fn try_insert_header( &mut self, header: &J::Header, + holder: Option, ) -> Result, Error> { let summary_maker = TransitionSummaryMaker::new(&*self)?; let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; @@ -181,29 +166,37 @@ impl Vertex { } None => self.parent = Some(parent_id), } + self.add_holder(holder); summary_maker.make(&*self) } pub fn try_insert_body( &mut self, header: &J::Header, + holder: Option, ) -> Result, Error> { let summary_maker = TransitionSummaryMaker::new(&*self)?; - self.try_insert_header(header)?; + self.try_insert_header(header, holder.clone())?; self.importance = Importance::Imported; + self.add_holder(holder); summary_maker.make(&*self) } pub fn try_insert_justification( &mut self, justification: J, + holder: Option, ) -> Result, Error> { let summary_maker = TransitionSummaryMaker::new(&*self)?; - self.try_insert_header(justification.header())?; + self.try_insert_header(justification.header(), holder.clone())?; if self.justification.is_none() { self.justification = Some(justification); self.know_most.clear(); } + // add justification holder + if let Some(peer_id) = holder { + self.know_most.insert(peer_id); + } summary_maker.make(&*self) } } From 3544ee91573fde4dc524d63c3d3284346c32f368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Mon, 9 Jan 2023 20:21:32 +0100 Subject: [PATCH 08/14] yet another Vertex --- finality-aleph/src/sync/forest/mod.rs | 40 +-- finality-aleph/src/sync/forest/vertex.rs | 370 ++++++++++++++--------- 2 files changed, 246 insertions(+), 164 deletions(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 9ca9c237b4..8e7cce8926 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -8,6 +8,18 @@ mod vertex; type BlockIdFor = <::Header as Header>::Identifier; +struct JustificationWithParent { + pub justification: J, + pub parent: BlockIdFor, +} + +impl JustificationWithParent { + fn new(justification: J) -> Option { + justification.header().parent_id().map(|id| Self { justification, parent: id.clone() }) + } +} + + pub enum VertexState<'a, I: PeerID, J: Justification> { Unknown, HopelessFork, @@ -22,34 +34,6 @@ pub enum RequestType { JustificationsBelow, } -// /// TODO: RETHINK -// impl From for Option { -// fn from(state: VertexState) -> Self { -// use VertexState::*; -// use Content::*; -// use Importance::*; -// use RequestType::{Header as RHeader, Body, JustificationsBelow}; -// match state { -// Unknown | HopelessFork | BelowMinimal | HighestFinalized => None, -// Candidate(Empty, Auxiliary) => Some(RHeader), -// Candidate(Empty, TopRequired) => Some(Body), -// Candidate(Empty, Required) => Some(Body), -// Candidate(Empty, Imported) => { -// error!(target: "aleph-sync", "Forbidden state combination: (Empty, Imported), interpreting as (Header, Imported)"); -// Some(JustificationsBelow) -// }, -// Candidate(Header, Auxiliary) => None, -// Candidate(Header, TopRequired) => Some(Body), -// Candidate(Header, Required) => Some(Body), -// Candidate(Header, Imported) => Some(JustificationsBelow), -// Candidate(Justification, Auxiliary) => { -// error!(target: "aleph-sync", "Forbidden state combination: (Justification, Auxiliary), interpreting as (Justification, _)"); -// Some(JustificationsBelow) -// }, -// Candidate(Justification, _) => Some(JustificationsBelow), -// } -// } -// } pub enum Error { Vertex(VertexError), diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 8d27dc47ef..4fcdc75d03 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -1,202 +1,300 @@ -use std::{collections::HashSet, marker::PhantomData}; +use std::collections::HashSet; -use super::{Header, Justification, PeerID}; +use super::{Header, Justification, JustificationWithParent, PeerID}; + +type BlockIDFor = <::Header as Header>::Identifier; pub enum Error { - ContentCorrupted, - InvalidHeader, - HeaderMissingParentID, + InvalidParentID, + JustificationImportance, } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -pub enum Importance { +pub enum EmptyImportance { Auxiliary, TopRequired, Required, - Imported, } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -pub enum Content { - Empty, - Header, - Justification, +pub enum HeaderImportance { + Auxiliary, + TopRequired, + Required, + Imported, } -type BlockIdFor = <::Header as Header>::Identifier; +impl From for HeaderImportance { + fn from(importance: EmptyImportance) -> Self { + match importance { + EmptyImportance::Auxiliary => Self::Auxiliary, + EmptyImportance::TopRequired => Self::TopRequired, + EmptyImportance::Required => Self::Required, + } + } +} -pub struct TransitionSummary { - pub gained_parent: bool, +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +pub enum JustificationImportance { + TopRequired, + Required, + Imported, } -impl TransitionSummary { - fn just_created() -> Self { - Self { - gained_parent: false, +impl From for Option { + fn from(importance: HeaderImportance) -> Self { + match importance { + HeaderImportance::Auxiliary => None, + HeaderImportance::TopRequired => Some(JustificationImportance::TopRequired), + HeaderImportance::Required => Some(JustificationImportance::Required), + HeaderImportance::Imported => Some(JustificationImportance::Imported), } } } -struct TransitionSummaryMaker { - content_before: Content, - importance_before: Importance, - phantom: PhantomData<(I, J)>, +struct EmptyVertex { + importance: EmptyImportance, + know_most: HashSet, } -impl TransitionSummaryMaker { - fn new(vertex_before: &Vertex) -> Result { - Ok(Self { - content_before: vertex_before.content()?, - importance_before: vertex_before.importance.clone(), - phantom: PhantomData, - }) - } - - fn make(self, vertex_after: &Vertex) -> Result, Error> { - use Content::*; - let (content_after, importance_after) = vertex_after.state()?; - Ok( - if self.content_before == content_after && self.importance_before == importance_after { - None - } else { - let gained_parent = matches!( - (self.content_before, content_after), - (Empty, Header | Justification) - ); - Some(TransitionSummary { gained_parent }) - }, - ) +struct HeaderVertex { + importance: HeaderImportance, + parent: BlockIDFor, + know_most: HashSet, +} + +impl HeaderVertex { + fn is_imported(&self) -> bool { + self.importance == HeaderImportance::Imported } } -pub struct Vertex { +struct JustificationVertex { + importance: JustificationImportance, + justification_with_parent: JustificationWithParent, know_most: HashSet, - importance: Importance, - parent: Option>, - justification: Option, } -impl Vertex { - pub fn new(holder: Option) -> (Self, TransitionSummary) { - let mut vertex = Self { - know_most: HashSet::new(), - importance: Importance::Auxiliary, - parent: None, - justification: None, - }; - vertex.add_holder(holder); - (vertex, TransitionSummary::just_created()) +impl JustificationVertex { + fn is_imported(&self) -> bool { + self.importance == JustificationImportance::Imported } +} + +pub enum Vertex { + Empty(EmptyVertex), + Header(HeaderVertex), + Justification(JustificationVertex), +} + +pub enum State { + Empty(EmptyImportance), + Header(HeaderImportance), + Justification(JustificationImportance), +} - fn content(&self) -> Result { - match (&self.parent, &self.justification) { - (Some(_), Some(_)) => Ok(Content::Justification), - (Some(_), None) => Ok(Content::Header), - (None, Some(_)) => Err(Error::ContentCorrupted), - (None, None) => Ok(Content::Empty), +impl State { + pub fn gained_parent(&self, new_state: &State) -> bool { + use State::*; + match (self, new_state) { + (Empty(_), Header(_)) | (Empty(_), Justification(_)) => true, + _ => false, } } +} - pub fn state(&self) -> Result<(Content, Importance), Error> { - Ok((self.content()?, self.importance.clone())) +impl Vertex { + pub fn new() -> Self { + Self::Empty(EmptyVertex{ importance: EmptyImportance::Auxiliary, know_most: HashSet::new()}) } - pub fn is_full(&self) -> Result { - Ok(self.state()? == (Content::Justification, Importance::Imported)) + pub fn state(&self) -> State { + use Vertex::*; + match self { + Empty(vertex) => State::Empty(vertex.importance.clone()), + Header(vertex) => State::Header(vertex.importance.clone()), + Justification(vertex) => State::Justification(vertex.importance.clone()), + } } - pub fn is_imported(&self) -> bool { - self.importance == Importance::Imported + pub fn is_full(&self) -> bool { + match self { + Self::Justification(vertex) => vertex.is_imported(), + _ => false, + } } - pub fn know_most(&self) -> &HashSet { - &self.know_most + pub fn is_imported(&self) -> bool { + match self { + Self::Empty(_) => false, + Self::Header(vertex) => vertex.is_imported(), + Self::Justification(vertex) => vertex.is_imported(), + } } - pub fn parent(&self) -> &Option> { - &self.parent + pub fn parent(&self) -> Option<&BlockIDFor> { + match self { + Self::Empty(vertex) => None, + Self::Header(vertex) => Some(&vertex.parent), + Self::Justification(vertex) => Some(&vertex.justification_with_parent.parent), + } } pub fn justification(self) -> Option { - self.justification + match self { + Self::Justification(vertex) => Some(vertex.justification_with_parent.justification), + _ => None, + } } - pub fn add_holder(&mut self, holder: Option) { - match (self.content(), holder) { - (Ok(Content::Empty), Some(peer_id)) | (Ok(Content::Header), Some(peer_id)) => { - self.know_most.insert(peer_id); - } - _ => (), - }; + pub fn know_most(&self) -> &HashSet { + match self { + Self::Empty(vertex) => &vertex.know_most, + Self::Header(vertex) => &vertex.know_most, + Self::Justification(vertex) => &vertex.know_most, + } } - // STATE CHANGING METHODS + pub fn try_set_top_required(&mut self) -> bool { + match self { + Self::Empty(vertex) => { + match vertex.importance { + EmptyImportance::Auxiliary => { + vertex.importance = EmptyImportance::TopRequired; + true + }, + _ => false, + } + }, + Self::Header(vertex) => { + match vertex.importance { + HeaderImportance::Auxiliary => { + vertex.importance = HeaderImportance::TopRequired; + true + }, + _ => false, + } + } + Self::Justification(_) => false, + } + } - pub fn try_set_top_required(&mut self) -> Result, Error> { - use Importance::*; - let summary_maker = TransitionSummaryMaker::new(&*self)?; - if self.importance == Auxiliary { - self.importance = TopRequired; + pub fn try_set_required(&mut self) -> bool { + match self { + Self::Empty(vertex) => { + match vertex.importance { + EmptyImportance::Auxiliary | EmptyImportance::TopRequired => { + vertex.importance = EmptyImportance::Required; + true + }, + _ => false, + } + }, + Self::Header(vertex) => { + match vertex.importance { + HeaderImportance::Auxiliary | HeaderImportance::TopRequired => { + vertex.importance = HeaderImportance::Required; + true + }, + _ => false, + } + } + Self::Justification(vertex) => { + match vertex.importance { + JustificationImportance::TopRequired => { + vertex.importance = JustificationImportance::Required; + true + }, + _ => false, + } + } } - summary_maker.make(&*self) } - pub fn try_set_required(&mut self) -> Result, Error> { - use Importance::*; - let summary_maker = TransitionSummaryMaker::new(&*self)?; - match self.importance { - Auxiliary | TopRequired => self.importance = Required, - _ => (), - }; - summary_maker.make(&*self) + fn check_parent(&self, parent_id: &BlockIDFor) -> Result<(), Error> { + match self { + Self::Empty(_) => Ok(()), + Self::Header(vertex) => match parent_id == &vertex.parent { + true => Ok(()), + false => Err(Error::InvalidParentID), + }, + Self::Justification(vertex) => match parent_id == &vertex.justification_with_parent.parent { + true => Ok(()), + false => Err(Error::InvalidParentID), + }, + } } - pub fn try_insert_header( - &mut self, - header: &J::Header, - holder: Option, - ) -> Result, Error> { - let summary_maker = TransitionSummaryMaker::new(&*self)?; - let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; - match &self.parent { - Some(id) => { - if id != &parent_id { - return Err(Error::InvalidHeader); - }; - } - None => self.parent = Some(parent_id), + pub fn try_insert_header(&mut self, parent_id: BlockIDFor, holder: Option) -> Result { + self.check_parent(&parent_id)?; + let output = match self { + Self::Empty(vertex) => { + *self = Self::Header(HeaderVertex { importance: vertex.importance.into(), parent: parent_id, know_most: vertex.know_most }); + Ok(true) + }, + _ => Ok(false), + }; + if let Some(holder) = holder { + match self { + Self::Empty(vertex) => vertex.know_most.insert(holder), + Self::Header(vertex) => vertex.know_most.insert(holder), + Self::Justification(_) => false, + }; } - self.add_holder(holder); - summary_maker.make(&*self) + output } - pub fn try_insert_body( - &mut self, - header: &J::Header, - holder: Option, - ) -> Result, Error> { - let summary_maker = TransitionSummaryMaker::new(&*self)?; - self.try_insert_header(header, holder.clone())?; - self.importance = Importance::Imported; - self.add_holder(holder); - summary_maker.make(&*self) + pub fn try_insert_body(&mut self, parent_id: BlockIDFor, holder: Option) -> Result { + self.check_parent(&parent_id)?; + let output = match self { + Self::Empty(vertex) => { + *self = Self::Header(HeaderVertex { importance: HeaderImportance::Imported, parent: parent_id, know_most: vertex.know_most }); + Ok(true) + }, + Self::Header(vertex) => match vertex.importance { + HeaderImportance::Imported => Ok(false), + _ => { + vertex.importance = HeaderImportance::Imported; + Ok(true) + }, + }, + Self::Justification(vertex) => match vertex.importance { + JustificationImportance::Imported => Ok(false), + _ => { + vertex.importance = JustificationImportance::Imported; + Ok(true) + }, + }, + }; + if let Some(holder) = holder { + match self { + Self::Empty(vertex) => vertex.know_most.insert(holder), + Self::Header(vertex) => vertex.know_most.insert(holder), + Self::Justification(_) => false, + }; + } + output } pub fn try_insert_justification( &mut self, - justification: J, + justification_with_parent: JustificationWithParent, holder: Option, - ) -> Result, Error> { - let summary_maker = TransitionSummaryMaker::new(&*self)?; - self.try_insert_header(justification.header(), holder.clone())?; - if self.justification.is_none() { - self.justification = Some(justification); - self.know_most.clear(); - } - // add justification holder - if let Some(peer_id) = holder { - self.know_most.insert(peer_id); + ) -> Result { + self.check_parent(&justification_with_parent.parent)?; + match self { + Self::Empty(_) | Self::Header(_) => { + *self = Self::Justification(JustificationVertex { + importance: match self { + Self::Empty(vertex) => Into::::into(vertex.importance).into(), + Self::Header(vertex) => vertex.importance.into(), + Self::Justification(vertex) => Some(vertex.importance.clone()), + }.ok_or(Error::JustificationImportance)?, + justification_with_parent, + know_most: HashSet::from_iter(holder.into_iter()) }); + Ok(true) + }, + Self::Justification(_) => Ok(false), } - summary_maker.make(&*self) } } From 17957e976276cbb2a44dff9f00cd790a0b47cf1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Tue, 10 Jan 2023 01:32:32 +0100 Subject: [PATCH 09/14] forest reimagined --- finality-aleph/src/sync/forest/mod.rs | 537 ++++++++++++----------- finality-aleph/src/sync/forest/vertex.rs | 215 +++++---- 2 files changed, 414 insertions(+), 338 deletions(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 8e7cce8926..7f55b05edd 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -1,52 +1,43 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::{ + hash_map::{Entry, OccupiedEntry, VacantEntry}, + HashMap, HashSet, +}; -use vertex::{Error as VertexError, Importance, TransitionSummary, Vertex}; +use vertex::{Error as VertexError, Importance, Vertex as Vertex_}; use super::{BlockIdentifier, Header, Justification, PeerID}; mod vertex; -type BlockIdFor = <::Header as Header>::Identifier; +type BlockIDFor = <::Header as Header>::Identifier; -struct JustificationWithParent { +pub struct JustificationWithParent { pub justification: J, - pub parent: BlockIdFor, + pub parent: BlockIDFor, } impl JustificationWithParent { fn new(justification: J) -> Option { - justification.header().parent_id().map(|id| Self { justification, parent: id.clone() }) + justification.header().parent_id().map(|id| Self { + justification, + parent: id, + }) } } - pub enum VertexState<'a, I: PeerID, J: Justification> { - Unknown, HopelessFork, BelowMinimal, HighestFinalized, - Candidate(&'a mut Vertex), -} - -pub enum RequestType { - Header, - Body, - JustificationsBelow, + Unknown(VacantEntry<'a, BlockIDFor, Vertex>), + Candidate(OccupiedEntry<'a, BlockIDFor, Vertex>), } - pub enum Error { Vertex(VertexError), - MissingParent, MissingVertex, - MissingChildrenHashSet, - TrunkMissingJustification, - RootPruned, - UnknownIDPresent, ShouldBePruned, InfiniteLoop, - HeaderMissingParentID, - ParentShouldBeImported, } impl From for Error { @@ -55,24 +46,38 @@ impl From for Error { } } +pub struct Vertex { + vertex: Vertex_, + children: HashSet>, +} + +impl Vertex { + fn new(holder: Option) -> Self { + Self { + vertex: Vertex_::new(holder), + children: HashSet::new(), + } + } +} + pub struct Forest { - vertices: HashMap, Vertex>, - children: HashMap, HashSet>>, - root_id: BlockIdFor, - compost_bin: HashSet>, + vertices: HashMap, Vertex>, + root_id: BlockIDFor, + root_children: HashSet>, + compost_bin: HashSet>, } impl Forest { - pub fn new(highest_justified: BlockIdFor) -> Self { + pub fn new(highest_justified: BlockIDFor) -> Self { Self { vertices: HashMap::new(), - children: HashMap::from([(highest_justified.clone(), HashSet::new())]), root_id: highest_justified, + root_children: HashSet::new(), compost_bin: HashSet::new(), } } - fn get_mut(&mut self, id: &BlockIdFor) -> VertexState { + fn get_mut(&mut self, id: &BlockIDFor) -> VertexState { use VertexState::*; if id == &self.root_id { HighestFinalized @@ -81,281 +86,287 @@ impl Forest { } else if self.compost_bin.contains(id) { HopelessFork } else { - match self.vertices.get_mut(id) { - Some(vertex) => Candidate(vertex), - None => Unknown, + match self.vertices.entry(id.clone()) { + Entry::Occupied(entry) => Candidate(entry), + Entry::Vacant(entry) => Unknown(entry), } } } - fn prune(&mut self, id: BlockIdFor) -> Result>, Error> { - if let VertexState::HighestFinalized = self.get_mut(&id) { - return Err(Error::RootPruned); + fn prune( + &mut self, + id: BlockIDFor, + modified: &mut HashSet>, + ) -> Result<(), Error> { + if !self.vertices.contains_key(&id) { + return Ok(()); } - let mut to_be_pruned: HashSet> = HashSet::new(); - let mut current = HashSet::from([id]); - let mut guard = self.vertices.len() as i64; - while !current.is_empty() { - to_be_pruned.extend(current.clone()); - let mut next_current = HashSet::new(); - for current_id in current.iter() { - next_current.extend( - self.children - .get(current_id) - .ok_or(Error::MissingChildrenHashSet)? - .clone(), - ); - } - current = next_current; - // avoid infinite loop - if guard < 0 { + let cap = self.vertices.len(); + let mut to_be_pruned = Vec::with_capacity(cap); + to_be_pruned.push(id); + let mut index: usize = 0; + while let Some(id) = to_be_pruned.get(index) { + to_be_pruned.extend( + self.vertices + .get(id) + .ok_or(Error::MissingVertex)? + .children + .iter() + .cloned(), + ); + index += 1; + if index > cap { return Err(Error::InfiniteLoop); } - guard -= 1; } for id in to_be_pruned.iter() { - self.children - .remove(id) - .ok_or(Error::MissingChildrenHashSet)?; self.vertices.remove(id).ok_or(Error::MissingVertex)?; } self.compost_bin.extend(to_be_pruned.clone()); - Ok(to_be_pruned) + modified.extend(to_be_pruned.into_iter()); + Ok(()) } - fn process_transition( + fn try_add_parent( &mut self, - id: BlockIdFor, - summary: Option, - modified: &mut HashSet>, + id: BlockIDFor, + modified: &mut HashSet>, ) -> Result<(), Error> { - use Importance::*; use VertexState::*; - if let Some(summary) = summary { - modified.insert(id.clone()); - if summary.gained_parent { - if let Candidate(vertex) = self.get_mut(&id) { - let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; - match self.get_mut(&parent_id) { - Unknown => return Err(Error::MissingParent), - HighestFinalized | Candidate(_) => self - .children - .get_mut(&parent_id) - .ok_or(Error::MissingChildrenHashSet)? - .insert(id.clone()), - HopelessFork | BelowMinimal => { - modified.extend(self.prune(id)?); - return Ok(()); - } - }; - }; - if let Candidate(vertex) = self.get_mut(&id) { - let (_, importance) = vertex.state()?; - match importance { - Required | TopRequired => modified.extend(self.set_required(&id)?), - Auxiliary | Imported => (), + if let Candidate(mut entry) = self.get_mut(&id) { + let vertex = entry.get_mut(); + let importance = vertex.vertex.state().importance(); + if let Some(parent_id) = vertex.vertex.parent().cloned() { + match self.get_mut(&parent_id) { + Unknown(_) => (), + HighestFinalized => { + self.root_children.insert(id); } - } - } - } + Candidate(mut entry) => { + entry.get_mut().children.insert(id); + match importance { + Importance::Required | Importance::TopRequired => { + self.set_required_with_ancestors(&parent_id, modified)? + } + Importance::Auxiliary | Importance::Imported => (), + }; + } + HopelessFork | BelowMinimal => self.prune(id, modified)?, + }; + }; + }; Ok(()) } - pub fn set_required(&mut self, id: &BlockIdFor) -> Result>, Error> { + fn set_required_with_ancestors( + &mut self, + id: &BlockIDFor, + modified: &mut HashSet>, + ) -> Result<(), Error> { use VertexState::{Candidate, HighestFinalized}; - let mut modified = HashSet::new(); let mut guard = id.number() as i64 - self.root_id.number() as i64; - if let Candidate(mut vertex) = self.get_mut(id) { - // if condition is false, then it's already required - // we proceed nevertheless, because we might've just set the parent - if vertex.try_set_top_required()?.is_some() { - modified.insert(id.clone()); - } - let mut id: BlockIdFor; - loop { - // check if has parent - id = match vertex.parent() { - Some(id) => id.clone(), - None => break, - }; - // check if we reached the root - vertex = match self.get_mut(&id) { - Candidate(vertex) => vertex, - HighestFinalized => break, - _ => return Err(Error::ShouldBePruned), - }; - // check if already required - match vertex.try_set_required()? { - Some(_) => modified.insert(id.clone()), - None => break, - }; - // avoid infinite loop - guard -= 1; - if guard < 0 { - return Err(Error::InfiniteLoop); + let mut id = id.clone(); + let mut vertex; + loop { + // check if we reached the root + match self.get_mut(&id) { + Candidate(mut entry) => { + vertex = entry.get_mut(); + // check if already required + match vertex.vertex.try_set_required() { + true => modified.insert(id.clone()), + false => break, + }; + // check if has parent + id = match vertex.vertex.parent() { + Some(id) => id.clone(), + None => break, + }; } + HighestFinalized => break, + _ => return Err(Error::ShouldBePruned), + }; + // avoid infinite loop + guard -= 1; + if guard < 0 { + return Err(Error::InfiniteLoop); } } - Ok(modified) + Ok(()) } - pub fn update_block_identifier( + fn bump_required( &mut self, - id: BlockIdFor, - holder: Option, - ) -> Result>, Error> { - let mut modified = HashSet::new(); - // insert vertex - let summary = match self.get_mut(&id) { - VertexState::Unknown => { - let (vertex, summary) = Vertex::new(holder); - if self.vertices.insert(id.clone(), vertex).is_some() - || self.children.insert(id.clone(), HashSet::new()).is_some() - { - return Err(Error::UnknownIDPresent); + id: &BlockIDFor, + modified: &mut HashSet>, + ) -> Result<(), Error> { + use VertexState::*; + if let Candidate(mut entry) = self.get_mut(id) { + if entry.get_mut().vertex.try_set_top_required() { + modified.insert(id.clone()); + if let Some(parent_id) = entry.get_mut().vertex.parent().cloned() { + self.set_required_with_ancestors(&parent_id, modified)?; } - Some(summary) } - _ => None, - }; - self.process_transition(id, summary, &mut modified)?; - Ok(modified) - } - - pub fn update_header( - &mut self, - header: &J::Header, - holder: Option, - ) -> Result>, Error> { - use VertexState::Candidate; - let id = header.id(); - let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; - let mut modified = self.update_block_identifier(parent_id, holder.clone())?; - modified.extend(self.update_block_identifier(id.clone(), holder.clone())?); - if let Candidate(vertex) = self.get_mut(&id) { - let summary = vertex.try_insert_header(header, holder)?; - self.process_transition(id, summary, &mut modified)?; } - Ok(modified) + Ok(()) } - pub fn update_body( + pub fn update_block_identifier( &mut self, - header: &J::Header, + id: BlockIDFor, holder: Option, - ) -> Result>, Error> { - use VertexState::*; - let id = header.id(); - let mut modified = self.update_header(header, holder.clone())?; - if let Candidate(vertex) = self.get_mut(&id) { - let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; - match self.get_mut(&parent_id) { - Unknown | HopelessFork | BelowMinimal => return Err(Error::MissingVertex), - HighestFinalized => (), - Candidate(parent_vertex) => { - if !parent_vertex.is_imported() { - return Err(Error::ParentShouldBeImported); - } - } - }; + required: bool, + ) -> Result>, Error> { + let mut modified = HashSet::new(); + match self.get_mut(&id) { + VertexState::Unknown(entry) => { + entry.insert(Vertex::new(holder)); + modified.insert(id.clone()); + } + VertexState::Candidate(mut entry) => { + entry.get_mut().vertex.add_empty_holder(holder); + } + _ => (), } - if let Candidate(vertex) = self.get_mut(&id) { - let summary = vertex.try_insert_body(header, holder)?; - self.process_transition(id, summary, &mut modified)?; + if required { + self.bump_required(&id, &mut modified)?; } Ok(modified) } - pub fn update_justification( - &mut self, - justification: J, - holder: Option, - ) -> Result>, Error> { - use VertexState::Candidate; - let header = justification.header(); - let id = header.id(); - let mut modified = self.update_header(header, holder.clone())?; - if let Candidate(vertex) = self.get_mut(&id) { - let summary = vertex.try_insert_justification(justification, holder)?; - self.process_transition(id, summary, &mut modified)?; - } - Ok(modified) - } + // pub fn update_header( + // &mut self, + // header: &J::Header, + // holder: Option, + // ) -> Result>, Error> { + // use VertexState::Candidate; + // let id = header.id(); + // let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; + // let mut modified = self.update_block_identifier(parent_id, holder.clone())?; + // modified.extend(self.update_block_identifier(id.clone(), holder.clone())?); + // if let Candidate(vertex) = self.get_mut(&id) { + // let summary = vertex.try_insert_header(header, holder)?; + // self.process_transition(id, summary, &mut modified)?; + // } + // Ok(modified) + // } - fn find_next_trunk_vertex( - &mut self, - id: &BlockIdFor, - ) -> Result>, Error> { - let children = self.children.get(id).ok_or(Error::MissingChildrenHashSet)?; - for child_id in children.clone().iter() { - if let VertexState::Candidate(vertex) = self.get_mut(child_id) { - if vertex.is_full()? { - return Ok(Some(child_id.clone())); - } - } - } - Ok(None) - } + // pub fn update_body( + // &mut self, + // header: &J::Header, + // holder: Option, + // ) -> Result>, Error> { + // use VertexState::*; + // let id = header.id(); + // let mut modified = self.update_header(header, holder.clone())?; + // if let Candidate(vertex) = self.get_mut(&id) { + // let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; + // match self.get_mut(&parent_id) { + // Unknown | HopelessFork | BelowMinimal => return Err(Error::MissingVertex), + // HighestFinalized => (), + // Candidate(parent_vertex) => { + // if !parent_vertex.is_imported() { + // return Err(Error::ParentShouldBeImported); + // } + // } + // }; + // } + // if let Candidate(vertex) = self.get_mut(&id) { + // let summary = vertex.try_insert_body(header, holder)?; + // self.process_transition(id, summary, &mut modified)?; + // } + // Ok(modified) + // } - #[allow(clippy::type_complexity)] - pub fn finalize(&mut self) -> Result, J)>>, Error> { - // find trunk - let mut trunk = vec![]; - let mut id = self.root_id.clone(); - while let Some(child_id) = self.find_next_trunk_vertex(&id)? { - trunk.push(child_id.clone()); - id = child_id; - } - // new root - let new_root_id = match trunk.last() { - Some(last) => last.clone(), - None => return Ok(None), - }; - // pruned branches don't have to be connected to the trunk! - let to_be_pruned: HashSet> = self - .vertices - .keys() - .filter(|x| x.number() <= new_root_id.number()) - .cloned() - .collect(); - for id in to_be_pruned.difference(&HashSet::from_iter(trunk.iter().cloned())) { - if self.vertices.contains_key(id) { - self.prune(id.clone())?; - } - } - // keep new root children, as we'll remove the new root in a moment - let new_root_children = self - .children - .get(&new_root_id) - .ok_or(Error::MissingChildrenHashSet)? - .clone(); - // remove trunk - let mut finalized = vec![]; - for id in trunk.into_iter() { - self.children - .remove(&id) - .ok_or(Error::MissingChildrenHashSet)?; - match self - .vertices - .remove(&id) - .ok_or(Error::MissingVertex)? - .justification() - { - Some(justification) => finalized.push((id, justification)), - None => return Err(Error::TrunkMissingJustification), - }; - } - // set new root - self.root_id = new_root_id.clone(); - self.children.insert(new_root_id, new_root_children); - // filter compost bin - self.compost_bin = self - .compost_bin - .drain() - .filter(|x| x.number() > self.root_id.number()) - .collect(); - Ok(Some(finalized)) - } + // pub fn update_justification( + // &mut self, + // justification: J, + // holder: Option, + // ) -> Result>, Error> { + // use VertexState::Candidate; + // let header = justification.header(); + // let id = header.id(); + // let mut modified = self.update_header(header, holder.clone())?; + // if let Candidate(vertex) = self.get_mut(&id) { + // let summary = vertex.try_insert_justification(justification, holder)?; + // self.process_transition(id, summary, &mut modified)?; + // } + // Ok(modified) + // } + + // fn find_next_trunk_vertex( + // &mut self, + // id: &BlockIDFor, + // ) -> Result>, Error> { + // let children = self.children.get(id).ok_or(Error::MissingChildrenHashSet)?; + // for child_id in children.clone().iter() { + // if let VertexState::Candidate(vertex) = self.get_mut(child_id) { + // if vertex.is_full()? { + // return Ok(Some(child_id.clone())); + // } + // } + // } + // Ok(None) + // } - // pub fn sth_request(...) {} + // #[allow(clippy::type_complexity)] + // pub fn finalize(&mut self) -> Result, J)>>, Error> { + // // find trunk + // let mut trunk = vec![]; + // let mut id = self.root_id.clone(); + // while let Some(child_id) = self.find_next_trunk_vertex(&id)? { + // trunk.push(child_id.clone()); + // id = child_id; + // } + // // new root + // let new_root_id = match trunk.last() { + // Some(last) => last.clone(), + // None => return Ok(None), + // }; + // // pruned branches don't have to be connected to the trunk! + // let to_be_pruned: HashSet> = self + // .vertices + // .keys() + // .filter(|x| x.number() <= new_root_id.number()) + // .cloned() + // .collect(); + // for id in to_be_pruned.difference(&HashSet::from_iter(trunk.iter().cloned())) { + // if self.vertices.contains_key(id) { + // self.prune(id.clone())?; + // } + // } + // // keep new root children, as we'll remove the new root in a moment + // let new_root_children = self + // .children + // .get(&new_root_id) + // .ok_or(Error::MissingChildrenHashSet)? + // .clone(); + // // remove trunk + // let mut finalized = vec![]; + // for id in trunk.into_iter() { + // self.children + // .remove(&id) + // .ok_or(Error::MissingChildrenHashSet)?; + // match self + // .vertices + // .remove(&id) + // .ok_or(Error::MissingVertex)? + // .justification() + // { + // Some(justification) => finalized.push((id, justification)), + // None => return Err(Error::TrunkMissingJustification), + // }; + // } + // // set new root + // self.root_id = new_root_id.clone(); + // self.children.insert(new_root_id, new_root_children); + // // filter compost bin + // self.compost_bin = self + // .compost_bin + // .drain() + // .filter(|x| x.number() > self.root_id.number()) + // .collect(); + // Ok(Some(finalized)) + // } } diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 4fcdc75d03..3d53638b69 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -24,8 +24,8 @@ pub enum HeaderImportance { Imported, } -impl From for HeaderImportance { - fn from(importance: EmptyImportance) -> Self { +impl From<&EmptyImportance> for HeaderImportance { + fn from(importance: &EmptyImportance) -> Self { match importance { EmptyImportance::Auxiliary => Self::Auxiliary, EmptyImportance::TopRequired => Self::TopRequired, @@ -41,8 +41,8 @@ pub enum JustificationImportance { Imported, } -impl From for Option { - fn from(importance: HeaderImportance) -> Self { +impl From<&HeaderImportance> for Option { + fn from(importance: &HeaderImportance) -> Self { match importance { HeaderImportance::Auxiliary => None, HeaderImportance::TopRequired => Some(JustificationImportance::TopRequired), @@ -52,12 +52,51 @@ impl From for Option { } } -struct EmptyVertex { +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +pub enum Importance { + Auxiliary, + TopRequired, + Required, + Imported, +} + +impl From<&EmptyImportance> for Importance { + fn from(importance: &EmptyImportance) -> Self { + match importance { + EmptyImportance::Auxiliary => Self::Auxiliary, + EmptyImportance::TopRequired => Self::TopRequired, + EmptyImportance::Required => Self::Required, + } + } +} + +impl From<&HeaderImportance> for Importance { + fn from(importance: &HeaderImportance) -> Self { + match importance { + HeaderImportance::Auxiliary => Self::Auxiliary, + HeaderImportance::TopRequired => Self::TopRequired, + HeaderImportance::Required => Self::Required, + HeaderImportance::Imported => Self::Imported, + } + } +} + +impl From<&JustificationImportance> for Importance { + fn from(importance: &JustificationImportance) -> Self { + match importance { + JustificationImportance::TopRequired => Self::TopRequired, + JustificationImportance::Required => Self::Required, + JustificationImportance::Imported => Self::Imported, + } + } +} + +pub struct EmptyVertex { importance: EmptyImportance, know_most: HashSet, } -struct HeaderVertex { +pub struct HeaderVertex { importance: HeaderImportance, parent: BlockIDFor, know_most: HashSet, @@ -69,7 +108,7 @@ impl HeaderVertex { } } -struct JustificationVertex { +pub struct JustificationVertex { importance: JustificationImportance, justification_with_parent: JustificationWithParent, know_most: HashSet, @@ -81,12 +120,7 @@ impl JustificationVertex { } } -pub enum Vertex { - Empty(EmptyVertex), - Header(HeaderVertex), - Justification(JustificationVertex), -} - +#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] pub enum State { Empty(EmptyImportance), Header(HeaderImportance), @@ -94,18 +128,28 @@ pub enum State { } impl State { - pub fn gained_parent(&self, new_state: &State) -> bool { + pub fn importance(&self) -> Importance { use State::*; - match (self, new_state) { - (Empty(_), Header(_)) | (Empty(_), Justification(_)) => true, - _ => false, + match self { + Empty(importance) => importance.into(), + Header(importance) => importance.into(), + Justification(importance) => importance.into(), } } } +pub enum Vertex { + Empty(EmptyVertex), + Header(HeaderVertex), + Justification(JustificationVertex), +} + impl Vertex { - pub fn new() -> Self { - Self::Empty(EmptyVertex{ importance: EmptyImportance::Auxiliary, know_most: HashSet::new()}) + pub fn new(holder: Option) -> Self { + Self::Empty(EmptyVertex { + importance: EmptyImportance::Auxiliary, + know_most: HashSet::from_iter(holder.into_iter()), + }) } pub fn state(&self) -> State { @@ -134,7 +178,7 @@ impl Vertex { pub fn parent(&self) -> Option<&BlockIDFor> { match self { - Self::Empty(vertex) => None, + Self::Empty(_) => None, Self::Header(vertex) => Some(&vertex.parent), Self::Justification(vertex) => Some(&vertex.justification_with_parent.parent), } @@ -157,57 +201,47 @@ impl Vertex { pub fn try_set_top_required(&mut self) -> bool { match self { - Self::Empty(vertex) => { - match vertex.importance { - EmptyImportance::Auxiliary => { - vertex.importance = EmptyImportance::TopRequired; - true - }, - _ => false, + Self::Empty(vertex) => match vertex.importance { + EmptyImportance::Auxiliary => { + vertex.importance = EmptyImportance::TopRequired; + true } + _ => false, }, - Self::Header(vertex) => { - match vertex.importance { - HeaderImportance::Auxiliary => { - vertex.importance = HeaderImportance::TopRequired; - true - }, - _ => false, + Self::Header(vertex) => match vertex.importance { + HeaderImportance::Auxiliary => { + vertex.importance = HeaderImportance::TopRequired; + true } - } + _ => false, + }, Self::Justification(_) => false, } } pub fn try_set_required(&mut self) -> bool { match self { - Self::Empty(vertex) => { - match vertex.importance { - EmptyImportance::Auxiliary | EmptyImportance::TopRequired => { - vertex.importance = EmptyImportance::Required; - true - }, - _ => false, + Self::Empty(vertex) => match vertex.importance { + EmptyImportance::Auxiliary | EmptyImportance::TopRequired => { + vertex.importance = EmptyImportance::Required; + true } + _ => false, }, - Self::Header(vertex) => { - match vertex.importance { - HeaderImportance::Auxiliary | HeaderImportance::TopRequired => { - vertex.importance = HeaderImportance::Required; - true - }, - _ => false, + Self::Header(vertex) => match vertex.importance { + HeaderImportance::Auxiliary | HeaderImportance::TopRequired => { + vertex.importance = HeaderImportance::Required; + true } - } - Self::Justification(vertex) => { - match vertex.importance { - JustificationImportance::TopRequired => { - vertex.importance = JustificationImportance::Required; - true - }, - _ => false, + _ => false, + }, + Self::Justification(vertex) => match vertex.importance { + JustificationImportance::TopRequired => { + vertex.importance = JustificationImportance::Required; + true } - } + _ => false, + }, } } @@ -218,20 +252,39 @@ impl Vertex { true => Ok(()), false => Err(Error::InvalidParentID), }, - Self::Justification(vertex) => match parent_id == &vertex.justification_with_parent.parent { - true => Ok(()), - false => Err(Error::InvalidParentID), - }, + Self::Justification(vertex) => { + match parent_id == &vertex.justification_with_parent.parent { + true => Ok(()), + false => Err(Error::InvalidParentID), + } + } } } - pub fn try_insert_header(&mut self, parent_id: BlockIDFor, holder: Option) -> Result { + pub fn add_empty_holder(&mut self, holder: Option) -> bool { + if let Some(holder) = holder { + if let Self::Empty(vertex) = self { + return vertex.know_most.insert(holder); + } + } + false + } + + pub fn try_insert_header( + &mut self, + parent_id: BlockIDFor, + holder: Option, + ) -> Result { self.check_parent(&parent_id)?; let output = match self { Self::Empty(vertex) => { - *self = Self::Header(HeaderVertex { importance: vertex.importance.into(), parent: parent_id, know_most: vertex.know_most }); + *self = Self::Header(HeaderVertex { + importance: (&vertex.importance).into(), + parent: parent_id, + know_most: vertex.know_most.clone(), + }); Ok(true) - }, + } _ => Ok(false), }; if let Some(holder) = holder { @@ -244,26 +297,34 @@ impl Vertex { output } - pub fn try_insert_body(&mut self, parent_id: BlockIDFor, holder: Option) -> Result { + pub fn try_insert_body( + &mut self, + parent_id: BlockIDFor, + holder: Option, + ) -> Result { self.check_parent(&parent_id)?; let output = match self { Self::Empty(vertex) => { - *self = Self::Header(HeaderVertex { importance: HeaderImportance::Imported, parent: parent_id, know_most: vertex.know_most }); + *self = Self::Header(HeaderVertex { + importance: HeaderImportance::Imported, + parent: parent_id, + know_most: vertex.know_most.clone(), + }); Ok(true) - }, + } Self::Header(vertex) => match vertex.importance { HeaderImportance::Imported => Ok(false), _ => { vertex.importance = HeaderImportance::Imported; Ok(true) - }, + } }, Self::Justification(vertex) => match vertex.importance { JustificationImportance::Imported => Ok(false), _ => { vertex.importance = JustificationImportance::Imported; Ok(true) - }, + } }, }; if let Some(holder) = holder { @@ -286,14 +347,18 @@ impl Vertex { Self::Empty(_) | Self::Header(_) => { *self = Self::Justification(JustificationVertex { importance: match self { - Self::Empty(vertex) => Into::::into(vertex.importance).into(), - Self::Header(vertex) => vertex.importance.into(), + Self::Empty(vertex) => { + (&Into::::into(&vertex.importance)).into() + } + Self::Header(vertex) => (&vertex.importance).into(), Self::Justification(vertex) => Some(vertex.importance.clone()), - }.ok_or(Error::JustificationImportance)?, + } + .ok_or(Error::JustificationImportance)?, justification_with_parent, - know_most: HashSet::from_iter(holder.into_iter()) }); + know_most: HashSet::from_iter(holder.into_iter()), + }); Ok(true) - }, + } Self::Justification(_) => Ok(false), } } From fca62968c3882cd71f05d942bd66bafa87386210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Le=C5=9Bniak?= Date: Tue, 10 Jan 2023 16:00:54 +0100 Subject: [PATCH 10/14] grown forest --- finality-aleph/src/sync/forest/mod.rs | 384 +++++++++++++---------- finality-aleph/src/sync/forest/vertex.rs | 46 ++- 2 files changed, 251 insertions(+), 179 deletions(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 7f55b05edd..a7c447c07c 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -3,7 +3,7 @@ use std::collections::{ HashMap, HashSet, }; -use vertex::{Error as VertexError, Importance, Vertex as Vertex_}; +use vertex::{Error as VertexError, Importance, State, Vertex as Vertex_}; use super::{BlockIdentifier, Header, Justification, PeerID}; @@ -16,16 +16,7 @@ pub struct JustificationWithParent { pub parent: BlockIDFor, } -impl JustificationWithParent { - fn new(justification: J) -> Option { - justification.header().parent_id().map(|id| Self { - justification, - parent: id, - }) - } -} - -pub enum VertexState<'a, I: PeerID, J: Justification> { +enum VertexHandle<'a, I: PeerID, J: Justification> { HopelessFork, BelowMinimal, HighestFinalized, @@ -33,16 +24,52 @@ pub enum VertexState<'a, I: PeerID, J: Justification> { Candidate(OccupiedEntry<'a, BlockIDFor, Vertex>), } -pub enum Error { +pub enum VertexState { + HopelessFork, + BelowMinimal, + HighestFinalized, + Unknown, + Candidate(State, HashSet), +} + +impl<'a, I: PeerID, J: Justification> From> for VertexState { + fn from(handle: VertexHandle<'a, I, J>) -> Self { + match handle { + VertexHandle::HopelessFork => Self::HopelessFork, + VertexHandle::BelowMinimal => Self::BelowMinimal, + VertexHandle::HighestFinalized => Self::HighestFinalized, + VertexHandle::Unknown(_) => Self::Unknown, + VertexHandle::Candidate(entry) => Self::Candidate( + entry.get().vertex.state(), + entry.get().vertex.know_most().clone(), + ), + } + } +} + +pub enum Critical { Vertex(VertexError), MissingVertex, ShouldBePruned, InfiniteLoop, } +pub enum Error { + Critical(Critical), + HeaderMissingParentID, + IncorrectParentState, + ParentNotImported, +} + impl From for Error { fn from(err: VertexError) -> Self { - Self::Vertex(err) + Self::Critical(Critical::Vertex(err)) + } +} + +impl From for Error { + fn from(err: Critical) -> Self { + Self::Critical(err) } } @@ -77,8 +104,8 @@ impl Forest { } } - fn get_mut(&mut self, id: &BlockIDFor) -> VertexState { - use VertexState::*; + fn get_mut(&mut self, id: &BlockIDFor) -> VertexHandle { + use VertexHandle::*; if id == &self.root_id { HighestFinalized } else if id.number() <= self.root_id.number() { @@ -95,32 +122,32 @@ impl Forest { fn prune( &mut self, - id: BlockIDFor, + id: &BlockIDFor, modified: &mut HashSet>, ) -> Result<(), Error> { - if !self.vertices.contains_key(&id) { + if !self.vertices.contains_key(id) { return Ok(()); } let cap = self.vertices.len(); let mut to_be_pruned = Vec::with_capacity(cap); - to_be_pruned.push(id); + to_be_pruned.push(id.clone()); let mut index: usize = 0; while let Some(id) = to_be_pruned.get(index) { to_be_pruned.extend( self.vertices .get(id) - .ok_or(Error::MissingVertex)? + .ok_or(Critical::MissingVertex)? .children .iter() .cloned(), ); index += 1; if index > cap { - return Err(Error::InfiniteLoop); + Err(Critical::InfiniteLoop)?; } } for id in to_be_pruned.iter() { - self.vertices.remove(id).ok_or(Error::MissingVertex)?; + self.vertices.remove(id).ok_or(Critical::MissingVertex)?; } self.compost_bin.extend(to_be_pruned.clone()); modified.extend(to_be_pruned.into_iter()); @@ -129,21 +156,21 @@ impl Forest { fn try_add_parent( &mut self, - id: BlockIDFor, + id: &BlockIDFor, modified: &mut HashSet>, ) -> Result<(), Error> { - use VertexState::*; - if let Candidate(mut entry) = self.get_mut(&id) { + use VertexHandle::*; + if let Candidate(mut entry) = self.get_mut(id) { let vertex = entry.get_mut(); let importance = vertex.vertex.state().importance(); if let Some(parent_id) = vertex.vertex.parent().cloned() { match self.get_mut(&parent_id) { Unknown(_) => (), HighestFinalized => { - self.root_children.insert(id); + self.root_children.insert(id.clone()); } Candidate(mut entry) => { - entry.get_mut().children.insert(id); + entry.get_mut().children.insert(id.clone()); match importance { Importance::Required | Importance::TopRequired => { self.set_required_with_ancestors(&parent_id, modified)? @@ -163,7 +190,7 @@ impl Forest { id: &BlockIDFor, modified: &mut HashSet>, ) -> Result<(), Error> { - use VertexState::{Candidate, HighestFinalized}; + use VertexHandle::{Candidate, HighestFinalized}; let mut guard = id.number() as i64 - self.root_id.number() as i64; let mut id = id.clone(); let mut vertex; @@ -184,12 +211,12 @@ impl Forest { }; } HighestFinalized => break, - _ => return Err(Error::ShouldBePruned), + _ => Err(Critical::ShouldBePruned)?, }; // avoid infinite loop guard -= 1; if guard < 0 { - return Err(Error::InfiniteLoop); + Err(Critical::InfiniteLoop)?; } } Ok(()) @@ -200,7 +227,7 @@ impl Forest { id: &BlockIDFor, modified: &mut HashSet>, ) -> Result<(), Error> { - use VertexState::*; + use VertexHandle::*; if let Candidate(mut entry) = self.get_mut(id) { if entry.get_mut().vertex.try_set_top_required() { modified.insert(id.clone()); @@ -212,6 +239,48 @@ impl Forest { Ok(()) } + fn bump_vertex( + &mut self, + id: &BlockIDFor, + holder: Option, + modified: &mut HashSet>, + ) { + match self.get_mut(id) { + VertexHandle::Unknown(entry) => { + entry.insert(Vertex::new(holder)); + modified.insert(id.clone()); + } + VertexHandle::Candidate(mut entry) => { + entry.get_mut().vertex.add_block_id_holder(holder); + } + _ => (), + } + } + + fn process_header( + &mut self, + header: &J::Header, + ) -> Result<(BlockIDFor, BlockIDFor), Error> { + Ok(( + header.id(), + header.parent_id().ok_or(Error::HeaderMissingParentID)?, + )) + } + + fn process_justification( + &mut self, + justification: J, + ) -> Result<(BlockIDFor, JustificationWithParent), Error> { + let (id, parent) = self.process_header(justification.header())?; + Ok(( + id, + JustificationWithParent { + justification, + parent, + }, + )) + } + pub fn update_block_identifier( &mut self, id: BlockIDFor, @@ -219,15 +288,35 @@ impl Forest { required: bool, ) -> Result>, Error> { let mut modified = HashSet::new(); - match self.get_mut(&id) { - VertexState::Unknown(entry) => { - entry.insert(Vertex::new(holder)); + self.bump_vertex(&id, holder.clone(), &mut modified); + if required { + self.bump_required(&id, &mut modified)?; + } + if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { + entry.get_mut().vertex.add_block_id_holder(holder); + } + Ok(modified) + } + + pub fn update_header( + &mut self, + header: &J::Header, + holder: Option, + required: bool, + ) -> Result>, Error> { + let mut modified = HashSet::new(); + let (id, parent_id) = self.process_header(header)?; + self.bump_vertex(&parent_id, holder.clone(), &mut modified); + self.bump_vertex(&id, holder.clone(), &mut modified); + if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { + if entry + .get_mut() + .vertex + .try_insert_header(parent_id.clone(), holder)? + { modified.insert(id.clone()); } - VertexState::Candidate(mut entry) => { - entry.get_mut().vertex.add_empty_holder(holder); - } - _ => (), + self.try_add_parent(&id, &mut modified)?; } if required { self.bump_required(&id, &mut modified)?; @@ -235,138 +324,97 @@ impl Forest { Ok(modified) } - // pub fn update_header( - // &mut self, - // header: &J::Header, - // holder: Option, - // ) -> Result>, Error> { - // use VertexState::Candidate; - // let id = header.id(); - // let parent_id = header.parent_id().ok_or(Error::HeaderMissingParentID)?; - // let mut modified = self.update_block_identifier(parent_id, holder.clone())?; - // modified.extend(self.update_block_identifier(id.clone(), holder.clone())?); - // if let Candidate(vertex) = self.get_mut(&id) { - // let summary = vertex.try_insert_header(header, holder)?; - // self.process_transition(id, summary, &mut modified)?; - // } - // Ok(modified) - // } - - // pub fn update_body( - // &mut self, - // header: &J::Header, - // holder: Option, - // ) -> Result>, Error> { - // use VertexState::*; - // let id = header.id(); - // let mut modified = self.update_header(header, holder.clone())?; - // if let Candidate(vertex) = self.get_mut(&id) { - // let parent_id = vertex.parent().clone().ok_or(Error::MissingParent)?; - // match self.get_mut(&parent_id) { - // Unknown | HopelessFork | BelowMinimal => return Err(Error::MissingVertex), - // HighestFinalized => (), - // Candidate(parent_vertex) => { - // if !parent_vertex.is_imported() { - // return Err(Error::ParentShouldBeImported); - // } - // } - // }; - // } - // if let Candidate(vertex) = self.get_mut(&id) { - // let summary = vertex.try_insert_body(header, holder)?; - // self.process_transition(id, summary, &mut modified)?; - // } - // Ok(modified) - // } + pub fn update_body( + &mut self, + header: &J::Header, + holder: Option, + ) -> Result>, Error> { + use VertexHandle::*; + let mut modified = HashSet::new(); + let (id, parent_id) = self.process_header(header)?; + self.bump_vertex(&parent_id, holder.clone(), &mut modified); + self.bump_vertex(&id, holder.clone(), &mut modified); + match self.get_mut(&parent_id) { + Candidate(entry) => { + if !entry.get().vertex.is_imported() { + Err(Error::ParentNotImported)?; + } + } + HighestFinalized => (), + Unknown(_) | HopelessFork | BelowMinimal => Err(Error::IncorrectParentState)?, + } + if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { + if entry + .get_mut() + .vertex + .try_insert_body(parent_id.clone(), holder)? + { + modified.insert(id.clone()); + } + self.try_add_parent(&id, &mut modified)?; + } + Ok(modified) + } - // pub fn update_justification( - // &mut self, - // justification: J, - // holder: Option, - // ) -> Result>, Error> { - // use VertexState::Candidate; - // let header = justification.header(); - // let id = header.id(); - // let mut modified = self.update_header(header, holder.clone())?; - // if let Candidate(vertex) = self.get_mut(&id) { - // let summary = vertex.try_insert_justification(justification, holder)?; - // self.process_transition(id, summary, &mut modified)?; - // } - // Ok(modified) - // } + pub fn update_justification( + &mut self, + justification: J, + holder: Option, + ) -> Result>, Error> { + let mut modified = HashSet::new(); + let (id, justification_with_parent) = self.process_justification(justification)?; + self.bump_vertex( + &justification_with_parent.parent, + holder.clone(), + &mut modified, + ); + self.bump_vertex(&id, holder.clone(), &mut modified); + self.bump_required(&id, &mut modified)?; + if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { + if entry + .get_mut() + .vertex + .try_insert_justification(justification_with_parent, holder)? + { + modified.insert(id.clone()); + } + self.try_add_parent(&id, &mut modified)?; + } + Ok(modified) + } - // fn find_next_trunk_vertex( - // &mut self, - // id: &BlockIDFor, - // ) -> Result>, Error> { - // let children = self.children.get(id).ok_or(Error::MissingChildrenHashSet)?; - // for child_id in children.clone().iter() { - // if let VertexState::Candidate(vertex) = self.get_mut(child_id) { - // if vertex.is_full()? { - // return Ok(Some(child_id.clone())); - // } - // } - // } - // Ok(None) - // } + #[allow(clippy::type_complexity)] + pub fn try_finalize(&mut self) -> Result>)>, Error> { + let mut modified = HashSet::new(); + for child_id in self.root_children.clone().iter() { + if let VertexHandle::Candidate(entry) = self.get_mut(child_id) { + if let Some(justification) = entry.get().vertex.is_full() { + let (new_root_id, vertex) = entry.remove_entry(); + modified.insert(new_root_id.clone()); + self.root_id = new_root_id; + self.root_children = vertex.children; + let to_be_pruned: Vec> = self + .vertices + .keys() + .filter(|k| k.number() <= self.root_id.number()) + .cloned() + .collect(); + for id in to_be_pruned.iter() { + self.prune(id, &mut modified)?; + } + self.compost_bin = self + .compost_bin + .drain() + .filter(|x| x.number() > self.root_id.number()) + .collect(); + return Ok(Some((justification, modified))); + } + } + } + Ok(None) + } - // #[allow(clippy::type_complexity)] - // pub fn finalize(&mut self) -> Result, J)>>, Error> { - // // find trunk - // let mut trunk = vec![]; - // let mut id = self.root_id.clone(); - // while let Some(child_id) = self.find_next_trunk_vertex(&id)? { - // trunk.push(child_id.clone()); - // id = child_id; - // } - // // new root - // let new_root_id = match trunk.last() { - // Some(last) => last.clone(), - // None => return Ok(None), - // }; - // // pruned branches don't have to be connected to the trunk! - // let to_be_pruned: HashSet> = self - // .vertices - // .keys() - // .filter(|x| x.number() <= new_root_id.number()) - // .cloned() - // .collect(); - // for id in to_be_pruned.difference(&HashSet::from_iter(trunk.iter().cloned())) { - // if self.vertices.contains_key(id) { - // self.prune(id.clone())?; - // } - // } - // // keep new root children, as we'll remove the new root in a moment - // let new_root_children = self - // .children - // .get(&new_root_id) - // .ok_or(Error::MissingChildrenHashSet)? - // .clone(); - // // remove trunk - // let mut finalized = vec![]; - // for id in trunk.into_iter() { - // self.children - // .remove(&id) - // .ok_or(Error::MissingChildrenHashSet)?; - // match self - // .vertices - // .remove(&id) - // .ok_or(Error::MissingVertex)? - // .justification() - // { - // Some(justification) => finalized.push((id, justification)), - // None => return Err(Error::TrunkMissingJustification), - // }; - // } - // // set new root - // self.root_id = new_root_id.clone(); - // self.children.insert(new_root_id, new_root_children); - // // filter compost bin - // self.compost_bin = self - // .compost_bin - // .drain() - // .filter(|x| x.number() > self.root_id.number()) - // .collect(); - // Ok(Some(finalized)) - // } + pub fn state(&mut self, id: &BlockIDFor) -> VertexState { + self.get_mut(id).into() + } } diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 3d53638b69..de94ed06fe 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -4,23 +4,35 @@ use super::{Header, Justification, JustificationWithParent, PeerID}; type BlockIDFor = <::Header as Header>::Identifier; +/// Vertex Error. pub enum Error { + /// Known parent ID differs from the new one. InvalidParentID, + /// Vertex must not be Auxiliary while adding the justificaiton. JustificationImportance, } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +/// Empty Vertex Importance. pub enum EmptyImportance { + /// Not required. Auxiliary, + /// Required, all children are Auxiliary. TopRequired, + /// Required. Required, } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +/// Importance of Vertex with Header. pub enum HeaderImportance { + /// Not required. Auxiliary, + /// Required, all children are Auxiliary. TopRequired, + /// Required. Required, + /// Block has been imported. Imported, } @@ -35,9 +47,13 @@ impl From<&EmptyImportance> for HeaderImportance { } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +/// Importance of Vertex with Header and Justification. pub enum JustificationImportance { + /// Required, all children are Auxiliary. TopRequired, + /// Required. Required, + /// Block has been imported. Imported, } @@ -53,10 +69,15 @@ impl From<&HeaderImportance> for Option { } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +/// Vertex Importance. pub enum Importance { + /// Not required. Auxiliary, + /// Required, all children are Auxiliary. TopRequired, + /// Required. Required, + /// Block has been imported. Imported, } @@ -91,11 +112,13 @@ impl From<&JustificationImportance> for Importance { } } +/// Empty Vertex. pub struct EmptyVertex { importance: EmptyImportance, know_most: HashSet, } +/// Vertex with added Header. pub struct HeaderVertex { importance: HeaderImportance, parent: BlockIDFor, @@ -108,6 +131,7 @@ impl HeaderVertex { } } +/// Vertex with added Header and Justification. pub struct JustificationVertex { importance: JustificationImportance, justification_with_parent: JustificationWithParent, @@ -121,13 +145,18 @@ impl JustificationVertex { } #[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] +/// Vertex state. Describes its content and importance. pub enum State { + /// Empty content. Empty(EmptyImportance), + /// Containing Header. Header(HeaderImportance), + /// Containing Header and Justification. Justification(JustificationImportance), } impl State { + /// Return the importance of the described Vertex. pub fn importance(&self) -> Importance { use State::*; match self { @@ -161,10 +190,12 @@ impl Vertex { } } - pub fn is_full(&self) -> bool { + pub fn is_full(&self) -> Option { match self { - Self::Justification(vertex) => vertex.is_imported(), - _ => false, + Self::Justification(vertex) => { + Some(vertex.justification_with_parent.justification.clone()) + } + _ => None, } } @@ -184,13 +215,6 @@ impl Vertex { } } - pub fn justification(self) -> Option { - match self { - Self::Justification(vertex) => Some(vertex.justification_with_parent.justification), - _ => None, - } - } - pub fn know_most(&self) -> &HashSet { match self { Self::Empty(vertex) => &vertex.know_most, @@ -261,7 +285,7 @@ impl Vertex { } } - pub fn add_empty_holder(&mut self, holder: Option) -> bool { + pub fn add_block_id_holder(&mut self, holder: Option) -> bool { if let Some(holder) = holder { if let Self::Empty(vertex) = self { return vertex.know_most.insert(holder); From 7947f5636031dddb334fcb8d4362f01613d76974 Mon Sep 17 00:00:00 2001 From: timorl Date: Fri, 13 Jan 2023 14:56:30 +0100 Subject: [PATCH 11/14] Trim --- finality-aleph/src/sync/forest/mod.rs | 462 ++++++++------------ finality-aleph/src/sync/forest/vertex.rs | 521 ++++++++--------------- finality-aleph/src/sync/mod.rs | 2 +- 3 files changed, 369 insertions(+), 616 deletions(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index a7c447c07c..f47d351ce4 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -3,108 +3,85 @@ use std::collections::{ HashMap, HashSet, }; -use vertex::{Error as VertexError, Importance, State, Vertex as Vertex_}; - -use super::{BlockIdentifier, Header, Justification, PeerID}; +use crate::sync::{BlockIdentifier, Header, Justification, PeerId}; mod vertex; -type BlockIDFor = <::Header as Header>::Identifier; +use vertex::{JustificationAddResult, Vertex}; + +type BlockIdFor = <::Header as Header>::Identifier; pub struct JustificationWithParent { pub justification: J, - pub parent: BlockIDFor, + pub parent: BlockIdFor, } -enum VertexHandle<'a, I: PeerID, J: Justification> { +enum VertexHandle<'a, I: PeerId, J: Justification> { HopelessFork, BelowMinimal, HighestFinalized, - Unknown(VacantEntry<'a, BlockIDFor, Vertex>), - Candidate(OccupiedEntry<'a, BlockIDFor, Vertex>), -} - -pub enum VertexState { - HopelessFork, - BelowMinimal, - HighestFinalized, - Unknown, - Candidate(State, HashSet), -} - -impl<'a, I: PeerID, J: Justification> From> for VertexState { - fn from(handle: VertexHandle<'a, I, J>) -> Self { - match handle { - VertexHandle::HopelessFork => Self::HopelessFork, - VertexHandle::BelowMinimal => Self::BelowMinimal, - VertexHandle::HighestFinalized => Self::HighestFinalized, - VertexHandle::Unknown(_) => Self::Unknown, - VertexHandle::Candidate(entry) => Self::Candidate( - entry.get().vertex.state(), - entry.get().vertex.know_most().clone(), - ), - } - } + Unknown(VacantEntry<'a, BlockIdFor, VertexWithChildren>), + Candidate(OccupiedEntry<'a, BlockIdFor, VertexWithChildren>), } -pub enum Critical { - Vertex(VertexError), - MissingVertex, - ShouldBePruned, - InfiniteLoop, +/// Our interest in a block referred to by a vertex, including the information about whom we expect to have the block. +#[derive(Clone, PartialEq, Eq)] +pub enum Interest { + /// We are not interested in this block. + Uninterested, + /// We would like to have this block. + Required(HashSet), + /// We would like to have this block and its the highest on its branch. + TopRequired(HashSet), } +/// What can go wrong when inserting data into the forest. +#[derive(Clone, PartialEq, Eq)] pub enum Error { - Critical(Critical), - HeaderMissingParentID, + HeaderMissingParentId, IncorrectParentState, + IncorrectVertexState, ParentNotImported, } -impl From for Error { - fn from(err: VertexError) -> Self { - Self::Critical(Critical::Vertex(err)) - } +pub struct VertexWithChildren { + vertex: Vertex, + children: HashSet>, } -impl From for Error { - fn from(err: Critical) -> Self { - Self::Critical(err) - } -} - -pub struct Vertex { - vertex: Vertex_, - children: HashSet>, -} - -impl Vertex { - fn new(holder: Option) -> Self { +impl VertexWithChildren { + fn new() -> Self { Self { - vertex: Vertex_::new(holder), + vertex: Vertex::new(), children: HashSet::new(), } } + + fn add_child(&mut self, child: BlockIdFor) { + self.children.insert(child); + } } -pub struct Forest { - vertices: HashMap, Vertex>, - root_id: BlockIDFor, - root_children: HashSet>, - compost_bin: HashSet>, +pub struct Forest { + vertices: HashMap, VertexWithChildren>, + top_required: HashSet>, + root_id: BlockIdFor, + root_children: HashSet>, + compost_bin: HashSet>, } -impl Forest { - pub fn new(highest_justified: BlockIDFor) -> Self { +impl Forest { + pub fn new(highest_justified: BlockIdFor) -> Self { Self { vertices: HashMap::new(), + top_required: HashSet::new(), root_id: highest_justified, root_children: HashSet::new(), compost_bin: HashSet::new(), } } - fn get_mut(&mut self, id: &BlockIDFor) -> VertexHandle { + fn get_mut(&mut self, id: &BlockIdFor) -> VertexHandle { use VertexHandle::*; if id == &self.root_id { HighestFinalized @@ -120,301 +97,228 @@ impl Forest { } } - fn prune( - &mut self, - id: &BlockIDFor, - modified: &mut HashSet>, - ) -> Result<(), Error> { - if !self.vertices.contains_key(id) { - return Ok(()); - } - let cap = self.vertices.len(); - let mut to_be_pruned = Vec::with_capacity(cap); - to_be_pruned.push(id.clone()); - let mut index: usize = 0; - while let Some(id) = to_be_pruned.get(index) { - to_be_pruned.extend( - self.vertices - .get(id) - .ok_or(Critical::MissingVertex)? - .children - .iter() - .cloned(), - ); - index += 1; - if index > cap { - Err(Critical::InfiniteLoop)?; + fn prune(&mut self, id: &BlockIdFor) { + self.top_required.remove(id); + if let Some(VertexWithChildren { children, .. }) = self.vertices.remove(id) { + self.compost_bin.insert(id.clone()); + for child in children { + self.prune(&child); } } - for id in to_be_pruned.iter() { - self.vertices.remove(id).ok_or(Critical::MissingVertex)?; - } - self.compost_bin.extend(to_be_pruned.clone()); - modified.extend(to_be_pruned.into_iter()); - Ok(()) } - fn try_add_parent( - &mut self, - id: &BlockIDFor, - modified: &mut HashSet>, - ) -> Result<(), Error> { + fn connect_parent(&mut self, id: &BlockIdFor) { use VertexHandle::*; if let Candidate(mut entry) = self.get_mut(id) { let vertex = entry.get_mut(); - let importance = vertex.vertex.state().importance(); + let required = vertex.vertex.required(); if let Some(parent_id) = vertex.vertex.parent().cloned() { match self.get_mut(&parent_id) { - Unknown(_) => (), + Unknown(entry) => { + entry + .insert(VertexWithChildren::new()) + .add_child(id.clone()); + if required { + self.set_required(&parent_id); + } + } HighestFinalized => { self.root_children.insert(id.clone()); } Candidate(mut entry) => { - entry.get_mut().children.insert(id.clone()); - match importance { - Importance::Required | Importance::TopRequired => { - self.set_required_with_ancestors(&parent_id, modified)? - } - Importance::Auxiliary | Importance::Imported => (), - }; + entry.get_mut().add_child(id.clone()); + if required { + self.set_required(&parent_id); + } } - HopelessFork | BelowMinimal => self.prune(id, modified)?, + HopelessFork | BelowMinimal => self.prune(id), }; }; }; - Ok(()) } - fn set_required_with_ancestors( - &mut self, - id: &BlockIDFor, - modified: &mut HashSet>, - ) -> Result<(), Error> { - use VertexHandle::{Candidate, HighestFinalized}; - let mut guard = id.number() as i64 - self.root_id.number() as i64; - let mut id = id.clone(); - let mut vertex; - loop { - // check if we reached the root - match self.get_mut(&id) { - Candidate(mut entry) => { - vertex = entry.get_mut(); - // check if already required - match vertex.vertex.try_set_required() { - true => modified.insert(id.clone()), - false => break, - }; - // check if has parent - id = match vertex.vertex.parent() { - Some(id) => id.clone(), - None => break, - }; + fn set_required(&mut self, id: &BlockIdFor) { + self.top_required.remove(id); + if let VertexHandle::Candidate(mut entry) = self.get_mut(id) { + let vertex = entry.get_mut(); + if vertex.vertex.set_required() { + if let Some(id) = vertex.vertex.parent().cloned() { + self.set_required(&id); } - HighestFinalized => break, - _ => Err(Critical::ShouldBePruned)?, - }; - // avoid infinite loop - guard -= 1; - if guard < 0 { - Err(Critical::InfiniteLoop)?; } } - Ok(()) } - fn bump_required( - &mut self, - id: &BlockIDFor, - modified: &mut HashSet>, - ) -> Result<(), Error> { - use VertexHandle::*; - if let Candidate(mut entry) = self.get_mut(id) { - if entry.get_mut().vertex.try_set_top_required() { - modified.insert(id.clone()); - if let Some(parent_id) = entry.get_mut().vertex.parent().cloned() { - self.set_required_with_ancestors(&parent_id, modified)?; + fn set_top_required(&mut self, id: &BlockIdFor) -> bool { + match self.get_mut(id) { + VertexHandle::Candidate(mut entry) => match entry.get_mut().vertex.set_required() { + true => { + if let Some(parent_id) = entry.get_mut().vertex.parent().cloned() { + self.set_required(&parent_id); + } + self.top_required.insert(id.clone()); + true } - } + false => false, + }, + _ => false, } - Ok(()) } - fn bump_vertex( - &mut self, - id: &BlockIDFor, - holder: Option, - modified: &mut HashSet>, - ) { - match self.get_mut(id) { - VertexHandle::Unknown(entry) => { - entry.insert(Vertex::new(holder)); - modified.insert(id.clone()); - } - VertexHandle::Candidate(mut entry) => { - entry.get_mut().vertex.add_block_id_holder(holder); - } - _ => (), - } + fn insert_id(&mut self, id: BlockIdFor, holder: Option) { + self.vertices + .entry(id) + .or_insert_with(VertexWithChildren::new) + .vertex + .add_block_holder(holder); } fn process_header( &mut self, header: &J::Header, - ) -> Result<(BlockIDFor, BlockIDFor), Error> { + ) -> Result<(BlockIdFor, BlockIdFor), Error> { Ok(( header.id(), - header.parent_id().ok_or(Error::HeaderMissingParentID)?, - )) - } - - fn process_justification( - &mut self, - justification: J, - ) -> Result<(BlockIDFor, JustificationWithParent), Error> { - let (id, parent) = self.process_header(justification.header())?; - Ok(( - id, - JustificationWithParent { - justification, - parent, - }, + header.parent_id().ok_or(Error::HeaderMissingParentId)?, )) } + /// Updates the provider block identifier, returns whether it became a new top required. pub fn update_block_identifier( &mut self, - id: BlockIDFor, + id: &BlockIdFor, holder: Option, required: bool, - ) -> Result>, Error> { - let mut modified = HashSet::new(); - self.bump_vertex(&id, holder.clone(), &mut modified); - if required { - self.bump_required(&id, &mut modified)?; + ) -> bool { + self.insert_id(id.clone(), holder); + match required { + true => self.set_top_required(id), + false => false, } - if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { - entry.get_mut().vertex.add_block_id_holder(holder); - } - Ok(modified) } + /// Updates the provided header, returns whether it became a new top required. pub fn update_header( &mut self, header: &J::Header, holder: Option, required: bool, - ) -> Result>, Error> { - let mut modified = HashSet::new(); + ) -> Result { let (id, parent_id) = self.process_header(header)?; - self.bump_vertex(&parent_id, holder.clone(), &mut modified); - self.bump_vertex(&id, holder.clone(), &mut modified); + self.insert_id(id.clone(), holder.clone()); if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { - if entry - .get_mut() - .vertex - .try_insert_header(parent_id.clone(), holder)? - { - modified.insert(id.clone()); - } - self.try_add_parent(&id, &mut modified)?; + entry.get_mut().vertex.insert_header(parent_id, holder); + self.connect_parent(&id); } - if required { - self.bump_required(&id, &mut modified)?; + match required { + true => Ok(self.set_top_required(&id)), + false => Ok(false), } - Ok(modified) } - pub fn update_body( - &mut self, - header: &J::Header, - holder: Option, - ) -> Result>, Error> { + /// Updates the vertex related to the provided header marking it as imported. Returns whether + /// it is now finalizable, or errors when it's impossible to do consistently. + pub fn update_body(&mut self, header: &J::Header) -> Result { use VertexHandle::*; - let mut modified = HashSet::new(); let (id, parent_id) = self.process_header(header)?; - self.bump_vertex(&parent_id, holder.clone(), &mut modified); - self.bump_vertex(&id, holder.clone(), &mut modified); + self.update_header(header, None, false)?; match self.get_mut(&parent_id) { Candidate(entry) => { - if !entry.get().vertex.is_imported() { - Err(Error::ParentNotImported)?; + if !entry.get().vertex.imported() { + return Err(Error::ParentNotImported); } } HighestFinalized => (), - Unknown(_) | HopelessFork | BelowMinimal => Err(Error::IncorrectParentState)?, + Unknown(_) | HopelessFork | BelowMinimal => return Err(Error::IncorrectParentState), } - if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { - if entry - .get_mut() - .vertex - .try_insert_body(parent_id.clone(), holder)? - { - modified.insert(id.clone()); - } - self.try_add_parent(&id, &mut modified)?; + match self.get_mut(&id) { + Candidate(mut entry) => Ok(entry.get_mut().vertex.insert_body(parent_id.clone())), + _ => Err(Error::IncorrectVertexState), } - Ok(modified) } + /// Updates the provided justification, returns whether either finalization is now possible or + /// the vertex became a new top required. pub fn update_justification( &mut self, justification: J, holder: Option, - ) -> Result>, Error> { - let mut modified = HashSet::new(); - let (id, justification_with_parent) = self.process_justification(justification)?; - self.bump_vertex( - &justification_with_parent.parent, - holder.clone(), - &mut modified, - ); - self.bump_vertex(&id, holder.clone(), &mut modified); - self.bump_required(&id, &mut modified)?; - if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { - if entry - .get_mut() - .vertex - .try_insert_justification(justification_with_parent, holder)? - { - modified.insert(id.clone()); + ) -> Result { + use JustificationAddResult::*; + let (id, parent_id) = self.process_header(justification.header())?; + self.update_header(justification.header(), None, false)?; + match self.get_mut(&id) { + VertexHandle::Candidate(mut entry) => { + match entry.get_mut().vertex.insert_justification( + parent_id.clone(), + justification, + holder, + ) { + Noop => Ok(Noop), + Required => { + self.top_required.insert(id.clone()); + self.set_required(&parent_id); + Ok(Required) + } + Finalizable => { + self.top_required.remove(&id); + Ok(Finalizable) + } + } } - self.try_add_parent(&id, &mut modified)?; + _ => Ok(Noop), } - Ok(modified) } - #[allow(clippy::type_complexity)] - pub fn try_finalize(&mut self) -> Result>)>, Error> { - let mut modified = HashSet::new(); - for child_id in self.root_children.clone().iter() { - if let VertexHandle::Candidate(entry) = self.get_mut(child_id) { - if let Some(justification) = entry.get().vertex.is_full() { - let (new_root_id, vertex) = entry.remove_entry(); - modified.insert(new_root_id.clone()); - self.root_id = new_root_id; - self.root_children = vertex.children; - let to_be_pruned: Vec> = self - .vertices - .keys() - .filter(|k| k.number() <= self.root_id.number()) - .cloned() - .collect(); - for id in to_be_pruned.iter() { - self.prune(id, &mut modified)?; + fn prune_level(&mut self, level: u32) { + let to_prune: Vec<_> = self + .vertices + .keys() + .filter(|k| k.number() <= level) + .cloned() + .collect(); + for id in to_prune.into_iter() { + self.prune(&id); + } + self.compost_bin.retain(|k| k.number() > level); + } + + /// Attempt to finalize one block, returns the correct justification if successful. + pub fn try_finalize(&mut self) -> Option { + for child_id in self.root_children.clone().into_iter() { + if let Some(VertexWithChildren { vertex, children }) = self.vertices.remove(&child_id) { + match vertex.ready() { + Ok(justification) => { + self.root_id = child_id; + self.root_children = children; + self.prune_level(self.root_id.number()); + return Some(justification); + } + Err(vertex) => { + self.vertices + .insert(child_id, VertexWithChildren { vertex, children }); } - self.compost_bin = self - .compost_bin - .drain() - .filter(|x| x.number() > self.root_id.number()) - .collect(); - return Ok(Some((justification, modified))); } } } - Ok(None) + None } - pub fn state(&mut self, id: &BlockIDFor) -> VertexState { - self.get_mut(id).into() + /// How much interest we have for the block. + pub fn state(&mut self, id: &BlockIdFor) -> Interest { + match self.get_mut(id) { + VertexHandle::Candidate(entry) => { + let vertex = &entry.get().vertex; + let know_most = vertex.know_most().clone(); + match vertex.required() { + true => match self.top_required.contains(id) { + true => Interest::TopRequired(know_most), + false => Interest::Required(know_most), + }, + false => Interest::Uninterested, + } + } + _ => Interest::Uninterested, + } } } diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index de94ed06fe..1203900ddc 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -1,389 +1,238 @@ use std::collections::HashSet; -use super::{Header, Justification, JustificationWithParent, PeerID}; +use crate::sync::{forest::BlockIdFor, Justification, PeerId}; -type BlockIDFor = <::Header as Header>::Identifier; - -/// Vertex Error. -pub enum Error { - /// Known parent ID differs from the new one. - InvalidParentID, - /// Vertex must not be Auxiliary while adding the justificaiton. - JustificationImportance, -} - -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -/// Empty Vertex Importance. -pub enum EmptyImportance { - /// Not required. - Auxiliary, - /// Required, all children are Auxiliary. - TopRequired, - /// Required. - Required, -} - -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -/// Importance of Vertex with Header. -pub enum HeaderImportance { - /// Not required. - Auxiliary, - /// Required, all children are Auxiliary. - TopRequired, - /// Required. - Required, - /// Block has been imported. - Imported, -} - -impl From<&EmptyImportance> for HeaderImportance { - fn from(importance: &EmptyImportance) -> Self { - match importance { - EmptyImportance::Auxiliary => Self::Auxiliary, - EmptyImportance::TopRequired => Self::TopRequired, - EmptyImportance::Required => Self::Required, - } - } -} - -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -/// Importance of Vertex with Header and Justification. -pub enum JustificationImportance { - /// Required, all children are Auxiliary. - TopRequired, - /// Required. - Required, - /// Block has been imported. - Imported, -} - -impl From<&HeaderImportance> for Option { - fn from(importance: &HeaderImportance) -> Self { - match importance { - HeaderImportance::Auxiliary => None, - HeaderImportance::TopRequired => Some(JustificationImportance::TopRequired), - HeaderImportance::Required => Some(JustificationImportance::Required), - HeaderImportance::Imported => Some(JustificationImportance::Imported), - } - } -} - -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -/// Vertex Importance. -pub enum Importance { - /// Not required. +#[derive(Clone, Copy, PartialEq, Eq)] +enum HeaderImportance { Auxiliary, - /// Required, all children are Auxiliary. - TopRequired, - /// Required. Required, - /// Block has been imported. Imported, } -impl From<&EmptyImportance> for Importance { - fn from(importance: &EmptyImportance) -> Self { - match importance { - EmptyImportance::Auxiliary => Self::Auxiliary, - EmptyImportance::TopRequired => Self::TopRequired, - EmptyImportance::Required => Self::Required, - } - } -} - -impl From<&HeaderImportance> for Importance { - fn from(importance: &HeaderImportance) -> Self { - match importance { - HeaderImportance::Auxiliary => Self::Auxiliary, - HeaderImportance::TopRequired => Self::TopRequired, - HeaderImportance::Required => Self::Required, - HeaderImportance::Imported => Self::Imported, - } - } -} - -impl From<&JustificationImportance> for Importance { - fn from(importance: &JustificationImportance) -> Self { - match importance { - JustificationImportance::TopRequired => Self::TopRequired, - JustificationImportance::Required => Self::Required, - JustificationImportance::Imported => Self::Imported, - } - } -} - -/// Empty Vertex. -pub struct EmptyVertex { - importance: EmptyImportance, - know_most: HashSet, -} - -/// Vertex with added Header. -pub struct HeaderVertex { - importance: HeaderImportance, - parent: BlockIDFor, - know_most: HashSet, -} - -impl HeaderVertex { - fn is_imported(&self) -> bool { - self.importance == HeaderImportance::Imported - } -} - -/// Vertex with added Header and Justification. -pub struct JustificationVertex { - importance: JustificationImportance, - justification_with_parent: JustificationWithParent, +#[derive(Clone, PartialEq, Eq)] +enum InnerVertex { + /// Empty Vertex. + Empty { required: bool }, + /// Vertex with added Header. + Header { + importance: HeaderImportance, + parent: BlockIdFor, + }, + /// Vertex with added Header and Justification. + Justification { + imported: bool, + justification: J, + parent: BlockIdFor, + }, +} + +/// The vomplete vertex, including metadata about peers that know most about the data it refers to. +#[derive(Clone, PartialEq, Eq)] +pub struct Vertex { + inner: InnerVertex, know_most: HashSet, } -impl JustificationVertex { - fn is_imported(&self) -> bool { - self.importance == JustificationImportance::Imported - } -} - -#[derive(Clone, std::cmp::PartialEq, std::cmp::Eq)] -/// Vertex state. Describes its content and importance. -pub enum State { - /// Empty content. - Empty(EmptyImportance), - /// Containing Header. - Header(HeaderImportance), - /// Containing Header and Justification. - Justification(JustificationImportance), +/// What can happen when we add a justification. +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum JustificationAddResult { + Noop, + Required, + Finalizable, } -impl State { - /// Return the importance of the described Vertex. - pub fn importance(&self) -> Importance { - use State::*; - match self { - Empty(importance) => importance.into(), - Header(importance) => importance.into(), - Justification(importance) => importance.into(), +impl Vertex { + /// Create a new empty vertex. + pub fn new() -> Self { + Vertex { + inner: InnerVertex::Empty { required: false }, + know_most: HashSet::new(), } } -} -pub enum Vertex { - Empty(EmptyVertex), - Header(HeaderVertex), - Justification(JustificationVertex), -} - -impl Vertex { - pub fn new(holder: Option) -> Self { - Self::Empty(EmptyVertex { - importance: EmptyImportance::Auxiliary, - know_most: HashSet::from_iter(holder.into_iter()), - }) - } - - pub fn state(&self) -> State { - use Vertex::*; - match self { - Empty(vertex) => State::Empty(vertex.importance.clone()), - Header(vertex) => State::Header(vertex.importance.clone()), - Justification(vertex) => State::Justification(vertex.importance.clone()), - } + /// Whether the vertex is required. + pub fn required(&self) -> bool { + use InnerVertex::*; + matches!( + self.inner, + Empty { required: true } + | Header { + importance: HeaderImportance::Required, + .. + } + | Justification { + imported: false, + .. + } + ) } - pub fn is_full(&self) -> Option { - match self { - Self::Justification(vertex) => { - Some(vertex.justification_with_parent.justification.clone()) - } - _ => None, - } + /// Whether the vertex is imported. + pub fn imported(&self) -> bool { + use InnerVertex::*; + matches!( + self.inner, + Header { + importance: HeaderImportance::Imported, + .. + } | Justification { imported: true, .. } + ) } - pub fn is_imported(&self) -> bool { - match self { - Self::Empty(_) => false, - Self::Header(vertex) => vertex.is_imported(), - Self::Justification(vertex) => vertex.is_imported(), + /// Deconstructs the vertex into a justification if it is ready to be imported, + /// i.e. the related block has already been imported, otherwise returns it. + pub fn ready(self) -> Result { + match self.inner { + InnerVertex::Justification { + imported: true, + justification, + .. + } => Ok(justification), + _ => Err(self), } } - pub fn parent(&self) -> Option<&BlockIDFor> { - match self { - Self::Empty(_) => None, - Self::Header(vertex) => Some(&vertex.parent), - Self::Justification(vertex) => Some(&vertex.justification_with_parent.parent), + /// The parent of the vertex, if known. + pub fn parent(&self) -> Option<&BlockIdFor> { + match &self.inner { + InnerVertex::Empty { .. } => None, + InnerVertex::Header { parent, .. } => Some(parent), + InnerVertex::Justification { parent, .. } => Some(parent), } } + /// The list of peers which know most about the data this vertex refers to. pub fn know_most(&self) -> &HashSet { - match self { - Self::Empty(vertex) => &vertex.know_most, - Self::Header(vertex) => &vertex.know_most, - Self::Justification(vertex) => &vertex.know_most, - } - } - - pub fn try_set_top_required(&mut self) -> bool { - match self { - Self::Empty(vertex) => match vertex.importance { - EmptyImportance::Auxiliary => { - vertex.importance = EmptyImportance::TopRequired; - true - } - _ => false, - }, - Self::Header(vertex) => match vertex.importance { - HeaderImportance::Auxiliary => { - vertex.importance = HeaderImportance::TopRequired; - true - } - _ => false, - }, - Self::Justification(_) => false, - } - } - - pub fn try_set_required(&mut self) -> bool { - match self { - Self::Empty(vertex) => match vertex.importance { - EmptyImportance::Auxiliary | EmptyImportance::TopRequired => { - vertex.importance = EmptyImportance::Required; - true - } - _ => false, - }, - Self::Header(vertex) => match vertex.importance { - HeaderImportance::Auxiliary | HeaderImportance::TopRequired => { - vertex.importance = HeaderImportance::Required; - true - } - _ => false, - }, - Self::Justification(vertex) => match vertex.importance { - JustificationImportance::TopRequired => { - vertex.importance = JustificationImportance::Required; - true - } - _ => false, - }, - } + &self.know_most } - fn check_parent(&self, parent_id: &BlockIDFor) -> Result<(), Error> { - match self { - Self::Empty(_) => Ok(()), - Self::Header(vertex) => match parent_id == &vertex.parent { - true => Ok(()), - false => Err(Error::InvalidParentID), - }, - Self::Justification(vertex) => { - match parent_id == &vertex.justification_with_parent.parent { - true => Ok(()), - false => Err(Error::InvalidParentID), - } + /// Set the vertex to be required, returns whether anything changed, i.e. the vertex was not + /// required or imported before. + pub fn set_required(&mut self) -> bool { + use InnerVertex::*; + match &self.inner { + Empty { required: false } => { + self.inner = Empty { required: true }; + true } + Header { + importance: HeaderImportance::Auxiliary, + parent, + } => { + self.inner = Header { + importance: HeaderImportance::Required, + parent: parent.clone(), + }; + true + } + _ => false, } } - pub fn add_block_id_holder(&mut self, holder: Option) -> bool { + /// Adds a peer that knows most about the block this vertex refers to. Does nothing if we + /// already have a justification. + pub fn add_block_holder(&mut self, holder: Option) { if let Some(holder) = holder { - if let Self::Empty(vertex) = self { - return vertex.know_most.insert(holder); + if !matches!(self.inner, InnerVertex::Justification { .. }) { + self.know_most.insert(holder); } } - false } - pub fn try_insert_header( - &mut self, - parent_id: BlockIDFor, - holder: Option, - ) -> Result { - self.check_parent(&parent_id)?; - let output = match self { - Self::Empty(vertex) => { - *self = Self::Header(HeaderVertex { - importance: (&vertex.importance).into(), - parent: parent_id, - know_most: vertex.know_most.clone(), - }); - Ok(true) - } - _ => Ok(false), - }; - if let Some(holder) = holder { - match self { - Self::Empty(vertex) => vertex.know_most.insert(holder), - Self::Header(vertex) => vertex.know_most.insert(holder), - Self::Justification(_) => false, + /// Adds the information the header provides to the vertex. + pub fn insert_header(&mut self, parent: BlockIdFor, holder: Option) { + self.add_block_holder(holder); + if let InnerVertex::Empty { required } = self.inner { + let importance = match required { + false => HeaderImportance::Auxiliary, + true => HeaderImportance::Required, }; + self.inner = InnerVertex::Header { importance, parent }; } - output } - pub fn try_insert_body( - &mut self, - parent_id: BlockIDFor, - holder: Option, - ) -> Result { - self.check_parent(&parent_id)?; - let output = match self { - Self::Empty(vertex) => { - *self = Self::Header(HeaderVertex { + /// Adds the information the header provides to the vertex and marks it as imported. Returns + /// whether finalization is now possible. + pub fn insert_body(&mut self, parent: BlockIdFor) -> bool { + use InnerVertex::*; + match &self.inner { + Empty { .. } | Header { .. } => { + self.inner = Header { + parent, importance: HeaderImportance::Imported, - parent: parent_id, - know_most: vertex.know_most.clone(), - }); - Ok(true) + }; + false } - Self::Header(vertex) => match vertex.importance { - HeaderImportance::Imported => Ok(false), - _ => { - vertex.importance = HeaderImportance::Imported; - Ok(true) - } - }, - Self::Justification(vertex) => match vertex.importance { - JustificationImportance::Imported => Ok(false), - _ => { - vertex.importance = JustificationImportance::Imported; - Ok(true) - } - }, - }; - if let Some(holder) = holder { - match self { - Self::Empty(vertex) => vertex.know_most.insert(holder), - Self::Header(vertex) => vertex.know_most.insert(holder), - Self::Justification(_) => false, - }; + Justification { + imported: false, + parent, + justification, + } => { + self.inner = Justification { + imported: true, + parent: parent.clone(), + justification: justification.clone(), + }; + true + } + _ => false, } - output } - pub fn try_insert_justification( + /// Adds a justification to the vertex. Returns whether either the finalization is now possible + /// or the vertex became required. + pub fn insert_justification( &mut self, - justification_with_parent: JustificationWithParent, + parent: BlockIdFor, + justification: J, holder: Option, - ) -> Result { - self.check_parent(&justification_with_parent.parent)?; - match self { - Self::Empty(_) | Self::Header(_) => { - *self = Self::Justification(JustificationVertex { - importance: match self { - Self::Empty(vertex) => { - (&Into::::into(&vertex.importance)).into() - } - Self::Header(vertex) => (&vertex.importance).into(), - Self::Justification(vertex) => Some(vertex.importance.clone()), - } - .ok_or(Error::JustificationImportance)?, - justification_with_parent, - know_most: HashSet::from_iter(holder.into_iter()), - }); - Ok(true) + ) -> JustificationAddResult { + use InnerVertex::*; + match self.inner { + Justification { .. } => { + if let Some(holder) = holder { + self.know_most.insert(holder); + } + JustificationAddResult::Noop + } + Empty { required: true } + | Header { + importance: HeaderImportance::Required, + .. + } => { + self.inner = Justification { + imported: false, + parent, + justification, + }; + self.know_most = holder.into_iter().collect(); + JustificationAddResult::Noop + } + Empty { required: false } + | Header { + importance: HeaderImportance::Auxiliary, + .. + } => { + self.inner = Justification { + imported: false, + parent, + justification, + }; + self.know_most = holder.into_iter().collect(); + JustificationAddResult::Required + } + Header { + importance: HeaderImportance::Imported, + .. + } => { + self.inner = Justification { + imported: true, + parent, + justification, + }; + // No need to modify know_most, as we now know everything we need. + JustificationAddResult::Finalizable } - Self::Justification(_) => Ok(false), } } } diff --git a/finality-aleph/src/sync/mod.rs b/finality-aleph/src/sync/mod.rs index d4d61caad7..604bc50d6e 100644 --- a/finality-aleph/src/sync/mod.rs +++ b/finality-aleph/src/sync/mod.rs @@ -11,7 +11,7 @@ mod ticker; const LOG_TARGET: &str = "aleph-block-sync"; /// The identifier of a connected peer. -pub trait PeerID: Clone + Hash + Eq {} +pub trait PeerId: Clone + Hash + Eq {} /// The identifier of a block, the least amount of knowledge we can have about a block. pub trait BlockIdentifier: Clone + Hash + Debug + Eq { From 0af74a8f71f7865d8d95677a6f99c879d985429b Mon Sep 17 00:00:00 2001 From: timorl Date: Mon, 16 Jan 2023 14:44:05 +0100 Subject: [PATCH 12/14] Tests --- finality-aleph/src/sync/forest/mod.rs | 342 ++++++++++++++++++++++- finality-aleph/src/sync/forest/vertex.rs | 244 +++++++++++++++- finality-aleph/src/sync/mock.rs | 104 +++++++ finality-aleph/src/sync/mod.rs | 4 + 4 files changed, 688 insertions(+), 6 deletions(-) create mode 100644 finality-aleph/src/sync/mock.rs diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index f47d351ce4..a2d56d1832 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -25,7 +25,7 @@ enum VertexHandle<'a, I: PeerId, J: Justification> { } /// Our interest in a block referred to by a vertex, including the information about whom we expect to have the block. -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq, Debug)] pub enum Interest { /// We are not interested in this block. Uninterested, @@ -36,7 +36,7 @@ pub enum Interest { } /// What can go wrong when inserting data into the forest. -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq, Debug)] pub enum Error { HeaderMissingParentId, IncorrectParentState, @@ -322,3 +322,341 @@ impl Forest { } } } + +#[cfg(test)] +mod tests { + use super::{Error, Forest, Interest::*, JustificationAddResult}; + use crate::sync::{ + mock::{MockHeader, MockJustification, MockPeerId}, + Header, Justification, + }; + + type MockForest = Forest; + + fn setup() -> (MockHeader, MockForest) { + let header = MockHeader::random_parentless(0); + let forest = Forest::new(header.id()); + (header, forest) + } + + #[test] + fn initially_empty() { + let (initial_header, mut forest) = setup(); + assert!(forest.try_finalize().is_none()); + assert_eq!(forest.state(&initial_header.id()), Uninterested); + } + + #[test] + fn accepts_first_unimportant_id() { + let (initial_header, mut forest) = setup(); + let child = initial_header.random_child(); + let peer_id = rand::random(); + assert!(!forest.update_block_identifier(&child.id(), Some(peer_id), false)); + assert!(forest.try_finalize().is_none()); + assert_eq!(forest.state(&child.id()), Uninterested); + } + + #[test] + fn accepts_first_important_id() { + let (initial_header, mut forest) = setup(); + let child = initial_header.random_child(); + let peer_id = rand::random(); + assert!(forest.update_block_identifier(&child.id(), Some(peer_id), true)); + assert!(forest.try_finalize().is_none()); + match forest.state(&child.id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + assert!(!forest.update_block_identifier(&child.id(), Some(peer_id), true)); + } + + #[test] + fn accepts_first_unimportant_header() { + let (initial_header, mut forest) = setup(); + let child = initial_header.random_child(); + let peer_id = rand::random(); + assert!(!forest + .update_header(&child, Some(peer_id), false) + .expect("header was correct")); + assert!(forest.try_finalize().is_none()); + assert_eq!(forest.state(&child.id()), Uninterested); + } + + #[test] + fn accepts_first_important_header() { + let (initial_header, mut forest) = setup(); + let child = initial_header.random_child(); + let peer_id = rand::random(); + assert!(forest + .update_header(&child, Some(peer_id), true) + .expect("header was correct")); + assert!(forest.try_finalize().is_none()); + match forest.state(&child.id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + assert!(!forest.update_block_identifier(&child.id(), Some(peer_id), true)); + } + + #[test] + fn rejects_parentless_header() { + let (_, mut forest) = setup(); + let parentless = MockHeader::random_parentless(43); + let peer_id = rand::random(); + assert!(matches!( + forest.update_header(&parentless, Some(peer_id), true), + Err(Error::HeaderMissingParentId) + )); + } + + #[test] + fn accepts_first_justification() { + let (initial_header, mut forest) = setup(); + let child = MockJustification::for_header(initial_header.random_child()); + let peer_id = rand::random(); + assert_eq!( + forest + .update_justification(child.clone(), Some(peer_id)) + .expect("header was correct"), + JustificationAddResult::Required + ); + assert!(forest.try_finalize().is_none()); + match forest.state(&child.header().id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + } + + #[test] + fn rejects_parentless_justification() { + let (_, mut forest) = setup(); + let parentless = MockJustification::for_header(MockHeader::random_parentless(43)); + let peer_id = rand::random(); + assert!(matches!( + forest.update_justification(parentless, Some(peer_id)), + Err(Error::HeaderMissingParentId) + )); + } + + #[test] + fn accepts_first_body() { + let (initial_header, mut forest) = setup(); + let child = initial_header.random_child(); + assert!(!forest.update_body(&child).expect("header was correct")); + assert!(forest.try_finalize().is_none()); + assert_eq!(forest.state(&child.id()), Uninterested); + } + + #[test] + fn finalizes_first_block() { + let (initial_header, mut forest) = setup(); + let child = MockJustification::for_header(initial_header.random_child()); + let peer_id = rand::random(); + assert_eq!( + forest + .update_justification(child.clone(), Some(peer_id)) + .expect("header was correct"), + JustificationAddResult::Required + ); + assert!(forest.try_finalize().is_none()); + match forest.state(&child.header().id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + assert!(forest + .update_body(child.header()) + .expect("header was correct")); + assert_eq!(forest.try_finalize().expect("the block is ready"), child); + } + + #[test] + fn prunes_forks() { + let (initial_header, mut forest) = setup(); + let child = MockJustification::for_header(initial_header.random_child()); + let fork_child = initial_header.random_child(); + let peer_id = rand::random(); + let fork_peer_id = rand::random(); + assert!(forest + .update_header(&fork_child, Some(fork_peer_id), true) + .expect("header was correct")); + match forest.state(&fork_child.id()) { + TopRequired(holders) => assert!(holders.contains(&fork_peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + assert_eq!( + forest + .update_justification(child.clone(), Some(peer_id)) + .expect("header was correct"), + JustificationAddResult::Required + ); + assert!(forest + .update_body(child.header()) + .expect("header was correct")); + assert_eq!(forest.try_finalize().expect("the block is ready"), child); + assert_eq!(forest.state(&fork_child.id()), Uninterested); + assert!(!forest + .update_header(&fork_child, Some(fork_peer_id), true) + .expect("header was correct")); + } + + #[test] + fn uninterested_in_forks() { + let (initial_header, mut forest) = setup(); + let fork_branch: Vec<_> = initial_header.random_branch().take(2).collect(); + for header in &fork_branch { + let peer_id = rand::random(); + assert!(forest + .update_header(header, Some(peer_id), true) + .expect("header was correct")); + match forest.state(&header.id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + } + let child = MockJustification::for_header(initial_header.random_child()); + let peer_id = rand::random(); + assert_eq!( + forest + .update_justification(child.clone(), Some(peer_id)) + .expect("header was correct"), + JustificationAddResult::Required + ); + assert!(forest + .update_body(child.header()) + .expect("header was correct")); + assert_eq!(forest.try_finalize().expect("the block is ready"), child); + for header in fork_branch { + assert_eq!(forest.state(&header.id()), Uninterested); + } + } + + #[test] + fn updates_interest_on_parent_connect() { + let (initial_header, mut forest) = setup(); + let branch: Vec<_> = initial_header.random_branch().take(4).collect(); + let header = &branch[0]; + let peer_id = rand::random(); + assert!(forest + .update_header(header, Some(peer_id), true) + .expect("header was correct")); + match forest.state(&header.id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + let header = &branch[1]; + let peer_id = rand::random(); + assert!(!forest + .update_header(header, Some(peer_id), false) + .expect("header was correct")); + assert_eq!(forest.state(&header.id()), Uninterested); + let header = &branch[3]; + let peer_id = rand::random(); + assert!(forest + .update_header(header, Some(peer_id), true) + .expect("header was correct")); + match forest.state(&header.id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + let header = &branch[2]; + let peer_id = rand::random(); + assert!(!forest + .update_header(header, Some(peer_id), false) + .expect("header was correct")); + for header in branch.iter().take(3) { + assert!(matches!(forest.state(&header.id()), Required(_))); + } + assert!(matches!(forest.state(&branch[3].id()), TopRequired(_))); + } + + const HUGE_BRANCH_LENGTH: usize = 1800; + + #[test] + fn finalizes_huge_branch() { + let (initial_header, mut forest) = setup(); + let justifications: Vec<_> = initial_header + .random_branch() + .map(MockJustification::for_header) + .take(HUGE_BRANCH_LENGTH) + .collect(); + for justification in &justifications { + let peer_id = rand::random(); + assert_eq!( + forest + .update_justification(justification.clone(), Some(peer_id)) + .expect("header was correct"), + JustificationAddResult::Required + ); + match forest.state(&justification.header().id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + assert!(forest + .update_body(justification.header()) + .expect("header was correct")); + } + for justification in justifications { + assert_eq!( + forest.try_finalize().expect("the block is ready"), + justification + ); + } + } + + #[test] + fn prunes_huge_branch() { + let (initial_header, mut forest) = setup(); + for header in initial_header.random_branch().take(HUGE_BRANCH_LENGTH) { + let peer_id = rand::random(); + assert!(!forest + .update_header(&header, Some(peer_id), false) + .expect("header was correct")); + assert!(forest.try_finalize().is_none()); + } + let child = MockJustification::for_header(initial_header.random_child()); + let peer_id = rand::random(); + assert_eq!( + forest + .update_justification(child.clone(), Some(peer_id)) + .expect("header was correct"), + JustificationAddResult::Required + ); + assert!(forest.try_finalize().is_none()); + match forest.state(&child.header().id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + assert!(forest + .update_body(child.header()) + .expect("header was correct")); + assert_eq!(forest.try_finalize().expect("the block is ready"), child); + } + + #[test] + fn updates_interest_on_huge_branch() { + let (initial_header, mut forest) = setup(); + let branch: Vec<_> = initial_header + .random_branch() + .take(HUGE_BRANCH_LENGTH) + .collect(); + for header in branch.iter().take(HUGE_BRANCH_LENGTH - 1) { + let peer_id = rand::random(); + assert!(!forest + .update_header(header, Some(peer_id), false) + .expect("header was correct")); + assert_eq!(forest.state(&header.id()), Uninterested); + } + let header = &branch[HUGE_BRANCH_LENGTH - 1]; + let peer_id = rand::random(); + assert!(forest + .update_header(header, Some(peer_id), true) + .expect("header was correct")); + match forest.state(&header.id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } + for header in branch.iter().take(HUGE_BRANCH_LENGTH - 1) { + assert!(matches!(forest.state(&header.id()), Required(_))); + } + } +} diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 1203900ddc..1204a289b2 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -2,14 +2,14 @@ use std::collections::HashSet; use crate::sync::{forest::BlockIdFor, Justification, PeerId}; -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Debug, Copy, PartialEq, Eq)] enum HeaderImportance { Auxiliary, Required, Imported, } -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] enum InnerVertex { /// Empty Vertex. Empty { required: bool }, @@ -27,14 +27,14 @@ enum InnerVertex { } /// The vomplete vertex, including metadata about peers that know most about the data it refers to. -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct Vertex { inner: InnerVertex, know_most: HashSet, } /// What can happen when we add a justification. -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum JustificationAddResult { Noop, Required, @@ -236,3 +236,239 @@ impl Vertex { } } } + +#[cfg(test)] +mod tests { + use super::{JustificationAddResult, Vertex}; + use crate::sync::{ + mock::{MockHeader, MockIdentifier, MockJustification, MockPeerId}, + Header, + }; + + type MockVertex = Vertex; + + #[test] + fn initially_empty() { + let vertex = MockVertex::new(); + assert!(!vertex.required()); + assert!(!vertex.imported()); + assert!(vertex.parent().is_none()); + assert!(vertex.know_most().is_empty()); + assert_eq!(vertex.clone().ready(), Err(vertex)); + } + + #[test] + fn empty_remembers_block_holders() { + let mut vertex = MockVertex::new(); + let peer_id = rand::random(); + vertex.add_block_holder(Some(peer_id)); + assert!(vertex.know_most().contains(&peer_id)); + } + + #[test] + fn empty_set_required() { + let mut vertex = MockVertex::new(); + assert!(vertex.set_required()); + assert!(vertex.required()); + assert!(!vertex.set_required()); + assert!(vertex.required()); + } + + #[test] + fn empty_to_header() { + let mut vertex = MockVertex::new(); + let peer_id = rand::random(); + let parent = MockIdentifier::new_random(43); + vertex.insert_header(parent.clone(), Some(peer_id)); + assert!(!vertex.required()); + assert!(!vertex.imported()); + assert_eq!(vertex.parent(), Some(&parent)); + assert!(vertex.know_most().contains(&peer_id)); + assert_eq!(vertex.clone().ready(), Err(vertex)); + } + + #[test] + fn header_remembers_block_holders() { + let mut vertex = MockVertex::new(); + let peer_id = rand::random(); + let parent = MockIdentifier::new_random(43); + vertex.insert_header(parent, Some(peer_id)); + let other_peer_id = rand::random(); + vertex.add_block_holder(Some(other_peer_id)); + assert!(vertex.know_most().contains(&peer_id)); + assert!(vertex.know_most().contains(&other_peer_id)); + } + + #[test] + fn header_set_required() { + let mut vertex = MockVertex::new(); + let peer_id = rand::random(); + let parent = MockIdentifier::new_random(43); + vertex.insert_header(parent, Some(peer_id)); + assert!(vertex.set_required()); + assert!(vertex.required()); + assert!(!vertex.set_required()); + assert!(vertex.required()); + } + + #[test] + fn header_still_required() { + let mut vertex = MockVertex::new(); + assert!(vertex.set_required()); + let peer_id = rand::random(); + let parent = MockIdentifier::new_random(43); + vertex.insert_header(parent, Some(peer_id)); + assert!(vertex.required()); + assert!(!vertex.set_required()); + assert!(vertex.required()); + } + + #[test] + fn empty_to_body() { + let mut vertex = MockVertex::new(); + let parent = MockIdentifier::new_random(43); + assert!(!vertex.insert_body(parent.clone())); + assert!(!vertex.required()); + assert!(vertex.imported()); + assert_eq!(vertex.parent(), Some(&parent)); + assert_eq!(vertex.clone().ready(), Err(vertex)); + } + + #[test] + fn header_to_body() { + let mut vertex = MockVertex::new(); + let peer_id = rand::random(); + let parent = MockIdentifier::new_random(43); + vertex.insert_header(parent.clone(), Some(peer_id)); + assert!(!vertex.insert_body(parent.clone())); + assert!(!vertex.required()); + assert!(vertex.imported()); + assert_eq!(vertex.parent(), Some(&parent)); + assert_eq!(vertex.clone().ready(), Err(vertex)); + } + + #[test] + fn body_set_required() { + let mut vertex = MockVertex::new(); + let parent = MockIdentifier::new_random(43); + assert!(!vertex.insert_body(parent)); + assert!(!vertex.set_required()); + assert!(!vertex.required()); + } + + #[test] + fn body_no_longer_required() { + let mut vertex = MockVertex::new(); + assert!(vertex.set_required()); + let parent = MockIdentifier::new_random(43); + assert!(!vertex.insert_body(parent)); + assert!(!vertex.required()); + } + + #[test] + fn empty_to_justification() { + let mut vertex = MockVertex::new(); + let parent_header = MockHeader::random_parentless(0); + let header = parent_header.random_child(); + let parent = header.parent_id().expect("born of a parent"); + let justification = MockJustification::for_header(header); + let peer_id = rand::random(); + assert_eq!( + vertex.insert_justification(parent.clone(), justification, Some(peer_id)), + JustificationAddResult::Required + ); + assert!(vertex.required()); + assert!(!vertex.imported()); + assert_eq!(vertex.parent(), Some(&parent)); + assert!(vertex.know_most().contains(&peer_id)); + assert_eq!(vertex.clone().ready(), Err(vertex)); + } + + #[test] + fn header_to_justification() { + let mut vertex = MockVertex::new(); + let parent_header = MockHeader::random_parentless(0); + let header = parent_header.random_child(); + let parent = header.parent_id().expect("born of a parent"); + let justification = MockJustification::for_header(header); + let peer_id = rand::random(); + vertex.insert_header(parent.clone(), Some(peer_id)); + assert_eq!( + vertex.insert_justification(parent.clone(), justification, None), + JustificationAddResult::Required + ); + assert!(vertex.required()); + assert!(!vertex.imported()); + assert_eq!(vertex.parent(), Some(&parent)); + assert!(vertex.know_most().is_empty()); + assert_eq!(vertex.clone().ready(), Err(vertex)); + } + + #[test] + fn body_to_justification() { + let mut vertex = MockVertex::new(); + let parent_header = MockHeader::random_parentless(0); + let header = parent_header.random_child(); + let parent = header.parent_id().expect("born of a parent"); + let justification = MockJustification::for_header(header); + assert!(!vertex.insert_body(parent.clone())); + assert_eq!( + vertex.insert_justification(parent.clone(), justification.clone(), None), + JustificationAddResult::Finalizable + ); + assert!(!vertex.required()); + assert!(vertex.imported()); + assert_eq!(vertex.parent(), Some(&parent)); + assert_eq!(vertex.ready(), Ok(justification)); + } + + #[test] + fn justification_set_required() { + let mut vertex = MockVertex::new(); + let parent_header = MockHeader::random_parentless(0); + let header = parent_header.random_child(); + let parent = header.parent_id().expect("born of a parent"); + let justification = MockJustification::for_header(header); + let peer_id = rand::random(); + assert_eq!( + vertex.insert_justification(parent, justification, Some(peer_id)), + JustificationAddResult::Required + ); + assert!(!vertex.set_required()); + assert!(vertex.required()); + } + + #[test] + fn justification_still_required() { + let mut vertex = MockVertex::new(); + let parent_header = MockHeader::random_parentless(0); + let header = parent_header.random_child(); + let parent = header.parent_id().expect("born of a parent"); + let justification = MockJustification::for_header(header); + let peer_id = rand::random(); + assert!(vertex.set_required()); + assert_eq!( + vertex.insert_justification(parent, justification, Some(peer_id)), + JustificationAddResult::Noop + ); + assert!(vertex.required()); + } + + #[test] + fn my_body_is_ready() { + let mut vertex = MockVertex::new(); + let parent_header = MockHeader::random_parentless(0); + let header = parent_header.random_child(); + let parent = header.parent_id().expect("born of a parent"); + let justification = MockJustification::for_header(header); + assert_eq!( + vertex.insert_justification(parent.clone(), justification.clone(), None), + JustificationAddResult::Required + ); + assert!(vertex.insert_body(parent.clone())); + assert!(!vertex.required()); + assert!(vertex.imported()); + assert_eq!(vertex.parent(), Some(&parent)); + assert_eq!(vertex.ready(), Ok(justification)); + } +} diff --git a/finality-aleph/src/sync/mock.rs b/finality-aleph/src/sync/mock.rs new file mode 100644 index 0000000000..084bd83933 --- /dev/null +++ b/finality-aleph/src/sync/mock.rs @@ -0,0 +1,104 @@ +use crate::sync::{BlockIdentifier, Header, Justification}; + +pub type MockPeerId = u32; + +#[derive(Clone, Hash, Debug, PartialEq, Eq)] +pub struct MockIdentifier { + number: u32, + hash: u32, +} + +impl MockIdentifier { + fn new(number: u32, hash: u32) -> Self { + MockIdentifier { number, hash } + } + + pub fn new_random(number: u32) -> Self { + MockIdentifier::new(number, rand::random()) + } +} + +impl BlockIdentifier for MockIdentifier { + fn number(&self) -> u32 { + self.number + } +} + +#[derive(Clone, Hash, Debug, PartialEq, Eq)] +pub struct MockHeader { + id: MockIdentifier, + parent: Option, +} + +impl MockHeader { + fn new(id: MockIdentifier, parent: Option) -> Self { + MockHeader { id, parent } + } + + pub fn random_parentless(number: u32) -> Self { + let id = MockIdentifier::new_random(number); + MockHeader { id, parent: None } + } + + pub fn random_child(&self) -> Self { + let id = MockIdentifier::new_random(self.id.number() + 1); + let parent = Some(self.id.clone()); + MockHeader { id, parent } + } + + pub fn random_branch(&self) -> impl Iterator { + RandomBranch { + parent: self.clone(), + } + } +} + +struct RandomBranch { + parent: MockHeader, +} + +impl Iterator for RandomBranch { + type Item = MockHeader; + + fn next(&mut self) -> Option { + let result = self.parent.random_child(); + self.parent = result.clone(); + Some(result) + } +} + +impl Header for MockHeader { + type Identifier = MockIdentifier; + + fn id(&self) -> Self::Identifier { + self.id.clone() + } + + fn parent_id(&self) -> Option { + self.parent.clone() + } +} + +#[derive(Clone, Hash, Debug, PartialEq, Eq)] +pub struct MockJustification { + header: MockHeader, +} + +impl MockJustification { + pub fn for_header(header: MockHeader) -> Self { + MockJustification { header } + } +} + +impl Justification for MockJustification { + type Header = MockHeader; + type Unverified = Self; + + fn header(&self) -> &Self::Header { + &self.header + } + + fn into_unverified(self) -> Self::Unverified { + self + } +} diff --git a/finality-aleph/src/sync/mod.rs b/finality-aleph/src/sync/mod.rs index 604bc50d6e..0d52ba4008 100644 --- a/finality-aleph/src/sync/mod.rs +++ b/finality-aleph/src/sync/mod.rs @@ -4,6 +4,8 @@ use std::{ }; mod forest; +#[cfg(test)] +mod mock; mod substrate; mod task_queue; mod ticker; @@ -13,6 +15,8 @@ const LOG_TARGET: &str = "aleph-block-sync"; /// The identifier of a connected peer. pub trait PeerId: Clone + Hash + Eq {} +impl PeerId for T {} + /// The identifier of a block, the least amount of knowledge we can have about a block. pub trait BlockIdentifier: Clone + Hash + Debug + Eq { /// The block number, useful when reasoning about hopeless forks. From 8452d68d49593887b27791363f51fe695911ee8c Mon Sep 17 00:00:00 2001 From: timorl Date: Thu, 19 Jan 2023 11:12:30 +0100 Subject: [PATCH 13/14] Better tests --- finality-aleph/src/sync/forest/mod.rs | 34 +++++++++++++++++++++--- finality-aleph/src/sync/forest/vertex.rs | 2 +- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index a2d56d1832..5392ef3100 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -447,6 +447,23 @@ mod tests { assert_eq!(forest.state(&child.id()), Uninterested); } + #[test] + fn rejects_body_when_parent_unimported() { + let (initial_header, mut forest) = setup(); + let child = initial_header.random_child(); + let grandchild = child.random_child(); + assert!(!forest + .update_header(&child, None, false) + .expect("header was correct")); + assert_eq!( + forest.update_body(&grandchild), + Err(Error::ParentNotImported) + ); + assert!(forest.try_finalize().is_none()); + assert_eq!(forest.state(&child.id()), Uninterested); + assert_eq!(forest.state(&grandchild.id()), Uninterested); + } + #[test] fn finalizes_first_block() { let (initial_header, mut forest) = setup(); @@ -606,12 +623,20 @@ mod tests { #[test] fn prunes_huge_branch() { let (initial_header, mut forest) = setup(); - for header in initial_header.random_branch().take(HUGE_BRANCH_LENGTH) { + let fork: Vec<_> = initial_header + .random_branch() + .take(HUGE_BRANCH_LENGTH) + .collect(); + for header in &fork { let peer_id = rand::random(); - assert!(!forest - .update_header(&header, Some(peer_id), false) + assert!(forest + .update_header(header, Some(peer_id), true) .expect("header was correct")); assert!(forest.try_finalize().is_none()); + match forest.state(&header.id()) { + TopRequired(holders) => assert!(holders.contains(&peer_id)), + other_state => panic!("Expected top required, got {:?}.", other_state), + } } let child = MockJustification::for_header(initial_header.random_child()); let peer_id = rand::random(); @@ -630,6 +655,9 @@ mod tests { .update_body(child.header()) .expect("header was correct")); assert_eq!(forest.try_finalize().expect("the block is ready"), child); + for header in &fork { + assert_eq!(forest.state(&header.id()), Uninterested); + } } #[test] diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 1204a289b2..ce4efa3b5d 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -26,7 +26,7 @@ enum InnerVertex { }, } -/// The vomplete vertex, including metadata about peers that know most about the data it refers to. +/// The complete vertex, including metadata about peers that know most about the data it refers to. #[derive(Clone, Debug, PartialEq, Eq)] pub struct Vertex { inner: InnerVertex, From eb9a20f7088a02d0c9e0fb7479fb9eac8a37905c Mon Sep 17 00:00:00 2001 From: timorl Date: Fri, 20 Jan 2023 15:11:23 +0100 Subject: [PATCH 14/14] Add max forest depth --- finality-aleph/src/sync/forest/mod.rs | 55 +++++++++++++++++++++------ 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 5392ef3100..1f457fbe1b 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -42,6 +42,7 @@ pub enum Error { IncorrectParentState, IncorrectVertexState, ParentNotImported, + TooNew, } pub struct VertexWithChildren { @@ -62,6 +63,10 @@ impl VertexWithChildren { } } +// How deep can the forest be, vaguely based on two sessions ahead, which is the most we expect to +// ever need worst case scenario. +const MAX_DEPTH: u32 = 1800; + pub struct Forest { vertices: HashMap, VertexWithChildren>, top_required: HashSet>, @@ -165,12 +170,16 @@ impl Forest { } } - fn insert_id(&mut self, id: BlockIdFor, holder: Option) { + fn insert_id(&mut self, id: BlockIdFor, holder: Option) -> Result<(), Error> { + if id.number() > self.root_id.number() + MAX_DEPTH { + return Err(Error::TooNew); + } self.vertices .entry(id) .or_insert_with(VertexWithChildren::new) .vertex .add_block_holder(holder); + Ok(()) } fn process_header( @@ -189,11 +198,11 @@ impl Forest { id: &BlockIdFor, holder: Option, required: bool, - ) -> bool { - self.insert_id(id.clone(), holder); + ) -> Result { + self.insert_id(id.clone(), holder)?; match required { - true => self.set_top_required(id), - false => false, + true => Ok(self.set_top_required(id)), + false => Ok(false), } } @@ -205,7 +214,7 @@ impl Forest { required: bool, ) -> Result { let (id, parent_id) = self.process_header(header)?; - self.insert_id(id.clone(), holder.clone()); + self.insert_id(id.clone(), holder.clone())?; if let VertexHandle::Candidate(mut entry) = self.get_mut(&id) { entry.get_mut().vertex.insert_header(parent_id, holder); self.connect_parent(&id); @@ -325,7 +334,7 @@ impl Forest { #[cfg(test)] mod tests { - use super::{Error, Forest, Interest::*, JustificationAddResult}; + use super::{Error, Forest, Interest::*, JustificationAddResult, MAX_DEPTH}; use crate::sync::{ mock::{MockHeader, MockJustification, MockPeerId}, Header, Justification, @@ -351,7 +360,9 @@ mod tests { let (initial_header, mut forest) = setup(); let child = initial_header.random_child(); let peer_id = rand::random(); - assert!(!forest.update_block_identifier(&child.id(), Some(peer_id), false)); + assert!(!forest + .update_block_identifier(&child.id(), Some(peer_id), false) + .expect("it's not too high")); assert!(forest.try_finalize().is_none()); assert_eq!(forest.state(&child.id()), Uninterested); } @@ -361,13 +372,31 @@ mod tests { let (initial_header, mut forest) = setup(); let child = initial_header.random_child(); let peer_id = rand::random(); - assert!(forest.update_block_identifier(&child.id(), Some(peer_id), true)); + assert!(forest + .update_block_identifier(&child.id(), Some(peer_id), true) + .expect("it's not too high")); assert!(forest.try_finalize().is_none()); match forest.state(&child.id()) { TopRequired(holders) => assert!(holders.contains(&peer_id)), other_state => panic!("Expected top required, got {:?}.", other_state), } - assert!(!forest.update_block_identifier(&child.id(), Some(peer_id), true)); + assert!(!forest + .update_block_identifier(&child.id(), Some(peer_id), true) + .expect("it's not too high")); + } + + #[test] + fn rejects_too_high_id() { + let (initial_header, mut forest) = setup(); + let too_high = initial_header + .random_branch() + .nth(MAX_DEPTH as usize) + .expect("the branch is infinite"); + let peer_id = rand::random(); + assert_eq!( + forest.update_block_identifier(&too_high.id(), Some(peer_id), true), + Err(Error::TooNew) + ); } #[test] @@ -395,7 +424,9 @@ mod tests { TopRequired(holders) => assert!(holders.contains(&peer_id)), other_state => panic!("Expected top required, got {:?}.", other_state), } - assert!(!forest.update_block_identifier(&child.id(), Some(peer_id), true)); + assert!(!forest + .update_block_identifier(&child.id(), Some(peer_id), true) + .expect("it's not too high")); } #[test] @@ -586,7 +617,7 @@ mod tests { assert!(matches!(forest.state(&branch[3].id()), TopRequired(_))); } - const HUGE_BRANCH_LENGTH: usize = 1800; + const HUGE_BRANCH_LENGTH: usize = MAX_DEPTH as usize; #[test] fn finalizes_huge_branch() {