diff --git a/Cargo.lock b/Cargo.lock index d7b10d49007a..c7571bf6b8c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1763,6 +1763,19 @@ dependencies = [ "termcolor", ] +[[package]] +name = "env_logger" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b2cf0344971ee6c64c31be0d530793fba457d322dfec2810c453d0ef228f9c3" +dependencies = [ + "atty", + "humantime 2.0.1", + "log", + "regex", + "termcolor", +] + [[package]] name = "environmental" version = "1.1.3" @@ -6946,16 +6959,18 @@ dependencies = [ name = "polkadot-service" version = "0.9.8" dependencies = [ + "assert_matches", "async-trait", "beefy-gadget", "beefy-primitives", - "env_logger 0.8.4", + "env_logger 0.9.0", "frame-system-rpc-runtime-api", "futures 0.3.15", "hex-literal", "kusama-runtime", "kvdb", "kvdb-rocksdb", + "log", "pallet-babe", "pallet-im-online", "pallet-mmr-primitives", @@ -6985,6 +7000,7 @@ dependencies = [ "polkadot-node-core-runtime-api", "polkadot-node-primitives", "polkadot-node-subsystem", + "polkadot-node-subsystem-test-helpers", "polkadot-node-subsystem-util", "polkadot-overseer", "polkadot-parachain", diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index 16ce617b7a3e..75410b5099b9 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -27,7 +27,7 @@ use polkadot_node_subsystem::{ ApprovalVotingMessage, RuntimeApiMessage, RuntimeApiRequest, ChainApiMessage, ApprovalDistributionMessage, CandidateValidationMessage, AvailabilityRecoveryMessage, ChainSelectionMessage, DisputeCoordinatorMessage, - ImportStatementsResult, + ImportStatementsResult, HighestApprovedAncestorBlock, BlockDescription, }, errors::RecoveryError, overseer::{self, SubsystemSender as _}, SubsystemContext, SubsystemError, SubsystemResult, SpawnedSubsystem, @@ -1180,7 +1180,7 @@ async fn handle_approved_ancestor( target: Hash, lower_bound: BlockNumber, wakeups: &Wakeups, -) -> SubsystemResult> { +) -> SubsystemResult> { const MAX_TRACING_WINDOW: usize = 200; const ABNORMAL_DEPTH_THRESHOLD: usize = 5; @@ -1228,6 +1228,8 @@ async fn handle_approved_ancestor( Vec::new() }; + let mut block_descriptions = Vec::new(); + let mut bits: BitVec = Default::default(); for (i, block_hash) in std::iter::once(target).chain(ancestry).enumerate() { // Block entries should be present as the assumption is that @@ -1259,8 +1261,10 @@ async fn handle_approved_ancestor( } } else if bits.len() <= ABNORMAL_DEPTH_THRESHOLD { all_approved_max = None; + block_descriptions.clear(); } else { all_approved_max = None; + block_descriptions.clear(); let unapproved: Vec<_> = entry.unapproved_candidates().collect(); tracing::debug!( @@ -1338,6 +1342,11 @@ async fn handle_approved_ancestor( } } } + block_descriptions.push(BlockDescription { + block_hash, + session: entry.session(), + candidates: entry.candidates().iter().map(|(_idx, candidate_hash)| *candidate_hash ).collect(), + }); } tracing::trace!( @@ -1366,8 +1375,19 @@ async fn handle_approved_ancestor( }, ); + // `reverse()` to obtain the ascending order from lowest to highest + // block within the candidates, which is the expected order + block_descriptions.reverse(); + + let all_approved_max = all_approved_max.map(|(hash, block_number)| { + HighestApprovedAncestorBlock{ + hash, + number: block_number, + descriptions: block_descriptions, + } + }); match all_approved_max { - Some((ref hash, ref number)) => { + Some(HighestApprovedAncestorBlock { ref hash, ref number, .. }) => { span.add_uint_tag("approved-number", *number as u64); span.add_string_fmt_debug_tag("approved-hash", hash); } diff --git a/node/core/approval-voting/src/old_tests.rs b/node/core/approval-voting/src/old_tests.rs index 6a2141dbc31f..4c6c6a230ebe 100644 --- a/node/core/approval-voting/src/old_tests.rs +++ b/node/core/approval-voting/src/old_tests.rs @@ -26,7 +26,7 @@ use polkadot_node_primitives::approval::{ RELAY_VRF_MODULO_CONTEXT, DelayTranche, }; use polkadot_node_subsystem_test_helpers::make_subsystem_context; -use polkadot_node_subsystem::messages::AllMessages; +use polkadot_node_subsystem::messages::{AllMessages, HighestApprovedAncestorBlock}; use sp_core::testing::TaskExecutor; use parking_lot::Mutex; @@ -1612,10 +1612,12 @@ fn approved_ancestor_all_approved() { let test_fut = Box::pin(async move { let overlay_db = OverlayedBackend::new(&db); - assert_eq!( + assert_matches!( handle_approved_ancestor(&mut ctx, &overlay_db, block_hash_4, 0, &Default::default()) - .await.unwrap(), - Some((block_hash_4, 4)), + .await, + Ok(Some(HighestApprovedAncestorBlock { hash, number, .. } )) => { + assert_eq!((block_hash_4, 4), (hash, number)); + } ) }); @@ -1686,10 +1688,12 @@ fn approved_ancestor_missing_approval() { let test_fut = Box::pin(async move { let overlay_db = OverlayedBackend::new(&db); - assert_eq!( + assert_matches!( handle_approved_ancestor(&mut ctx, &overlay_db, block_hash_4, 0, &Default::default()) - .await.unwrap(), - Some((block_hash_2, 2)), + .await, + Ok(Some(HighestApprovedAncestorBlock { hash, number, .. })) => { + assert_eq!((block_hash_2, 2), (hash, number)); + } ) }); diff --git a/node/core/dispute-coordinator/src/lib.rs b/node/core/dispute-coordinator/src/lib.rs index 50d2d7f78253..db35c49803f9 100644 --- a/node/core/dispute-coordinator/src/lib.rs +++ b/node/core/dispute-coordinator/src/lib.rs @@ -35,7 +35,7 @@ use polkadot_node_subsystem::{ errors::{ChainApiError, RuntimeApiError}, messages::{ ChainApiMessage, DisputeCoordinatorMessage, DisputeDistributionMessage, - DisputeParticipationMessage, ImportStatementsResult, + DisputeParticipationMessage, ImportStatementsResult, BlockDescription, } }; use polkadot_node_subsystem_util::rolling_session_window::{ @@ -961,14 +961,16 @@ fn make_dispute_message( ).map_err(MakeDisputeMessageError::InvalidStatementCombination) } +/// Determine the the best block and its block number. +/// Assumes `block_descriptions` are sorted from the one +/// with the lowest `BlockNumber` to the highest. fn determine_undisputed_chain( overlay_db: &mut OverlayedBackend<'_, impl Backend>, base_number: BlockNumber, - block_descriptions: Vec<(Hash, SessionIndex, Vec)>, + block_descriptions: Vec, ) -> Result, Error> { let last = block_descriptions.last() - - .map(|e| (base_number + block_descriptions.len() as BlockNumber, e.0)); + .map(|e| (base_number + block_descriptions.len() as BlockNumber, e.block_hash)); // Fast path for no disputes. let recent_disputes = match overlay_db.load_recent_disputes()? { @@ -984,14 +986,14 @@ fn determine_undisputed_chain( ) }; - for (i, (_, session, candidates)) in block_descriptions.iter().enumerate() { + for (i, BlockDescription { session, candidates, .. }) in block_descriptions.iter().enumerate() { if candidates.iter().any(|c| is_possibly_invalid(*session, *c)) { if i == 0 { return Ok(None); } else { return Ok(Some(( base_number + i as BlockNumber, - block_descriptions[i - 1].0, + block_descriptions[i - 1].block_hash, ))); } } diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs index b3610df2f043..a3b8c669eae2 100644 --- a/node/core/dispute-coordinator/src/tests.rs +++ b/node/core/dispute-coordinator/src/tests.rs @@ -22,6 +22,7 @@ use polkadot_primitives::v1::{BlakeTwo256, HashT, ValidatorId, Header, SessionIn use polkadot_node_subsystem::{jaeger, ActiveLeavesUpdate, ActivatedLeaf, LeafStatus}; use polkadot_node_subsystem::messages::{ AllMessages, ChainApiMessage, RuntimeApiMessage, RuntimeApiRequest, + BlockDescription, }; use polkadot_node_subsystem_test_helpers::{make_subsystem_context, TestSubsystemContextHandle}; use sp_core::testing::TaskExecutor; @@ -672,10 +673,10 @@ fn finality_votes_ignore_disputed_candidates() { let block_hash_b = Hash::repeat_byte(0x0b); virtual_overseer.send(FromOverseer::Communication { - msg: DisputeCoordinatorMessage::DetermineUndisputedChain { + msg: DisputeCoordinatorMessage::DetermineUndisputedChain{ base_number: 10, block_descriptions: vec![ - (block_hash_a, session, vec![candidate_hash]), + BlockDescription { block_hash: block_hash_a, session, candidates: vec![candidate_hash] }, ], tx, }, @@ -688,8 +689,8 @@ fn finality_votes_ignore_disputed_candidates() { msg: DisputeCoordinatorMessage::DetermineUndisputedChain { base_number: 10, block_descriptions: vec![ - (block_hash_a, session, vec![]), - (block_hash_b, session, vec![candidate_hash]), + BlockDescription { block_hash: block_hash_a, session, candidates: vec![] }, + BlockDescription { block_hash: block_hash_b, session, candidates: vec![candidate_hash] }, ], tx, }, @@ -804,7 +805,7 @@ fn supermajority_valid_dispute_may_be_finalized() { msg: DisputeCoordinatorMessage::DetermineUndisputedChain { base_number: 10, block_descriptions: vec![ - (block_hash_a, session, vec![candidate_hash]), + BlockDescription { block_hash: block_hash_a, session, candidates: vec![candidate_hash] }, ], tx, }, @@ -817,8 +818,8 @@ fn supermajority_valid_dispute_may_be_finalized() { msg: DisputeCoordinatorMessage::DetermineUndisputedChain { base_number: 10, block_descriptions: vec![ - (block_hash_a, session, vec![]), - (block_hash_b, session, vec![candidate_hash]), + BlockDescription { block_hash: block_hash_a, session, candidates: vec![] }, + BlockDescription { block_hash: block_hash_b, session, candidates: vec![candidate_hash] }, ], tx, }, diff --git a/node/service/Cargo.toml b/node/service/Cargo.toml index 14ddc256ec67..82e7903162d2 100644 --- a/node/service/Cargo.toml +++ b/node/service/Cargo.toml @@ -112,7 +112,10 @@ polkadot-statement-distribution = { path = "../network/statement-distribution", [dev-dependencies] polkadot-test-client = { path = "../test/client" } -env_logger = "0.8.4" +polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } +env_logger = "0.9.0" +log = "0.4.14" +assert_matches = "1.5.0" [features] default = ["db", "full-node"] diff --git a/node/service/src/grandpa_support.rs b/node/service/src/grandpa_support.rs index 8e0affc7264c..5ffcab34cb0a 100644 --- a/node/service/src/grandpa_support.rs +++ b/node/service/src/grandpa_support.rs @@ -18,23 +18,24 @@ use std::sync::Arc; -use sp_runtime::traits::{Block as BlockT, NumberFor}; -use sp_runtime::generic::BlockId; use sp_runtime::traits::Header as _; +use sp_runtime::traits::{Block as BlockT, NumberFor}; + +use crate::HeaderProvider; #[cfg(feature = "full-node")] use polkadot_primitives::v1::Hash; /// Returns the block hash of the block at the given `target_number` by walking /// backwards from the given `current_header`. -pub(super) fn walk_backwards_to_target_block( - backend: &B, +pub(super) fn walk_backwards_to_target_block( + backend: &HP, target_number: NumberFor, current_header: &Block::Header, ) -> Result<(Block::Hash, NumberFor), sp_blockchain::Error> where Block: BlockT, - B: sp_blockchain::HeaderBackend, + HP: HeaderProvider, { let mut target_hash = current_header.hash(); let mut target_header = current_header.clone(); @@ -54,7 +55,7 @@ where target_hash = *target_header.parent_hash(); target_header = backend - .header(BlockId::Hash(target_hash))? + .header(target_hash)? .expect("Header known to exist due to the existence of one of its descendants; qed"); } } @@ -69,7 +70,7 @@ pub(crate) struct PauseAfterBlockFor(pub(crate) N, pub(crate) N); impl grandpa::VotingRule for PauseAfterBlockFor> where Block: BlockT, - B: sp_blockchain::HeaderBackend, + B: sp_blockchain::HeaderBackend + 'static, { fn restrict_vote( &self, diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index c6d1241d8da1..e08af558157b 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2017-2020 Parity Technologies (UK) Ltd. +// Copyright 2017-2021 Parity Technologies (UK) Ltd. // This file is part of Polkadot. // Polkadot is free software: you can redistribute it and/or modify @@ -34,6 +34,9 @@ pub use self::overseer::{ create_default_subsystems, }; +#[cfg(test)] +mod tests; + #[cfg(feature = "full-node")] use { tracing::info, @@ -51,7 +54,6 @@ use { sp_trie::PrefixedMemoryDB, sc_client_api::ExecutorProvider, grandpa::{self, FinalityProofProvider as GrandpaFinalityProofProvider}, - sp_runtime::traits::Header as HeaderT, }; #[cfg(feature = "full-node")] @@ -103,7 +105,18 @@ pub use service::{ }; pub use service::config::{DatabaseConfig, PrometheusConfig}; pub use sp_api::{ApiRef, Core as CoreApi, ConstructRuntimeApi, ProvideRuntimeApi, StateBackend}; -pub use sp_runtime::traits::{DigestFor, HashFor, NumberFor, Block as BlockT, self as runtime_traits, BlakeTwo256}; +pub use sp_runtime::{ + generic, + traits::{ + DigestFor, + HashFor, + NumberFor, + Block as BlockT, + Header as HeaderT, + self as runtime_traits, + BlakeTwo256, + }, +}; #[cfg(feature = "kusama-native")] pub use kusama_runtime; @@ -114,9 +127,76 @@ pub use rococo_runtime; pub use westend_runtime; /// The maximum number of active leaves we forward to the [`Overseer`] on startup. -#[cfg(any(test,feature = "full-node"))] +#[cfg(any(test, feature = "full-node"))] const MAX_ACTIVE_LEAVES: usize = 4; +/// Provides the header and block number for a hash. +/// +/// Decouples `sc_client_api::Backend` and `sp_blockchain::HeaderBackend`. +pub trait HeaderProvider: Send + Sync + 'static +where + Block: BlockT, + Error: std::fmt::Debug + Send + Sync + 'static, +{ + /// Obtain the header for a hash. + fn header( + &self, + hash: ::Hash, + ) -> Result::Header>, Error>; + /// Obtain the block number for a hash. + fn number( + &self, + hash: ::Hash, + ) -> Result::Header as HeaderT>::Number>, Error>; +} + +impl HeaderProvider for T +where + Block: BlockT, + T: sp_blockchain::HeaderBackend + 'static, +{ + fn header( + &self, + hash: Block::Hash, + ) -> sp_blockchain::Result::Header>> { + >::header( + self, + generic::BlockId::::Hash(hash), + ) + } + fn number( + &self, + hash: Block::Hash, + ) -> sp_blockchain::Result::Header as HeaderT>::Number>> { + >::number(self, hash) + } +} + +/// Decoupling the provider. +/// +/// Mandated since `trait HeaderProvider` can only be +/// implemented once for a generic `T`. +pub trait HeaderProviderProvider: Send + Sync + 'static +where + Block: BlockT, +{ + type Provider: HeaderProvider + 'static; + + fn header_provider(&self) -> &Self::Provider; +} + +impl HeaderProviderProvider for T +where + Block: BlockT, + T: sc_client_api::Backend + 'static, +{ + type Provider = >::Blockchain; + + fn header_provider(&self) -> &Self::Provider { + self.blockchain() + } +} + #[derive(thiserror::Error, Debug)] pub enum Error { #[error(transparent)] @@ -205,8 +285,12 @@ fn set_prometheus_registry(config: &mut Configuration) -> Result<(), Error> { /// Initialize the `Jeager` collector. The destination must listen /// on the given address and port for `UDP` packets. -#[cfg(any(test,feature = "full-node"))] -fn jaeger_launch_collector_with_agent(spawner: impl SpawnNamed, config: &Configuration, agent: Option) -> Result<(), Error> { +#[cfg(any(test, feature = "full-node"))] +fn jaeger_launch_collector_with_agent( + spawner: impl SpawnNamed, + config: &Configuration, + agent: Option, +) -> Result<(), Error> { if let Some(agent) = agent { let cfg = jaeger::JaegerConfig::builder() .agent(agent) @@ -219,7 +303,7 @@ fn jaeger_launch_collector_with_agent(spawner: impl SpawnNamed, config: &Configu } #[cfg(feature = "full-node")] -type FullSelectChain = relay_chain_selection::SelectRelayChain; +type FullSelectChain = relay_chain_selection::SelectRelayChainWithFallback; #[cfg(feature = "full-node")] type FullGrandpaBlockImport = grandpa::GrandpaBlockImport< FullBackend, Block, FullClient, FullSelectChain @@ -303,7 +387,7 @@ fn new_partial( jaeger_launch_collector_with_agent(task_manager.spawn_handle(), &*config, jaeger_agent)?; - let select_chain = relay_chain_selection::SelectRelayChain::new( + let select_chain = relay_chain_selection::SelectRelayChainWithFallback::new( backend.clone(), Handle::new_disconnected(), polkadot_node_subsystem_util::metrics::Metrics::register(config.prometheus_registry())?, @@ -506,7 +590,7 @@ where .unwrap_or_default() .into_iter() .filter_map(|hash| { - let number = client.number(hash).ok()??; + let number = HeaderBackend::number(client, hash).ok()??; // Only consider leaves that are in maximum an uncle of the best block. if number < best_block.number().saturating_sub(1) { diff --git a/node/service/src/relay_chain_selection.rs b/node/service/src/relay_chain_selection.rs index 0116e2a03ae6..aebd5238cc78 100644 --- a/node/service/src/relay_chain_selection.rs +++ b/node/service/src/relay_chain_selection.rs @@ -35,19 +35,16 @@ #![cfg(feature = "full-node")] -use { - polkadot_primitives::v1::{ - Hash, BlockNumber, Block as PolkadotBlock, Header as PolkadotHeader, - }, - polkadot_subsystem::messages::{ApprovalVotingMessage, ChainSelectionMessage}, - polkadot_node_subsystem_util::metrics::{self, prometheus}, - polkadot_overseer::{Handle, OverseerHandle}, - futures::channel::oneshot, - consensus_common::{Error as ConsensusError, SelectChain}, - sp_blockchain::HeaderBackend, - sp_runtime::generic::BlockId, - std::sync::Arc, +use polkadot_primitives::v1::{ + Hash, BlockNumber, Block as PolkadotBlock, Header as PolkadotHeader, }; +use polkadot_subsystem::messages::{ApprovalVotingMessage, HighestApprovedAncestorBlock, ChainSelectionMessage, DisputeCoordinatorMessage}; +use polkadot_node_subsystem_util::metrics::{self, prometheus}; +use futures::channel::oneshot; +use consensus_common::{Error as ConsensusError, SelectChain}; +use std::sync::Arc; +use polkadot_overseer::{AllMessages, Handle, OverseerHandle}; +use super::{HeaderProvider, HeaderProviderProvider}; /// The maximum amount of unfinalized blocks we are willing to allow due to approval checking /// or disputes. @@ -109,25 +106,120 @@ impl Metrics { } /// A chain-selection implementation which provides safety for relay chains. -pub struct SelectRelayChain { - backend: Arc, - overseer: Handle, +pub struct SelectRelayChainWithFallback< + B: sc_client_api::Backend, +> { // A fallback to use in case the overseer is disconnected. // // This is used on relay chains which have not yet enabled // parachains as well as situations where the node is offline. fallback: sc_consensus::LongestChain, + selection: SelectRelayChain< + B, + Handle, + >, +} + +impl Clone for SelectRelayChainWithFallback +where + B: sc_client_api::Backend, + SelectRelayChain< + B, + Handle, + >: Clone, +{ + fn clone(&self) -> Self { + Self { + fallback: self.fallback.clone(), + selection: self.selection.clone(), + } + } +} + + +impl SelectRelayChainWithFallback +where + B: sc_client_api::Backend + 'static, +{ + /// Create a new [`SelectRelayChainWithFallback`] wrapping the given chain backend + /// and a handle to the overseer. + pub fn new(backend: Arc, overseer: Handle, metrics: Metrics) -> Self { + SelectRelayChainWithFallback { + fallback: sc_consensus::LongestChain::new(backend.clone()), + selection: SelectRelayChain::new( + backend, + overseer, + metrics, + ), + } + } +} + +impl SelectRelayChainWithFallback +where + B: sc_client_api::Backend + 'static, +{ + /// Given an overseer handle, this connects the [`SelectRelayChainWithFallback`]'s + /// internal handle and its clones to the same overseer. + pub fn connect_to_overseer( + &mut self, + handle: OverseerHandle, + ) { + self.selection.overseer.connect_to_overseer(handle); + } +} + + +#[async_trait::async_trait] +impl SelectChain for SelectRelayChainWithFallback +where + B: sc_client_api::Backend + 'static, +{ + async fn leaves(&self) -> Result, ConsensusError> { + if self.selection.overseer.is_disconnected() { + return self.fallback.leaves().await + } + + self.selection.leaves().await + } + + async fn best_chain(&self) -> Result { + if self.selection.overseer.is_disconnected() { + return self.fallback.best_chain().await + } + self.selection.best_chain().await + } + + async fn finality_target( + &self, + target_hash: Hash, + maybe_max_number: Option, + ) -> Result, ConsensusError> { + if self.selection.overseer.is_disconnected() { + return self.fallback.finality_target(target_hash, maybe_max_number).await + } + self.selection.finality_target(target_hash, maybe_max_number).await + } +} + + +/// A chain-selection implementation which provides safety for relay chains +/// but does not handle situations where the overseer is not yet connected. +pub struct SelectRelayChain { + backend: Arc, + overseer: OH, metrics: Metrics, } -impl SelectRelayChain - where B: sc_client_api::backend::Backend + 'static +impl SelectRelayChain +where + B: HeaderProviderProvider, + OH: OverseerHandleT, { /// Create a new [`SelectRelayChain`] wrapping the given chain backend /// and a handle to the overseer. - pub fn new(backend: Arc, overseer: Handle, metrics: Metrics) -> Self { + pub fn new(backend: Arc, overseer: OH, metrics: Metrics) -> Self { SelectRelayChain { - fallback: sc_consensus::LongestChain::new(backend.clone()), backend, overseer, metrics, @@ -135,7 +227,7 @@ impl SelectRelayChain } fn block_header(&self, hash: Hash) -> Result { - match self.backend.blockchain().header(BlockId::Hash(hash)) { + match HeaderProvider::header(self.backend.header_provider(), hash) { Ok(Some(header)) => Ok(header), Ok(None) => Err(ConsensusError::ChainLookup(format!( "Missing header with hash {:?}", @@ -150,7 +242,7 @@ impl SelectRelayChain } fn block_number(&self, hash: Hash) -> Result { - match self.backend.blockchain().number(hash) { + match HeaderProvider::number(self.backend.header_provider(), hash) { Ok(Some(number)) => Ok(number), Ok(None) => Err(ConsensusError::ChainLookup(format!( "Missing number with hash {:?}", @@ -165,25 +257,15 @@ impl SelectRelayChain } } -impl SelectRelayChain { - /// Given an overseer handle, connects the [`SelectRelayChain`]'s - /// internal handle and its clones to the same overseer. - pub fn connect_to_overseer( - &mut self, - handle: OverseerHandle, - ) { - self.overseer.connect_to_overseer(handle); - } -} - -impl Clone for SelectRelayChain - where B: sc_client_api::backend::Backend + 'static +impl Clone for SelectRelayChain +where + B: HeaderProviderProvider + Send + Sync, + OH: OverseerHandleT, { - fn clone(&self) -> SelectRelayChain { + fn clone(&self) -> Self { SelectRelayChain { backend: self.backend.clone(), overseer: self.overseer.clone(), - fallback: self.fallback.clone(), metrics: self.metrics.clone(), } } @@ -199,17 +281,32 @@ enum Error { EmptyLeaves, } + +/// Decoupling trait for the overseer handle. +/// +/// Required for testing purposes. +#[async_trait::async_trait] +pub trait OverseerHandleT: Clone + Send + Sync { + async fn send_msg>(&mut self, msg: M, origin: &'static str); +} + #[async_trait::async_trait] -impl SelectChain for SelectRelayChain - where B: sc_client_api::backend::Backend + 'static +impl OverseerHandleT for Handle { + async fn send_msg>(&mut self, msg: M, origin: &'static str) { + Handle::send_msg(self, msg, origin).await + } +} + + +#[async_trait::async_trait] +impl SelectChain for SelectRelayChain +where + B: HeaderProviderProvider, + OH: OverseerHandleT, { /// Get all leaves of the chain, i.e. block hashes that are suitable to /// build upon and have no suitable children. async fn leaves(&self) -> Result, ConsensusError> { - if self.overseer.is_disconnected() { - return self.fallback.leaves().await - } - let (tx, rx) = oneshot::channel(); self.overseer @@ -226,10 +323,6 @@ impl SelectChain for SelectRelayChain /// Among all leaves, pick the one which is the best chain to build upon. async fn best_chain(&self) -> Result { - if self.overseer.is_disconnected() { - return self.fallback.best_chain().await - } - // The Chain Selection subsystem is supposed to treat the finalized // block as the best leaf in the case that there are no viable // leaves, so this should not happen in practice. @@ -257,10 +350,6 @@ impl SelectChain for SelectRelayChain target_hash: Hash, maybe_max_number: Option, ) -> Result, ConsensusError> { - if self.overseer.is_disconnected() { - return self.fallback.finality_target(target_hash, maybe_max_number).await - } - let mut overseer = self.overseer.clone(); let subchain_head = { @@ -305,7 +394,7 @@ impl SelectChain for SelectRelayChain subchain_head } else { let (ancestor_hash, _) = crate::grandpa_support::walk_backwards_to_target_block( - self.backend.blockchain(), + self.backend.header_provider(), max, &subchain_header, ).map_err(|e| ConsensusError::ChainLookup(format!("{:?}", e)))?; @@ -319,7 +408,7 @@ impl SelectChain for SelectRelayChain let initial_leaf_number = self.block_number(initial_leaf)?; // 2. Constrain according to `ApprovedAncestor`. - let (subchain_head, subchain_number) = { + let (subchain_head, subchain_number, subchain_block_descriptions) = { let (tx, rx) = oneshot::channel(); overseer.send_msg( @@ -336,17 +425,45 @@ impl SelectChain for SelectRelayChain .map_err(|e| ConsensusError::Other(Box::new(e)))? { // No approved ancestors means target hash is maximal vote. - None => (target_hash, target_number), - Some((s_h, s_n)) => (s_h, s_n), + None => (target_hash, target_number, Vec::new()), + Some(HighestApprovedAncestorBlock { + number, hash, descriptions + }) => (hash, number, descriptions), } }; + // Prevent sending flawed data to the dispute-coordinator. + if Some(subchain_block_descriptions.len() as _) != subchain_number.checked_sub(target_number) { + tracing::error!( + LOG_TARGET, + present_block_descriptions = subchain_block_descriptions.len(), + target_number, + subchain_number, + "Mismatch of anticipated block descriptions and block number difference.", + ); + return Ok(Some(target_hash)); + } + let lag = initial_leaf_number.saturating_sub(subchain_number); self.metrics.note_approval_checking_finality_lag(lag); // 3. Constrain according to disputes: - // TODO: https://github.com/paritytech/polkadot/issues/3164 - self.metrics.note_disputes_finality_lag(0); + 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); // 4. Apply the maximum safeguard to the finality lag. if lag > MAX_FINALITY_LAG { @@ -361,7 +478,7 @@ impl SelectChain for SelectRelayChain // Otherwise we're looking for a descendant. let initial_leaf_header = self.block_header(initial_leaf)?; let (forced_target, _) = crate::grandpa_support::walk_backwards_to_target_block( - self.backend.blockchain(), + self.backend.header_provider(), safe_target, &initial_leaf_header, ).map_err(|e| ConsensusError::ChainLookup(format!("{:?}", e)))?; diff --git a/node/service/src/tests.rs b/node/service/src/tests.rs new file mode 100644 index 000000000000..84fe018de20a --- /dev/null +++ b/node/service/src/tests.rs @@ -0,0 +1,715 @@ +// Copyright 2021 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +use super::relay_chain_selection::*; +use super::*; + +use futures::channel::oneshot::Receiver; +use polkadot_node_primitives::approval::{VRFOutput, VRFProof}; +use polkadot_node_subsystem_test_helpers as test_helpers; +use polkadot_node_subsystem_util::TimeoutExt; +use polkadot_subsystem::messages::AllMessages; +use polkadot_subsystem::messages::BlockDescription; +use polkadot_test_client::Sr25519Keyring; +use sp_consensus_babe::digests::CompatibleDigestItem; +use sp_consensus_babe::digests::{PreDigest, SecondaryVRFPreDigest}; +use sp_consensus_babe::Transcript; +use sp_runtime::testing::*; +use sp_runtime::DigestItem; +use std::collections::{BTreeMap, HashMap, HashSet}; +use std::iter::IntoIterator; + +use assert_matches::assert_matches; +use std::sync::Arc; +use std::time::Duration; + +use consensus_common::SelectChain; +use futures::channel::oneshot; +use futures::prelude::*; +use polkadot_primitives::v1::{Block, BlockNumber, Hash, Header}; +use polkadot_subsystem::messages::{ + ApprovalVotingMessage, ChainSelectionMessage, DisputeCoordinatorMessage, HighestApprovedAncestorBlock, +}; + +use polkadot_node_subsystem_test_helpers::TestSubsystemSender; +use polkadot_overseer::{SubsystemContext, SubsystemSender}; + +type VirtualOverseer = test_helpers::TestSubsystemContextHandle; + +#[async_trait::async_trait] +impl OverseerHandleT for TestSubsystemSender { + async fn send_msg>(&mut self, msg: M, _origin: &'static str) { + TestSubsystemSender::send_message(self, msg.into()).await; + } +} + +struct TestHarness { + virtual_overseer: VirtualOverseer, + case_vars: CaseVars, + /// The result of `fn finality_target` will be injected into the + /// harness scope via this channel. + finality_target_rx: Receiver>, +} + +#[derive(Default)] +struct HarnessConfig; + +fn test_harness>(case_vars: CaseVars, test: impl FnOnce(TestHarness) -> T) { + let _ = env_logger::builder().is_test(true).filter_level(log::LevelFilter::Trace).try_init(); + + let pool = sp_core::testing::TaskExecutor::new(); + let (mut context, virtual_overseer) = test_helpers::make_subsystem_context(pool); + + let (finality_target_tx, finality_target_rx) = oneshot::channel::>(); + + let select_relay_chain = SelectRelayChain::::new( + Arc::new(case_vars.chain.clone()), + context.sender().clone(), + Default::default(), + ); + + 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(); + finality_target_tx.send(best).unwrap(); + () + }; + + let test_fut = test(TestHarness { virtual_overseer, case_vars, finality_target_rx }); + + futures::pin_mut!(test_fut); + futures::pin_mut!(selection_process); + + futures::executor::block_on(future::join( + async move { + let _overseer = test_fut.await; + }, + selection_process, + )) + .1; +} + +async fn overseer_recv(overseer: &mut VirtualOverseer) -> AllMessages { + let msg = overseer_recv_with_timeout(overseer, TIMEOUT) + .await + .expect(&format!("{:?} is enough to receive messages.", TIMEOUT)); + + tracing::trace!("Received message:\n{:?}", &msg); + + msg +} +async fn overseer_recv_with_timeout(overseer: &mut VirtualOverseer, timeout: Duration) -> Option { + tracing::trace!("Waiting for message..."); + overseer.recv().timeout(timeout).await +} + +const TIMEOUT: Duration = Duration::from_millis(2000); + +// used for generating assignments where the validity of the VRF doesn't matter. +fn garbage_vrf() -> (VRFOutput, VRFProof) { + let key = Sr25519Keyring::Alice.pair(); + let key = key.as_ref(); + + let (o, p, _) = key.vrf_sign(Transcript::new(b"test-garbage")); + (VRFOutput(o.to_output()), VRFProof(p)) +} + +// nice to have +const A1: Hash = Hash::repeat_byte(0xA1); +const A2: Hash = Hash::repeat_byte(0xA2); +const A3: Hash = Hash::repeat_byte(0xA3); +const A5: Hash = Hash::repeat_byte(0xA5); + +const B2: Hash = Hash::repeat_byte(0xB2); +const B3: Hash = Hash::repeat_byte(0xB3); + +/// Representation of a local representation +/// to extract information for finalization target +/// extraction. +#[derive(Debug, Default, Clone)] +struct TestChainStorage { + blocks_by_hash: HashMap, + blocks_at_height: BTreeMap>, + disputed_blocks: HashSet, + approved_blocks: HashSet, + heads: HashSet, +} + +impl TestChainStorage { + /// Fill the [`HighestApprovedAncestor`] structure with mostly + /// correct data. + pub fn highest_approved_ancestors( + &self, + minimum_block_number: BlockNumber, + leaf: Hash, + ) -> Option { + let hash = leaf; + let number = self.blocks_by_hash.get(&leaf)?.number; + + let mut descriptions = Vec::new(); + let mut block_hash = leaf; + let mut highest_approved_ancestor = None; + + while let Some(block) = self.blocks_by_hash.get(&block_hash) { + if minimum_block_number >= block.number { + break; + } + descriptions.push(BlockDescription { + session: 1 as _, // dummy, not checked + block_hash, + candidates: vec![], // not relevant for any test cases + }); + if !self.approved_blocks.contains(&block_hash) { + highest_approved_ancestor = None; + descriptions.clear(); + } else if highest_approved_ancestor.is_none() { + descriptions.clear(); + highest_approved_ancestor = Some(block_hash); + } + let next = block.parent_hash(); + if &leaf != next { + block_hash = *next; + } else { + break; + } + } + + if highest_approved_ancestor.is_none() { + return None; + } + + Some(HighestApprovedAncestorBlock { + hash, + number, + descriptions: descriptions.into_iter().rev().collect(), + }) + } + + /// Traverse backwards from leave down to block number. + fn undisputed_chain(&self, base_blocknumber: BlockNumber, highest_approved_block_hash: Hash) -> Option { + if self.disputed_blocks.is_empty() { + return Some(highest_approved_block_hash); + } + + let mut undisputed_chain = Some(highest_approved_block_hash); + let mut block_hash = highest_approved_block_hash; + while let Some(block) = self.blocks_by_hash.get(&block_hash) { + block_hash = block.hash(); + if ChainBuilder::GENESIS_HASH == block_hash { + return None; + } + let next = block.parent_hash(); + if self.disputed_blocks.contains(&block_hash) { + undisputed_chain = Some(*next); + } + if block.number() == &base_blocknumber { + return None; + } + block_hash = *next; + } + undisputed_chain + } +} + +impl HeaderProvider for TestChainStorage { + fn header(&self, hash: Hash) -> sp_blockchain::Result> { + Ok(self.blocks_by_hash.get(&hash).cloned()) + } + fn number(&self, hash: Hash) -> sp_blockchain::Result> { + self.header(hash).map(|opt| opt.map(|h| h.number)) + } +} + +impl HeaderProviderProvider for TestChainStorage { + type Provider = Self; + fn header_provider(&self) -> &Self { + self + } +} + +#[derive(Debug, Clone)] +struct ChainBuilder(pub TestChainStorage); + +impl ChainBuilder { + const GENESIS_HASH: Hash = Hash::repeat_byte(0xff); + const GENESIS_PARENT_HASH: Hash = Hash::repeat_byte(0x00); + + pub fn new() -> Self { + let mut builder = Self(TestChainStorage::default()); + let _ = builder.add_block_inner(Self::GENESIS_HASH, Self::GENESIS_PARENT_HASH, 0); + builder + } + + pub fn add_block<'a>(&'a mut self, hash: Hash, parent_hash: Hash, number: u32) -> &'a mut Self { + assert!(number != 0, "cannot add duplicate genesis block"); + assert!(hash != Self::GENESIS_HASH, "cannot add block with genesis hash"); + assert!(parent_hash != Self::GENESIS_PARENT_HASH, "cannot add block with genesis parent hash"); + assert!(self.0.blocks_by_hash.len() < u8::MAX.into()); + self.add_block_inner(hash, parent_hash, number) + } + + fn add_block_inner<'a>(&'a mut self, hash: Hash, parent_hash: Hash, number: u32) -> &'a mut Self { + let header = ChainBuilder::make_header(parent_hash, number); + assert!(self.0.blocks_by_hash.insert(hash, header).is_none(), "block with hash {:?} already exists", hash,); + self.0.blocks_at_height.entry(number).or_insert_with(Vec::new).push(hash); + self + } + + pub fn fast_forward_approved( + &mut self, + branch_tag: u8, + parent: Hash, + block_number: BlockNumber, + ) -> Hash { + let block = self.fast_forward(branch_tag, parent, block_number); + let _ = self.0.approved_blocks.insert(block); + block + } + + /// Add a relay chain block that contains a disputed parachain block. + /// For simplicity this is not modeled explicitly. + pub fn fast_forward_disputed( + &mut self, + branch_tag: u8, + parent: Hash, + block_number: BlockNumber, + ) -> Hash { + let block = self.fast_forward_approved(branch_tag, parent, block_number); + let _ = self.0.disputed_blocks.insert(block); + block + } + + pub fn fast_forward(&mut self, branch_tag: u8, parent: Hash, block_number: BlockNumber) -> Hash { + let hash = Hash::repeat_byte((block_number as u8 | branch_tag) as u8); + let _ = self.add_block(hash, parent, block_number); + hash + } + + pub fn set_heads(&mut self, heads: impl IntoIterator) { + self.0.heads = heads.into_iter().collect(); + } + + pub fn init(self) -> TestChainStorage { + self.0 + } + + fn make_header(parent_hash: Hash, number: u32) -> Header { + let digest = { + let mut digest = Digest::default(); + let (vrf_output, vrf_proof) = garbage_vrf(); + digest.push(DigestItem::babe_pre_digest(PreDigest::SecondaryVRF(SecondaryVRFPreDigest { + authority_index: 0, + slot: 1.into(), // slot, unused + vrf_output, + vrf_proof, + }))); + digest + }; + + Header { digest, extrinsics_root: Default::default(), number, state_root: Default::default(), parent_hash } + } +} + +/// Generalized sequence of the test, based on +/// the messages being sent out by the `fn finality_target` +/// Depends on a particular `target_hash` +/// that is passed to `finality_target` block number. +async fn test_skeleton( + chain: &TestChainStorage, + virtual_overseer: &mut VirtualOverseer, + target_block_hash: Hash, + best_chain_containing_block: Option, + highest_approved_ancestor_block: Option, + undisputed_chain: Option, +) { + let undisputed_chain = undisputed_chain.map(|x| (chain.number(x).unwrap().unwrap(), x)); + + tracing::trace!("best leaf response: {:?}", undisputed_chain); + assert_matches!( + overseer_recv( + virtual_overseer + ).await, + AllMessages::ChainSelection(ChainSelectionMessage::BestLeafContaining( + target_hash, + tx, + )) + => { + assert_eq!(target_block_hash, target_hash, "TestIntegrity: target hashes always match. qed"); + tx.send(best_chain_containing_block.clone()).unwrap(); + } + ); + + if best_chain_containing_block.is_none() { + return; + } + + tracing::trace!("approved ancestor response: {:?}", undisputed_chain); + assert_matches!( + overseer_recv( + virtual_overseer + ).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::ApprovedAncestor(_block_hash, _block_number, tx)) + => { + tx.send(highest_approved_ancestor_block.clone()).unwrap(); + } + ); + + tracing::trace!("determine undisputed chain response: {:?}", undisputed_chain); + assert_matches!( + overseer_recv( + virtual_overseer + ).await, + AllMessages::DisputeCoordinator( + DisputeCoordinatorMessage::DetermineUndisputedChain { + base_number: _, + block_descriptions: _, + tx, + } + ) => { + tx.send(undisputed_chain).unwrap(); + }); +} + +/// Straight forward test case, where the test is not +/// for integrity, but for different block relation structures. +fn run_specialized_test_w_harness CaseVars>(case_var_provider: F) { + test_harness(case_var_provider(), |test_harness| async move { + let TestHarness { + mut virtual_overseer, + finality_target_rx, + case_vars: + CaseVars { + chain, + target_block, + best_chain_containing_block, + highest_approved_ancestor_block, + undisputed_chain, + expected_finality_target_result, + }, + .. + } = test_harness; + + // Verify test integrity: the provided highest approved + // ancestor must match the chain derived one. + let highest_approved_ancestor_w_desc = best_chain_containing_block.and_then(|best_chain_containing_block| { + let min_blocknumber = + chain.blocks_by_hash.get(&best_chain_containing_block).map(|x| x.number).unwrap_or_default(); + let highest_approved_ancestor_w_desc = + chain.highest_approved_ancestors(min_blocknumber, best_chain_containing_block); + if let (Some(highest_approved_ancestor_w_desc), Some(highest_approved_ancestor_block)) = + (&highest_approved_ancestor_w_desc, highest_approved_ancestor_block) + { + assert_eq!( + highest_approved_ancestor_block, highest_approved_ancestor_w_desc.hash, + "TestCaseIntegrity: Provided and expected approved ancestor hash mismatch: {:?} vs {:?}", + highest_approved_ancestor_block, highest_approved_ancestor_w_desc.hash, + ); + } + highest_approved_ancestor_w_desc + }); + if let Some(haacwd) = &highest_approved_ancestor_w_desc { + let expected = chain.undisputed_chain(haacwd.number, haacwd.hash); + assert_eq!( + expected, undisputed_chain, + "TestCaseIntegrity: Provided and anticipated undisputed chain mismatch: {:?} vs {:?}", + undisputed_chain, expected, + ) + } + + test_skeleton( + &chain, + &mut virtual_overseer, + target_block, + best_chain_containing_block, + highest_approved_ancestor_w_desc, + undisputed_chain, + ) + .await; + + assert_matches!(finality_target_rx.await, + Ok( + finality_target_val, + ) => assert_eq!(expected_finality_target_result, finality_target_val)); + + virtual_overseer + }); +} + +/// All variables relevant for a test case. +#[derive(Clone, Debug)] +struct CaseVars { + /// Chain test _case_ definition. + chain: TestChainStorage, + + /// The target block to be finalized. + target_block: Hash, + + /// Response to the `target_block` request, must be a chain-head. + /// `None` if no such chain exists. + best_chain_containing_block: Option, + + /// Resulting best estimate, before considering + /// the disputed state of blocks. + highest_approved_ancestor_block: Option, + + /// Equal to the previous, unless there are disputes. + /// The backtracked version of this must _never_ + /// contain a disputed block. + undisputed_chain: Option, + + /// The returned value by `fn finality_target`. + expected_finality_target_result: Option, +} + +/// ```raw +/// genesis -- 0xA1 --- 0xA2 --- 0xA3 --- 0xA4(!avail) --- 0xA5(!avail) +/// \ +/// `- 0xB2 +/// ``` +fn chain_0() -> CaseVars { + let head: Hash = ChainBuilder::GENESIS_HASH; + let mut builder = ChainBuilder::new(); + + let a1 = builder.fast_forward_approved(0xA0, head, 1); + let a2 = builder.fast_forward_approved(0xA0, a1, 2); + let a3 = builder.fast_forward_approved(0xA0, a2, 3); + let a4 = builder.fast_forward(0xA0, a3, 4); + let a5 = builder.fast_forward(0xA0, a4, 5); + + let b1 = builder.fast_forward_approved(0xB0, a1, 2); + let b2 = builder.fast_forward_approved(0xB0, b1, 3); + let b3 = builder.fast_forward_approved(0xB0, b2, 4); + + builder.set_heads(vec![a5, b3]); + + CaseVars { + chain: builder.init(), + target_block: A1, + best_chain_containing_block: Some(A5), + highest_approved_ancestor_block: Some(A3), + undisputed_chain: Some(A2), + expected_finality_target_result: Some(A2), + } +} + +/// ```raw +/// genesis -- 0xA1 --- 0xA2(disputed) --- 0xA3 +/// \ +/// `- 0xB2 --- 0xB3(!available) +/// ``` +fn chain_1() -> CaseVars { + let head: Hash = ChainBuilder::GENESIS_HASH; + let mut builder = ChainBuilder::new(); + + let a1 = builder.fast_forward_approved(0xA0, head, 1); + let a2 = builder.fast_forward_disputed(0xA0, a1, 2); + let a3 = builder.fast_forward_approved(0xA0, a2, 3); + + let b2 = builder.fast_forward_approved(0xB0, a1, 2); + let b3 = builder.fast_forward_approved(0xB0, b2, 3); + + builder.set_heads(vec![a3, b3]); + + CaseVars { + chain: builder.init(), + target_block: A1, + best_chain_containing_block: Some(B3), + highest_approved_ancestor_block: Some(B2), + undisputed_chain: Some(B2), + expected_finality_target_result: Some(B2), + } +} + +/// ```raw +/// genesis -- 0xA1 --- 0xA2(disputed) --- 0xA3 +/// \ +/// `- 0xB2 --- 0xB3 +/// ``` +fn chain_2() -> CaseVars { + let head: Hash = ChainBuilder::GENESIS_HASH; + let mut builder = ChainBuilder::new(); + + let a1 = builder.fast_forward_approved(0xA0, head, 1); + let a2 = builder.fast_forward_disputed(0xA0, a1, 2); + let a3 = builder.fast_forward_approved(0xA0, a2, 3); + + let b2 = builder.fast_forward_approved(0xB0, a1, 2); + let b3 = builder.fast_forward_approved(0xB0, b2, 3); + + builder.set_heads(vec![a3, b3]); + + CaseVars { + chain: builder.init(), + target_block: A3, + best_chain_containing_block: Some(A3), + highest_approved_ancestor_block: Some(A3), + undisputed_chain: Some(A1), + expected_finality_target_result: Some(A1), + } +} + +/// ```raw +/// genesis -- 0xA1 --- 0xA2 --- 0xA3(disputed) +/// \ +/// `- 0xB2 --- 0xB3 +/// ``` +fn chain_3() -> CaseVars { + let head: Hash = ChainBuilder::GENESIS_HASH; + let mut builder = ChainBuilder::new(); + + let a1 = builder.fast_forward_approved(0xA0, head, 1); + let a2 = builder.fast_forward_approved(0xA0, a1, 2); + let a3 = builder.fast_forward_disputed(0xA0, a2, 3); + + let b2 = builder.fast_forward_approved(0xB0, a1, 2); + let b3 = builder.fast_forward_approved(0xB0, b2, 3); + + builder.set_heads(vec![a3, b3]); + + CaseVars { + chain: builder.init(), + target_block: A2, + best_chain_containing_block: Some(A3), + highest_approved_ancestor_block: Some(A3), + undisputed_chain: Some(A2), + expected_finality_target_result: Some(A2), + } +} + +/// ```raw +/// genesis -- 0xA1 --- 0xA2 --- 0xA3(disputed) +/// \ +/// `- 0xB2 --- 0xB3 +/// +/// ? --- NEX(does_not_exist) +/// ``` +fn chain_4() -> CaseVars { + let head: Hash = ChainBuilder::GENESIS_HASH; + let mut builder = ChainBuilder::new(); + + let a1 = builder.fast_forward_approved(0xA0, head, 1); + let a2 = builder.fast_forward_approved(0xA0, a1, 2); + let a3 = builder.fast_forward_disputed(0xA0, a2, 3); + + let b2 = builder.fast_forward_approved(0xB0, a1, 2); + let b3 = builder.fast_forward_approved(0xB0, b2, 3); + + builder.set_heads(vec![a3, b3]); + + let does_not_exist = Hash::repeat_byte(0xCC); + CaseVars { + chain: builder.init(), + target_block: does_not_exist, + best_chain_containing_block: None, + highest_approved_ancestor_block: None, + undisputed_chain: None, + expected_finality_target_result: Some(does_not_exist), + } +} + +/// ```raw +/// genesis -- 0xA1 --- 0xA2 +/// ``` +fn chain_5() -> CaseVars { + let head: Hash = ChainBuilder::GENESIS_HASH; + let mut builder = ChainBuilder::new(); + + let a1 = builder.fast_forward_approved(0xA0, head, 1); + let a2 = builder.fast_forward_approved(0xA0, a1, 2); + + builder.set_heads(vec![a2]); + + CaseVars { + chain: builder.init(), + target_block: A2, + best_chain_containing_block: Some(A2), + highest_approved_ancestor_block: Some(A2), + undisputed_chain: Some(A2), + expected_finality_target_result: Some(A2), + } +} + +/// ```raw +/// genesis -- 0xB2 -- 0xD2 -- .. -- 0xD8 -- 0xC8(unapproved) -- .. -- 0xCF(unapproved) +/// ``` +fn chain_6() -> CaseVars { + let head: Hash = ChainBuilder::GENESIS_HASH; + let mut builder = ChainBuilder::new(); + + let b1 = builder.fast_forward_approved(0xB0, head, 1); + + let mut previous = b1; + let mut approved = b1; + for block_number in 2_u32..16 { + if block_number <= 8 { + previous = builder.fast_forward_approved(0xD0, previous, block_number as _); + approved = previous; + } else { + previous = builder.fast_forward(0xA0, previous, block_number as _); + } + } + let leaf = previous; + + builder.set_heads(vec![leaf]); + + let chain = builder.init(); + + tracing::trace!(highest_approved = ?chain.highest_approved_ancestors(1, leaf)); + tracing::trace!(undisputed = ?chain.undisputed_chain(1, approved)); + CaseVars { + chain, + target_block: b1, + best_chain_containing_block: Some(leaf), + highest_approved_ancestor_block: Some(approved), + undisputed_chain: Some(approved), + expected_finality_target_result: Some(approved), + } +} + +#[test] +fn chain_sel_0() { + run_specialized_test_w_harness(chain_0); +} + +#[test] +fn chain_sel_1() { + run_specialized_test_w_harness(chain_1); +} + +#[test] +fn chain_sel_2() { + run_specialized_test_w_harness(chain_2); +} + +#[test] +fn chain_sel_3() { + run_specialized_test_w_harness(chain_3); +} + +#[test] +fn chain_sel_4_target_hash_value_not_contained() { + run_specialized_test_w_harness(chain_4); +} + +#[test] +fn chain_sel_5_best_is_target_hash() { + run_specialized_test_w_harness(chain_5); +} + +#[test] +fn chain_sel_6_approval_lag() { + run_specialized_test_w_harness(chain_6); +} diff --git a/node/subsystem-types/src/messages.rs b/node/subsystem-types/src/messages.rs index 5a3d6333a010..ba4693cc1914 100644 --- a/node/subsystem-types/src/messages.rs +++ b/node/subsystem-types/src/messages.rs @@ -249,7 +249,7 @@ pub enum DisputeCoordinatorMessage { /// The number of the lowest possible block to vote on. base_number: BlockNumber, /// Descriptions of all the blocks counting upwards from the block after the base number - block_descriptions: Vec<(Hash, SessionIndex, Vec)>, + block_descriptions: Vec, /// A response channel - `None` to vote on base, `Some` to vote higher. tx: oneshot::Sender>, } @@ -789,6 +789,33 @@ pub enum ApprovalCheckError { Internal(Hash, CandidateHash), } + +/// Describes a relay-chain block by the para-chain candidates +/// it includes. +#[derive(Clone, Debug)] +pub struct BlockDescription { + /// The relay-chain block hash. + pub block_hash: Hash, + /// The session index of this block. + pub session: SessionIndex, + /// The set of para-chain candidates. + pub candidates: Vec, +} + +/// Response type to `ApprovalVotingMessage::ApprovedAncestor`. +#[derive(Clone, Debug)] +pub struct HighestApprovedAncestorBlock { + /// The block hash of the highest viable ancestor. + pub hash: Hash, + /// The block number of the highest viable ancestor. + pub number: BlockNumber, + /// Block descriptions in the direct path between the + /// initially provided hash and the highest viable ancestor. + /// Primarily for use with `DetermineUndisputedChain`. + /// Must be sorted from lowest to highest block number. + pub descriptions: Vec, +} + /// Message to the Approval Voting subsystem. #[derive(Debug)] pub enum ApprovalVotingMessage { @@ -814,7 +841,7 @@ pub enum ApprovalVotingMessage { /// /// It can also return the same block hash, if that is acceptable to vote upon. /// Return `None` if the input hash is unrecognized. - ApprovedAncestor(Hash, BlockNumber, oneshot::Sender>), + ApprovedAncestor(Hash, BlockNumber, oneshot::Sender>), } /// Message to the Approval Distribution subsystem.