Safe TreeRoute constructor (#12691)

* Safe TreeRoute constructor
* Remove test duplicate
* Better tree route error info
This commit is contained in:
Davide Galassi
2022-11-11 16:22:26 +01:00
committed by GitHub
parent 3e6cd742e0
commit 8c423baf32
3 changed files with 43 additions and 21 deletions
+8
View File
@@ -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();
@@ -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());
@@ -255,65 +260,66 @@ 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);
@@ -321,49 +327,49 @@ mod enactment_state_tests {
#[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);
}
@@ -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).