Skip to content

Commit

Permalink
approval-distribution: Fix handling of conclude
Browse files Browse the repository at this point in the history
After
moonbeam-foundation@53373a9,
approval-distribution did not terminate anymore if Conclude signal was
received.

This should have been caught by the subsystem tests, but it
wasn't because the subsystem is also exiting on error when the channels
are dropped so the test overseer was dropped which made the susbystem
exit and masked the problem.

This pr fixes both the test and the subsystem.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
  • Loading branch information
alexggh committed Aug 15, 2024
1 parent 069f8a6 commit d6364c6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
11 changes: 8 additions & 3 deletions polkadot/node/network/approval-distribution/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2508,14 +2508,18 @@ impl ApprovalDistribution {
};


self.handle_from_orchestra(message, &mut approval_voting_sender, &mut network_sender, state, rng).await;
if self.handle_from_orchestra(message, &mut approval_voting_sender, &mut network_sender, state, rng).await {
return;
}

},
}
}
}

/// Handles a from orchestra message received by approval distribution subystem.
///
/// Returns `true` if the subsystem should be stopped.
pub async fn handle_from_orchestra<
N: overseer::SubsystemSender<NetworkBridgeTxMessage>,
A: overseer::SubsystemSender<ApprovalVotingMessage>,
Expand All @@ -2526,7 +2530,7 @@ impl ApprovalDistribution {
network_sender: &mut N,
state: &mut State,
rng: &mut (impl CryptoRng + Rng),
) {
) -> bool {
match message {
FromOrchestra::Communication { msg } =>
Self::handle_incoming(
Expand Down Expand Up @@ -2555,8 +2559,9 @@ impl ApprovalDistribution {
gum::trace!(target: LOG_TARGET, number = %number, "finalized signal");
state.handle_block_finalized(network_sender, &self.metrics, number).await;
},
FromOrchestra::Signal(OverseerSignal::Conclude) => return,
FromOrchestra::Signal(OverseerSignal::Conclude) => return true,
}
false
}

async fn handle_incoming<
Expand Down
12 changes: 9 additions & 3 deletions polkadot/node/network/approval-distribution/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,13 @@ fn test_harness<T: Future<Output = VirtualOverseer>>(
let subsystem = ApprovalDistribution::new(Default::default());
{
let mut rng = rand_chacha::ChaCha12Rng::seed_from_u64(12345);

let subsystem =
subsystem.run_inner(context, &mut state, REPUTATION_CHANGE_TEST_INTERVAL, &mut rng);
let (tx, rx) = oneshot::channel();
let subsystem = async {
subsystem
.run_inner(context, &mut state, REPUTATION_CHANGE_TEST_INTERVAL, &mut rng)
.await;
tx.send(()).expect("Fail to notify subystem is done");
};

let test_fut = test_fn(virtual_overseer);

Expand All @@ -76,6 +80,8 @@ fn test_harness<T: Future<Output = VirtualOverseer>>(
.timeout(TIMEOUT)
.await
.expect("Conclude send timeout");
let _ =
rx.timeout(Duration::from_secs(2)).await.expect("Subsystem did not conclude");
},
subsystem,
));
Expand Down

0 comments on commit d6364c6

Please sign in to comment.