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

Commit

Permalink
Safe TreeRoute constructor (#12691)
Browse files Browse the repository at this point in the history 
* Safe TreeRoute constructor
* Remove test duplicate
* Better tree route error info
  • Loading branch information
@davxy
davxy authored Nov 11, 2022
1 parent 67e3f9b commit e6768a3
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 21 deletions.
8 changes: 8 additions & 0 deletions client/db/src/lib.rs
View file
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
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
14 changes: 11 additions & 3 deletions primitives/blockchain/src/header_metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,17 @@ 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(format!(
"TreeRoute pivot ({}) should be less than route length ({})",
pivot,
route.len()
))
}
}

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

0 comments on commit e6768a3

Please sign in to comment.