Fix block announcement validation (#171)

* Fix block announce

* Fix compilation

collator/src/lib.rs

@@ -29,9 +29,9 @@ use cumulus_primitives::{
 };
 use cumulus_runtime::ParachainBlockData;
 
-use sc_client_api::{BlockBackend, BlockchainEvents, Finalizer, StateBackend, UsageProvider};
+use sc_client_api::{BlockBackend, Finalizer, StateBackend, UsageProvider};
 use sc_service::Configuration;
-use sp_api::{ApiExt, ProvideRuntimeApi};
+use sp_api::ApiExt;
 use sp_blockchain::HeaderBackend;
 use sp_consensus::{
 	BlockImport, BlockImportParams, BlockOrigin, BlockStatus, Environment, Error as ConsensusError,
@@ -41,15 +41,15 @@ use sp_core::traits::SpawnNamed;
 use sp_inherents::{InherentData, InherentDataProviders};
 use sp_runtime::{
 	generic::BlockId,
-	traits::{Block as BlockT, HashFor, Header as HeaderT},
+	traits::{BlakeTwo256, Block as BlockT, Header as HeaderT},
 };
 
 use polkadot_collator::{
 	BuildParachainContext, Network as CollatorNetwork, ParachainContext, RuntimeApiCollection,
 };
 use polkadot_primitives::v0::{
-	self as parachain, BlockData, GlobalValidationData, Id as ParaId, LocalValidationData,
+	self as parachain, Block as PBlock, BlockData, DownwardMessage, GlobalValidationData,
-	Block as PBlock, DownwardMessage, Hash as PHash,
+	Hash as PHash, Id as ParaId, LocalValidationData,
 };
 
 use codec::{Decode, Encode};
@@ -321,11 +321,7 @@ where
 			let (header, extrinsics) = block.deconstruct();
 
 			// Create the parachain block data for the validators.
-			let b = ParachainBlockData::<Block>::new(
+			let b = ParachainBlockData::<Block>::new(header.clone(), extrinsics, proof);
-				header.clone(),
-				extrinsics,
-				proof,
-			);
 
 			let mut block_import_params = BlockImportParams::new(BlockOrigin::Own, header);
 			block_import_params.body = Some(b.extrinsics().to_vec());
@@ -431,23 +427,90 @@ where
 {
 	type ParachainContext = Collator<Block, PF, BI, BS>;
 
-	fn build<PClient, Spawner, Extrinsic>(
+	fn build<Spawner>(
 		self,
-		polkadot_client: Arc<PClient>,
+		polkadot_client: polkadot_collator::Client,
 		spawner: Spawner,
 		polkadot_network: impl CollatorNetwork + Clone + 'static,
 	) -> Result<Self::ParachainContext, ()>
 	where
-		PClient: ProvideRuntimeApi<PBlock>
-			+ BlockchainEvents<PBlock>
-			+ HeaderBackend<PBlock>
-			+ Send
-			+ Sync
-			+ 'static,
-		PClient::Api: RuntimeApiCollection<Extrinsic>,
-		<PClient::Api as ApiExt<PBlock>>::StateBackend: StateBackend<HashFor<PBlock>>,
 		Spawner: SpawnNamed + Clone + Send + Sync + 'static,
-		Extrinsic: codec::Codec + Send + Sync + 'static,
+	{
+		let CollatorBuilder {
+			proposer_factory,
+			inherent_data_providers,
+			block_import,
+			block_status,
+			para_id,
+			client,
+			announce_block,
+			delayed_block_announce_validator,
+			_marker,
+		} = self;
+		polkadot_client.execute_with(CollatorBuilderWithClient {
+			spawner,
+			polkadot_network,
+			proposer_factory,
+			inherent_data_providers,
+			block_import,
+			block_status,
+			para_id,
+			client,
+			announce_block,
+			delayed_block_announce_validator,
+			_marker,
+		})
+	}
+}
+
+pub struct CollatorBuilderWithClient<Block: BlockT, PF, BI, Backend, Client, BS, Spawner, Network> {
+	proposer_factory: PF,
+	inherent_data_providers: InherentDataProviders,
+	block_import: BI,
+	block_status: Arc<BS>,
+	para_id: ParaId,
+	client: Arc<Client>,
+	announce_block: Arc<dyn Fn(Block::Hash, Vec<u8>) + Send + Sync>,
+	delayed_block_announce_validator: DelayedBlockAnnounceValidator<Block>,
+	_marker: PhantomData<(Block, Backend)>,
+	spawner: Spawner,
+	polkadot_network: Network,
+}
+
+impl<Block: BlockT, PF, BI, Backend, Client, BS, Spawner, Network>
+	polkadot_service::ExecuteWithClient
+	for CollatorBuilderWithClient<Block, PF, BI, Backend, Client, BS, Spawner, Network>
+where
+	PF: Environment<Block> + Send + 'static,
+	BI: BlockImport<Block, Error = sp_consensus::Error, Transaction = TransactionFor<PF, Block>>
+		+ Send
+		+ Sync
+		+ 'static,
+	Backend: sc_client_api::Backend<Block> + 'static,
+	Client: Finalizer<Block, Backend>
+		+ UsageProvider<Block>
+		+ HeaderBackend<Block>
+		+ Send
+		+ Sync
+		+ BlockBackend<Block>
+		+ 'static,
+	for<'a> &'a Client: BlockImport<Block>,
+	BS: BlockBackend<Block>,
+	Spawner: SpawnNamed + Clone + Send + Sync + 'static,
+	Network: CollatorNetwork + Clone + 'static,
+{
+	type Output = Result<Collator<Block, PF, BI, BS>, ()>;
+
+	fn execute_with_client<PClient, Api, PBackend>(
+		self,
+		polkadot_client: Arc<PClient>,
+	) -> Self::Output
+	where
+		<Api as ApiExt<PBlock>>::StateBackend: sp_api::StateBackend<BlakeTwo256>,
+		PBackend: sc_client_api::Backend<PBlock>,
+		PBackend::State: StateBackend<BlakeTwo256>,
+		Api: RuntimeApiCollection<StateBackend = PBackend::State>,
+		PClient: polkadot_service::AbstractClient<PBlock, PBackend, Api = Api> + 'static,
 	{
 		self.delayed_block_announce_validator
 			.set(Box::new(JustifiedBlockAnnounceValidator::new(
@@ -463,15 +526,16 @@ where
 				}
 			};
 
-		spawner.spawn("cumulus-follow-polkadot", follow.map(|_| ()).boxed());
+		self.spawner
+			.spawn("cumulus-follow-polkadot", follow.map(|_| ()).boxed());
 
 		Ok(Collator::new(
 			self.proposer_factory,
 			self.inherent_data_providers,
-			polkadot_network,
+			self.polkadot_network,
 			self.block_import,
 			self.block_status,
-			Arc::new(spawner),
+			Arc::new(self.spawner),
 			self.announce_block,
 		))
 	}
@@ -618,7 +682,7 @@ mod tests {
 		);
 		let context = builder
 			.build(
-				Arc::new(
+				polkadot_service::Client::Polkadot(Arc::new(
 					substrate_test_client::TestClientBuilder::<_, _, _, ()>::default()
 						.build_with_native_executor::<polkadot_service::polkadot_runtime::RuntimeApi, _>(
 							Some(NativeExecutor::<polkadot_service::PolkadotExecutor>::new(
@@ -628,7 +692,7 @@ mod tests {
 							)),
 						)
 						.0,
-				),
+				)),
 				spawner,
 				DummyCollatorNetwork,
 			)

network/src/lib.rs

@@ -32,9 +32,7 @@ use sp_runtime::{
 
 use polkadot_collator::Network as CollatorNetwork;
 use polkadot_network::legacy::gossip::{GossipMessage, GossipStatement};
-use polkadot_primitives::v0::{
+use polkadot_primitives::v0::{Block as PBlock, Hash as PHash, Id as ParaId, ParachainHost};
-	Id as ParaId, ParachainHost, Block as PBlock, Hash as PHash,
-};
 use polkadot_statement_table::v0::{SignedStatement, Statement};
 use polkadot_validation::check_statement;
 
@@ -244,11 +242,13 @@ impl<B: BlockT> BlockAnnounceValidator<B> for DelayedBlockAnnounceValidator<B> {
 		header: &B::Header,
 		data: &[u8],
 	) -> Result<Validation, Box<dyn std::error::Error + Send>> {
-		self.0
+		match self.0.lock().as_mut() {
-			.lock()
+			Some(validator) => validator.validate(header, data),
-			.as_mut()
+			None => {
-			.expect("BlockAnnounceValidator is set before validating the first announcement; qed")
+				log::warn!("BlockAnnounce validator not yet set, rejecting block announcement");
-			.validate(header, data)
+				Ok(Validation::Failure)
+			}
+		}
 	}
 }
 

runtime/src/validate_block/tests.rs

@@ -62,7 +62,7 @@ fn call_validate_block(
 
 	executor
 		.call_in_wasm(
-			&WASM_BINARY,
+			&WASM_BINARY.expect("You need to build the WASM binaries to run the tests!"),
 			None,
 			"validate_block",
 			&params,

test/parachain/Cargo.toml

@@ -40,6 +40,7 @@ sc-network = { git = "https://github.com/paritytech/substrate", branch = "cumulu
 sc-basic-authorship = { git = "https://github.com/paritytech/substrate", branch = "cumulus-branch", version = "0.8.0-rc5" }
 sp-timestamp = { git = "https://github.com/paritytech/substrate", branch = "cumulus-branch" }
 sp-trie = { git = "https://github.com/paritytech/substrate", branch = "cumulus-branch" }
+sp-api = { git = "https://github.com/paritytech/substrate", branch = "cumulus-branch" }
 sc-finality-grandpa = { git = "https://github.com/paritytech/substrate", branch = "cumulus-branch" }
 sc-informant = { git = "https://github.com/paritytech/substrate", branch = "cumulus-branch" }
 sc-chain-spec = { git = "https://github.com/paritytech/substrate", branch = "cumulus-branch" }

test/parachain/build.rs

@@ -14,29 +14,9 @@
 // You should have received a copy of the GNU General Public License
 // along with Cumulus.  If not, see <http://www.gnu.org/licenses/>.
 
-use std::{env, path::PathBuf};
-
 use substrate_build_script_utils::{generate_cargo_keys, rerun_if_git_head_changed};
 
 fn main() {
 	generate_cargo_keys();
 	rerun_if_git_head_changed();
-
-	let mut manifest_dir = PathBuf::from(
-		env::var("CARGO_MANIFEST_DIR").expect("`CARGO_MANIFEST_DIR` is always set by cargo."),
-	);
-
-	while manifest_dir.parent().is_some() {
-		if manifest_dir.join(".git/HEAD").exists() {
-			println!(
-				"cargo:rerun-if-changed={}",
-				manifest_dir.join(".git/HEAD").display()
-			);
-			return;
-		}
-
-		manifest_dir.pop();
-	}
-
-	println!("cargo:warning=Could not find `.git/HEAD` from manifest dir!");
 }

test/parachain/runtime/build.rs

@@ -19,7 +19,7 @@ use wasm_builder_runner::WasmBuilder;
 fn main() {
 	WasmBuilder::new()
 		.with_current_project()
-		.with_wasm_builder_from_crates("1.0.11")
+		.with_wasm_builder_from_crates("2.0.0")
 		.export_heap_base()
 		.import_memory()
 		.build()

test/parachain/src/chain_spec.rs

@@ -83,7 +83,7 @@ fn testnet_genesis(
 ) -> GenesisConfig {
 	GenesisConfig {
 		frame_system: Some(SystemConfig {
-			code: WASM_BINARY.to_vec(),
+			code: WASM_BINARY.expect("WASM binary was not build, please build it!").to_vec(),
 			changes_trie_config: Default::default(),
 		}),
 		pallet_balances: Some(BalancesConfig {

test/parachain/src/integration_test.rs

@@ -42,9 +42,7 @@ static INTEGRATION_TEST_ALLOWED_TIME: Option<&str> = option_env!("INTEGRATION_TE
 #[tokio::test]
 #[ignore]
 async fn integration_test() {
-	let task_executor: TaskExecutor = (|fut, _| {
+	let task_executor: TaskExecutor = (|fut, _| spawn(fut).map(|_| ())).into();
-		spawn(fut).map(|_| ())
-	}).into();
 
 	// start alice
 	let mut alice =
@@ -82,7 +80,10 @@ async fn integration_test() {
 				Info {
 					scheduling: Scheduling::Always,
 				},
-				parachain_runtime::WASM_BINARY.to_vec().into(),
+				parachain_runtime::WASM_BINARY
+					.expect("You need to build the WASM binary to run this test!")
+					.to_vec()
+					.into(),
 				genesis_state.into(),
 			)),
 		)));
@@ -104,7 +105,8 @@ async fn integration_test() {
 		let parachain_config =
 			parachain_config(task_executor.clone(), Charlie, vec![], para_id).unwrap();
 		let (_service, charlie_client) =
-			crate::service::run_collator(parachain_config, key, polkadot_config, para_id, true).unwrap();
+			crate::service::run_collator(parachain_config, key, polkadot_config, para_id, true)
+				.unwrap();
 		sleep(Duration::from_secs(3)).await;
 		charlie_client.wait_for_blocks(4).await;
 

test/parachain/src/service.rs

@@ -16,9 +16,9 @@
 
 use ansi_term::Color;
 use cumulus_collator::{prepare_collator_config, CollatorBuilder};
-use cumulus_network::DelayedBlockAnnounceValidator;
+use cumulus_network::{DelayedBlockAnnounceValidator, JustifiedBlockAnnounceValidator};
 use futures::{future::ready, FutureExt};
-use polkadot_primitives::v0::CollatorPair;
+use polkadot_primitives::v0::{CollatorPair, Block as PBlock, Id as ParaId};
 use sc_executor::native_executor_instance;
 pub use sc_executor::NativeExecutor;
 use sc_informant::OutputFormat;
@@ -26,7 +26,8 @@ use sc_service::{Configuration, PartialComponents, TaskManager, TFullBackend, TF
 use std::sync::Arc;
 use sp_core::crypto::Pair;
 use sp_trie::PrefixedMemoryDB;
-use sp_runtime::traits::BlakeTwo256;
+use sp_runtime::traits::{BlakeTwo256, Block as BlockT};
+use polkadot_service::{AbstractClient, RuntimeApiCollection};
 
 // Our native executor instance.
 native_executor_instance!(
@@ -206,11 +207,11 @@ pub fn run_collator(
 			.spawn("polkadot", polkadot_future);
 	} else {
 		let is_light = matches!(polkadot_config.role, Role::Light);
-		let builder = polkadot_service::NodeBuilder::new(polkadot_config);
+		let (mut polkadot_task_manager, client, _) = if is_light {
-		let mut polkadot_task_manager = if is_light {
+			Err("Light client not supported.".into())
-			return Err("Light client not supported.".into());
 		} else {
-			builder.build_full(
+			polkadot_service::build_full(
+				polkadot_config,
 				Some((key.public(), id)),
 				None,
 				false,
@@ -221,6 +222,7 @@ pub fn run_collator(
 		let polkadot_future = async move {
 			polkadot_task_manager.future().await.expect("polkadot essential task failed");
 		};
+		client.execute_with(SetDelayedBlockAnnounceValidator { block_announce_validator, para_id: id });
 
 		task_manager
 			.spawn_essential_handle()
@@ -229,3 +231,22 @@ pub fn run_collator(
 
 	Ok((task_manager, client))
 }
+
+struct SetDelayedBlockAnnounceValidator<B: BlockT> {
+	block_announce_validator: DelayedBlockAnnounceValidator<B>,
+	para_id: ParaId,
+}
+
+impl<B: BlockT> polkadot_service::ExecuteWithClient for SetDelayedBlockAnnounceValidator<B> {
+	type Output = ();
+
+	fn execute_with_client<Client, Api, Backend>(self, client: Arc<Client>) -> Self::Output
+		where<Api as sp_api::ApiExt<PBlock>>::StateBackend: sp_api::StateBackend<BlakeTwo256>,
+		Backend: sc_client_api::Backend<PBlock>,
+		Backend::State: sp_api::StateBackend<BlakeTwo256>,
+		Api: RuntimeApiCollection<StateBackend = Backend::State>,
+		Client: AbstractClient<PBlock, Backend, Api = Api> + 'static
+	{
+		self.block_announce_validator.set(Box::new(JustifiedBlockAnnounceValidator::new(client, self.para_id)));
+	}
+}

test/runtime/build.rs

@@ -19,7 +19,7 @@ use wasm_builder_runner::WasmBuilder;
 fn main() {
 	WasmBuilder::new()
 		.with_current_project()
-		.with_wasm_builder_from_crates("1.0.11")
+		.with_wasm_builder_from_crates("2.0.0")
 		.export_heap_base()
 		.import_memory()
 		.build()