From d71d2c572259605ee1c9f92b8d3e0a1de150098c Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 9 Mar 2023 09:42:19 +0100 Subject: [PATCH] Adding is_descendent_of defenitions and fork tree testing with blockqueues (#1198) * Adding fork-tree to worker * Cargo fmt and clippy * taplo fmt * adding in default impl for clippy and removing senseless comments * minor fmt * Cherry picking and merging * Cherry picking test of is_descendent_builder * Removing comments and cleanup * Adding in build_queue_header helper * Adding in imports for SidechainBlockBuilderTrait * Cargo fmt taplo fmt and clippy * Adding some documentation to new structures * cargo fmt * Addressing comments * Refactoring for pr * Fix incorrect comment * fixing docstring * cargo fmt * Moving errors to correct file * refactor from comments half * Refactoring for Chris comments * Missing import * cargo fmt * Minor fixes for `is_descendant_builder` (#1206) * properly feature gate the `is_descendant_of_builder` * [header_db] improve traitbounds * Update sidechain/consensus/common/src/is_descendant_of_builder.rs Co-authored-by: clangenb <37865735+clangenb@users.noreply.github.com> * Update sidechain/consensus/common/src/is_descendant_of_builder.rs Co-authored-by: clangenb <37865735+clangenb@users.noreply.github.com> * Update sidechain/consensus/common/src/is_descendant_of_builder.rs Co-authored-by: clangenb <37865735+clangenb@users.noreply.github.com> * Update sidechain/consensus/common/src/is_descendant_of_builder.rs Co-authored-by: clangenb <37865735+clangenb@users.noreply.github.com> --------- Co-authored-by: clangenb <37865735+clangenb@users.noreply.github.com> --- sidechain/consensus/common/src/header_db.rs | 44 +++ .../common/src/is_descendant_of_builder.rs | 133 +++++++++ .../common/src/is_descendent_of_builder.rs | 35 --- sidechain/consensus/common/src/lib.rs | 6 +- .../mocks/block_import_queue_worker_mock.rs | 259 +++++++++++++----- .../consensus/common/src/test/mocks/mod.rs | 1 + sidechain/fork-tree/src/lib.rs | 5 + 7 files changed, 384 insertions(+), 99 deletions(-) create mode 100644 sidechain/consensus/common/src/header_db.rs create mode 100644 sidechain/consensus/common/src/is_descendant_of_builder.rs delete mode 100644 sidechain/consensus/common/src/is_descendent_of_builder.rs diff --git a/sidechain/consensus/common/src/header_db.rs b/sidechain/consensus/common/src/header_db.rs new file mode 100644 index 0000000000..f15acd5028 --- /dev/null +++ b/sidechain/consensus/common/src/header_db.rs @@ -0,0 +1,44 @@ +/* + Copyright 2021 Integritee AG and Supercomputing Systems AG + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ +use itp_types::H256; +use its_primitives::traits::Header as HeaderT; +use std::{collections::HashMap, convert::From, hash::Hash as HashT}; + +/// Normally implemented on the `client` in substrate. +/// Is a trait which can offer methods for interfacing with a block Database. +pub trait HeaderDbTrait { + type Header: HeaderT; + /// Retrieves Header for the corresponding block hash. + fn header(&self, hash: &H256) -> Option; +} + +/// A mocked Header Database which allows you to take a Block Hash and Query a Block Header. +pub struct HeaderDb(pub HashMap); + +impl HeaderDbTrait for HeaderDb +where + // TODO: the H256 trait bounds are needed because: #1203 + Hash: PartialEq + HashT + Into + From + core::cmp::Eq + Clone, + Header: HeaderT + Clone, +{ + type Header = Header; + + fn header(&self, hash: &H256) -> Option { + let header = self.0.get(&Hash::from(*hash))?; + Some(header.clone()) + } +} diff --git a/sidechain/consensus/common/src/is_descendant_of_builder.rs b/sidechain/consensus/common/src/is_descendant_of_builder.rs new file mode 100644 index 0000000000..a6c8edc75f --- /dev/null +++ b/sidechain/consensus/common/src/is_descendant_of_builder.rs @@ -0,0 +1,133 @@ +/* + Copyright 2021 Integritee AG and Supercomputing Systems AG + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ +use crate::header_db::HeaderDbTrait; +use core::{hash::Hash as HashT, marker::PhantomData}; +use itp_types::H256; +use its_primitives::traits::Header as HeaderT; + +pub struct IsDescendantOfBuilder(PhantomData<(Hash, HeaderDb, Error)>); + +impl<'a, Hash, HeaderDb, Error> IsDescendantOfBuilder +where + Error: From<()>, + Hash: PartialEq + HashT + Default + Into + From + Clone, + HeaderDb: HeaderDbTrait, +{ + /// Builds the `is_descendant_of` closure for the fork-tree + /// used when adding and removing nodes from the tree. + pub fn build_is_descendant_of( + current: Option<(&'a Hash, &'a Hash)>, + header_db: &'a HeaderDb, + ) -> impl Fn(&Hash, &Hash) -> Result + 'a { + move |base, head| { + // If the base is equal to the proposed head, then the head is for sure not a descendant of the base. + if base == head { + return Ok(false) + } + + let mut head = head; + if let Some((current_hash, current_parent_hash)) = current { + // If the current hash is equal to the base, then it will not be a descendant of base. + if current_hash == base { + return Ok(false) + } + + // If the current hash is the head and the parent is the base, then we know that + // this current hash is the descendant of the parent. Otherwise we can set the + // head to the parent and find the lowest common ancestor between `head` + /// and `base` in the tree. + if current_hash == head { + if current_parent_hash == base { + return Ok(true) + } else { + head = current_parent_hash; + } + } + } + + let ancestor = + >::find_lowest_common_ancestor( + head, base, header_db, + )?; + Ok(ancestor == *base) + } + } +} + +pub struct LowestCommonAncestorFinder(PhantomData<(Hash, HeaderDb)>); + +impl LowestCommonAncestorFinder +where + Hash: PartialEq + Default + Into + From + Clone, + HeaderDb: HeaderDbTrait, +{ + /// Used by the `build_is_descendant_of` to find the LCA of two nodes in the fork-tree. + fn find_lowest_common_ancestor(a: &Hash, b: &Hash, header_db: &HeaderDb) -> Result { + let header_1 = header_db.header(&a.clone().into()).ok_or(())?; + let header_2 = header_db.header(&b.clone().into()).ok_or(())?; + let mut blocknum_1 = header_1.block_number(); + let mut blocknum_2 = header_2.block_number(); + let mut parent_1 = Hash::from(header_1.parent_hash()); + let mut parent_2 = Hash::from(header_2.parent_hash()); + + if *a == parent_2 { + // Then a is the common ancestor of b and it means it is itself the ancestor + return Ok(parent_2) + } + + if *b == parent_1 { + // Then b is the common ancestor of a and it means it is itself the ancestor + return Ok(parent_1) + } + + while blocknum_1 > blocknum_2 { + // This means block 1 is further down in the tree than block 2 + let new_parent = header_db.header(&parent_1.clone().into()).ok_or(())?; + + if new_parent.block_number() >= blocknum_2 { + blocknum_1 = new_parent.block_number(); + parent_1 = Hash::from(new_parent.parent_hash()); + } else { + break + } + } + + while blocknum_2 > blocknum_1 { + // This means block 2 is further down in the tree than block 1 + let new_parent = header_db.header(&parent_2.clone().into()).ok_or(())?; + + if new_parent.block_number() >= blocknum_1 { + blocknum_2 = new_parent.block_number(); + parent_2 = Hash::from(new_parent.parent_hash()); + } else { + break + } + } + + // At this point will be at equal height + while parent_1 != parent_2 { + // go up on both nodes + let new_header_1 = header_db.header(&parent_1.into()).ok_or(())?; + let new_header_2 = header_db.header(&parent_2.into()).ok_or(())?; + parent_1 = Hash::from(new_header_1.parent_hash()); + parent_2 = Hash::from(new_header_2.parent_hash()); + } + + // Return any Parent node Hash as in worst case scenario it is the root which is shared amongst all + Ok(parent_1) + } +} diff --git a/sidechain/consensus/common/src/is_descendent_of_builder.rs b/sidechain/consensus/common/src/is_descendent_of_builder.rs deleted file mode 100644 index a90b3acff0..0000000000 --- a/sidechain/consensus/common/src/is_descendent_of_builder.rs +++ /dev/null @@ -1,35 +0,0 @@ -#[cfg(test)] -use std::marker::PhantomData; - -#[cfg(test)] -pub struct IsDescendentOfBuilder(PhantomData); -#[cfg(test)] -impl<'a, Hash: PartialEq> IsDescendentOfBuilder { - #[cfg(test)] - /// Build the `is_descendent_of` closure for the fork-tree structure - /// to utilize when adding and removing nodes from the tree. - pub fn build_is_descendent_of( - _curr_block: Option<(&Hash, &Hash)>, - ) -> impl Fn(&Hash, &Hash) -> Result + 'a { - move |_base, _head| Ok(true) - } -} - -#[cfg(test)] -pub struct LowestCommonAncestorFinder(PhantomData); -#[cfg(test)] -impl LowestCommonAncestorFinder { - #[cfg(test)] - /// Used by the `build_is_descendent_of` to find the LCA of two nodes in the fork-tree. - pub fn find_lowest_common_ancestor(_a: Hash, _b: Hash) -> Hash { - Default::default() - } -} - -#[cfg(test)] -#[test] -fn test_build_is_descendent_of_works() { - let is_descendent_of = >::build_is_descendent_of(None); - let my_result = is_descendent_of(&42u64, &42u64); - assert_eq!(my_result, Ok(true)); -} diff --git a/sidechain/consensus/common/src/lib.rs b/sidechain/consensus/common/src/lib.rs index 4b0352fd47..c6a708c9e1 100644 --- a/sidechain/consensus/common/src/lib.rs +++ b/sidechain/consensus/common/src/lib.rs @@ -36,9 +36,13 @@ mod block_import; mod block_import_confirmation_handler; mod block_import_queue_worker; mod error; -mod is_descendent_of_builder; +mod header_db; mod peer_block_sync; +// The feature flag will be removed once we use the module outside of tests. +#[cfg(test)] +mod is_descendant_of_builder; + #[cfg(test)] mod test; diff --git a/sidechain/consensus/common/src/test/mocks/block_import_queue_worker_mock.rs b/sidechain/consensus/common/src/test/mocks/block_import_queue_worker_mock.rs index 455dbaaa73..fb2b0d8bcc 100644 --- a/sidechain/consensus/common/src/test/mocks/block_import_queue_worker_mock.rs +++ b/sidechain/consensus/common/src/test/mocks/block_import_queue_worker_mock.rs @@ -1,32 +1,29 @@ -use crate::{ - test::mocks::verifier_mock::VerifierMock, - BlockImport, - Error, - Result, - BlockImportQueueWorker, - SyncBlockFromPeer, - IsDescendentOfBuilder, -}; -use its_test::{ - sidechain_block_builder::SidechainBlockBuilderTrait, - sidechain_block_builder::SidechainBlockBuilder, - sidechain_block_data_builder::SidechainBlockDataBuilder as SidechainBlockData, - sidechain_header_builder::SidechainHeaderBuilder as SidechainHeader, -}; +/* + Copyright 2021 Integritee AG and Supercomputing Systems AG + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ +use crate::{header_db::HeaderDb, is_descendant_of_builder::IsDescendantOfBuilder}; use core::marker::PhantomData; -use itp_sgx_crypto::aes::Aes; -use itp_sgx_externalities::SgxExternalities; -use itp_test::mock::onchain_mock::OnchainMock; -use itp_types::{H256}; -use its_primitives::traits::{ShardIdentifierFor, SignedBlock as SignedSidechainBlockTrait}; -use sp_core::Pair; -use itp_block_import_queue::PopFromBlockQueue; +use fork_tree::ForkTree; +use itp_types::H256; use its_primitives::{ traits::{Block as BlockT, Header as HeaderT}, - types::{block_data::BlockData, header::SidechainHeader as Header, Block, SignedBlock} + types::{header::SidechainHeader as Header, Block}, }; -use sp_runtime::traits::Block as ParentchainBlockTrait; -use std::{collections::VecDeque, sync::RwLock}; +use its_test::sidechain_block_builder::{SidechainBlockBuilder, SidechainBlockBuilderTrait}; +use std::collections::VecDeque; #[derive(Default)] pub struct BlockQueueBuilder { @@ -37,32 +34,29 @@ pub struct BlockQueueBuilder { impl BlockQueueBuilder where Builder: SidechainBlockBuilderTrait + Default, - B: BlockT + From + B: BlockT + From, { - fn new() -> Self { - Self { - queue: VecDeque::new(), - _phantom_data: PhantomData::default(), - } + Self { queue: VecDeque::new(), _phantom_data: PhantomData::default() } } /// Allows definining a mock queue based and assumes that a genesis block /// will need to be appended to the queue as the first item. /// Returns: BuiltQueue - fn build_queue(&mut self, f: impl Fn(VecDeque) -> VecDeque) -> VecDeque { - self.add_genesis_block_to_queue(); - f(self.queue.clone()) + fn build_queue(self, f: impl FnOnce(VecDeque) -> VecDeque) -> VecDeque { + f(self.queue) } - fn add_genesis_block_to_queue(&mut self) { + fn add_genesis_block_to_queue(self) -> Self { + let mut self_mut = self; let genesis_header = Header { - block_number: 0, - parent_hash: H256::from_slice(&[0; 32]), - ..Default::default() - }; + block_number: 0, + parent_hash: H256::from_slice(&[0; 32]), + ..Default::default() + }; let block: B = Builder::default().with_header(genesis_header).build().into(); - self.queue.push_back(block); + self_mut.queue.push_back(block); + self_mut } } @@ -74,57 +68,196 @@ pub trait BlockQueueHeaderBuild { pub struct BlockQueueHeaderBuilder(PhantomData<(BlockNumber, Hash)>); -impl BlockQueueHeaderBuild for BlockQueueHeaderBuilder +impl BlockQueueHeaderBuild + for BlockQueueHeaderBuilder where BlockNumber: Into, Hash: Into, { type QueueHeader = Header; + /// Helper trait to build a Header for a BlockQueue. fn build_queue_header(block_number: BlockNumber, parent_hash: Hash) -> Self::QueueHeader { Header { block_number: block_number.into(), parent_hash: parent_hash.into(), + block_data_hash: H256::random(), ..Default::default() } } } +#[derive(Debug)] +pub enum TestError { + Error, +} + +impl From<()> for TestError { + fn from(_a: ()) -> Self { + TestError::Error + } +} + +impl std::fmt::Display for TestError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "TestError") + } +} + +impl std::error::Error for TestError {} + +#[cfg(test)] mod tests { use super::*; + fn fork_tree_from_header_queue(queue: VecDeque) -> ForkTree + where + B: BlockT, + { + // Store all block_headers in db + let db = HeaderDb::( + queue.iter().map(|block| (block.hash(), *block.header())).collect(), + ); + + // Import into forktree + let is_descendant_of = + , TestError>>::build_is_descendant_of(None, &db); + let mut tree = >::new(); + queue.iter().for_each(|block| { + let _ = tree + .import(block.header().hash(), block.header().block_number(), (), &is_descendant_of) + .unwrap(); + }); + tree + } + #[test] fn process_sequential_queue_no_forks() { + // Construct a queue which is sequential with 5 members all with distinct block numbers and parents + let mut queue = >::new() + .add_genesis_block_to_queue() + .build_queue(|mut queue| { + for i in 1..5 { + let parent_header = queue.back().unwrap().header(); + let header = >::build_queue_header( + i, + parent_header.hash(), + ); + queue.push_back(SidechainBlockBuilder::default().with_header(header).build()); + } + queue + }); - let queue = >::new().build_queue(|mut queue| { - for i in 1..5 { - let parent_header = queue.back().unwrap().header(); - let header = >::build_queue_header(i, parent_header.hash()); - queue.push_back(SidechainBlockBuilder::default().with_header(header).build()); - } - queue - }); + // queue -> [0, 1, 2, 3, 4] + assert_eq!(queue.len(), 5); + + let mut tree = fork_tree_from_header_queue::(queue.clone()); - // TODO: Add blocks to the fork-tree and assert that everything is correct + // We have a tree which looks like this. H0 is the only root. // - // H1 - H2 - H3 - H4 - H5 + // H0 - H1 - H2 - H3 - H4 // - todo!(); - println!("Process Sequential Queue With No Forks"); + + // We see that the only root of this tree is so far H0 + assert_eq!(tree.roots_hash_and_number(), vec![(&queue.front().unwrap().header.hash(), &0)]); + + // Now finalize H0 and so the new Root should be H1 + tree.finalize_root(&queue.front().unwrap().header.hash()).unwrap(); + let _ = queue.pop_front(); + assert_eq!(tree.roots_hash_and_number(), vec![(&queue.front().unwrap().header.hash(), &1)]); } #[test] fn process_sequential_queue_with_forks() { - // TODO: Make sure this works correctly + // Construct a queue which is sequential and every odd member has 2 block numbers which are the same + let mut queue = >::new() + .add_genesis_block_to_queue() + .build_queue(|mut queue| { + for i in 1..8 { + let parent_header = queue.back().unwrap().header(); + if i % 2 == 0 && i != 1 { + // 1 is not even want all odds to have 2 of the same block_number + let header = >::build_queue_header( + i, + parent_header.hash(), + ); + queue.push_back( + SidechainBlockBuilder::default().with_header(header).build(), + ); + } else { + // build a Queue with 2 headers which are of the same block_number + let headers = vec![ + >::build_queue_header( + i, + parent_header.hash(), + ), + >::build_queue_header( + i, + parent_header.hash(), + ), + ]; + headers.iter().for_each(|header| { + queue.push_back( + SidechainBlockBuilder::default().with_header(*header).build(), + ); + }); + } + } + queue + }); + + // queue -> [0, 1, 1, 2, 3, 3, 4, 5, 5, 6, 7, 7] + assert_eq!(queue.len(), 12); + + let mut tree = fork_tree_from_header_queue::(queue.clone()); + + // We have a tree which looks like the following + // - (H5, B3).. + // / + // - (H3, B2) + // / \ + // - (H1, B1) - (H4, B3).. + // / + // / + // (H0, B0) + // \ + // \ + // - (H2, B1).. // - // - H2.. - // / - // H1.. - H4.. - // \ / - // - H3.. - // \ - // - H5.. // - todo!(); - println!("Process Sequential Queue with Forks") + + // H0 is the first root + assert_eq!(tree.roots_hash_and_number(), vec![(&queue.front().unwrap().header.hash(), &0)]); + + // Now if we finalize H0 we should see 2 roots H1 and H2 + tree.finalize_root(&queue.front().unwrap().header.hash()).unwrap(); + let _ = queue.pop_front(); + assert_eq!( + tree.roots_hash_and_number(), + vec![(&queue[1].header.hash(), &1), (&queue[0].header.hash(), &1)] + ); + + // If we finalize (H1, B1) then we should see one roots (H3, B2) + let _ = queue.pop_front(); // remove (H1, B1) + tree.finalize_root(&queue.front().unwrap().header.hash()).unwrap(); + let _ = queue.pop_front(); // remove (H2, B1) + assert_eq!(tree.roots_hash_and_number(), vec![(&queue[0].header.hash(), &2)]); + + // If we finalize (H3, B2) we should see two roots (H4, B3), (H5, B3) + tree.finalize_root(&queue.front().unwrap().header.hash()).unwrap(); + let _ = queue.pop_front(); // remove (H3, B2) + assert_eq!( + tree.roots_hash_and_number(), + vec![(&queue[1].header.hash(), &3), (&queue[0].header.hash(), &3)] + ); + } + + #[test] + fn process_non_sequential_queue_without_forks() { + // TODO + } + + #[test] + fn process_non_sequential_queue_with_forks() { + // TODO } } diff --git a/sidechain/consensus/common/src/test/mocks/mod.rs b/sidechain/consensus/common/src/test/mocks/mod.rs index 39137ee822..1408ce9402 100644 --- a/sidechain/consensus/common/src/test/mocks/mod.rs +++ b/sidechain/consensus/common/src/test/mocks/mod.rs @@ -15,6 +15,7 @@ */ +pub mod block_import_queue_worker_mock; pub mod block_importer_mock; pub mod confirm_block_import_mock; pub mod verifier_mock; diff --git a/sidechain/fork-tree/src/lib.rs b/sidechain/fork-tree/src/lib.rs index 1f7fa76a4a..0af11b653b 100644 --- a/sidechain/fork-tree/src/lib.rs +++ b/sidechain/fork-tree/src/lib.rs @@ -188,6 +188,11 @@ where self.roots.iter().map(|node| (&node.hash, &node.number, &node.data)) } + /// Iterates over the roots and gives just the hash and block number + pub fn roots_hash_and_number(&self) -> Vec<(&H, &N)> { + self.roots.iter().map(|node| (&node.hash, &node.number)).collect::>() + } + fn node_iter(&self) -> impl Iterator> { // we need to reverse the order of roots to maintain the expected // ordering since the iterator uses a stack to track state.