diff --git a/node/service/src/relay_chain_selection.rs b/node/service/src/relay_chain_selection.rs index f499c14921e2..5364f633d1dc 100644 --- a/node/service/src/relay_chain_selection.rs +++ b/node/service/src/relay_chain_selection.rs @@ -178,10 +178,15 @@ where target_hash: Hash, maybe_max_number: Option, ) -> Result, 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 SelectChain for SelectRelayChain +impl SelectRelayChain where B: HeaderProviderProvider, 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, maybe_max_number: Option, ) -> Result, 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::(), - ) - .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::(), + ) + .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 { diff --git a/node/service/src/tests.rs b/node/service/src/tests.rs index 42f1794cd79b..69dcd80b1217 100644 --- a/node/service/src/tests.rs +++ b/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>( 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);