Reject blocks without justification which don't have the best number (#105)

* Initial commit

Forked at: 6c74beab7b
Parent branch: origin/master

* Reject blocks without justification

* Revert "Reject blocks without justification"

This reverts commit ee60e12097939d4ccfe987a71db9a876319ae5ba.

* WIP

Forked at: 6c74beab7b
Parent branch: origin/master

* WIP

Forked at: 6c74beab7b
Parent branch: origin/master

* WIP

Forked at: 6c74beab7b
Parent branch: origin/master

* CLEANUP

Forked at: 6c74beab7b
Parent branch: origin/master

* WIP

Forked at: 6c74beab7b
Parent branch: origin/master

* WIP

Forked at: 6c74beab7b
Parent branch: origin/master

* CLEANUP

Forked at: 6c74beab7b
Parent branch: origin/master

* WIP

Forked at: 6c74beab7b
Parent branch: origin/master

* WIP

Forked at: 6c74beab7b
Parent branch: origin/master

* Move HeadData to primitives

* Update network/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update network/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* CLEANUP

Forked at: 6c74beab7b
Parent branch: origin/master

* fix

* CLEANUP

Forked at: 6c74beab7b
Parent branch: origin/master

* messages

* for the greater good

* Update primitives/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update network/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update network/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update network/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

cumulus/Cargo.lock

@@ -930,6 +930,7 @@ dependencies = [
 name = "cumulus-network"
 version = "0.1.0"
 dependencies = [
+ "cumulus-primitives",
  "cumulus-test-runtime",
  "futures 0.3.4",
  "log 0.4.8",

cumulus/collator/src/lib.rs

@@ -20,7 +20,7 @@ use cumulus_network::{
 	DelayedBlockAnnounceValidator, JustifiedBlockAnnounceValidator, WaitToAnnounce,
 };
 use cumulus_primitives::{
-	inherents::VALIDATION_FUNCTION_PARAMS_IDENTIFIER as VFP_IDENT,
+	HeadData, inherents::VALIDATION_FUNCTION_PARAMS_IDENTIFIER as VFP_IDENT,
 	validation_function_params::ValidationFunctionParams,
 };
 use cumulus_runtime::ParachainBlockData;
@@ -56,12 +56,6 @@ use std::{fmt::Debug, marker::PhantomData, sync::Arc, time::Duration, pin::Pin};
 
 use parking_lot::Mutex;
 
-/// The head data of the parachain, stored in the relay chain.
-#[derive(Decode, Encode, Debug)]
-struct HeadData<Block: BlockT> {
-	header: Block::Header,
-}
-
 /// The implementation of the Cumulus `Collator`.
 pub struct Collator<Block: BlockT, PF, BI> {
 	proposer_factory: Arc<Mutex<PF>>,
@@ -340,7 +334,10 @@ where
 		+ Sync
 		+ 'static,
 	Backend: sc_client_api::Backend<Block> + 'static,
-	Client: Finalizer<Block, Backend> + UsageProvider<Block> + Send + Sync + 'static,
+	Client: Finalizer<Block, Backend> + UsageProvider<Block> + HeaderBackend<Block>
+		+ Send
+		+ Sync
+		+ 'static,
 {
 	type ParachainContext = Collator<Block, PF, BI>;
 
@@ -359,7 +356,7 @@ where
 		Extrinsic: codec::Codec + Send + Sync + 'static,
 	{
 		self.delayed_block_announce_validator.set(
-			Box::new(JustifiedBlockAnnounceValidator::new(polkadot_client.clone())),
+			Box::new(JustifiedBlockAnnounceValidator::new(polkadot_client.clone(), self.para_id)),
 		);
 
 		let follow =

cumulus/network/Cargo.toml

@@ -19,6 +19,9 @@ polkadot-statement-table = { git = "https://github.com/paritytech/polkadot", bra
 polkadot-validation = { git = "https://github.com/paritytech/polkadot", branch = "cumulus-branch" }
 polkadot-network = { git = "https://github.com/paritytech/polkadot", branch = "cumulus-branch" }
 
+# cumulus deps
+cumulus-primitives = { path = "../primitives" }
+
 # other deps
 codec = { package = "parity-scale-codec", version = "1.3.0", features = [ "derive" ] }
 futures = { version = "0.3.1", features = ["compat"] }

cumulus/network/src/lib.rs

@@ -24,17 +24,19 @@ mod tests;
 use sp_api::ProvideRuntimeApi;
 use sp_blockchain::{Error as ClientError, HeaderBackend};
 use sp_consensus::block_validation::{BlockAnnounceValidator, Validation};
-use sp_runtime::{generic::BlockId, traits::Block as BlockT};
+use sp_runtime::{generic::BlockId, traits::{Block as BlockT, Header as HeaderT}};
 
 use polkadot_collator::Network as CollatorNetwork;
 use polkadot_network::legacy::gossip::{GossipMessage, GossipStatement};
 use polkadot_primitives::{
-	parachain::ParachainHost,
+	parachain::{ParachainHost, Id as ParaId},
 	Block as PBlock, Hash as PHash,
 };
 use polkadot_statement_table::{SignedStatement, Statement};
 use polkadot_validation::check_statement;
 
+use cumulus_primitives::HeadData;
+
 use codec::{Decode, Encode};
 use futures::{pin_mut, select, StreamExt};
 use futures::channel::oneshot;
@@ -54,13 +56,15 @@ use parking_lot::Mutex;
 pub struct JustifiedBlockAnnounceValidator<B, P> {
 	phantom: PhantomData<B>,
 	polkadot_client: Arc<P>,
+	para_id: ParaId,
 }
 
 impl<B, P> JustifiedBlockAnnounceValidator<B, P> {
-	pub fn new(polkadot_client: Arc<P>) -> Self {
+	pub fn new(polkadot_client: Arc<P>, para_id: ParaId) -> Self {
 		Self {
 			phantom: Default::default(),
 			polkadot_client,
+			para_id,
 		}
 	}
 }
@@ -75,9 +79,34 @@ where
 		header: &B::Header,
 		mut data: &[u8],
 	) -> Result<Validation, Box<dyn std::error::Error + Send>> {
-		// If no data is provided the announce is valid.
+		let runtime_api = self.polkadot_client.runtime_api();
+		let polkadot_info = self.polkadot_client.info();
+
 		if data.is_empty() {
-			return Ok(Validation::Success);
+			// Check if block is equal or higher than best (this requires a justification)
+			let runtime_api_block_id = BlockId::Hash(polkadot_info.best_hash);
+			let block_number = header.number();
+
+			let local_validation_data = runtime_api
+				.local_validation_data(&runtime_api_block_id, self.para_id)
+				.map_err(|e| Box::new(ClientError::Msg(format!("{:?}", e))) as Box<_>)?
+				.ok_or_else(|| {
+					Box::new(ClientError::Msg("Could not find parachain head in relay chain".into())) as Box<_>
+				})?;
+			let parent_head = HeadData::<B>::decode(&mut &local_validation_data.parent_head.0[..])
+				.map_err(|e| Box::new(ClientError::Msg(format!("Failed to decode parachain head: {:?}", e))) as Box<_>)?;
+			let known_best_number = parent_head.header.number();
+
+			return Ok(if block_number >= known_best_number {
+				trace!(
+					target: "cumulus-network",
+					"validation failed because a justification is needed if the block at the top of the chain."
+				);
+
+				Validation::Failure
+			} else {
+				Validation::Success
+			});
 		}
 
 		// Check data is a gossip message.
@@ -108,7 +137,7 @@ where
 		} = gossip_statement;
 
 		// Check that the relay chain parent of the block is the relay chain head
-		let best_number = self.polkadot_client.info().best_number;
+		let best_number = polkadot_info.best_number;
 
 		match self.polkadot_client.number(relay_chain_leaf) {
 			Err(err) => {
@@ -133,7 +162,6 @@ where
 			},
 		}
 
-		let runtime_api = self.polkadot_client.runtime_api();
 		let runtime_api_block_id = BlockId::Hash(relay_chain_leaf);
 		let signing_context = runtime_api
 			.signing_context(&runtime_api_block_id)

cumulus/network/src/tests.rs

@@ -48,12 +48,15 @@ fn make_validator_and_client() -> (
 	let builder = TestClientBuilder::new();
 	let client = Arc::new(TestApi::new(Arc::new(builder.build())));
 
-	(JustifiedBlockAnnounceValidator::new(client.clone()), client)
+	(
+		JustifiedBlockAnnounceValidator::new(client.clone(), ParaId::from(56)),
+		client,
+	)
 }
 
 fn default_header() -> Header {
 	Header {
-		number: Default::default(),
+		number: 1,
 		digest: Default::default(),
 		extrinsics_root: Default::default(),
 		parent_hash: Default::default(),
@@ -91,13 +94,34 @@ fn make_gossip_message_and_header(
 }
 
 #[test]
-fn valid_if_no_data() {
+fn valid_if_no_data_and_less_than_best_known_number() {
 	let mut validator = make_validator();
-	let header = default_header();
+	let header = Header {
+		number: 0,
+		..default_header()
+	};
+	let res = validator.validate(&header, &[]);
 
-	assert!(
+	assert_eq!(
-		matches!(validator.validate(&header, &[]), Ok(Validation::Success)),
+		res.unwrap(),
-		"validating without data is always a success",
+		Validation::Success,
+		"validating without data with block number < best known number is always a success",
+	);
+}
+
+#[test]
+fn invalid_if_no_data_exceeds_best_known_number() {
+	let mut validator = make_validator();
+	let header = Header {
+		number: 1,
+		..default_header()
+	};
+	let res = validator.validate(&header, &[]);
+
+	assert_eq!(
+		res.unwrap(),
+		Validation::Failure,
+		"validation fails if no justification and block number >= best known number",
 	);
 }
 
@@ -399,7 +423,10 @@ sp_api::mock_impl_runtime_apis! {
 		}
 
 		fn local_validation_data(_: ParaId) -> Option<LocalValidationData> {
-			Some(Default::default())
+			Some(LocalValidationData {
+				parent_head: HeadData::<Block> { header: default_header() }.encode().into(),
+				..Default::default()
+			})
 		}
 
 		fn get_heads(_: Vec<<PBlock as BlockT>::Extrinsic>) -> Option<Vec<AbridgedCandidateReceipt>> {

cumulus/primitives/src/lib.rs

@@ -20,6 +20,9 @@
 
 pub mod validation_function_params;
 
+use codec::{Decode, Encode};
+use sp_runtime::traits::Block as BlockT;
+
 /// Identifiers and types related to Cumulus Inherents
 pub mod inherents {
 	use sp_inherents::InherentIdentifier;
@@ -64,3 +67,9 @@ pub trait UpwardMessageSender {
 	/// Returns an error if sending failed.
 	fn send_upward_message(msg: &()) -> Result<(), ()>;
 }
+
+/// The head data of the parachain, stored in the relay chain.
+#[derive(Decode, Encode, Debug)]
+pub struct HeadData<Block: BlockT> {
+	pub header: Block::Header,
+}