Skip to content
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
baed83b
Adding fork-tree to worker
coax1d Feb 28, 2023
b7a6a37
Rebasing
coax1d Feb 28, 2023
8686ccc
Cargo fmt and clippy
coax1d Feb 28, 2023
4533b6b
taplo fmt
coax1d Feb 28, 2023
111b7c8
adding in default impl for clippy and removing senseless comments
coax1d Mar 1, 2023
07dcd74
minor fmt
coax1d Mar 1, 2023
fdbe2c6
Cherry picking and merging
coax1d Mar 1, 2023
de3d31a
Cherry picking test of is_descendent_builder
coax1d Mar 1, 2023
26daddc
Removing comments and cleanup
coax1d Mar 1, 2023
cfe8a6a
Adding in build_queue_header helper
coax1d Mar 1, 2023
56d63cb
Adding in imports for SidechainBlockBuilderTrait
coax1d Mar 1, 2023
9882418
Cargo fmt taplo fmt and clippy
coax1d Mar 1, 2023
80a1c1e
Adding some documentation to new structures
coax1d Mar 1, 2023
35f055e
cargo fmt
coax1d Mar 1, 2023
e88a6f0
Rebasing
coax1d Mar 1, 2023
473ce64
Addressing comments
coax1d Mar 1, 2023
97128ef
Refactoring for pr
coax1d Mar 1, 2023
01a574c
Fix incorrect comment
coax1d Mar 1, 2023
5d666cd
fixing docstring
coax1d Mar 1, 2023
2e6ab83
rebasing
coax1d Mar 1, 2023
c452dc2
cargo fmt
coax1d Mar 1, 2023
ae8143a
Moving errors to correct file
coax1d Mar 2, 2023
d88d4ea
refactor from comments half
coax1d Mar 6, 2023
8dc3830
Refactoring for Chris comments
coax1d Mar 6, 2023
8620d4e
Merge branch 'master' into adding_is_descendent_of_fork_tree_testing
coax1d Mar 7, 2023
3fddc34
Missing import
coax1d Mar 7, 2023
511b8cf
cargo fmt
coax1d Mar 7, 2023
45afc72
Merge branch 'master' into adding_is_descendent_of_fork_tree_testing
coax1d Mar 7, 2023
3eccee1
Minor fixes for `is_descendant_builder` (#1206)
clangenb Mar 8, 2023
d35d025
Update sidechain/consensus/common/src/is_descendant_of_builder.rs
coax1d Mar 8, 2023
47b57fd
Update sidechain/consensus/common/src/is_descendant_of_builder.rs
coax1d Mar 8, 2023
6202662
Update sidechain/consensus/common/src/is_descendant_of_builder.rs
coax1d Mar 8, 2023
8b6669a
Update sidechain/consensus/common/src/is_descendant_of_builder.rs
coax1d Mar 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions sidechain/consensus/common/src/header_db.rs
Original file line number Diff line number Diff line change
@@ -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, types::header::SidechainHeader};
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<Self::Header>;
}

/// A mocked Header Database which allows you to take a Block Hash and Query a Block Header.
pub struct HeaderDb<Hash, Header>(pub HashMap<Hash, Header>);

impl<Hash, Header> HeaderDbTrait for HeaderDb<Hash, Header>
where
// TODO: the H256 trait bounds are needed because: #1203
Hash: PartialEq + HashT + Into<H256> + From<H256> + core::cmp::Eq + Clone,
Header: HeaderT + Clone + Into<SidechainHeader>,
{
type Header = SidechainHeader;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the Into<SidechainHeader> and just use:

type Header = Header

Copy link
Copy Markdown
Contributor Author

@coax1d coax1d Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed or else you get this

error[E0277]: the trait bound `SidechainHeader: From<Header>` is not satisfied
  --> sidechain/consensus/common/src/header_db.rs:42:8
   |
42 |         Some(header.clone().into())
   |              ^^^^^^^^^^^^^^ ---- required by a bound introduced by this call
   |              |
   |              the trait `From<Header>` is not implemented for `SidechainHeader`
   |
   = note: required for `Header` to implement `Into<SidechainHeader>`
help: consider extending the `where` clause, but there might be an alternative better way to express this requirement
   |
36 |     Header: HeaderT + Clone, SidechainHeader: From<Header>
   |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0277`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that you don't need .into() when Header = SidechainHeader. Or a different question, why do you assume that the Header and Sidechainheader are not effectively the same type?


fn header(&self, hash: &H256) -> Option<Self::Header> {
let header = self.0.get(&Hash::from(*hash))?;
Some(header.clone().into())
}
}
136 changes: 136 additions & 0 deletions sidechain/consensus/common/src/is_descendant_of_builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
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;

#[allow(dead_code)]
pub struct IsDescendantOfBuilder<Hash, HeaderDb, Error>(PhantomData<(Hash, HeaderDb, Error)>);

impl<'a, Hash, HeaderDb, Error> IsDescendantOfBuilder<Hash, HeaderDb, Error>
where
Error: From<()>,
Hash: PartialEq + HashT + Default + Into<H256> + From<H256> + Clone,
HeaderDb: HeaderDbTrait,
{
Comment on lines +25 to +29
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for all these comments here, they help a lot. Regardless, I have to tell you that we are very strict with documentation/comments. They should always be:

  1. be full sentences, generally start with a verb in the 3rd person if it describes a function. (Comments don't always need to be full sentences.)
  2. use correct punctuation
  3. be as succinct as possible

Hence, I will add some comments to this file. I hope you can cope with my pickiness. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont mind the pickiness might just take me a second to get it exactly correct though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hahah, thanks for your understanding! :D

/// Build the `is_descendant_of` closure for the fork-tree structure
/// to utilize when adding and removing nodes from the tree.
#[allow(dead_code)]
pub fn build_is_descendant_of(
current: Option<(&'a Hash, &'a Hash)>,
header_db: &'a HeaderDb,
) -> impl Fn(&Hash, &Hash) -> Result<bool, Error> + 'a {
move |base, head| {
// If the base is equal to the proposed head then the head is forsure 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 =
<LowestCommonAncestorFinder<Hash, HeaderDb>>::find_lowest_common_ancestor(
head, base, header_db,
)?;
Ok(ancestor == *base)
}
}
}

#[allow(dead_code)]
pub struct LowestCommonAncestorFinder<Hash, HeaderDb>(PhantomData<(Hash, HeaderDb)>);

impl<Hash, HeaderDb> LowestCommonAncestorFinder<Hash, HeaderDb>
where
Hash: PartialEq + Default + Into<H256> + From<H256> + Clone,
HeaderDb: HeaderDbTrait,
{
/// Used by the `build_is_descendant_of` to find the LCA of two nodes in the fork-tree.
#[allow(dead_code)]
fn find_lowest_common_ancestor(a: &Hash, b: &Hash, header_db: &HeaderDb) -> Result<Hash, ()> {
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)
}
}
35 changes: 0 additions & 35 deletions sidechain/consensus/common/src/is_descendent_of_builder.rs

This file was deleted.

3 changes: 2 additions & 1 deletion sidechain/consensus/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ mod block_import;
mod block_import_confirmation_handler;
mod block_import_queue_worker;
mod error;
mod is_descendent_of_builder;
mod header_db;
mod is_descendant_of_builder;
mod peer_block_sync;

#[cfg(test)]
Expand Down
Loading