Switch to custom BlockAnnounceData (#310)

* Switch to custom `BlockAnnounceData`

Instead of sending a `SignedFullStatement` this switches to a new struct
`BlockAnnounceData` that is being send alongside the block announcement.
The signed full statement contains the candidate commitments, meaning it
could be a full runtime upgrade that we send alongside a block
announcement... To prevent this, we now only send the candidate receipt
and the compact statement.

* Update to latest polkadot

cumulus/network/Cargo.toml

@@ -21,6 +21,7 @@ polkadot-overseer = { git = "https://github.com/paritytech/polkadot", branch = "
 polkadot-node-primitives = { git = "https://github.com/paritytech/polkadot", branch = "master" }
 polkadot-node-subsystem = { git = "https://github.com/paritytech/polkadot", branch = "master" }
 polkadot-service = { git = "https://github.com/paritytech/polkadot", branch = "master" }
+polkadot-parachain = { git = "https://github.com/paritytech/polkadot", branch = "master" }
 
 # Cumulus deps
 cumulus-primitives = { path = "../primitives" }

cumulus/network/src/lib.rs

@@ -41,8 +41,8 @@ use polkadot_node_primitives::{SignedFullStatement, Statement};
 use polkadot_node_subsystem::messages::StatementDistributionMessage;
 use polkadot_overseer::OverseerHandler;
 use polkadot_primitives::v1::{
-	Block as PBlock, Hash as PHash, Id as ParaId, OccupiedCoreAssumption, ParachainHost,
+	Block as PBlock, CandidateReceipt, CompactStatement, Hash as PHash, Id as ParaId,
-	SigningContext,
+	OccupiedCoreAssumption, ParachainHost, SignedStatement, SigningContext,
 };
 use polkadot_service::ClientHandle;
 
@@ -54,7 +54,7 @@ use futures::{
 };
 use log::trace;
 
-use std::{fmt, marker::PhantomData, pin::Pin, sync::Arc};
+use std::{fmt, marker::PhantomData, pin::Pin, sync::Arc, convert::TryFrom};
 
 use wait_on_relay_chain_block::WaitOnRelayChainBlock;
 
@@ -70,6 +70,118 @@ impl fmt::Display for BlockAnnounceError {
 	}
 }
 
+/// The data that we attach to a block announcement.
+///
+/// This will be used to prove that a header belongs to a block that is probably being backed by
+/// the relay chain.
+#[derive(Encode, Decode, Debug)]
+pub struct BlockAnnounceData {
+	receipt: CandidateReceipt,
+	statement: SignedStatement,
+}
+
+impl BlockAnnounceData {
+	/// Validate that the receipt, statement and announced header match.
+	///
+	/// This will not check the signature, for this you should use [`BlockAnnounceData::check_signature`].
+	fn validate(&self, encoded_header: Vec<u8>) -> Result<(), BlockAnnounceError> {
+		let candidate_hash = if let CompactStatement::Candidate(h) = self.statement.payload() {
+			h
+		} else {
+			return Err(BlockAnnounceError(
+				"`CompactStatement` isn't the candidate variant!".into(),
+			));
+		};
+
+		if *candidate_hash != self.receipt.hash() {
+			return Err(BlockAnnounceError(
+				"Receipt candidate hash doesn't match candidate hash in statement".into(),
+			));
+		}
+
+		if polkadot_parachain::primitives::HeadData(encoded_header).hash() != self.receipt.descriptor.para_head
+		{
+			return Err(BlockAnnounceError(
+				"Receipt para head hash doesn't match the hash of the header in the block announcement".into(),
+			));
+		}
+
+		Ok(())
+	}
+
+	/// Check the signature of the statement.
+	///
+	/// Returns an `Err(_)` if it failed.
+	fn check_signature<P>(&self, relay_chain_client: &Arc<P>) -> Result<(), BlockAnnounceError>
+	where
+		P: ProvideRuntimeApi<PBlock> + Send + Sync + 'static,
+		P::Api: ParachainHost<PBlock>,
+	{
+		let runtime_api = relay_chain_client.runtime_api();
+		let validator_index = self.statement.validator_index();
+
+		let runtime_api_block_id = BlockId::Hash(self.receipt.descriptor.relay_parent);
+		let session_index = match runtime_api.session_index_for_child(&runtime_api_block_id) {
+			Ok(r) => r,
+			Err(e) => {
+				return Err(BlockAnnounceError(format!("{:?}", e)));
+			}
+		};
+
+		let signing_context = SigningContext {
+			parent_hash: self.receipt.descriptor.relay_parent,
+			session_index,
+		};
+
+		// Check that the signer is a legit validator.
+		let authorities = match runtime_api.validators(&runtime_api_block_id) {
+			Ok(r) => r,
+			Err(e) => {
+				return Err(BlockAnnounceError(format!("{:?}", e)));
+			}
+		};
+		let signer = match authorities.get(validator_index as usize) {
+			Some(r) => r,
+			None => {
+				return Err(BlockAnnounceError(
+					"block accouncement justification signer is a validator index out of bound"
+						.to_string(),
+				));
+			}
+		};
+
+		// Check statement is correctly signed.
+		if self
+			.statement
+			.check_signature(&signing_context, &signer)
+			.is_err()
+		{
+			return Err(BlockAnnounceError(
+				"block announcement justification signature is invalid".to_string(),
+			));
+		}
+
+		Ok(())
+	}
+}
+
+impl TryFrom<SignedFullStatement> for BlockAnnounceData {
+	type Error = ();
+
+	fn try_from(stmt: SignedFullStatement) -> Result<BlockAnnounceData, ()> {
+		let receipt = if let Statement::Seconded(receipt) = stmt.payload() {
+			receipt.to_plain()
+		} else {
+			return Err(());
+		};
+
+		Ok(BlockAnnounceData {
+			receipt,
+			statement: stmt.convert_payload(),
+		})
+	}
+}
+
 /// Parachain specific block announce validator.
 ///
 /// This block announce validator is required if the parachain is running
@@ -89,8 +201,8 @@ impl fmt::Display for BlockAnnounceError {
 /// For each block announcement that is received, the generic block announcement validation
 /// will call this validator and provides the extra data that was attached to the announcement.
 /// We call this extra data `justification`.
-/// It is expected that the attached data is a SCALE encoded [`SignedFullStatement`]. The
+/// It is expected that the attached data is a SCALE encoded [`BlockAnnounceData`]. The
-/// statement is checked to be a [`Statement::Seconded`] and that it is signed by an active
+/// statement is checked to be a [`CompactStatement::Candidate`] and that it is signed by an active
 /// parachain validator.
 ///
 /// If no justification was provided we check if the block announcement is at the tip of the known
@@ -213,13 +325,14 @@ where
 				.boxed();
 		}
 
-		let signed_stmt = match SignedFullStatement::decode(&mut data) {
+		let block_announce_data = match BlockAnnounceData::decode(&mut data) {
 			Ok(r) => r,
-			Err(_) => return ready(Err(Box::new(BlockAnnounceError(
+			Err(_) => {
-				"cannot decode block announcement justification, must be a `SignedFullStatement`"
+				return ready(Err(Box::new(BlockAnnounceError(
-					.into(),
+					"Can not decode the `BlockAnnounceData`".into(),
 				)) as Box<_>))
-			.boxed(),
+				.boxed()
+			}
 		};
 
 		let relay_chain_client = self.relay_chain_client.clone();
@@ -227,72 +340,20 @@ where
 		let wait_on_relay_chain_block = self.wait_on_relay_chain_block.clone();
 
 		async move {
-			// Check statement is a candidate statement.
+			block_announce_data
-			let candidate_receipt = match signed_stmt.payload() {
+				.validate(header_encoded)
-				Statement::Seconded(ref candidate_receipt) => candidate_receipt,
+				.map_err(|e| Box::new(e) as Box<_>)?;
-				_ => {
-					return Err(Box::new(BlockAnnounceError(
-						"block announcement justification must be a `Statement::Seconded`".into(),
-					)) as Box<_>)
-				}
-			};
 
-			// Check the header in the candidate_receipt match header given header.
+			let relay_parent = block_announce_data.receipt.descriptor.relay_parent;
-			if header_encoded != candidate_receipt.commitments.head_data.0 {
-				return Err(Box::new(BlockAnnounceError(
-					"block announcement header does not match the one justified".into(),
-				)) as Box<_>);
-			}
-
-			let relay_parent = &candidate_receipt.descriptor.relay_parent;
 
 			wait_on_relay_chain_block
-				.wait_on_relay_chain_block(*relay_parent)
+				.wait_on_relay_chain_block(relay_parent)
 				.await
 				.map_err(|e| Box::new(BlockAnnounceError(e.to_string())) as Box<_>)?;
 
-			let runtime_api = relay_chain_client.runtime_api();
+			block_announce_data
-			let validator_index = signed_stmt.validator_index();
+				.check_signature(&relay_chain_client)
-
+				.map_err(|e| Box::new(e) as Box<_>)?;
-			let runtime_api_block_id = BlockId::Hash(*relay_parent);
-			let session_index = match runtime_api.session_index_for_child(&runtime_api_block_id) {
-				Ok(r) => r,
-				Err(e) => {
-					return Err(Box::new(BlockAnnounceError(format!("{:?}", e))) as Box<_>);
-				}
-			};
-
-			let signing_context = SigningContext {
-				parent_hash: *relay_parent,
-				session_index,
-			};
-
-			// Check that the signer is a legit validator.
-			let authorities = match runtime_api.validators(&runtime_api_block_id) {
-				Ok(r) => r,
-				Err(e) => {
-					return Err(Box::new(BlockAnnounceError(format!("{:?}", e))) as Box<_>);
-				}
-			};
-			let signer = match authorities.get(validator_index as usize) {
-				Some(r) => r,
-				None => {
-					return Err(Box::new(BlockAnnounceError(
-						"block accouncement justification signer is a validator index out of bound"
-							.to_string(),
-					)) as Box<_>);
-				}
-			};
-
-			// Check statement is correctly signed.
-			if signed_stmt
-				.check_signature(&signing_context, &signer)
-				.is_err()
-			{
-				return Err(Box::new(BlockAnnounceError(
-					"block announcement justification signature is invalid".to_string(),
-				)) as Box<_>);
-			}
 
 			Ok(Validation::Success { is_new_best: true })
 		}
@@ -484,9 +545,11 @@ async fn wait_to_announce<Block: BlockT>(
 		.await;
 
 	while let Some(statement) = receiver.next().await {
-		match &statement.payload() {
+		match statement.payload() {
 			Statement::Seconded(c) if &c.descriptor.pov_hash == &pov_hash => {
-				announce_block(block_hash, statement.encode());
+				if let Ok(data) = BlockAnnounceData::try_from(statement) {
+					announce_block(block_hash, data.encode());
+				}
 
 				break;
 			}

cumulus/network/src/tests.rs

@@ -132,6 +132,7 @@ async fn make_gossip_message_and_header(
 		},
 		descriptor: CandidateDescriptor {
 			relay_parent,
+			para_head: polkadot_parachain::primitives::HeadData(header.encode()).hash(),
 			..Default::default()
 		},
 	};
@@ -192,7 +193,7 @@ fn check_statement_is_encoded_correctly() {
 	check_error(res, |error| {
 		matches!(
 			error,
-			BlockAnnounceError(x) if x.contains("must be a `SignedFullStatement`")
+			BlockAnnounceError(x) if x.contains("Can not decode the `BlockAnnounceData`")
 		)
 	});
 }
@@ -202,7 +203,7 @@ fn check_signer_is_legit_validator() {
 	let (mut validator, api) = make_validator_and_api();
 
 	let (signed_statement, header) = block_on(make_gossip_message_and_header_using_genesis(api, 1));
-	let data = signed_statement.encode();
+	let data = BlockAnnounceData::try_from(signed_statement).unwrap().encode();
 
 	let res = block_on(validator.validate(&header, &data))
 		.err()
@@ -220,7 +221,7 @@ fn check_statement_is_correctly_signed() {
 
 	let (signed_statement, header) = block_on(make_gossip_message_and_header_using_genesis(api, 0));
 
-	let mut data = signed_statement.encode();
+	let mut data = BlockAnnounceData::try_from(signed_statement).unwrap().encode();
 
 	// The signature comes at the end of the type, so change a bit to make the signature invalid.
 	let last = data.len() - 1;
@@ -270,7 +271,10 @@ fn check_statement_seconded() {
 		&alice_public.into(),
 	))
 	.expect("Signs statement");
-	let data = signed_statement.encode();
+	let data = BlockAnnounceData {
+		receipt: Default::default(),
+		statement: signed_statement.convert_payload(),
+	}.encode();
 
 	let res = block_on(validator.validate(&header, &data))
 		.err()
@@ -279,7 +283,7 @@ fn check_statement_seconded() {
 	check_error(res, |error| {
 		matches!(
 			error,
-			BlockAnnounceError(x) if x.contains("must be a `Statement::Seconded`")
+			BlockAnnounceError(x) if x.contains("`CompactStatement` isn't the candidate variant")
 		)
 	});
 }
@@ -290,7 +294,7 @@ fn check_header_match_candidate_receipt_header() {
 
 	let (signed_statement, mut header) =
 		block_on(make_gossip_message_and_header_using_genesis(api, 0));
-	let data = signed_statement.encode();
+	let data = BlockAnnounceData::try_from(signed_statement).unwrap().encode();
 	header.number = 300;
 
 	let res = block_on(validator.validate(&header, &data))
@@ -300,7 +304,7 @@ fn check_header_match_candidate_receipt_header() {
 	check_error(res, |error| {
 		matches!(
 			error,
-			BlockAnnounceError(x) if x.contains("header does not match")
+			BlockAnnounceError(x) if x.contains("Receipt para head hash doesn't match")
 		)
 	});
 }
@@ -323,7 +327,7 @@ fn relay_parent_not_imported_when_block_announce_is_processed() {
 
 		let (signed_statement, header) = make_gossip_message_and_header(api, block.hash(), 0).await;
 
-		let data = signed_statement.encode();
+		let data = BlockAnnounceData::try_from(signed_statement).unwrap().encode();
 
 		let mut validation = validator.validate(&header, &data);