diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 7cc6ef54a06ee..7061e9d29af6c 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -2222,6 +2222,40 @@ mod tests { } } + #[test] + fn test_tree_route_regression() { + // NOTE: this is a test for a regression introduced in #3665, the result + // of tree_route would be erroneously computed, since it was taking into + // account the `ancestor` in `CachedHeaderMetadata` for the comparison. + // in this test we simulate the same behavior with the side-effect + // triggering the issue being eviction of a previously fetched record + // from the cache, therefore this test is dependent on the LRU cache + // size for header metadata, which is currently set to 5000 elements. + let backend = Backend::::new_test(10000, 10000); + let blockchain = backend.blockchain(); + + let genesis = insert_header(&backend, 0, Default::default(), Vec::new(), Default::default()); + + let block100 = (1..=100).fold(genesis, |parent, n| { + insert_header(&backend, n, parent, Vec::new(), Default::default()) + }); + + let block7000 = (101..=7000).fold(block100, |parent, n| { + insert_header(&backend, n, parent, Vec::new(), Default::default()) + }); + + // This will cause the ancestor of `block100` to be set to `genesis` as a side-effect. + lowest_common_ancestor(blockchain, genesis, block100).unwrap(); + + // While traversing the tree we will have to do 6900 calls to + // `header_metadata`, which will make sure we will exhaust our cache + // which only takes 5000 elements. In particular, the `CachedHeaderMetadata` struct for + // block #100 will be evicted and will get a new value (with ancestor set to its parent). + let tree_route = tree_route(blockchain, block100, block7000).unwrap(); + + assert!(tree_route.retracted().is_empty()); + } + #[test] fn test_leaves_with_complex_block_tree() { let backend: Arc> = Arc::new(Backend::new_test(20, 20)); diff --git a/core/client/header-metadata/src/lib.rs b/core/client/header-metadata/src/lib.rs index cce45f264e8ea..a8c3886020e27 100644 --- a/core/client/header-metadata/src/lib.rs +++ b/core/client/header-metadata/src/lib.rs @@ -122,7 +122,7 @@ pub fn tree_route>( // numbers are equal now. walk backwards until the block is the same - while to != from { + while to.hash != from.hash { to_branch.push(HashAndNumber { number: to.number, hash: to.hash, @@ -257,7 +257,7 @@ impl HeaderMetadata for HeaderMetadataCache { } /// Cached header metadata. Used to efficiently traverse the tree. -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Clone)] pub struct CachedHeaderMetadata { /// Hash of the header. pub hash: Block::Hash,