diff --git a/cumulus/client/network/src/lib.rs b/cumulus/client/network/src/lib.rs index 674c569fd6..eff58b2683 100644 --- a/cumulus/client/network/src/lib.rs +++ b/cumulus/client/network/src/lib.rs @@ -34,6 +34,7 @@ use sp_runtime::{ }; use polkadot_node_primitives::{SignedFullStatement, Statement}; +use polkadot_parachain::primitives::HeadData; use polkadot_primitives::v1::{ Block as PBlock, CandidateReceipt, CompactStatement, Hash as PHash, Id as ParaId, OccupiedCoreAssumption, ParachainHost, SignedStatement, SigningContext, @@ -55,7 +56,7 @@ use wait_on_relay_chain_block::WaitOnRelayChainBlock; mod tests; mod wait_on_relay_chain_block; -const LOG_TARGET: &str = "cumulus-network"; +const LOG_TARGET: &str = "sync::cumulus"; type BoxedError = Box; @@ -102,9 +103,7 @@ impl BlockAnnounceData { return Err(Validation::Failure { disconnect: true }); } - if polkadot_parachain::primitives::HeadData(encoded_header).hash() - != self.receipt.descriptor.para_head - { + if HeadData(encoded_header).hash() != self.receipt.descriptor.para_head { tracing::debug!( target: LOG_TARGET, "Receipt para head hash doesn't match the hash of the header in the block announcement", @@ -223,19 +222,19 @@ impl TryFrom for BlockAnnounceData { /// chain. If it is at the tip, it is required to provide a justification or otherwise we reject /// it. However, if the announcement is for a block below the tip the announcement is accepted /// as it probably comes from a node that is currently syncing the chain. -pub struct BlockAnnounceValidator { +pub struct BlockAnnounceValidator { phantom: PhantomData, - relay_chain_client: Arc

, + relay_chain_client: Arc, relay_chain_backend: Arc, para_id: ParaId, relay_chain_sync_oracle: Box, wait_on_relay_chain_block: WaitOnRelayChainBlock, } -impl BlockAnnounceValidator { +impl BlockAnnounceValidator { /// Create a new [`BlockAnnounceValidator`]. pub fn new( - relay_chain_client: Arc

, + relay_chain_client: Arc, para_id: ParaId, relay_chain_sync_oracle: Box, relay_chain_backend: Arc, @@ -255,14 +254,54 @@ impl BlockAnnounceValidator { } } -impl BlockAnnounceValidator +impl BlockAnnounceValidator where - P: ProvideRuntimeApi + Send + Sync + 'static, - P::Api: ParachainHost, + R: ProvideRuntimeApi + Send + Sync + 'static, + R::Api: ParachainHost, B: Backend + 'static, // Rust bug: https://github.com/rust-lang/rust/issues/24159 sc_client_api::StateBackendFor: sc_client_api::StateBackend>, { + /// Get the included block of the given parachain in the relay chain. + fn included_block( + relay_chain_client: &R, + block_id: &BlockId, + para_id: ParaId, + ) -> Result { + let validation_data = relay_chain_client + .runtime_api() + .persisted_validation_data(block_id, para_id, OccupiedCoreAssumption::TimedOut) + .map_err(|e| Box::new(BlockAnnounceError(format!("{:?}", e))) as Box<_>)? + .ok_or_else(|| { + Box::new(BlockAnnounceError( + "Could not find parachain head in relay chain".into(), + )) as Box<_> + })?; + let para_head = + Block::Header::decode(&mut &validation_data.parent_head.0[..]).map_err(|e| { + Box::new(BlockAnnounceError(format!( + "Failed to decode parachain head: {:?}", + e + ))) as Box<_> + })?; + + Ok(para_head) + } + + /// Get the backed block hash of the given parachain in the relay chain. + fn backed_block_hash( + relay_chain_client: &R, + block_id: &BlockId, + para_id: ParaId, + ) -> Result, BoxedError> { + let candidate_receipt = relay_chain_client + .runtime_api() + .candidate_pending_availability(block_id, para_id) + .map_err(|e| Box::new(BlockAnnounceError(format!("{:?}", e))) as Box<_>)?; + + Ok(candidate_receipt.map(|cr| cr.descriptor.para_head)) + } + /// Handle a block announcement with empty data (no statement) attached to it. fn handle_empty_block_announce_data( &self, @@ -278,32 +317,30 @@ where let runtime_api_block_id = BlockId::Hash(relay_chain_info.best_hash); let block_number = header.number(); - let local_validation_data = relay_chain_client - .runtime_api() - .persisted_validation_data( - &runtime_api_block_id, - para_id, - OccupiedCoreAssumption::TimedOut, - ) - .map_err(|e| Box::new(BlockAnnounceError(format!("{:?}", e))) as Box<_>)? - .ok_or_else(|| { - Box::new(BlockAnnounceError( - "Could not find parachain head in relay chain".into(), - )) as Box<_> - })?; - let parent_head = Block::Header::decode(&mut &local_validation_data.parent_head.0[..]) - .map_err(|e| { - Box::new(BlockAnnounceError(format!( - "Failed to decode parachain head: {:?}", - e - ))) as Box<_> - })?; - let known_best_number = parent_head.number(); + let best_head = + Self::included_block(&*relay_chain_client, &runtime_api_block_id, para_id)?; + let known_best_number = best_head.number(); + let backed_block = || + Self::backed_block_hash(&*relay_chain_client, &runtime_api_block_id, para_id); - if block_number >= known_best_number { - tracing::trace!( - target: "cumulus-network", - "validation failed because a justification is needed if the block at the top of the chain." + if best_head == header { + tracing::debug!( + target: LOG_TARGET, + "Announced block matches best block.", + ); + + Ok(Validation::Success { is_new_best: true }) + } else if Some(HeadData(header.encode()).hash()) == backed_block()? { + tracing::debug!( + target: LOG_TARGET, + "Announced block matches latest backed block.", + ); + + Ok(Validation::Success { is_new_best: true }) + } else if block_number >= known_best_number { + tracing::debug!( + target: LOG_TARGET, + "Validation failed because a justification is needed if the block at the top of the chain." ); Ok(Validation::Failure { disconnect: false }) @@ -501,7 +538,7 @@ impl WaitToAnnounce { self.spawner.spawn( "cumulus-wait-to-announce", async move { - tracing::trace!( + tracing::debug!( target: "cumulus-network", "waiting for announce block in a background task...", ); @@ -509,7 +546,7 @@ impl WaitToAnnounce { wait_to_announce::(block_hash, pov_hash, announce_block, signed_stmt_recv) .await; - tracing::trace!( + tracing::debug!( target: "cumulus-network", "block announcement finished", ); diff --git a/cumulus/client/network/src/tests.rs b/cumulus/client/network/src/tests.rs index d0c72dd83c..997583fda0 100644 --- a/cumulus/client/network/src/tests.rs +++ b/cumulus/client/network/src/tests.rs @@ -15,7 +15,7 @@ // along with Polkadot. If not, see . use super::*; -use cumulus_test_service::runtime::{Block, Header}; +use cumulus_test_service::runtime::{Block, Header, Hash}; use futures::{executor::block_on, poll, task::Poll}; use polkadot_node_primitives::{SignedFullStatement, Statement}; use polkadot_primitives::v1::{ @@ -37,6 +37,7 @@ use sp_keyring::Sr25519Keyring; use sp_keystore::{testing::KeyStore, SyncCryptoStore, SyncCryptoStorePtr}; use sp_runtime::RuntimeAppPublic; use std::collections::BTreeMap; +use parking_lot::Mutex; fn check_error(error: crate::BoxedError, check_error: impl Fn(&BlockAnnounceError) -> bool) { let error = *error @@ -173,6 +174,7 @@ fn invalid_if_no_data_exceeds_best_known_number() { let mut validator = make_validator_and_api().0; let header = Header { number: 1, + state_root: Hash::random(), ..default_header() }; let res = block_on(validator.validate(&header, &[])); @@ -184,6 +186,18 @@ fn invalid_if_no_data_exceeds_best_known_number() { ); } +#[test] +fn valid_if_no_data_and_block_matches_best_known_block() { + let mut validator = make_validator_and_api().0; + let res = block_on(validator.validate(&default_header(), &[])); + + assert_eq!( + res.unwrap(), + Validation::Success { is_new_best: true }, + "validation is successful when the block hash matches the best known block", + ); +} + #[test] fn check_statement_is_encoded_correctly() { let mut validator = make_validator_and_api().0; @@ -331,13 +345,33 @@ fn relay_parent_not_imported_when_block_announce_is_processed() { }); } +/// Ensures that when we receive a block announcement without a statement included, while the block +/// is not yet included by the node checking the announcement, but the node is already backed. +#[test] +fn block_announced_without_statement_and_block_only_backed() { + block_on(async move { + let (mut validator, api) = make_validator_and_api(); + api.data.lock().has_pending_availability = true; + + let header = default_header(); + + let validation = validator.validate(&header, &[]); + + assert!(matches!( + validation.await, + Ok(Validation::Success { is_new_best: true }) + )); + }); +} + #[derive(Default)] struct ApiData { validators: Vec, + has_pending_availability: bool, } struct TestApi { - data: Arc, + data: Arc>, relay_client: Arc, relay_backend: Arc, } @@ -348,9 +382,10 @@ impl TestApi { let relay_backend = builder.backend(); Self { - data: Arc::new(ApiData { + data: Arc::new(Mutex::new(ApiData { validators: vec![Sr25519Keyring::Alice.public().into()], - }), + has_pending_availability: false, + })), relay_client: Arc::new(builder.build()), relay_backend, } @@ -359,7 +394,7 @@ impl TestApi { #[derive(Default)] struct RuntimeApi { - data: Arc, + data: Arc>, } impl ProvideRuntimeApi for TestApi { @@ -376,7 +411,7 @@ impl ProvideRuntimeApi for TestApi { sp_api::mock_impl_runtime_apis! { impl ParachainHost for RuntimeApi { fn validators(&self) -> Vec { - self.data.validators.clone() + self.data.lock().validators.clone() } fn validator_groups(&self) -> (Vec>, GroupRotationInfo) { @@ -407,7 +442,19 @@ sp_api::mock_impl_runtime_apis! { } fn candidate_pending_availability(&self, _: ParaId) -> Option> { - None + if self.data.lock().has_pending_availability { + Some(CommittedCandidateReceipt { + descriptor: CandidateDescriptor { + para_head: polkadot_parachain::primitives::HeadData( + default_header().encode(), + ).hash(), + ..Default::default() + }, + ..Default::default() + }) + } else { + None + } } fn candidate_events(&self) -> Vec> {