Bypass chain-selection subsystem until disputes are enabled. (#3676)

* cleanup of rob's patch

* depend on feature disputes, both lag and query

* typo

* make it work

TODO move the fallback into selection

* option

* Update node/service/src/relay_chain_selection.rs

* remove trait implementation and fix errors

* hotfix: tests

* fmt

* remove unused trait

* make pub(crate) for test

* disable tests with disputed blocks

* Fix warnings

Co-authored-by: Robert Habermeier <rphmeier@gmail.com>
Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com>

polkadot/node/service/src/relay_chain_selection.rs

@@ -178,10 +178,15 @@ where
 		target_hash: Hash,
 		maybe_max_number: Option<BlockNumber>,
 	) -> Result<Option<Hash>, ConsensusError> {
+		let longest_chain_best =
+			self.fallback.finality_target(target_hash, maybe_max_number).await?;
+
 		if self.selection.overseer.is_disconnected() {
-			return self.fallback.finality_target(target_hash, maybe_max_number).await
+			return Ok(longest_chain_best)
 		}
-		self.selection.finality_target(target_hash, maybe_max_number).await
+		self.selection
+			.finality_target_with_fallback(target_hash, longest_chain_best, maybe_max_number)
+			.await
 	}
 }
 
@@ -268,8 +273,7 @@ impl OverseerHandleT for Handle {
 	}
 }
 
-#[async_trait::async_trait]
-impl<B, OH> SelectChain<PolkadotBlock> for SelectRelayChain<B, OH>
+impl<B, OH> SelectRelayChain<B, OH>
 where
 	B: HeaderProviderProvider<PolkadotBlock>,
 	OH: OverseerHandleT,
@@ -313,14 +317,15 @@ where
 	///
 	/// It will also constrain the chain to only chains which are fully
 	/// approved, and chains which contain no disputes.
-	async fn finality_target(
+	pub(crate) async fn finality_target_with_fallback(
 		&self,
 		target_hash: Hash,
+		best_leaf: Option<Hash>,
 		maybe_max_number: Option<BlockNumber>,
 	) -> Result<Option<Hash>, ConsensusError> {
 		let mut overseer = self.overseer.clone();
 
-		let subchain_head = {
+		let subchain_head = if cfg!(feature = "disputes") {
 			let (tx, rx) = oneshot::channel();
 			overseer
 				.send_msg(
@@ -339,6 +344,11 @@ where
 				None => return Ok(Some(target_hash)),
 				Some(best) => best,
 			}
+		} else {
+			match best_leaf {
+				None => return Ok(Some(target_hash)),
+				Some(best_leaf) => best_leaf,
+			}
 		};
 
 		let target_number = self.block_number(target_hash)?;
@@ -419,27 +429,32 @@ where
 		let lag = initial_leaf_number.saturating_sub(subchain_number);
 		self.metrics.note_approval_checking_finality_lag(lag);
 
-		// 3. Constrain according to disputes:
-		let (tx, rx) = oneshot::channel();
-		overseer
-			.send_msg(
-				DisputeCoordinatorMessage::DetermineUndisputedChain {
-					base_number: target_number,
-					block_descriptions: subchain_block_descriptions,
-					tx,
-				},
-				std::any::type_name::<Self>(),
-			)
-			.await;
-		let (subchain_number, subchain_head) = rx
-			.await
-			.map_err(Error::OverseerDisconnected)
-			.map_err(|e| ConsensusError::Other(Box::new(e)))?
-			.unwrap_or_else(|| (subchain_number, subchain_head));
+		let lag = if cfg!(feature = "disputes") {
+			// 3. Constrain according to disputes:
+			let (tx, rx) = oneshot::channel();
+			overseer
+				.send_msg(
+					DisputeCoordinatorMessage::DetermineUndisputedChain {
+						base_number: target_number,
+						block_descriptions: subchain_block_descriptions,
+						tx,
+					},
+					std::any::type_name::<Self>(),
+				)
+				.await;
+			let (subchain_number, _subchain_head) = rx
+				.await
+				.map_err(Error::OverseerDisconnected)
+				.map_err(|e| ConsensusError::Other(Box::new(e)))?
+				.unwrap_or_else(|| (subchain_number, subchain_head));
 
-		// The the total lag accounting for disputes.
-		let lag_disputes = initial_leaf_number.saturating_sub(subchain_number);
-		self.metrics.note_disputes_finality_lag(lag_disputes);
+			// The the total lag accounting for disputes.
+			let lag_disputes = initial_leaf_number.saturating_sub(subchain_number);
+			self.metrics.note_disputes_finality_lag(lag_disputes);
+			lag_disputes
+		} else {
+			lag
+		};
 
 		// 4. Apply the maximum safeguard to the finality lag.
 		if lag > MAX_FINALITY_LAG {

polkadot/node/service/src/tests.rs

@@ -1,3 +1,4 @@
+#![allow(dead_code)]
 // Copyright 2021 Parity Technologies (UK) Ltd.
 // This file is part of Polkadot.
 
@@ -35,7 +36,6 @@ use std::{
 use assert_matches::assert_matches;
 use std::{sync::Arc, time::Duration};
 
-use consensus_common::SelectChain;
 use futures::{channel::oneshot, prelude::*};
 use polkadot_primitives::v1::{Block, BlockNumber, Hash, Header};
 use polkadot_subsystem::messages::{
@@ -88,7 +88,10 @@ fn test_harness<T: Future<Output = VirtualOverseer>>(
 
 	let target_hash = case_vars.target_block.clone();
 	let selection_process = async move {
-		let best = select_relay_chain.finality_target(target_hash, None).await.unwrap();
+		let best = select_relay_chain
+			.finality_target_with_fallback(target_hash, Some(target_hash), None)
+			.await
+			.unwrap();
 		finality_target_tx.send(best).unwrap();
 		()
 	};
@@ -722,36 +725,43 @@ fn chain_6() -> CaseVars {
 	}
 }
 
+#[cfg(feature = "disputes")]
 #[test]
 fn chain_sel_0() {
 	run_specialized_test_w_harness(chain_0);
 }
 
+#[cfg(feature = "disputes")]
 #[test]
 fn chain_sel_1() {
 	run_specialized_test_w_harness(chain_1);
 }
 
+#[cfg(feature = "disputes")]
 #[test]
 fn chain_sel_2() {
 	run_specialized_test_w_harness(chain_2);
 }
 
+#[cfg(feature = "disputes")]
 #[test]
 fn chain_sel_3() {
 	run_specialized_test_w_harness(chain_3);
 }
 
+#[cfg(feature = "disputes")]
 #[test]
 fn chain_sel_4_target_hash_value_not_contained() {
 	run_specialized_test_w_harness(chain_4);
 }
 
+#[cfg(feature = "disputes")]
 #[test]
 fn chain_sel_5_best_is_target_hash() {
 	run_specialized_test_w_harness(chain_5);
 }
 
+#[cfg(feature = "disputes")]
 #[test]
 fn chain_sel_6_approval_lag() {
 	run_specialized_test_w_harness(chain_6);