From cca141e7f3c48c51ac3821366fff617966a71676 Mon Sep 17 00:00:00 2001
From: Tsvetomir Dimitrov <tsvetomir@parity.io>
Date: Mon, 9 Jan 2023 11:58:29 +0200
Subject: [PATCH] Fix flaky test in `dispute-coordinator`

https://github.com/paritytech/polkadot/pull/6494 updates disputes
participation priority on Active Leaves update. This operation might
trigger participation in some cases and as a result some of the message
ordering is not as nice as it used to be.

As a side effect of this `resume_dispute_without_local_statement` was
failing occasionally. The solution is not to expect that `BlockNumber`,
`CandidateEvents`, `FetchOnChainVotes` and `Ancestors` messages are
executed after `FinalizedBlockNumber` and in any specific order.

This should be okay as the code is in helper function and doesn't affect
the actual test behaviour.

Fixes https://github.com/paritytech/polkadot/issues/6514
---
 node/core/dispute-coordinator/src/tests.rs | 80 +++++++++-------------
 1 file changed, 34 insertions(+), 46 deletions(-)

diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs
index 023c95d5e23c..b5c2a6bd8e3f 100644
--- a/node/core/dispute-coordinator/src/tests.rs
+++ b/node/core/dispute-coordinator/src/tests.rs
@@ -395,57 +395,45 @@ impl TestState {
 					);
 					finished_steps.got_scraping_information = true;
 					tx.send(Ok(0)).unwrap();
-
-					// If the activated block number is > 1 the scraper will ask for block ancestors. Handle this case.
-					if block_number > 1 {
-						assert_matches!(
-							overseer_recv(virtual_overseer).await,
-							AllMessages::ChainApi(ChainApiMessage::Ancestors{
-								hash,
-								k,
-								response_channel,
-							}) => {
-								assert_eq!(hash, block_hash);	// A bit restrictive, remove if it causes problems.
-								let target_header = self.headers.get(&hash).expect("The function is called for this block so it should exist");
-								let mut response = Vec::new();
-								for i in target_header.number.saturating_sub(k as u32)..target_header.number {
-									response.push(self.block_num_to_header.get(&i).expect("headers and block_num_to_header should always be in sync").clone());
-								}
-								let _ = response_channel.send(Ok(response));
-							}
-						);
-					}
-
-					assert_matches!(
-					overseer_recv(virtual_overseer).await,
-					AllMessages::RuntimeApi(RuntimeApiMessage::Request(
-							_new_leaf,
-							RuntimeApiRequest::CandidateEvents(tx),
-							)) => {
-						tx.send(Ok(candidate_events.clone())).unwrap();
-					}
-					);
-					gum::trace!("After answering runtime api request");
-					assert_matches!(
-					overseer_recv(virtual_overseer).await,
-					AllMessages::RuntimeApi(RuntimeApiMessage::Request(
-							_new_leaf,
-							RuntimeApiRequest::FetchOnChainVotes(tx),
-							)) => {
-						//add some `BackedCandidates` or resolved disputes here as needed
-						tx.send(Ok(Some(ScrapedOnChainVotes {
-							session,
-							backing_validators_per_candidate: Vec::default(),
-							disputes: MultiDisputeStatementSet::default(),
-						}))).unwrap();
-					}
-					);
-					gum::trace!("After answering runtime API request (votes)");
 				},
 				AllMessages::ChainApi(ChainApiMessage::BlockNumber(hash, tx)) => {
 					let block_num = self.headers.get(&hash).map(|header| header.number);
 					tx.send(Ok(block_num)).unwrap();
 				},
+				AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+					_new_leaf,
+					RuntimeApiRequest::CandidateEvents(tx),
+				)) => {
+					tx.send(Ok(candidate_events.clone())).unwrap();
+				},
+				AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+					_new_leaf,
+					RuntimeApiRequest::FetchOnChainVotes(tx),
+				)) => {
+					//add some `BackedCandidates` or resolved disputes here as needed
+					tx.send(Ok(Some(ScrapedOnChainVotes {
+						session,
+						backing_validators_per_candidate: Vec::default(),
+						disputes: MultiDisputeStatementSet::default(),
+					})))
+					.unwrap();
+				},
+				AllMessages::ChainApi(ChainApiMessage::Ancestors { hash, k, response_channel }) => {
+					let target_header = self
+						.headers
+						.get(&hash)
+						.expect("The function is called for this block so it should exist");
+					let mut response = Vec::new();
+					for i in target_header.number.saturating_sub(k as u32)..target_header.number {
+						response.push(
+							self.block_num_to_header
+								.get(&i)
+								.expect("headers and block_num_to_header should always be in sync")
+								.clone(),
+						);
+					}
+					let _ = response_channel.send(Ok(response));
+				},
 				msg => {
 					panic!("Received unexpected message in `handle_sync_queries`: {:?}", msg);
 				},