From 66d578769d90d5d67ee42d797c82023a9fe554ee Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Tue, 27 Jul 2021 14:03:32 +0200 Subject: [PATCH 1/2] change ActiveLeaves to contain at most one activated --- Cargo.lock | 2 - node/core/av-store/src/tests.rs | 45 ++--- .../availability-distribution/Cargo.toml | 1 - .../src/tests/state.rs | 7 +- node/network/availability-recovery/Cargo.toml | 1 - .../availability-recovery/src/tests.rs | 166 +++++++----------- node/network/bridge/src/lib.rs | 2 +- node/network/bridge/src/tests.rs | 62 +++---- .../src/collator_side/tests.rs | 17 +- .../dispute-distribution/src/tests/mod.rs | 28 +-- .../statement-distribution/src/tests.rs | 36 ++-- node/overseer/src/lib.rs | 16 +- node/overseer/src/tests.rs | 54 +++--- node/subsystem-types/src/lib.rs | 21 +-- 14 files changed, 193 insertions(+), 265 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c7571bf6b8c8..2b6844666a56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5932,7 +5932,6 @@ dependencies = [ "polkadot-primitives", "rand 0.8.4", "sc-network", - "smallvec 1.6.1", "sp-application-crypto", "sp-core", "sp-keyring", @@ -5962,7 +5961,6 @@ dependencies = [ "polkadot-primitives", "rand 0.8.4", "sc-network", - "smallvec 1.6.1", "sp-application-crypto", "sp-core", "sp-keyring", diff --git a/node/core/av-store/src/tests.rs b/node/core/av-store/src/tests.rs index baaa13abbf4e..c453b4135d14 100644 --- a/node/core/av-store/src/tests.rs +++ b/node/core/av-store/src/tests.rs @@ -255,15 +255,12 @@ fn runtime_api_error_does_not_stop_the_subsystem() { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { - hash: new_leaf, - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: vec![].into(), - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: new_leaf, + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let header = Header { @@ -806,15 +803,12 @@ fn we_dont_miss_anything_if_import_notifications_are_missed() { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { - hash: new_leaf, - number: 4, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: vec![].into(), - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: new_leaf, + number: 4, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; assert_matches!( @@ -1139,15 +1133,12 @@ async fn import_leaf( overseer_signal( virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { - hash: new_leaf, - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: vec![].into(), - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: new_leaf, + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; assert_matches!( diff --git a/node/network/availability-distribution/Cargo.toml b/node/network/availability-distribution/Cargo.toml index e39817aedef2..5c39886a9535 100644 --- a/node/network/availability-distribution/Cargo.toml +++ b/node/network/availability-distribution/Cargo.toml @@ -31,4 +31,3 @@ sc-network = { git = "https://github.com/paritytech/substrate", branch = "master futures-timer = "3.0.2" assert_matches = "1.4.0" maplit = "1.0" -smallvec = "1.6.1" diff --git a/node/network/availability-distribution/src/tests/state.rs b/node/network/availability-distribution/src/tests/state.rs index cceb6b95cff1..f07000d21cda 100644 --- a/node/network/availability-distribution/src/tests/state.rs +++ b/node/network/availability-distribution/src/tests/state.rs @@ -18,7 +18,6 @@ use std::{collections::{HashMap, HashSet}, sync::Arc, time::Duration}; use polkadot_node_subsystem_util::TimeoutExt; use polkadot_subsystem_testhelpers::TestSubsystemContextHandle; -use smallvec::smallvec; use futures::{FutureExt, channel::oneshot, SinkExt, channel::mpsc, StreamExt}; use futures_timer::Delay; @@ -171,13 +170,13 @@ impl TestState { self .relay_chain.iter().zip(advanced) .map(|(old, new)| ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { + activated: Some(ActivatedLeaf { hash: new.clone(), number: 1, status: LeafStatus::Fresh, span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![old.clone()], + }), + deactivated: vec![old.clone()].into(), }).collect::>() }; diff --git a/node/network/availability-recovery/Cargo.toml b/node/network/availability-recovery/Cargo.toml index c88b9fd98cdb..104750ef023e 100644 --- a/node/network/availability-recovery/Cargo.toml +++ b/node/network/availability-recovery/Cargo.toml @@ -24,7 +24,6 @@ assert_matches = "1.4.0" env_logger = "0.8.4" futures-timer = "3.0.2" log = "0.4.11" -smallvec = "1.5.1" sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/node/network/availability-recovery/src/tests.rs b/node/network/availability-recovery/src/tests.rs index d98de4f2bb06..3a72e222ed6a 100644 --- a/node/network/availability-recovery/src/tests.rs +++ b/node/network/availability-recovery/src/tests.rs @@ -20,7 +20,6 @@ use std::sync::Arc; use futures::{executor, future}; use futures_timer::Delay; use assert_matches::assert_matches; -use smallvec::smallvec; use parity_scale_codec::Encode; @@ -446,15 +445,12 @@ fn availability_is_recovered_from_chunks_if_no_group_provided() { test_harness_fast_path(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -528,15 +524,12 @@ fn availability_is_recovered_from_chunks_even_if_backing_group_supplied_if_chunk test_harness_chunks_only(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -610,15 +603,12 @@ fn bad_merkle_path_leads_to_recovery_error() { test_harness_fast_path(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -667,15 +657,12 @@ fn wrong_chunk_index_leads_to_recovery_error() { test_harness_fast_path(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -741,15 +728,12 @@ fn invalid_erasure_coding_leads_to_invalid_error() { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -789,15 +773,12 @@ fn fast_path_backing_group_recovers() { test_harness_fast_path(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -842,15 +823,12 @@ fn no_answers_in_fast_path_causes_chunk_requests() { test_harness_fast_path(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -905,15 +883,12 @@ fn task_canceled_when_receivers_dropped() { test_harness_chunks_only(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, _) = oneshot::channel(); @@ -948,15 +923,12 @@ fn chunks_retry_until_all_nodes_respond() { test_harness_chunks_only(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -1007,15 +979,12 @@ fn returns_early_if_we_have_the_data() { test_harness_chunks_only(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); @@ -1045,15 +1014,12 @@ fn does_not_query_local_validator() { test_harness_chunks_only(|mut virtual_overseer| async move { overseer_signal( &mut virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: smallvec![ActivatedLeaf { - hash: test_state.current.clone(), - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }], - deactivated: smallvec![], - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.current.clone(), + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; let (tx, rx) = oneshot::channel(); diff --git a/node/network/bridge/src/lib.rs b/node/network/bridge/src/lib.rs index 031343cd76e4..e5efcd6c502e 100644 --- a/node/network/bridge/src/lib.rs +++ b/node/network/bridge/src/lib.rs @@ -401,7 +401,7 @@ where tracing::trace!( target: LOG_TARGET, action = "ActiveLeaves", - num_activated = %activated.len(), + has_activated = activated.is_some(), num_deactivated = %deactivated.len(), ); diff --git a/node/network/bridge/src/tests.rs b/node/network/bridge/src/tests.rs index 7a90e299aa64..06f110176bf4 100644 --- a/node/network/bridge/src/tests.rs +++ b/node/network/bridge/src/tests.rs @@ -1336,31 +1336,22 @@ fn our_view_updates_decreasing_order_and_limited_to_max() { // to show that we're still connected on the collation protocol, send a view update. - let hashes = (0..MAX_VIEW_HEADS * 3).map(|i| Hash::repeat_byte(i as u8)); - - virtual_overseer.send( - FromOverseer::Signal(OverseerSignal::ActiveLeaves( - // These are in reverse order, so the subsystem must sort internally to - // get the correct view. - ActiveLeavesUpdate { - activated: hashes.enumerate().map(|(i, h)| ActivatedLeaf { - hash: h, + let hashes = (0..MAX_VIEW_HEADS + 1).map(|i| Hash::repeat_byte(i as u8)); + + for (i, hash) in hashes.enumerate().rev() { + // These are in reverse order, so the subsystem must sort internally to + // get the correct view. + virtual_overseer.send( + FromOverseer::Signal(OverseerSignal::ActiveLeaves( + ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash, number: i as _, status: LeafStatus::Fresh, span: Arc::new(jaeger::Span::Disabled), - }).rev().collect(), - deactivated: Default::default(), - } - )) - ).await; - - let view_heads = (MAX_VIEW_HEADS * 2 .. MAX_VIEW_HEADS * 3).rev() - .map(|i| (Hash::repeat_byte(i as u8), Arc::new(jaeger::Span::Disabled)) ); - - let our_view = OurView::new( - view_heads, - 0, - ); + }), + )) + ).await; + } assert_matches!( virtual_overseer.recv().await, @@ -1375,15 +1366,26 @@ fn our_view_updates_decreasing_order_and_limited_to_max() { ) ); - assert_sends_validation_event_to_all( - NetworkBridgeEvent::OurViewChange(our_view.clone()), - &mut virtual_overseer, - ).await; + let our_views = (1..=MAX_VIEW_HEADS).rev() + .map(|start| OurView::new( + (start..=MAX_VIEW_HEADS) + .rev() + .map(|i| (Hash::repeat_byte(i as u8), Arc::new(jaeger::Span::Disabled))), + 0, + )); + + for our_view in our_views { + assert_sends_validation_event_to_all( + NetworkBridgeEvent::OurViewChange(our_view.clone()), + &mut virtual_overseer, + ).await; + + assert_sends_collation_event_to_all( + NetworkBridgeEvent::OurViewChange(our_view), + &mut virtual_overseer, + ).await; + } - assert_sends_collation_event_to_all( - NetworkBridgeEvent::OurViewChange(our_view), - &mut virtual_overseer, - ).await; virtual_overseer }); } diff --git a/node/network/collator-protocol/src/collator_side/tests.rs b/node/network/collator-protocol/src/collator_side/tests.rs index 3b610a9ca873..cb97da8b9397 100644 --- a/node/network/collator-protocol/src/collator_side/tests.rs +++ b/node/network/collator-protocol/src/collator_side/tests.rs @@ -286,15 +286,12 @@ async fn setup_system(virtual_overseer: &mut VirtualOverseer, test_state: &TestS overseer_signal( virtual_overseer, - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { - hash: test_state.relay_parent, - number: 1, - status: LeafStatus::Fresh, - span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: [][..].into(), - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: test_state.relay_parent, + number: 1, + status: LeafStatus::Fresh, + span: Arc::new(jaeger::Span::Disabled), + })), ).await; overseer_send( @@ -579,7 +576,7 @@ fn advertise_and_send_collation() { assert_matches!( overseer_recv(&mut virtual_overseer).await, AllMessages::NetworkBridge(NetworkBridgeMessage::ReportPeer(bad_peer, _)) => { - assert_eq!(bad_peer, peer); + assert_eq!(bad_peer, peer); } ); assert_matches!( diff --git a/node/network/dispute-distribution/src/tests/mod.rs b/node/network/dispute-distribution/src/tests/mod.rs index f973536a46b2..369178afa9c4 100644 --- a/node/network/dispute-distribution/src/tests/mod.rs +++ b/node/network/dispute-distribution/src/tests/mod.rs @@ -614,19 +614,21 @@ async fn activate_leaf( let has_active_disputes = !active_disputes.is_empty(); handle.send(FromOverseer::Signal( OverseerSignal::ActiveLeaves( - ActiveLeavesUpdate { - activated: [ActivatedLeaf { - hash: activate, - number: 10, - status: LeafStatus::Fresh, - span: Arc::new(Span::Disabled), - }][..] - .into(), - deactivated: deactivate.into_iter().collect(), - } - - ))) - .await; + ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: activate, + number: 10, + status: LeafStatus::Fresh, + span: Arc::new(Span::Disabled), + })), + + )).await; + if let Some(deactivated) = deactivate { + handle.send(FromOverseer::Signal( + OverseerSignal::ActiveLeaves( + ActiveLeavesUpdate::stop_work(deactivated), + ) + )).await; + } assert_matches!( handle.recv().await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( diff --git a/node/network/statement-distribution/src/tests.rs b/node/network/statement-distribution/src/tests.rs index a59957d8af82..5c0b37793dee 100644 --- a/node/network/statement-distribution/src/tests.rs +++ b/node/network/statement-distribution/src/tests.rs @@ -652,15 +652,14 @@ fn receiving_from_one_sends_to_another_and_to_candidate_backing() { let test_fut = async move { // register our active heads. - handle.send(FromOverseer::Signal(OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { + handle.send(FromOverseer::Signal( + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { hash: hash_a, number: 1, status: LeafStatus::Fresh, span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: vec![].into(), - }))).await; + })), + )).await; assert_matches!( handle.recv().await, @@ -828,15 +827,14 @@ fn receiving_large_statement_from_one_sends_to_another_and_to_candidate_backing( }).await; // register our active heads. - handle.send(FromOverseer::Signal(OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { + handle.send(FromOverseer::Signal( + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { hash: hash_a, number: 1, status: LeafStatus::Fresh, span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: vec![].into(), - }))).await; + })), + )).await; assert_matches!( handle.recv().await, @@ -1300,15 +1298,14 @@ fn share_prioritizes_backing_group() { }).await; // register our active heads. - handle.send(FromOverseer::Signal(OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { + handle.send(FromOverseer::Signal( + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { hash: hash_a, number: 1, status: LeafStatus::Fresh, span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: vec![].into(), - }))).await; + })), + )).await; assert_matches!( handle.recv().await, @@ -1556,15 +1553,14 @@ fn peer_cant_flood_with_large_statements() { }).await; // register our active heads. - handle.send(FromOverseer::Signal(OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: vec![ActivatedLeaf { + handle.send(FromOverseer::Signal( + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { hash: hash_a, number: 1, status: LeafStatus::Fresh, span: Arc::new(jaeger::Span::Disabled), - }].into(), - deactivated: vec![].into(), - }))).await; + })), + )).await; assert_matches!( handle.recv().await, diff --git a/node/overseer/src/lib.rs b/node/overseer/src/lib.rs index 4a6bfc23f517..8081b2660c78 100644 --- a/node/overseer/src/lib.rs +++ b/node/overseer/src/lib.rs @@ -706,31 +706,27 @@ where /// Stop the overseer. async fn stop(mut self) { let _ = self.wait_terminate( - OverseerSignal::Conclude, - ::std::time::Duration::from_secs(1_u64) - ).await; + OverseerSignal::Conclude, + Duration::from_secs(1_u64) + ).await; } /// Run the `Overseer`. pub async fn run(mut self) -> SubsystemResult<()> { - let mut update = ActiveLeavesUpdate::default(); - + // Notify about active leaves on startup before starting the loop for (hash, number) in std::mem::take(&mut self.leaves) { let _ = self.active_leaves.insert(hash, number); if let Some((span, status)) = self.on_head_activated(&hash, None) { - update.activated.push(ActivatedLeaf { + let update = ActiveLeavesUpdate::start_work(ActivatedLeaf { hash, number, status, span, }); + self.broadcast_signal(OverseerSignal::ActiveLeaves(update)).await?; } } - if !update.is_empty() { - self.broadcast_signal(OverseerSignal::ActiveLeaves(update)).await?; - } - loop { select! { msg = self.events_rx.select_next_some() => { diff --git a/node/overseer/src/tests.rs b/node/overseer/src/tests.rs index 9921b8906589..b435dabdaae1 100644 --- a/node/overseer/src/tests.rs +++ b/node/overseer/src/tests.rs @@ -427,21 +427,21 @@ fn overseer_start_stop_works() { status: LeafStatus::Fresh, })), OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: [ActivatedLeaf { + activated: Some(ActivatedLeaf { hash: second_block_hash, number: 2, span: Arc::new(jaeger::Span::Disabled), status: LeafStatus::Fresh, - }].as_ref().into(), + }), deactivated: [first_block_hash].as_ref().into(), }), OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: [ActivatedLeaf { + activated: Some(ActivatedLeaf { hash: third_block_hash, number: 3, span: Arc::new(jaeger::Span::Disabled), status: LeafStatus::Fresh, - }].as_ref().into(), + }), deactivated: [second_block_hash].as_ref().into(), }), ]; @@ -530,23 +530,18 @@ fn overseer_finalize_works() { handle.block_finalized(third_block).await; let expected_heartbeats = vec![ - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: [ - ActivatedLeaf { - hash: first_block_hash, - number: 1, - span: Arc::new(jaeger::Span::Disabled), - status: LeafStatus::Fresh, - }, - ActivatedLeaf { - hash: second_block_hash, - number: 2, - span: Arc::new(jaeger::Span::Disabled), - status: LeafStatus::Fresh, - }, - ].as_ref().into(), - ..Default::default() - }), + 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: 2, + span: Arc::new(jaeger::Span::Disabled), + status: LeafStatus::Fresh, + })), OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { deactivated: [first_block_hash, second_block_hash].as_ref().into(), ..Default::default() @@ -630,17 +625,12 @@ fn do_not_send_empty_leaves_update_on_block_finalization() { handle.block_imported(imported_block.clone()).await; let expected_heartbeats = vec![ - OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { - activated: [ - ActivatedLeaf { - hash: imported_block.hash, - number: imported_block.number, - span: Arc::new(jaeger::Span::Disabled), - status: LeafStatus::Fresh, - } - ].as_ref().into(), - ..Default::default() - }), + OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf { + hash: imported_block.hash, + number: imported_block.number, + span: Arc::new(jaeger::Span::Disabled), + status: LeafStatus::Fresh, + })), OverseerSignal::BlockFinalized(finalized_block.hash, 1), ]; diff --git a/node/subsystem-types/src/lib.rs b/node/subsystem-types/src/lib.rs index cba7fa43ae02..7524a795cfa4 100644 --- a/node/subsystem-types/src/lib.rs +++ b/node/subsystem-types/src/lib.rs @@ -90,8 +90,8 @@ pub struct ActivatedLeaf { /// Note that the activated and deactivated fields indicate deltas, not complete sets. #[derive(Clone, Default)] pub struct ActiveLeavesUpdate { - /// New relay chain blocks of interest. - pub activated: SmallVec<[ActivatedLeaf; ACTIVE_LEAVES_SMALLVEC_CAPACITY]>, + /// New relay chain block of interest. + pub activated: Option, /// Relay chain block hashes no longer of interest. pub deactivated: SmallVec<[Hash; ACTIVE_LEAVES_SMALLVEC_CAPACITY]>, } @@ -99,7 +99,7 @@ pub struct ActiveLeavesUpdate { impl ActiveLeavesUpdate { /// Create a `ActiveLeavesUpdate` with a single activated hash pub fn start_work(activated: ActivatedLeaf) -> Self { - Self { activated: [activated][..].into(), ..Default::default() } + Self { activated: Some(activated), ..Default::default() } } /// Create a `ActiveLeavesUpdate` with a single deactivated hash @@ -109,7 +109,7 @@ impl ActiveLeavesUpdate { /// Is this update empty and doesn't contain any information? pub fn is_empty(&self) -> bool { - self.activated.is_empty() && self.deactivated.is_empty() + self.activated.is_none() && self.deactivated.is_empty() } } @@ -118,23 +118,16 @@ impl PartialEq for ActiveLeavesUpdate { /// /// Instead, it means equality when `activated` and `deactivated` are considered as sets. fn eq(&self, other: &Self) -> bool { - self.activated.len() == other.activated.len() && self.deactivated.len() == other.deactivated.len() - && self.activated.iter().all(|a| other.activated.iter().any(|o| a.hash == o.hash)) + self.activated.as_ref().map(|a| a.hash) == other.activated.as_ref().map(|a| a.hash) + && self.deactivated.len() == other.deactivated.len() && self.deactivated.iter().all(|a| other.deactivated.contains(a)) } } impl fmt::Debug for ActiveLeavesUpdate { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - struct Activated<'a>(&'a [ActivatedLeaf]); - impl fmt::Debug for Activated<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_list().entries(self.0.iter().map(|e| e.hash)).finish() - } - } - f.debug_struct("ActiveLeavesUpdate") - .field("activated", &Activated(&self.activated)) + .field("activated", &self.activated) .field("deactivated", &self.deactivated) .finish() } From 54395b478dd66dbb6f79c9e220f32a2d48955acf Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Tue, 27 Jul 2021 15:43:16 +0200 Subject: [PATCH 2/2] fix test --- .../dispute-distribution/src/tests/mod.rs | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/node/network/dispute-distribution/src/tests/mod.rs b/node/network/dispute-distribution/src/tests/mod.rs index 369178afa9c4..1a8b0468066e 100644 --- a/node/network/dispute-distribution/src/tests/mod.rs +++ b/node/network/dispute-distribution/src/tests/mod.rs @@ -614,21 +614,17 @@ async fn activate_leaf( let has_active_disputes = !active_disputes.is_empty(); handle.send(FromOverseer::Signal( OverseerSignal::ActiveLeaves( - ActiveLeavesUpdate::start_work(ActivatedLeaf { - hash: activate, - number: 10, - status: LeafStatus::Fresh, - span: Arc::new(Span::Disabled), - })), - - )).await; - if let Some(deactivated) = deactivate { - handle.send(FromOverseer::Signal( - OverseerSignal::ActiveLeaves( - ActiveLeavesUpdate::stop_work(deactivated), - ) - )).await; - } + ActiveLeavesUpdate { + activated: Some(ActivatedLeaf { + hash: activate, + number: 10, + status: LeafStatus::Fresh, + span: Arc::new(Span::Disabled), + }), + deactivated: deactivate.into_iter().collect(), + } + ))) + .await; assert_matches!( handle.recv().await, AllMessages::RuntimeApi(RuntimeApiMessage::Request(