Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Safe TreeRoute constructor #12691

Merged
merged 4 commits into from
Nov 11, 2022
Merged

Safe TreeRoute constructor #12691

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0edc4c
Safe TreeRoute constructor
davxy Nov 11, 2022
1546399
Remove test duplicate
davxy Nov 11, 2022
ad4e048
Remove useless dynamic String for error
davxy Nov 11, 2022
12ea2a0
Better tree route error info
davxy Nov 11, 2022
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
8 changes: 8 additions & 0 deletions client/db/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2796,6 +2796,14 @@ pub(crate) mod tests {
let b1 = insert_header(&backend, 1, block0, None, H256::from([1; 32]));
let b2 = insert_header(&backend, 2, b1, None, Default::default());

{
let tree_route = tree_route(blockchain, a1, a1).unwrap();

assert_eq!(tree_route.common_block().hash, a1);
assert!(tree_route.retracted().is_empty());
assert!(tree_route.enacted().is_empty());
}

{
let tree_route = tree_route(blockchain, a3, b2).unwrap();

Expand Down
42 changes: 24 additions & 18 deletions client/transaction-pool/src/enactment_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,19 @@ mod enactment_state_tests {
}
};

Ok(TreeRoute::new(vec, pivot))
TreeRoute::new(vec, pivot)
}

mod mock_tree_route_tests {
use super::*;

/// asserts that tree routes are equal
fn assert_treeroute_eq(expected: TreeRoute<Block>, result: TreeRoute<Block>) {
fn assert_treeroute_eq(
expected: Result<TreeRoute<Block>, String>,
result: Result<TreeRoute<Block>, String>,
) {
let expected = expected.unwrap();
let result = result.unwrap();
assert_eq!(result.common_block().hash, expected.common_block().hash);
assert_eq!(result.enacted().len(), expected.enacted().len());
assert_eq!(result.retracted().len(), expected.retracted().len());
Expand All @@ -255,115 +260,116 @@ mod enactment_state_tests {
}

// some tests for mock tree_route function

#[test]
fn tree_route_mock_test_01() {
let result = tree_route(b1().hash, a().hash).expect("tree route exists");
let result = tree_route(b1().hash, a().hash);
let expected = TreeRoute::new(vec![b1(), a()], 1);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_02() {
let result = tree_route(a().hash, b1().hash).expect("tree route exists");
let result = tree_route(a().hash, b1().hash);
let expected = TreeRoute::new(vec![a(), b1()], 0);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_03() {
let result = tree_route(a().hash, c2().hash).expect("tree route exists");
let result = tree_route(a().hash, c2().hash);
let expected = TreeRoute::new(vec![a(), b2(), c2()], 0);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_04() {
let result = tree_route(e2().hash, a().hash).expect("tree route exists");
let result = tree_route(e2().hash, a().hash);
let expected = TreeRoute::new(vec![e2(), d2(), c2(), b2(), a()], 4);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_05() {
let result = tree_route(d1().hash, b1().hash).expect("tree route exists");
let result = tree_route(d1().hash, b1().hash);
let expected = TreeRoute::new(vec![d1(), c1(), b1()], 2);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_06() {
let result = tree_route(d2().hash, b2().hash).expect("tree route exists");
let result = tree_route(d2().hash, b2().hash);
let expected = TreeRoute::new(vec![d2(), c2(), b2()], 2);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_07() {
let result = tree_route(b1().hash, d1().hash).expect("tree route exists");
let result = tree_route(b1().hash, d1().hash);
let expected = TreeRoute::new(vec![b1(), c1(), d1()], 0);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_08() {
let result = tree_route(b2().hash, d2().hash).expect("tree route exists");
let result = tree_route(b2().hash, d2().hash);
let expected = TreeRoute::new(vec![b2(), c2(), d2()], 0);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_09() {
let result = tree_route(e2().hash, e1().hash).expect("tree route exists");
let result = tree_route(e2().hash, e1().hash);
let expected =
TreeRoute::new(vec![e2(), d2(), c2(), b2(), a(), b1(), c1(), d1(), e1()], 4);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_10() {
let result = tree_route(e1().hash, e2().hash).expect("tree route exists");
let result = tree_route(e1().hash, e2().hash);
let expected =
TreeRoute::new(vec![e1(), d1(), c1(), b1(), a(), b2(), c2(), d2(), e2()], 4);
assert_treeroute_eq(result, expected);
}
#[test]
fn tree_route_mock_test_11() {
let result = tree_route(b1().hash, c2().hash).expect("tree route exists");
let result = tree_route(b1().hash, c2().hash);
let expected = TreeRoute::new(vec![b1(), a(), b2(), c2()], 1);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_12() {
let result = tree_route(d2().hash, b1().hash).expect("tree route exists");
let result = tree_route(d2().hash, b1().hash);
let expected = TreeRoute::new(vec![d2(), c2(), b2(), a(), b1()], 3);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_13() {
let result = tree_route(c2().hash, e1().hash).expect("tree route exists");
let result = tree_route(c2().hash, e1().hash);
let expected = TreeRoute::new(vec![c2(), b2(), a(), b1(), c1(), d1(), e1()], 2);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_14() {
let result = tree_route(b1().hash, b1().hash).expect("tree route exists");
let result = tree_route(b1().hash, b1().hash);
let expected = TreeRoute::new(vec![b1()], 0);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_15() {
let result = tree_route(b2().hash, b2().hash).expect("tree route exists");
let result = tree_route(b2().hash, b2().hash);
let expected = TreeRoute::new(vec![b2()], 0);
assert_treeroute_eq(result, expected);
}

#[test]
fn tree_route_mock_test_16() {
let result = tree_route(a().hash, a().hash).expect("tree route exists");
let result = tree_route(a().hash, a().hash);
let expected = TreeRoute::new(vec![a()], 0);
assert_treeroute_eq(result, expected);
}
Expand Down
10 changes: 7 additions & 3 deletions primitives/blockchain/src/header_metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,13 @@ pub struct TreeRoute<Block: BlockT> {
impl<Block: BlockT> TreeRoute<Block> {
/// Creates a new `TreeRoute`.
///
/// It is required that `pivot >= route.len()`, otherwise it may panics.
pub fn new(route: Vec<HashAndNumber<Block>>, pivot: usize) -> Self {
TreeRoute { route, pivot }
/// To preserve the structure safety invariats it is required that `pivot < route.len()`.
pub fn new(route: Vec<HashAndNumber<Block>>, pivot: usize) -> Result<Self, String> {
if pivot < route.len() {
Ok(TreeRoute { route, pivot })
} else {
Err("TreeRoute pivot should be less than route length".into())
davxy marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the relevant part

}

/// Get a slice of all retracted blocks in reverse order (towards common ancestor).
Expand Down