From dca7ce3db3b2894f1c246236247a14d90ba6ace7 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Wed, 7 Oct 2020 16:16:51 +0200 Subject: [PATCH 01/22] Fix import --- utils/fork-tree/src/lib.rs | 67 +++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 1d01c53417649..0c83b3576b6a3 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -696,43 +696,56 @@ mod node_implementation { pub fn import( &mut self, - mut hash: H, - mut number: N, - mut data: V, + hash: H, + number: N, + data: V, is_descendent_of: &F, ) -> Result, Error> where E: fmt::Debug, F: Fn(&H, &H) -> Result, { - if self.hash == hash { - return Err(Error::Duplicate); - }; + let mut stack: Vec<(&Self, bool)> = vec![(&self, true)]; - if number <= self.number { return Ok(Some((hash, number, data))); } + while !stack.is_empty() { + let (node, first_time) = stack.pop().unwrap(); - for node in self.children.iter_mut() { - match node.import(hash, number, data, is_descendent_of)? { - Some((h, n, d)) => { - hash = h; - number = n; - data = d; - }, - None => return Ok(None), + if first_time { + if node.hash == hash { + return Err(Error::Duplicate); + } + + if number <= node.number { + continue; + } + + if !node.children.is_empty() { + stack.push((node, false)); + + for child in node.children.iter().rev() { + stack.push((child, true)); + } + + continue; + } } - } - if is_descendent_of(&self.hash, &hash)? { - self.children.push(Node { - data, - hash: hash, - number: number, - children: Vec::new(), - }); - - Ok(None) - } else { - Ok(Some((hash, number, data))) + if is_descendent_of(&node.hash, &hash)? { + #[allow(mutable_transmutes)] + let node = unsafe { + std::mem::transmute::<&Self, &mut Self>(node) + }; + + node.children.push(Node { + data, + hash: hash, + number: number, + children: Vec::new(), + }); + return Ok(None); + } } + + return Ok(Some((hash, number, data))) } /// Find a node in the tree that is the deepest ancestor of the given From 21f7e45f22018715c7945cedef26936a61ac914a Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Wed, 7 Oct 2020 16:21:06 +0200 Subject: [PATCH 02/22] Add custom decode impl --- utils/fork-tree/src/lib.rs | 110 ++++++++++++++++++++++++++++++++++++- 1 file changed, 109 insertions(+), 1 deletion(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 0c83b3576b6a3..c8c0b07c796ba 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -643,7 +643,7 @@ mod node_implementation { Abort, } - #[derive(Clone, Debug, Decode, Encode, PartialEq)] + #[derive(Clone, Debug, Encode, PartialEq)] pub struct Node { pub hash: H, pub number: N, @@ -651,6 +651,42 @@ mod node_implementation { pub children: Vec>, } + impl Decode for Node { + fn decode(input: &mut I) -> Result { + let complete = |node: &Self| { + node.children.len() == node.children.capacity() + }; + + let mut stack = Vec::new(); + + // Loop until we've got a single completely decoded node on the stack. + while !(stack.len() == 1 && stack.last().map(complete).unwrap_or(false)) { + // If the top-most node is complete, pop it and push it as a child of the node + // beneath it. + if stack.last().map(complete).unwrap_or(false) { + let last = stack.pop().expect("We already checked this"); + let latest = stack.last_mut().expect("We know there were 2 items on the stack"); + latest.children.push(last); + continue; + } + + // Otherwise, decode a node and push it onto the stack. + let hash = H::decode(input)?; + let number = N::decode(input)?; + let data = V::decode(input)?; + // `Vec`s use a compacted u32 for capacity. + let capacity = codec::Compact::::decode(input)?; + + stack.push(Node { + hash, number, data, + children: Vec::with_capacity(capacity.0 as usize), + }); + } + + Ok(stack.pop().expect("checked")) + } + } + impl Node { /// Rebalance the tree, i.e. sort child nodes by max branch depth (decreasing). pub fn rebalance(&mut self) { @@ -719,6 +755,8 @@ mod node_implementation { } if !node.children.is_empty() { + // We want to check all children depth-first. + stack.push((node, false)); for child in node.children.iter().rev() { @@ -996,6 +1034,67 @@ mod test { (tree, is_descendent_of) } + fn test_fork_tree_2() -> (ForkTree, impl Fn(&String, &String) -> Result) { + let mut tree = ForkTree::new(); + + // + // - B - C - D - E + // / + // / - G + // / / + // A - F - H - I + // \ + // - L - M + // \ + // - O + // \ + // — J - K + // + // (where N is not a part of fork tree) + let is_descendent_of = |base: &String, block: &String| -> Result { + let letters = vec!["B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "O"]; + match (&base[..], &block[..]) { + ("A", b) => Ok(letters.into_iter().any(|n| n == b)), + ("B", b) => Ok(b == "C" || b == "D" || b == "E"), + ("C", b) => Ok(b == "D" || b == "E"), + ("D", b) => Ok(b == "E"), + ("E", _) => Ok(false), + ("F", b) => Ok(b == "G" || b == "H" || b == "I" || b == "L" || b == "M" || b == "O"), + ("G", _) => Ok(false), + ("H", b) => Ok(b == "I" || b == "L" || b == "M" || b == "O"), + ("I", _) => Ok(false), + ("J", b) => Ok(b == "K"), + ("K", _) => Ok(false), + ("L", b) => Ok(b == "M" || b == "O"), + ("M", _) => Ok(false), + ("O", _) => Ok(false), + ("0", _) => Ok(true), + _ => Ok(false), + } + }; + + tree.import("A".into(), 1, (), &is_descendent_of).unwrap(); + + tree.import("B".into(), 2, (), &is_descendent_of).unwrap(); + tree.import("C".into(), 3, (), &is_descendent_of).unwrap(); + tree.import("D".into(), 4, (), &is_descendent_of).unwrap(); + tree.import("E".into(), 5, (), &is_descendent_of).unwrap(); + + tree.import("F".into(), 2, (), &is_descendent_of).unwrap(); + tree.import("G".into(), 3, (), &is_descendent_of).unwrap(); + + tree.import("H".into(), 3, (), &is_descendent_of).unwrap(); + tree.import("I".into(), 4, (), &is_descendent_of).unwrap(); + tree.import("L".into(), 4, (), &is_descendent_of).unwrap(); + tree.import("M".into(), 5, (), &is_descendent_of).unwrap(); + tree.import("O".into(), 5, (), &is_descendent_of).unwrap(); + + tree.import("J".into(), 2, (), &is_descendent_of).unwrap(); + tree.import("K".into(), 3, (), &is_descendent_of).unwrap(); + + (tree, is_descendent_of) + } + #[test] fn import_doesnt_revert() { let (mut tree, is_descendent_of) = test_fork_tree(); @@ -1573,4 +1672,13 @@ mod test { ["A", "B", "C", "D", "E", "F", "H", "L", "M", "O", "I", "G", "J", "K"] ); } + + #[test] + fn test_serialization() { + use codec::*; + let (tree, _is_descendent_of) = test_fork_tree_2(); + let encoded = tree.encode(); + let decoded = ForkTree::decode(&mut &encoded[..]).unwrap(); + assert_eq!(tree, decoded); + } } From 3ab6abb1ab919b95b9cf29688be52eab1e2378ff Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Thu, 8 Oct 2020 12:39:26 +0200 Subject: [PATCH 03/22] Remove serialization tests --- utils/fork-tree/src/lib.rs | 70 -------------------------------------- 1 file changed, 70 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index c8c0b07c796ba..cc675a64f17a0 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -1034,67 +1034,6 @@ mod test { (tree, is_descendent_of) } - fn test_fork_tree_2() -> (ForkTree, impl Fn(&String, &String) -> Result) { - let mut tree = ForkTree::new(); - - // - // - B - C - D - E - // / - // / - G - // / / - // A - F - H - I - // \ - // - L - M - // \ - // - O - // \ - // — J - K - // - // (where N is not a part of fork tree) - let is_descendent_of = |base: &String, block: &String| -> Result { - let letters = vec!["B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "O"]; - match (&base[..], &block[..]) { - ("A", b) => Ok(letters.into_iter().any(|n| n == b)), - ("B", b) => Ok(b == "C" || b == "D" || b == "E"), - ("C", b) => Ok(b == "D" || b == "E"), - ("D", b) => Ok(b == "E"), - ("E", _) => Ok(false), - ("F", b) => Ok(b == "G" || b == "H" || b == "I" || b == "L" || b == "M" || b == "O"), - ("G", _) => Ok(false), - ("H", b) => Ok(b == "I" || b == "L" || b == "M" || b == "O"), - ("I", _) => Ok(false), - ("J", b) => Ok(b == "K"), - ("K", _) => Ok(false), - ("L", b) => Ok(b == "M" || b == "O"), - ("M", _) => Ok(false), - ("O", _) => Ok(false), - ("0", _) => Ok(true), - _ => Ok(false), - } - }; - - tree.import("A".into(), 1, (), &is_descendent_of).unwrap(); - - tree.import("B".into(), 2, (), &is_descendent_of).unwrap(); - tree.import("C".into(), 3, (), &is_descendent_of).unwrap(); - tree.import("D".into(), 4, (), &is_descendent_of).unwrap(); - tree.import("E".into(), 5, (), &is_descendent_of).unwrap(); - - tree.import("F".into(), 2, (), &is_descendent_of).unwrap(); - tree.import("G".into(), 3, (), &is_descendent_of).unwrap(); - - tree.import("H".into(), 3, (), &is_descendent_of).unwrap(); - tree.import("I".into(), 4, (), &is_descendent_of).unwrap(); - tree.import("L".into(), 4, (), &is_descendent_of).unwrap(); - tree.import("M".into(), 5, (), &is_descendent_of).unwrap(); - tree.import("O".into(), 5, (), &is_descendent_of).unwrap(); - - tree.import("J".into(), 2, (), &is_descendent_of).unwrap(); - tree.import("K".into(), 3, (), &is_descendent_of).unwrap(); - - (tree, is_descendent_of) - } - #[test] fn import_doesnt_revert() { let (mut tree, is_descendent_of) = test_fork_tree(); @@ -1672,13 +1611,4 @@ mod test { ["A", "B", "C", "D", "E", "F", "H", "L", "M", "O", "I", "G", "J", "K"] ); } - - #[test] - fn test_serialization() { - use codec::*; - let (tree, _is_descendent_of) = test_fork_tree_2(); - let encoded = tree.encode(); - let decoded = ForkTree::decode(&mut &encoded[..]).unwrap(); - assert_eq!(tree, decoded); - } } From 8cd0640111a828ca499e792f33f5d4e1d2193676 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Thu, 8 Oct 2020 12:45:04 +0200 Subject: [PATCH 04/22] Use while pop --- utils/fork-tree/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index cc675a64f17a0..42e8ef5be644f 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -742,9 +742,7 @@ mod node_implementation { { let mut stack: Vec<(&Self, bool)> = vec![(&self, true)]; - while !stack.is_empty() { - let (node, first_time) = stack.pop().unwrap(); - + while let Some((node, first_time)) = stack.pop() { if first_time { if node.hash == hash { return Err(Error::Duplicate); From 82867294110481d421f77bddb04aef10c0674e09 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Thu, 8 Oct 2020 15:51:37 +0200 Subject: [PATCH 05/22] Remvoe decode --- utils/fork-tree/src/lib.rs | 38 +------------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 42e8ef5be644f..63648cb5ee8ad 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -643,7 +643,7 @@ mod node_implementation { Abort, } - #[derive(Clone, Debug, Encode, PartialEq)] + #[derive(Clone, Debug, Decode, Encode, PartialEq)] pub struct Node { pub hash: H, pub number: N, @@ -651,42 +651,6 @@ mod node_implementation { pub children: Vec>, } - impl Decode for Node { - fn decode(input: &mut I) -> Result { - let complete = |node: &Self| { - node.children.len() == node.children.capacity() - }; - - let mut stack = Vec::new(); - - // Loop until we've got a single completely decoded node on the stack. - while !(stack.len() == 1 && stack.last().map(complete).unwrap_or(false)) { - // If the top-most node is complete, pop it and push it as a child of the node - // beneath it. - if stack.last().map(complete).unwrap_or(false) { - let last = stack.pop().expect("We already checked this"); - let latest = stack.last_mut().expect("We know there were 2 items on the stack"); - latest.children.push(last); - continue; - } - - // Otherwise, decode a node and push it onto the stack. - let hash = H::decode(input)?; - let number = N::decode(input)?; - let data = V::decode(input)?; - // `Vec`s use a compacted u32 for capacity. - let capacity = codec::Compact::::decode(input)?; - - stack.push(Node { - hash, number, data, - children: Vec::with_capacity(capacity.0 as usize), - }); - } - - Ok(stack.pop().expect("checked")) - } - } - impl Node { /// Rebalance the tree, i.e. sort child nodes by max branch depth (decreasing). pub fn rebalance(&mut self) { From de8b504fed32516de1923ba570811944230ffdd7 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Thu, 8 Oct 2020 16:38:31 +0200 Subject: [PATCH 06/22] Does this work?? --- utils/fork-tree/src/lib.rs | 64 +++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 63648cb5ee8ad..6db7b730e856e 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -768,48 +768,40 @@ mod node_implementation { F: Fn(&H, &H) -> Result, P: Fn(&V) -> bool, { - // stop searching this branch - if *number < self.number { - return Ok(FindOutcome::Failure(false)); - } + let mut stack: Vec<(&Self, Option, bool)> = vec![(self, None, true)]; + let mut indices = Vec::new(); + + let mut found = false; - let mut known_descendent_of = false; - - // continue depth-first search through all children - for (i, node) in self.children.iter().enumerate() { - // found node, early exit - match node.find_node_index_where(hash, number, is_descendent_of, predicate)? { - FindOutcome::Abort => return Ok(FindOutcome::Abort), - FindOutcome::Found(mut x) => { - x.push(i); - return Ok(FindOutcome::Found(x)) - }, - FindOutcome::Failure(true) => { - // if the block was a descendent of this child, - // then it cannot be a descendent of any others, - // so we don't search them. - known_descendent_of = true; - break; - }, - FindOutcome::Failure(false) => {}, + while let Some((node, index, first_time)) = stack.pop() { + // stop searching this branch + if *number < node.number { + continue; } - } - // node not found in any of the descendents, if the node we're - // searching for is a descendent of this node then we will stop the - // search here, since there aren't any more children and we found - // the correct node so we don't want to backtrack. - let is_descendent_of = known_descendent_of || is_descendent_of(&self.hash, hash)?; - if is_descendent_of { - // if the predicate passes we return the node - if predicate(&self.data) { - return Ok(FindOutcome::Found(Vec::new())); + if first_time && !found { + if !node.children.is_empty() { + stack.push((node, index, false)); + + for (i, node) in node.children.iter().enumerate().rev() { + stack.push((node, Some(i), true)); + } + + continue; + } + } + + if is_descendent_of(&node.hash, hash)? { + if predicate(&node.data) { + if let Some(index) = index { + indices.push(index); + } + found = true; + } } } - // otherwise, tell our ancestor that we failed, and whether - // the block was a descendent. - Ok(FindOutcome::Failure(is_descendent_of)) + return Ok(FindOutcome::Found(indices)) } /// Find a node in the tree that is the deepest ancestor of the given From 6a0c36c0a9330c1ad02078a82cdad7b70fd5e904 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Fri, 16 Oct 2020 11:58:48 +0200 Subject: [PATCH 07/22] Add find_index_where tests --- utils/fork-tree/src/lib.rs | 84 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 63648cb5ee8ad..02f63f7efbef1 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -633,6 +633,7 @@ mod node_implementation { use super::*; /// The outcome of a search within a node. + #[derive(Debug, PartialEq)] pub enum FindOutcome { // this is the node we were looking for. Found(T), @@ -996,6 +997,89 @@ mod test { (tree, is_descendent_of) } + #[test] + fn find_node_index_where() { + let (tree, is_descendent_of) = test_fork_tree(); + + assert_eq!( + tree.find_node_index_where(&"B", &2, &is_descendent_of, &|&()| true), + Ok(Some(vec![0])) + ); + + assert_eq!( + tree.find_node_index_where(&"C", &3, &is_descendent_of, &|&()| true), + Ok(Some(vec![0, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"D", &4, &is_descendent_of, &|&()| true), + Ok(Some(vec![0, 0, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"E", &5, &is_descendent_of, &|&()| true), + Ok(Some(vec![0, 0, 0, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"F", &2, &is_descendent_of, &|&()| true), + Ok(Some(vec![0])) + ); + + assert_eq!( + tree.find_node_index_where(&"G", &3, &is_descendent_of, &|&()| true), + Ok(Some(vec![1, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"H", &3, &is_descendent_of, &|&()| true), + Ok(Some(vec![1, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"I", &4, &is_descendent_of, &|&()| true), + Ok(Some(vec![1, 1, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"L", &4, &is_descendent_of, &|&()| true), + Ok(Some(vec![1, 1, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"M", &5, &is_descendent_of, &|&()| true), + Ok(Some(vec![1, 1, 1, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"O", &5, &is_descendent_of, &|&()| true), + Ok(Some(vec![1, 1, 1, 0])) + ); + + assert_eq!( + tree.find_node_index_where(&"J", &2, &is_descendent_of, &|&()| true), + Ok(Some(vec![0])) + ); + + assert_eq!( + tree.find_node_index_where(&"K", &3, &is_descendent_of, &|&()| true), + Ok(Some(vec![2, 0])) + ); + + for i in 0 .. 10 { + assert_eq!( + tree.find_node_index_where(&"A", &i, &is_descendent_of, &|&()| true), + Ok(None) + ); + } + + assert_eq!( + tree.find_node_index_where(&"B", &0, &is_descendent_of, &|&()| true), + Ok(None), + ); + } + + #[test] fn import_doesnt_revert() { let (mut tree, is_descendent_of) = test_fork_tree(); From 12b859839683e78e8b65795599e508ac89a7c566 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Fri, 16 Oct 2020 12:37:27 +0200 Subject: [PATCH 08/22] Might be working --- utils/fork-tree/src/lib.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 23164558777a4..57f58577e9fb0 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -770,13 +770,14 @@ mod node_implementation { P: Fn(&V) -> bool, { let mut stack: Vec<(&Self, Option, bool)> = vec![(self, None, true)]; - let mut indices = Vec::new(); let mut found = false; + let mut last: Option>> = None; while let Some((node, index, first_time)) = stack.pop() { // stop searching this branch if *number < node.number { + last = Some(FindOutcome::Failure(false)); continue; } @@ -784,25 +785,39 @@ mod node_implementation { if !node.children.is_empty() { stack.push((node, index, false)); - for (i, node) in node.children.iter().enumerate().rev() { - stack.push((node, Some(i), true)); + for (i, child) in node.children.iter().enumerate().rev() { + stack.push((child, Some(i), true)); } continue; } } - if is_descendent_of(&node.hash, hash)? { + let is_descendent_of = is_descendent_of(&node.hash, hash)?; + + if is_descendent_of { if predicate(&node.data) { - if let Some(index) = index { - indices.push(index); - } + last = match last.take() { + Some(FindOutcome::Found(mut vec)) => { + if let Some(index) = index { + vec.push(index); + } + Some(FindOutcome::Found(vec)) + }, + _ => Some(FindOutcome::Found({ + if let Some(index) = index { + vec![index] + } else { + vec![] + } + })) + }; found = true; } } } - return Ok(FindOutcome::Found(indices)) + return Ok(last.unwrap_or_else(|| FindOutcome::Failure(false))); } /// Find a node in the tree that is the deepest ancestor of the given @@ -1061,7 +1076,8 @@ mod test { for i in 0 .. 10 { assert_eq!( tree.find_node_index_where(&"A", &i, &is_descendent_of, &|&()| true), - Ok(None) + Ok(None), + "{}", i ); } From d7f84482f1604c15c2ec9762a81ab8e1876b5ab3 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Fri, 16 Oct 2020 15:33:53 +0200 Subject: [PATCH 09/22] Fingers crossed! --- utils/fork-tree/src/lib.rs | 82 +++++++++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 57f58577e9fb0..73dd4106a44cf 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -796,7 +796,9 @@ mod node_implementation { let is_descendent_of = is_descendent_of(&node.hash, hash)?; if is_descendent_of { - if predicate(&node.data) { + found |= predicate(&node.data); + + if found { last = match last.take() { Some(FindOutcome::Found(mut vec)) => { if let Some(index) = index { @@ -815,6 +817,12 @@ mod node_implementation { found = true; } } + + if found { + while stack.last().map(|(.., first)| *first).unwrap_or(false) { + stack.pop(); + } + } } return Ok(last.unwrap_or_else(|| FindOutcome::Failure(false))); @@ -1665,4 +1673,76 @@ mod test { ["A", "B", "C", "D", "E", "F", "H", "L", "M", "O", "I", "G", "J", "K"] ); } + + fn test_fork_tree_with_values<'a>() -> (ForkTree<&'a str, u64, u64>, impl Fn(&&str, &&str) -> Result) { + let mut tree = ForkTree::new(); + + // + // - B - C - D - E + // / + // / - G + // / / + // A - F - H - I + // \ + // - L - M + // \ + // - O + // \ + // — J - K + // + // (where N is not a part of fork tree) + let is_descendent_of = |base: &&str, block: &&str| -> Result { + let letters = vec!["B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "O"]; + match (*base, *block) { + ("A", b) => Ok(letters.into_iter().any(|n| n == b)), + ("B", b) => Ok(b == "C" || b == "D" || b == "E"), + ("C", b) => Ok(b == "D" || b == "E"), + ("D", b) => Ok(b == "E"), + ("E", _) => Ok(false), + ("F", b) => Ok(b == "G" || b == "H" || b == "I" || b == "L" || b == "M" || b == "O"), + ("G", _) => Ok(false), + ("H", b) => Ok(b == "I" || b == "L" || b == "M" || b == "O"), + ("I", _) => Ok(false), + ("J", b) => Ok(b == "K"), + ("K", _) => Ok(false), + ("L", b) => Ok(b == "M" || b == "O"), + ("M", _) => Ok(false), + ("O", _) => Ok(false), + ("0", _) => Ok(true), + _ => Ok(false), + } + }; + + tree.import("A", 1, 10, &is_descendent_of).unwrap(); + + tree.import("B", 2, 9, &is_descendent_of).unwrap(); + tree.import("C", 3, 8, &is_descendent_of).unwrap(); + tree.import("D", 4, 7, &is_descendent_of).unwrap(); + tree.import("E", 5, 6, &is_descendent_of).unwrap(); + + tree.import("F", 2, 5, &is_descendent_of).unwrap(); + tree.import("G", 3, 4, &is_descendent_of).unwrap(); + + tree.import("H", 3, 3, &is_descendent_of).unwrap(); + tree.import("I", 4, 2, &is_descendent_of).unwrap(); + tree.import("L", 4, 1, &is_descendent_of).unwrap(); + tree.import("M", 5, 2, &is_descendent_of).unwrap(); + tree.import("O", 5, 3, &is_descendent_of).unwrap(); + + tree.import("J", 2, 4, &is_descendent_of).unwrap(); + tree.import("K", 3, 11, &is_descendent_of).unwrap(); + + (tree, is_descendent_of) + } + + #[test] + fn find_node_where_value() { + let (tree, d) = test_fork_tree_with_values(); + assert_eq!( + tree.find_node_where(&"M", &5, &d, &|&n| n == 1 || n == 2) + .map(|opt| opt.map(|node| node.hash)), + Ok(Some("L")), + "{:?}", tree.find_node_index_where(&"M", &5, &d, &|&n| n == 1 || n == 2) + ); + } } From c7d405f6edef37a85a0ac50b938f51724d6478be Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Fri, 16 Oct 2020 16:51:35 +0200 Subject: [PATCH 10/22] Add more tests --- utils/fork-tree/src/lib.rs | 101 +++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 02f63f7efbef1..9340f2f66398c 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -1657,4 +1657,105 @@ mod test { ["A", "B", "C", "D", "E", "F", "H", "L", "M", "O", "I", "G", "J", "K"] ); } + + fn test_fork_tree_with_values<'a>() -> (ForkTree<&'a str, u64, u64>, impl Fn(&&str, &&str) -> Result) { + let mut tree = ForkTree::new(); + + // + // - B - C - D - E + // / + // / - G + // / / + // A - F - H - I + // \ + // - L - M + // \ + // - O + // \ + // — J - K + // + // (where N is not a part of fork tree) + let is_descendent_of = |base: &&str, block: &&str| -> Result { + let letters = vec!["B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "O"]; + match (*base, *block) { + ("A", b) => Ok(letters.into_iter().any(|n| n == b)), + ("B", b) => Ok(b == "C" || b == "D" || b == "E"), + ("C", b) => Ok(b == "D" || b == "E"), + ("D", b) => Ok(b == "E"), + ("E", _) => Ok(false), + ("F", b) => Ok(b == "G" || b == "H" || b == "I" || b == "L" || b == "M" || b == "O"), + ("G", _) => Ok(false), + ("H", b) => Ok(b == "I" || b == "L" || b == "M" || b == "O"), + ("I", _) => Ok(false), + ("J", b) => Ok(b == "K"), + ("K", _) => Ok(false), + ("L", b) => Ok(b == "M" || b == "O"), + ("M", _) => Ok(false), + ("O", _) => Ok(false), + ("0", _) => Ok(true), + _ => Ok(false), + } + }; + + tree.import("A", 1, 10, &is_descendent_of).unwrap(); + + tree.import("B", 2, 9, &is_descendent_of).unwrap(); + tree.import("C", 3, 8, &is_descendent_of).unwrap(); + tree.import("D", 4, 7, &is_descendent_of).unwrap(); + tree.import("E", 5, 6, &is_descendent_of).unwrap(); + + tree.import("F", 2, 5, &is_descendent_of).unwrap(); + tree.import("G", 3, 4, &is_descendent_of).unwrap(); + + tree.import("H", 3, 3, &is_descendent_of).unwrap(); + tree.import("I", 4, 2, &is_descendent_of).unwrap(); + tree.import("L", 4, 1, &is_descendent_of).unwrap(); + tree.import("M", 5, 2, &is_descendent_of).unwrap(); + tree.import("O", 5, 3, &is_descendent_of).unwrap(); + + tree.import("J", 2, 4, &is_descendent_of).unwrap(); + tree.import("K", 3, 11, &is_descendent_of).unwrap(); + + (tree, is_descendent_of) + } + + #[test] + fn find_node_where_value() { + let (tree, d) = test_fork_tree_with_values(); + assert_eq!( + tree.find_node_where(&"M", &5, &d, &|&n| n == 1 || n == 2) + .map(|opt| opt.map(|node| node.hash)), + Ok(Some("L")), + "{:?}", tree.find_node_index_where(&"M", &5, &d, &|&n| n == 1 || n == 2) + ); + } + + #[test] + fn find_node_where_specific_value() { + let mut tree = ForkTree::new(); + + // + // A - B + // \ + // — C + // + let is_descendent_of = |base: &&str, block: &&str| -> Result { + match (*base, *block) { + ("A", b) => Ok(b == "B" || b == "C" || b == "D"), + ("B", b) | ("C", b) => Ok(b == "D"), + ("0", _) => Ok(true), + _ => Ok(false), + } + }; + + tree.import("A", 1, 1, &is_descendent_of).unwrap(); + tree.import("B", 2, 2, &is_descendent_of).unwrap(); + tree.import("C", 2, 4, &is_descendent_of).unwrap(); + + assert_eq!( + tree.find_node_where(&"D", &3, &is_descendent_of, &|&n| n == 4) + .map(|opt| opt.map(|node| node.hash)), + Ok(Some("C")) + ); + } } From 490b393671ed8feb60c6efb7fa09193915070131 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 19 Oct 2020 12:44:28 +0200 Subject: [PATCH 11/22] Tidy up a little --- utils/fork-tree/src/lib.rs | 61 +++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 73dd4106a44cf..818922b06f772 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -731,22 +731,22 @@ mod node_implementation { } if is_descendent_of(&node.hash, &hash)? { - #[allow(mutable_transmutes)] - let node = unsafe { - std::mem::transmute::<&Self, &mut Self>(node) - }; - - node.children.push(Node { - data, - hash: hash, - number: number, - children: Vec::new(), - }); + let node = &mut (node as *const Self as *mut Self); + + unsafe { + (**node).children.push(Node { + data, + hash: hash, + number: number, + children: Vec::new(), + }); + } + return Ok(None); } } - return Ok(Some((hash, number, data))) + Ok(Some((hash, number, data))) } /// Find a node in the tree that is the deepest ancestor of the given @@ -772,25 +772,22 @@ mod node_implementation { let mut stack: Vec<(&Self, Option, bool)> = vec![(self, None, true)]; let mut found = false; - let mut last: Option>> = None; + let mut indices: Option> = None; while let Some((node, index, first_time)) = stack.pop() { // stop searching this branch if *number < node.number { - last = Some(FindOutcome::Failure(false)); continue; } - if first_time && !found { - if !node.children.is_empty() { - stack.push((node, index, false)); - - for (i, child) in node.children.iter().enumerate().rev() { - stack.push((child, Some(i), true)); - } + if first_time && !found && !node.children.is_empty() { + stack.push((node, index, false)); - continue; + for (i, child) in node.children.iter().enumerate().rev() { + stack.push((child, Some(i), true)); } + + continue; } let is_descendent_of = is_descendent_of(&node.hash, hash)?; @@ -799,20 +796,18 @@ mod node_implementation { found |= predicate(&node.data); if found { - last = match last.take() { - Some(FindOutcome::Found(mut vec)) => { + indices = match indices.take() { + Some(mut vec) => { if let Some(index) = index { vec.push(index); } - Some(FindOutcome::Found(vec)) + Some(vec) }, - _ => Some(FindOutcome::Found({ - if let Some(index) = index { - vec![index] - } else { - vec![] - } - })) + _ => Some(if let Some(index) = index { + vec![index] + } else { + vec![] + }) }; found = true; } @@ -825,7 +820,7 @@ mod node_implementation { } } - return Ok(last.unwrap_or_else(|| FindOutcome::Failure(false))); + Ok(indices.map(FindOutcome::Found).unwrap_or(FindOutcome::Failure(false))) } /// Find a node in the tree that is the deepest ancestor of the given From ca7fe7ff5593199380425a14a5144f84f3da23ff Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 19 Oct 2020 15:35:51 +0200 Subject: [PATCH 12/22] Add special case --- utils/fork-tree/src/lib.rs | 65 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 818922b06f772..a846b7d63a6ac 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -795,6 +795,12 @@ mod node_implementation { if is_descendent_of { found |= predicate(&node.data); + if !found { + while stack.last().map(|(.., first)| *first).unwrap_or(false) { + stack.pop(); + } + } + if found { indices = match indices.take() { Some(mut vec) => { @@ -1740,4 +1746,63 @@ mod test { "{:?}", tree.find_node_index_where(&"M", &5, &d, &|&n| n == 1 || n == 2) ); } + + #[test] + fn find_node_where_returns_none_on_bad_is_descendent_of() { + let mut tree = ForkTree::new(); + + // + // A - B + // \ + // — C + // + let is_descendent_of = |base: &&str, block: &&str| -> Result { + match (*base, *block) { + ("A", b) => Ok(b == "B" || b == "C" || b == "D"), + ("B", b) | ("C", b) => Ok(b == "D"), + ("0", _) => Ok(true), + _ => Ok(false), + } + }; + + tree.import("A", 1, 1, &is_descendent_of).unwrap(); + tree.import("B", 2, 2, &is_descendent_of).unwrap(); + tree.import("C", 2, 4, &is_descendent_of).unwrap(); + + assert_eq!( + tree.find_node_where(&"D", &3, &is_descendent_of, &|&n| n == 4) + .map(|opt| opt.map(|node| node.hash)), + Ok(None) + ); + } + + #[test] + fn find_node_where_returns_none_on_bad_is_descendent_of_2() { + let mut tree = ForkTree::new(); + + // + // A - B + // \ + // — C + // + let is_descendent_of = |base: &&str, block: &&str| -> Result { + match (*base, *block) { + ("A", b) => Ok(b == "B" || b == "C" || b == "D"), + ("B", b) | ("C", b) => Ok(b == "D"), + ("0", _) => Ok(true), + _ => Ok(false), + } + }; + + tree.import("A", 1, 1, &is_descendent_of).unwrap(); + tree.import("B", 2, 2, &is_descendent_of).unwrap(); + tree.import("C", 2, 4, &is_descendent_of).unwrap(); + + assert_eq!( + tree.find_node_where(&"D", &3, &is_descendent_of, &|&n| n == 1) + .map(|opt| opt.map(|node| node.hash)), + Ok(Some("A")) + ); + } + } From c8066906834519027f6b6421302db889b472ced2 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Thu, 22 Oct 2020 15:08:46 +0200 Subject: [PATCH 13/22] Add custom decode impl --- utils/fork-tree/src/lib.rs | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index a846b7d63a6ac..78b573d13f2b8 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -644,7 +644,7 @@ mod node_implementation { Abort, } - #[derive(Clone, Debug, Decode, Encode, PartialEq)] + #[derive(Clone, Debug, Encode, PartialEq)] pub struct Node { pub hash: H, pub number: N, @@ -652,6 +652,42 @@ mod node_implementation { pub children: Vec>, } + impl Decode for Node { + fn decode(input: &mut I) -> Result { + let complete = |node: &Self| { + node.children.len() == node.children.capacity() + }; + + let mut stack = Vec::new(); + + // Loop until we've got a single completely decoded node on the stack. + while !(stack.len() == 1 && stack.last().map(complete).unwrap_or(false)) { + // If the top-most node is complete, pop it and push it as a child of the node + // beneath it. + if stack.last().map(complete).unwrap_or(false) { + let last = stack.pop().expect("We already checked this"); + let latest = stack.last_mut().expect("We know there were 2 items on the stack"); + latest.children.push(last); + continue; + } + + // Otherwise, decode a node and push it onto the stack. + let hash = H::decode(input)?; + let number = N::decode(input)?; + let data = V::decode(input)?; + // `Vec`s use a compacted u32 for capacity. + let capacity = codec::Compact::::decode(input)?; + + stack.push(Node { + hash, number, data, + children: Vec::with_capacity(capacity.0 as usize), + }); + } + + Ok(stack.pop().expect("checked")) + } + } + impl Node { /// Rebalance the tree, i.e. sort child nodes by max branch depth (decreasing). pub fn rebalance(&mut self) { From 87777830a170172df766b20e5229682be1639bad Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 26 Oct 2020 16:16:01 +0100 Subject: [PATCH 14/22] Fix line widths --- utils/fork-tree/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 78b573d13f2b8..ec6563ccec1a5 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -1711,7 +1711,9 @@ mod test { ); } - fn test_fork_tree_with_values<'a>() -> (ForkTree<&'a str, u64, u64>, impl Fn(&&str, &&str) -> Result) { + fn test_fork_tree_with_values<'a>() -> (ForkTree<&'a str, u64, u64>, impl Fn(&&str, &&str) + -> Result) + { let mut tree = ForkTree::new(); // From 76d6688476b5c7777b73cc18cf4513a38a0ff000 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 27 Oct 2020 13:26:32 +0100 Subject: [PATCH 15/22] Add values to default test fork tree --- utils/fork-tree/src/lib.rs | 145 +++++++++++-------------------------- 1 file changed, 41 insertions(+), 104 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index ec6563ccec1a5..8c26275b33c33 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -988,7 +988,7 @@ mod test { impl std::error::Error for TestError {} - fn test_fork_tree<'a>() -> (ForkTree<&'a str, u64, ()>, impl Fn(&&str, &&str) -> Result) { + fn test_fork_tree<'a>() -> (ForkTree<&'a str, u64, u64>, impl Fn(&&str, &&str) -> Result) { let mut tree = ForkTree::new(); // @@ -1027,24 +1027,24 @@ mod test { } }; - tree.import("A", 1, (), &is_descendent_of).unwrap(); + tree.import("A", 1, 10, &is_descendent_of).unwrap(); - tree.import("B", 2, (), &is_descendent_of).unwrap(); - tree.import("C", 3, (), &is_descendent_of).unwrap(); - tree.import("D", 4, (), &is_descendent_of).unwrap(); - tree.import("E", 5, (), &is_descendent_of).unwrap(); + tree.import("B", 2, 9, &is_descendent_of).unwrap(); + tree.import("C", 3, 8, &is_descendent_of).unwrap(); + tree.import("D", 4, 7, &is_descendent_of).unwrap(); + tree.import("E", 5, 6, &is_descendent_of).unwrap(); - tree.import("F", 2, (), &is_descendent_of).unwrap(); - tree.import("G", 3, (), &is_descendent_of).unwrap(); + tree.import("F", 2, 5, &is_descendent_of).unwrap(); + tree.import("G", 3, 4, &is_descendent_of).unwrap(); - tree.import("H", 3, (), &is_descendent_of).unwrap(); - tree.import("I", 4, (), &is_descendent_of).unwrap(); - tree.import("L", 4, (), &is_descendent_of).unwrap(); - tree.import("M", 5, (), &is_descendent_of).unwrap(); - tree.import("O", 5, (), &is_descendent_of).unwrap(); + tree.import("H", 3, 3, &is_descendent_of).unwrap(); + tree.import("I", 4, 2, &is_descendent_of).unwrap(); + tree.import("L", 4, 1, &is_descendent_of).unwrap(); + tree.import("M", 5, 2, &is_descendent_of).unwrap(); + tree.import("O", 5, 3, &is_descendent_of).unwrap(); - tree.import("J", 2, (), &is_descendent_of).unwrap(); - tree.import("K", 3, (), &is_descendent_of).unwrap(); + tree.import("J", 2, 4, &is_descendent_of).unwrap(); + tree.import("K", 3, 11, &is_descendent_of).unwrap(); (tree, is_descendent_of) } @@ -1054,80 +1054,80 @@ mod test { let (tree, is_descendent_of) = test_fork_tree(); assert_eq!( - tree.find_node_index_where(&"B", &2, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"B", &2, &is_descendent_of, &|_| true), Ok(Some(vec![0])) ); assert_eq!( - tree.find_node_index_where(&"C", &3, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"C", &3, &is_descendent_of, &|_| true), Ok(Some(vec![0, 0])) ); assert_eq!( - tree.find_node_index_where(&"D", &4, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"D", &4, &is_descendent_of, &|_| true), Ok(Some(vec![0, 0, 0])) ); assert_eq!( - tree.find_node_index_where(&"E", &5, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"E", &5, &is_descendent_of, &|_| true), Ok(Some(vec![0, 0, 0, 0])) ); assert_eq!( - tree.find_node_index_where(&"F", &2, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"F", &2, &is_descendent_of, &|_| true), Ok(Some(vec![0])) ); assert_eq!( - tree.find_node_index_where(&"G", &3, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"G", &3, &is_descendent_of, &|_| true), Ok(Some(vec![1, 0])) ); assert_eq!( - tree.find_node_index_where(&"H", &3, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"H", &3, &is_descendent_of, &|_| true), Ok(Some(vec![1, 0])) ); assert_eq!( - tree.find_node_index_where(&"I", &4, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"I", &4, &is_descendent_of, &|_| true), Ok(Some(vec![1, 1, 0])) ); assert_eq!( - tree.find_node_index_where(&"L", &4, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"L", &4, &is_descendent_of, &|_| true), Ok(Some(vec![1, 1, 0])) ); assert_eq!( - tree.find_node_index_where(&"M", &5, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"M", &5, &is_descendent_of, &|_| true), Ok(Some(vec![1, 1, 1, 0])) ); assert_eq!( - tree.find_node_index_where(&"O", &5, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"O", &5, &is_descendent_of, &|_| true), Ok(Some(vec![1, 1, 1, 0])) ); assert_eq!( - tree.find_node_index_where(&"J", &2, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"J", &2, &is_descendent_of, &|_| true), Ok(Some(vec![0])) ); assert_eq!( - tree.find_node_index_where(&"K", &3, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"K", &3, &is_descendent_of, &|_| true), Ok(Some(vec![2, 0])) ); for i in 0 .. 10 { assert_eq!( - tree.find_node_index_where(&"A", &i, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"A", &i, &is_descendent_of, &|_| true), Ok(None), "{}", i ); } assert_eq!( - tree.find_node_index_where(&"B", &0, &is_descendent_of, &|&()| true), + tree.find_node_index_where(&"B", &0, &is_descendent_of, &|_| true), Ok(None), ); } @@ -1145,7 +1145,7 @@ mod test { ); assert_eq!( - tree.import("A", 1, (), &is_descendent_of), + tree.import("A", 1, 1, &is_descendent_of), Err(Error::Revert), ); } @@ -1155,22 +1155,22 @@ mod test { let (mut tree, is_descendent_of) = test_fork_tree(); assert_eq!( - tree.import("A", 1, (), &is_descendent_of), + tree.import("A", 1, 1, &is_descendent_of), Err(Error::Duplicate), ); assert_eq!( - tree.import("I", 4, (), &is_descendent_of), + tree.import("I", 4, 1, &is_descendent_of), Err(Error::Duplicate), ); assert_eq!( - tree.import("G", 3, (), &is_descendent_of), + tree.import("G", 3, 1, &is_descendent_of), Err(Error::Duplicate), ); assert_eq!( - tree.import("K", 3, (), &is_descendent_of), + tree.import("K", 3, 1, &is_descendent_of), Err(Error::Duplicate), ); } @@ -1244,7 +1244,7 @@ mod test { // finalizing "A" opens up three possible forks assert_eq!( tree.finalize(&"A", 1, &is_descendent_of), - Ok(FinalizationResult::Changed(Some(()))), + Ok(FinalizationResult::Changed(Some(10))), ); assert_eq!( @@ -1272,12 +1272,12 @@ mod test { // after finalizing "F" we can finalize "H" assert_eq!( tree.finalize(&"F", 2, &is_descendent_of), - Ok(FinalizationResult::Changed(Some(()))), + Ok(FinalizationResult::Changed(Some(5))), ); assert_eq!( tree.finalize(&"H", 3, &is_descendent_of), - Ok(FinalizationResult::Changed(Some(()))), + Ok(FinalizationResult::Changed(Some(3))), ); assert_eq!( @@ -1311,7 +1311,7 @@ mod test { // finalizing "A" opens up three possible forks assert_eq!( tree.finalize_with_ancestors(&"A", 1, &is_descendent_of), - Ok(FinalizationResult::Changed(Some(()))), + Ok(FinalizationResult::Changed(Some(10))), ); assert_eq!( @@ -1325,7 +1325,7 @@ mod test { // 3) finalizes the just opened root H (H -> I + L) assert_eq!( tree.finalize_with_ancestors(&"H", 3, &is_descendent_of), - Ok(FinalizationResult::Changed(Some(()))), + Ok(FinalizationResult::Changed(Some(3))), ); assert_eq!( @@ -1711,72 +1711,9 @@ mod test { ); } - fn test_fork_tree_with_values<'a>() -> (ForkTree<&'a str, u64, u64>, impl Fn(&&str, &&str) - -> Result) - { - let mut tree = ForkTree::new(); - - // - // - B - C - D - E - // / - // / - G - // / / - // A - F - H - I - // \ - // - L - M - // \ - // - O - // \ - // — J - K - // - // (where N is not a part of fork tree) - let is_descendent_of = |base: &&str, block: &&str| -> Result { - let letters = vec!["B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "O"]; - match (*base, *block) { - ("A", b) => Ok(letters.into_iter().any(|n| n == b)), - ("B", b) => Ok(b == "C" || b == "D" || b == "E"), - ("C", b) => Ok(b == "D" || b == "E"), - ("D", b) => Ok(b == "E"), - ("E", _) => Ok(false), - ("F", b) => Ok(b == "G" || b == "H" || b == "I" || b == "L" || b == "M" || b == "O"), - ("G", _) => Ok(false), - ("H", b) => Ok(b == "I" || b == "L" || b == "M" || b == "O"), - ("I", _) => Ok(false), - ("J", b) => Ok(b == "K"), - ("K", _) => Ok(false), - ("L", b) => Ok(b == "M" || b == "O"), - ("M", _) => Ok(false), - ("O", _) => Ok(false), - ("0", _) => Ok(true), - _ => Ok(false), - } - }; - - tree.import("A", 1, 10, &is_descendent_of).unwrap(); - - tree.import("B", 2, 9, &is_descendent_of).unwrap(); - tree.import("C", 3, 8, &is_descendent_of).unwrap(); - tree.import("D", 4, 7, &is_descendent_of).unwrap(); - tree.import("E", 5, 6, &is_descendent_of).unwrap(); - - tree.import("F", 2, 5, &is_descendent_of).unwrap(); - tree.import("G", 3, 4, &is_descendent_of).unwrap(); - - tree.import("H", 3, 3, &is_descendent_of).unwrap(); - tree.import("I", 4, 2, &is_descendent_of).unwrap(); - tree.import("L", 4, 1, &is_descendent_of).unwrap(); - tree.import("M", 5, 2, &is_descendent_of).unwrap(); - tree.import("O", 5, 3, &is_descendent_of).unwrap(); - - tree.import("J", 2, 4, &is_descendent_of).unwrap(); - tree.import("K", 3, 11, &is_descendent_of).unwrap(); - - (tree, is_descendent_of) - } - #[test] fn find_node_where_value() { - let (tree, d) = test_fork_tree_with_values(); + let (tree, d) = test_fork_tree(); assert_eq!( tree.find_node_where(&"M", &5, &d, &|&n| n == 1 || n == 2) .map(|opt| opt.map(|node| node.hash)), From 9cf44e051d8d9dd7e562953e538161a424daa2ba Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 27 Oct 2020 13:34:52 +0100 Subject: [PATCH 16/22] Switch to alternate implementations --- utils/fork-tree/src/lib.rs | 272 +++++++++++++++++-------------------- 1 file changed, 126 insertions(+), 146 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 8c26275b33c33..3374a623f2349 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -22,6 +22,7 @@ use std::cmp::Reverse; use std::fmt; +use std::hash::Hash; use codec::{Decode, Encode}; /// Error occurred when iterating with the tree. @@ -84,7 +85,7 @@ pub struct ForkTree { } impl ForkTree where - H: PartialEq + Clone, + H: Eq + Clone + Hash, N: Ord + Clone, V: Clone, { @@ -169,7 +170,7 @@ impl ForkTree where } impl ForkTree where - H: PartialEq, + H: Eq + Hash + Clone, N: Ord, { /// Create a new empty tree. @@ -190,9 +191,52 @@ impl ForkTree where /// for at any point will likely be in one of the deepest chains (i.e. the /// longest ones). pub fn rebalance(&mut self) { - self.roots.sort_by_key(|n| Reverse(n.max_depth())); - for root in &mut self.roots { - root.rebalance(); + use std::collections::HashMap; + use std::iter::FromIterator; + + let mut current_max_depth = 1; + let mut max_depths = HashMap::new(); + let mut stack = Vec::from_iter(self.roots.iter().map(|n| (n, 1, false))); + + while let Some((node, depth, expanded)) = stack.pop() { + // we've reached a leaf, record the max depth for this branch + if node.children.is_empty() { + current_max_depth = depth; + max_depths.insert(node.hash.clone(), depth); + continue; + } + + // this is not a leaf node and it has not been expanded yet + // we should reset the current max depth to the current depth + // since we will begin exploring a new branch + if !expanded { + for child in &node.children { + stack.push((node, depth, true)); + stack.push((child, depth + 1, false)); + } + stack.push((node, depth, true)); + current_max_depth = depth; + continue; + } + + // we are backtracking so we need to update our depth with + // the maximum from all our children + let max_depth = max_depths.entry(node.hash.clone()).or_default(); + if current_max_depth > *max_depth { + *max_depth = current_max_depth; + } + } + + // iterate the tree in depth-first search and use the max depths + // map to sort each intermediary node + let mut stack = Vec::from_iter(self.roots.iter_mut()); + while let Some(node) = stack.pop() { + node.children.sort_by_key(|n| { + let max_depth = max_depths.get(&n.hash).expect(""); + Reverse(max_depth) + }); + + stack.extend(node.children.iter_mut()) } } @@ -204,9 +248,9 @@ impl ForkTree where /// Returns `true` if the imported node is a root. pub fn import( &mut self, - mut hash: H, - mut number: N, - mut data: V, + hash: H, + number: N, + data: V, is_descendent_of: &F, ) -> Result> where E: std::error::Error, @@ -218,31 +262,40 @@ impl ForkTree where } } - for root in self.roots.iter_mut() { - if root.hash == hash { - return Err(Error::Duplicate); - } + let is_descendent_of = |base: &H, target: &H| { + Ok(base == target || is_descendent_of(base, target)?) + }; - match root.import(hash, number, data, is_descendent_of)? { - Some((h, n, d)) => { - hash = h; - number = n; - data = d; - }, - None => return Ok(false), + let is_root = match self.find_node_where_mut(&hash, &number, &is_descendent_of, &|_| true)? { + Some(node) => { + if node.hash == hash { + return Err(Error::Duplicate) + } + + node.children.push(Node { + hash, + number, + data, + children: Vec::new(), + }); + + false } - } + None => { + self.roots.push(Node { + hash, + number, + data, + children: Vec::new(), + }); - self.roots.push(Node { - data, - hash: hash, - number: number, - children: Vec::new(), - }); + true + } + }; self.rebalance(); - Ok(true) + Ok(is_root) } /// Iterates over the existing roots in the tree. @@ -689,25 +742,6 @@ mod node_implementation { } impl Node { - /// Rebalance the tree, i.e. sort child nodes by max branch depth (decreasing). - pub fn rebalance(&mut self) { - self.children.sort_by_key(|n| Reverse(n.max_depth())); - for child in &mut self.children { - child.rebalance(); - } - } - - /// Finds the max depth among all branches descendent from this node. - pub fn max_depth(&self) -> usize { - let mut max = 0; - - for node in &self.children { - max = node.max_depth().max(max) - } - - max + 1 - } - /// Map node data into values of new types. pub fn map( self, @@ -731,60 +765,6 @@ mod node_implementation { } } - pub fn import( - &mut self, - hash: H, - number: N, - data: V, - is_descendent_of: &F, - ) -> Result, Error> - where E: fmt::Debug, - F: Fn(&H, &H) -> Result, - { - let mut stack: Vec<(&Self, bool)> = vec![(&self, true)]; - - while let Some((node, first_time)) = stack.pop() { - if first_time { - if node.hash == hash { - return Err(Error::Duplicate); - } - - if number <= node.number { - continue; - } - - if !node.children.is_empty() { - // We want to check all children depth-first. - - stack.push((node, false)); - - for child in node.children.iter().rev() { - stack.push((child, true)); - } - - continue; - } - } - - if is_descendent_of(&node.hash, &hash)? { - let node = &mut (node as *const Self as *mut Self); - - unsafe { - (**node).children.push(Node { - data, - hash: hash, - number: number, - children: Vec::new(), - }); - } - - return Ok(None); - } - } - - Ok(Some((hash, number, data))) - } - /// Find a node in the tree that is the deepest ancestor of the given /// block hash which also passes the given predicate, backtracking /// when the predicate fails. @@ -801,68 +781,68 @@ mod node_implementation { is_descendent_of: &F, predicate: &P, ) -> Result>, Error> - where E: std::error::Error, - F: Fn(&H, &H) -> Result, - P: Fn(&V) -> bool, + where E: std::error::Error, + F: Fn(&H, &H) -> Result, + P: Fn(&V) -> bool, { - let mut stack: Vec<(&Self, Option, bool)> = vec![(self, None, true)]; - - let mut found = false; - let mut indices: Option> = None; + let mut stack = vec![(self, None, false)]; + let mut indices = Vec::new(); + + while let Some((node, index, expanded)) = stack.pop() { + // if we have already expanded this node we drop the index + // for the tree path + if expanded { + indices.pop(); + } - while let Some((node, index, first_time)) = stack.pop() { - // stop searching this branch + // skip to the next node as we have traveled too deep if *number < node.number { continue; } - if first_time && !found && !node.children.is_empty() { - stack.push((node, index, false)); - - for (i, child) in node.children.iter().enumerate().rev() { - stack.push((child, Some(i), true)); + // if we haven't expanded this node yet and it has children + // let's keep traversing in depth + if !expanded && !node.children.is_empty() { + // we re-add this node to the stack + // noting that it has already been expanded + stack.push((node, index, true)); + + // we also need add the index of this node + // in the tree path + if let Some(idx) = index { + indices.push(idx); } + // add all children to the stack + stack.extend(node.children.iter().enumerate().map(|(i, n)| (n, Some(i), false))); + continue; } - let is_descendent_of = is_descendent_of(&node.hash, hash)?; - - if is_descendent_of { - found |= predicate(&node.data); - - if !found { - while stack.last().map(|(.., first)| *first).unwrap_or(false) { - stack.pop(); + // there's nothing left to expand in this branch, so before we backtrack + // we need to test the current node. + if is_descendent_of(&node.hash, hash)? { + // we're in the right branch + // and this node passes the predicate + if predicate(&node.data) { + // we must re-add its index (if any) since it was popped + // in the beginning of this iteration, and we also need + // to reverse the tree path + if let Some(idx) = index { + indices.push(idx); + indices.reverse(); + + return Ok(FindOutcome::Found(indices)); + } else { + // if there's no index it means that the found node + // is `self` so we return an empty tree path + return Ok(FindOutcome::Found(Vec::new())); } } - - if found { - indices = match indices.take() { - Some(mut vec) => { - if let Some(index) = index { - vec.push(index); - } - Some(vec) - }, - _ => Some(if let Some(index) = index { - vec![index] - } else { - vec![] - }) - }; - found = true; - } - } - - if found { - while stack.last().map(|(.., first)| *first).unwrap_or(false) { - stack.pop(); - } } } - Ok(indices.map(FindOutcome::Found).unwrap_or(FindOutcome::Failure(false))) + Ok(FindOutcome::Failure(false)) } /// Find a node in the tree that is the deepest ancestor of the given From 89c9b210c81198f1bc512ed3b9e61ed1d6c3deee Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 27 Oct 2020 14:02:53 +0100 Subject: [PATCH 17/22] Fix sc-consensus-epochs and sc-finality-grandpa --- client/consensus/epochs/src/lib.rs | 4 ++-- client/consensus/epochs/src/migration.rs | 2 +- client/finality-grandpa/rpc/src/report.rs | 3 ++- client/finality-grandpa/src/authorities.rs | 7 ++++--- client/finality-grandpa/src/aux_schema.rs | 3 ++- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/client/consensus/epochs/src/lib.rs b/client/consensus/epochs/src/lib.rs index acb07dd668a3c..a2e79e0c90621 100644 --- a/client/consensus/epochs/src/lib.rs +++ b/client/consensus/epochs/src/lib.rs @@ -312,7 +312,7 @@ fn fake_head_hash + AsMut<[u8]> + Clone>(parent_hash: &H) -> H { } impl Default for EpochChanges where - Hash: PartialEq + Ord, + Hash: Eq + Ord + Clone + std::hash::Hash, Number: Ord, { fn default() -> Self { @@ -321,7 +321,7 @@ impl Default for EpochChanges where } impl EpochChanges where - Hash: PartialEq + Ord + AsRef<[u8]> + AsMut<[u8]> + Copy, + Hash: Eq + Ord + AsRef<[u8]> + AsMut<[u8]> + Copy + std::hash::Hash, Number: Ord + One + Zero + Add + Copy, { /// Create a new epoch change. diff --git a/client/consensus/epochs/src/migration.rs b/client/consensus/epochs/src/migration.rs index e4717b5584e0e..e031b2d024604 100644 --- a/client/consensus/epochs/src/migration.rs +++ b/client/consensus/epochs/src/migration.rs @@ -32,7 +32,7 @@ pub struct EpochChangesV0 { pub type EpochChangesForV0 = EpochChangesV0<::Hash, NumberFor, Epoch>; impl EpochChangesV0 where - Hash: PartialEq + Ord + Copy, + Hash: Eq + Ord + Copy + std::hash::Hash, Number: Ord + Copy, { /// Create a new value of this type from raw. diff --git a/client/finality-grandpa/rpc/src/report.rs b/client/finality-grandpa/rpc/src/report.rs index a635728cb938a..bb06fcdaf61f7 100644 --- a/client/finality-grandpa/rpc/src/report.rs +++ b/client/finality-grandpa/rpc/src/report.rs @@ -20,6 +20,7 @@ use std::{ collections::{BTreeSet, HashSet}, fmt::Debug, ops::Add, + hash::Hash, }; use serde::{Deserialize, Serialize}; @@ -41,7 +42,7 @@ pub trait ReportVoterState { impl ReportAuthoritySet for SharedAuthoritySet where N: Add + Ord + Clone + Debug, - H: Clone + Debug + Eq, + H: Clone + Debug + Eq + Hash, { fn get(&self) -> (u64, HashSet) { let current_voters: HashSet = self diff --git a/client/finality-grandpa/src/authorities.rs b/client/finality-grandpa/src/authorities.rs index 2de169fc8285a..f6677c07007cf 100644 --- a/client/finality-grandpa/src/authorities.rs +++ b/client/finality-grandpa/src/authorities.rs @@ -30,6 +30,7 @@ use std::cmp::Ord; use std::fmt::Debug; use std::ops::Add; use std::sync::Arc; +use std::hash::Hash; /// Error type returned on operations on the `AuthoritySet`. #[derive(Debug, derive_more::Display)] @@ -88,7 +89,7 @@ impl SharedAuthoritySet { impl SharedAuthoritySet where N: Add + Ord + Clone + Debug, - H: Clone + Debug + H: Clone + Debug + Hash, { /// Get the earliest limit-block number that's higher or equal to the given /// min number, if any. @@ -156,7 +157,7 @@ pub struct AuthoritySet { impl AuthoritySet where - H: PartialEq, + H: Eq + Hash + Clone, N: Ord, { // authority sets must be non-empty and all weights must be greater than 0 @@ -206,7 +207,7 @@ where impl AuthoritySet where N: Add + Ord + Clone + Debug, - H: Clone + Debug, + H: Clone + Debug + Hash, { /// Returns the block hash and height at which the next pending change in /// the given chain (i.e. it includes `best_hash`) was signalled, `None` if diff --git a/client/finality-grandpa/src/aux_schema.rs b/client/finality-grandpa/src/aux_schema.rs index 4ed96d058ac6b..b4554bbf0233b 100644 --- a/client/finality-grandpa/src/aux_schema.rs +++ b/client/finality-grandpa/src/aux_schema.rs @@ -18,6 +18,7 @@ use std::fmt::Debug; use std::sync::Arc; +use std::hash::Hash; use parity_scale_codec::{Encode, Decode}; use sc_client_api::backend::AuxStore; use sp_blockchain::{Result as ClientResult, Error as ClientError}; @@ -70,7 +71,7 @@ struct V0AuthoritySet { } impl Into> for V0AuthoritySet -where H: Clone + Debug + PartialEq, +where H: Clone + Debug + Eq + Hash, N: Clone + Debug + Ord, { fn into(self) -> AuthoritySet { From cc5baf337b9456e9cb80b6c828486259572b7d83 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 27 Oct 2020 16:04:45 +0100 Subject: [PATCH 18/22] Update test values --- utils/fork-tree/src/lib.rs | 66 +++++++++++++------------------------- 1 file changed, 22 insertions(+), 44 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 3374a623f2349..64c4c77125eb7 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -1070,22 +1070,22 @@ mod test { assert_eq!( tree.find_node_index_where(&"I", &4, &is_descendent_of, &|_| true), - Ok(Some(vec![1, 1, 0])) + Ok(Some(vec![0, 1, 0])) ); assert_eq!( tree.find_node_index_where(&"L", &4, &is_descendent_of, &|_| true), - Ok(Some(vec![1, 1, 0])) + Ok(Some(vec![0, 1, 0])) ); assert_eq!( tree.find_node_index_where(&"M", &5, &is_descendent_of, &|_| true), - Ok(Some(vec![1, 1, 1, 0])) + Ok(Some(vec![0, 0, 1, 0])) ); assert_eq!( tree.find_node_index_where(&"O", &5, &is_descendent_of, &|_| true), - Ok(Some(vec![1, 1, 1, 0])) + Ok(Some(vec![0, 0, 1, 0])) ); assert_eq!( @@ -1262,7 +1262,7 @@ mod test { assert_eq!( tree.roots().map(|(h, n, _)| (h.clone(), n.clone())).collect::>(), - vec![("I", 4), ("L", 4)], + vec![("L", 4), ("I", 4)], ); // finalizing a node from another fork that isn't part of the tree clears the tree @@ -1310,7 +1310,7 @@ mod test { assert_eq!( tree.roots().map(|(h, n, _)| (h.clone(), n.clone())).collect::>(), - vec![("I", 4), ("L", 4)], + vec![("L", 4), ("I", 4)], ); assert_eq!( @@ -1483,12 +1483,19 @@ mod test { tree.iter().map(|(h, n, _)| (h.clone(), n.clone())).collect::>(), vec![ ("A", 1), - ("B", 2), ("C", 3), ("D", 4), ("E", 5), - ("F", 2), - ("G", 3), - ("H", 3), ("I", 4), - ("L", 4), ("M", 5), ("O", 5), - ("J", 2), ("K", 3) + ("B", 2), + ("C", 3), + ("D", 4), + ("E", 5), + ("F", 2), + ("H", 3), + ("L", 4), + ("M", 5), + ("O", 5), + ("I", 4), + ("G", 3), + ("J", 2), + ("K", 3), ], ); } @@ -1610,7 +1617,7 @@ mod test { assert_eq!( removed.map(|(hash, _, _)| hash).collect::>(), - vec!["A", "F", "G", "H", "I", "L", "M", "O", "J", "K"] + vec!["A", "F", "H", "L", "M", "O", "I", "G", "J", "K"] ); let removed = tree.prune( @@ -1677,7 +1684,7 @@ mod test { assert_eq!( tree.iter().map(|(h, _, _)| *h).collect::>(), - vec!["A", "B", "C", "D", "E", "F", "G", "H", "I", "L", "M", "O", "J", "K"], + vec!["A", "B", "C", "D", "E", "F", "H", "L", "M", "O", "I", "G", "J", "K"], ); // after rebalancing the tree we should iterate in preorder exploring @@ -1703,36 +1710,7 @@ mod test { } #[test] - fn find_node_where_returns_none_on_bad_is_descendent_of() { - let mut tree = ForkTree::new(); - - // - // A - B - // \ - // — C - // - let is_descendent_of = |base: &&str, block: &&str| -> Result { - match (*base, *block) { - ("A", b) => Ok(b == "B" || b == "C" || b == "D"), - ("B", b) | ("C", b) => Ok(b == "D"), - ("0", _) => Ok(true), - _ => Ok(false), - } - }; - - tree.import("A", 1, 1, &is_descendent_of).unwrap(); - tree.import("B", 2, 2, &is_descendent_of).unwrap(); - tree.import("C", 2, 4, &is_descendent_of).unwrap(); - - assert_eq!( - tree.find_node_where(&"D", &3, &is_descendent_of, &|&n| n == 4) - .map(|opt| opt.map(|node| node.hash)), - Ok(None) - ); - } - - #[test] - fn find_node_where_returns_none_on_bad_is_descendent_of_2() { + fn find_node_where_value_2() { let mut tree = ForkTree::new(); // From c441b6103d53a36b37f7c1a25599c67df5a8d88d Mon Sep 17 00:00:00 2001 From: Ashley Date: Wed, 4 Nov 2020 14:37:57 +0100 Subject: [PATCH 19/22] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- utils/fork-tree/src/lib.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 64c4c77125eb7..261c512a06891 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -211,10 +211,12 @@ impl ForkTree where // since we will begin exploring a new branch if !expanded { for child in &node.children { + // we always backtrack to the parent after exploring one branch + // so that we can update its max depth before exploring the rest of + // the branches stack.push((node, depth, true)); stack.push((child, depth + 1, false)); } - stack.push((node, depth, true)); current_max_depth = depth; continue; } @@ -232,7 +234,11 @@ impl ForkTree where let mut stack = Vec::from_iter(self.roots.iter_mut()); while let Some(node) = stack.pop() { node.children.sort_by_key(|n| { - let max_depth = max_depths.get(&n.hash).expect(""); + let max_depth = max_depths.get(&n.hash).expect( + "fails on unknown hash; \ + all tree node hashes have been added previously; \ + we only fetch tree node hashes; qed" + ); Reverse(max_depth) }); @@ -737,7 +743,7 @@ mod node_implementation { }); } - Ok(stack.pop().expect("checked")) + Ok(stack.pop().expect("fails if stack is empty; exit condition of loop above ensures stack has one element; qed")) } } @@ -1112,7 +1118,6 @@ mod test { ); } - #[test] fn import_doesnt_revert() { let (mut tree, is_descendent_of) = test_fork_tree(); @@ -1737,5 +1742,4 @@ mod test { Ok(Some("A")) ); } - } From d5046fafbab8b513f3713a67b5cfdec141f47e41 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Wed, 4 Nov 2020 14:53:12 +0100 Subject: [PATCH 20/22] Write out decode proofs --- utils/fork-tree/src/lib.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index 261c512a06891..f3d7f94be2e13 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -720,12 +720,21 @@ mod node_implementation { let mut stack = Vec::new(); // Loop until we've got a single completely decoded node on the stack. - while !(stack.len() == 1 && stack.last().map(complete).unwrap_or(false)) { + while stack.len() != 1 || !stack.last().map(complete).unwrap_or(false) { // If the top-most node is complete, pop it and push it as a child of the node // beneath it. if stack.last().map(complete).unwrap_or(false) { - let last = stack.pop().expect("We already checked this"); - let latest = stack.last_mut().expect("We know there were 2 items on the stack"); + let last = stack.pop().expect( + "If the stack was empty, the above statement would have returned none; qed" + ); + let latest = stack.last_mut().expect( + "In the above while statment, we check if the stack contains a number of \ + items different fron 1 OR if the top item is not complete; \ + In the above if statement, we check that the top item is complete; \ + Therefore, the number of items has to be different fron 1; \ + The only case where there are 0 items on the stack is at the start; \ + Therefore theremust be 2 or more items on the stack; QED" + ); latest.children.push(last); continue; } From 14dd951a547b037aeac74497d884d1e4dc5f7eab Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Wed, 4 Nov 2020 15:04:47 +0100 Subject: [PATCH 21/22] Add decoding test --- utils/fork-tree/src/lib.rs | 69 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/utils/fork-tree/src/lib.rs b/utils/fork-tree/src/lib.rs index f3d7f94be2e13..c4ac34f6af38b 100644 --- a/utils/fork-tree/src/lib.rs +++ b/utils/fork-tree/src/lib.rs @@ -971,6 +971,7 @@ impl Iterator for RemovedIterator { #[cfg(test)] mod test { use super::{FinalizationResult, ForkTree, Error}; + use codec::{Encode, Decode}; #[derive(Debug, PartialEq)] struct TestError; @@ -1751,4 +1752,72 @@ mod test { Ok(Some("A")) ); } + + #[test] + fn encoding_and_decoding_works() { + let tree = { + let mut tree = ForkTree::::new(); + + // + // - B - C - D - E + // / + // / - G + // / / + // A - F - H - I + // \ + // - L - M + // \ + // - O + // \ + // — J - K + // + // (where N is not a part of fork tree) + let is_descendent_of = |base: &String, block: &String| -> Result { + let letters = vec!["B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "O"]; + match (&base[..], &block[..]) { + ("A", b) => Ok(letters.into_iter().any(|n| n == b)), + ("B", b) => Ok(b == "C" || b == "D" || b == "E"), + ("C", b) => Ok(b == "D" || b == "E"), + ("D", b) => Ok(b == "E"), + ("E", _) => Ok(false), + ("F", b) => Ok(b == "G" || b == "H" || b == "I" || b == "L" || b == "M" || b == "O"), + ("G", _) => Ok(false), + ("H", b) => Ok(b == "I" || b == "L" || b == "M" || b == "O"), + ("I", _) => Ok(false), + ("J", b) => Ok(b == "K"), + ("K", _) => Ok(false), + ("L", b) => Ok(b == "M" || b == "O"), + ("M", _) => Ok(false), + ("O", _) => Ok(false), + ("0", _) => Ok(true), + _ => Ok(false), + } + }; + + tree.import("A".into(), 1, 10, &is_descendent_of).unwrap(); + + tree.import("B".into(), 2, 9, &is_descendent_of).unwrap(); + tree.import("C".into(), 3, 8, &is_descendent_of).unwrap(); + tree.import("D".into(), 4, 7, &is_descendent_of).unwrap(); + tree.import("E".into(), 5, 6, &is_descendent_of).unwrap(); + + tree.import("F".into(), 2, 5, &is_descendent_of).unwrap(); + tree.import("G".into(), 3, 4, &is_descendent_of).unwrap(); + + tree.import("H".into(), 3, 3, &is_descendent_of).unwrap(); + tree.import("I".into(), 4, 2, &is_descendent_of).unwrap(); + tree.import("L".into(), 4, 1, &is_descendent_of).unwrap(); + tree.import("M".into(), 5, 2, &is_descendent_of).unwrap(); + tree.import("O".into(), 5, 3, &is_descendent_of).unwrap(); + + tree.import("J".into(), 2, 4, &is_descendent_of).unwrap(); + tree.import("K".into(), 3, 11, &is_descendent_of).unwrap(); + + tree + }; + + let encoded = tree.encode(); + let decoded = ForkTree::decode(&mut &encoded[..]).unwrap(); + assert_eq!(tree, decoded); + } } From fa18a1a6c8778b94e15460a19391ba9dd0d673bd Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 9 Nov 2020 12:49:54 +0100 Subject: [PATCH 22/22] Fix grandpa test :D --- client/finality-grandpa/src/authorities.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/finality-grandpa/src/authorities.rs b/client/finality-grandpa/src/authorities.rs index f6677c07007cf..4a2d63ab947ec 100644 --- a/client/finality-grandpa/src/authorities.rs +++ b/client/finality-grandpa/src/authorities.rs @@ -794,8 +794,8 @@ mod tests { delay_kind: DelayKind::Finalized, }; - authorities.add_pending_change(change_a.clone(), &static_is_descendent_of(true)).unwrap(); - authorities.add_pending_change(change_b.clone(), &static_is_descendent_of(true)).unwrap(); + authorities.add_pending_change(change_a.clone(), &static_is_descendent_of(false)).unwrap(); + authorities.add_pending_change(change_b.clone(), &static_is_descendent_of(false)).unwrap(); assert_eq!( authorities.pending_changes().collect::>(),