diff --git a/node/overseer/src/lib.rs b/node/overseer/src/lib.rs index 961e1e659f42..3a1e5a31458d 100644 --- a/node/overseer/src/lib.rs +++ b/node/overseer/src/lib.rs @@ -674,7 +674,9 @@ where let mut update = ActiveLeavesUpdate::default(); self.active_leaves.retain(|h, n| { - if *n <= block.number { + // prune all orphaned leaves, but don't prune + // the finalized block if it is itself a leaf. + if *n <= block.number && *h != block.hash { update.deactivated.push(*h); false } else { diff --git a/node/overseer/src/tests.rs b/node/overseer/src/tests.rs index 2ab2e01afe96..39eb91e0f6d6 100644 --- a/node/overseer/src/tests.rs +++ b/node/overseer/src/tests.rs @@ -574,6 +574,103 @@ fn overseer_finalize_works() { }); } +// Tests that finalization of an active leaf doesn't remove it from +// the leaves set. +#[test] +fn overseer_finalize_leaf_preserves_it() { + let spawner = sp_core::testing::TaskExecutor::new(); + + executor::block_on(async move { + let first_block_hash = [1; 32].into(); + let second_block_hash = [2; 32].into(); + + let first_block = + BlockInfo { hash: first_block_hash, parent_hash: [0; 32].into(), number: 1 }; + let second_block = + BlockInfo { hash: second_block_hash, parent_hash: [42; 32].into(), number: 1 }; + + let (tx_5, mut rx_5) = metered::channel(64); + let (tx_6, mut rx_6) = metered::channel(64); + + // start with two forks at height 1. + + let (overseer, handle) = dummy_overseer_builder(spawner, MockSupportsParachains, None) + .unwrap() + .replace_candidate_validation(move |_| TestSubsystem5(tx_5)) + .replace_candidate_backing(move |_| TestSubsystem6(tx_6)) + .leaves(block_info_to_pair(vec![first_block.clone(), second_block])) + .build() + .unwrap(); + let mut handle = Handle::new(handle); + + let overseer_fut = overseer.run().fuse(); + pin_mut!(overseer_fut); + + let mut ss5_results = Vec::new(); + let mut ss6_results = Vec::new(); + + // This should stop work on the second block, but only the + // second block. + handle.block_finalized(first_block).await; + + let expected_heartbeats = vec![ + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: first_block_hash, + number: 1, + span: Arc::new(jaeger::Span::Disabled), + status: LeafStatus::Fresh, + })), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: second_block_hash, + number: 1, + span: Arc::new(jaeger::Span::Disabled), + status: LeafStatus::Fresh, + })), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { + deactivated: [second_block_hash].as_ref().into(), + ..Default::default() + }), + OverseerSignal::BlockFinalized(first_block_hash, 1), + ]; + + loop { + select! { + res = overseer_fut => { + assert!(res.is_ok()); + break; + }, + res = rx_5.next() => { + if let Some(res) = res { + ss5_results.push(res); + } + } + res = rx_6.next() => { + if let Some(res) = res { + ss6_results.push(res); + } + } + complete => break, + } + + if ss5_results.len() == expected_heartbeats.len() && + ss6_results.len() == expected_heartbeats.len() + { + handle.stop().await; + } + } + + assert_eq!(ss5_results.len(), expected_heartbeats.len()); + assert_eq!(ss6_results.len(), expected_heartbeats.len()); + + // Notifications on finality for multiple blocks at once + // may be received in different orders. + for expected in expected_heartbeats { + assert!(ss5_results.contains(&expected)); + assert!(ss6_results.contains(&expected)); + } + }); +} + #[test] fn do_not_send_empty_leaves_update_on_block_finalization() { let spawner = sp_core::testing::TaskExecutor::new();