Reject blocks without justification which don't have the best number

collator/src/lib.rs

@@ -340,7 +340,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 +362,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.client.clone())),
 		);
 
 		let follow =

network/src/lib.rs

@@ -24,7 +24,7 @@ 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, One}};
 
 use polkadot_collator::Network as CollatorNetwork;
 use polkadot_network::legacy::gossip::{GossipMessage, GossipStatement};
@@ -51,32 +51,48 @@ use parking_lot::Mutex;
 /// the justification.
 ///
 /// Note: if no justification is provided the annouce is considered valid.
-pub struct JustifiedBlockAnnounceValidator<B, P> {
+pub struct JustifiedBlockAnnounceValidator<B, P, C> {
 	phantom: PhantomData<B>,
 	polkadot_client: Arc<P>,
+	parachain_client: Arc<C>,
 }
 
-impl<B, P> JustifiedBlockAnnounceValidator<B, P> {
-	pub fn new(polkadot_client: Arc<P>) -> Self {
+impl<B, P, C> JustifiedBlockAnnounceValidator<B, P, C> {
+	pub fn new(polkadot_client: Arc<P>, parachain_client: Arc<C>) -> Self {
 		Self {
 			phantom: Default::default(),
 			polkadot_client,
+			parachain_client,
 		}
 	}
 }
 
-impl<B: BlockT, P> BlockAnnounceValidator<B> for JustifiedBlockAnnounceValidator<B, P>
+impl<B: BlockT, P, C> BlockAnnounceValidator<B> for JustifiedBlockAnnounceValidator<B, P, C>
 where
 	P: ProvideRuntimeApi<PBlock> + HeaderBackend<PBlock>,
 	P::Api: ParachainHost<PBlock>,
+	C: HeaderBackend<B>,
 {
 	fn validate(
 		&mut self,
 		header: &B::Header,
 		mut data: &[u8],
 	) -> Result<Validation, Box<dyn std::error::Error + Send>> {
-		// If no data is provided the announce is valid.
+		// If no data is provided the announce is probably valid
 		if data.is_empty() {
+			// Check if block is one higher than best
+			let best_number = self.parachain_client.info().best_number;
+			let block_number: <<B as BlockT>::Header as HeaderT>::Number = *header.number();
+
+			if !(block_number == best_number + One::one() || block_number == best_number) {
+				trace!(
+					target: "cumulus-network",
+					"validation failed because the block number is not the best block number or one higher",
+				);
+
+				return Ok(Validation::Failure);
+			}
+
 			return Ok(Validation::Success);
 		}
 

network/src/tests.rs

@@ -15,7 +15,7 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use super::*;
-use cumulus_test_runtime::{Block, Header};
+use cumulus_test_runtime::{Block, Hash, Header};
 use polkadot_primitives::{
 	parachain::{
 		AbridgedCandidateReceipt, Chain, CollatorId, DutyRoster, GlobalValidationSchedule,
@@ -35,25 +35,28 @@ use sp_core::H256;
 use sp_keyring::Sr25519Keyring;
 use sp_runtime::traits::{Block as BlockT, NumberFor, Zero};
 
-fn make_validator() -> JustifiedBlockAnnounceValidator<Block, TestApi> {
+fn make_validator() -> JustifiedBlockAnnounceValidator<Block, TestApi, TestApi> {
 	let (validator, _client) = make_validator_and_client();
 
 	validator
 }
 
 fn make_validator_and_client() -> (
-	JustifiedBlockAnnounceValidator<Block, TestApi>,
+	JustifiedBlockAnnounceValidator<Block, TestApi, TestApi>,
 	Arc<TestApi>,
 ) {
 	let builder = TestClientBuilder::new();
 	let client = Arc::new(TestApi::new(Arc::new(builder.build())));
 
-	(JustifiedBlockAnnounceValidator::new(client.clone()), client)
+	(
+		JustifiedBlockAnnounceValidator::new(client.clone(), client.clone()),
+		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,53 @@ fn make_gossip_message_and_header(
 }
 
 #[test]
-fn valid_if_no_data() {
+fn valid_if_no_data_and_best_number() {
 	let mut validator = make_validator();
 	let header = default_header();
 
 	assert!(
 		matches!(validator.validate(&header, &[]), Ok(Validation::Success)),
-		"validating without data is always a success",
+		"validating without data is a success",
+	);
+}
+
+#[test]
+fn invalid_if_no_data_but_not_best_number() {
+	let mut validator = make_validator();
+	let header = Header {
+		number: 0,
+		..default_header()
+	};
+	let res = validator.validate(&header, &[]);
+
+	assert_eq!(
+		res.unwrap(),
+		Validation::Failure,
+		"validation fails if no justification and not the best number",
+	);
+
+	let header = Header {
+		number: 2,
+		..default_header()
+	};
+	let res = validator.validate(&header, &[]);
+
+	assert_eq!(
+		res.unwrap(),
+		Validation::Success,
+		"validating without data with block number = best + 1 is always a success",
+	);
+
+	let header = Header {
+		number: 3,
+		..default_header()
+	};
+	let res = validator.validate(&header, &[]);
+
+	assert_eq!(
+		res.unwrap(),
+		Validation::Failure,
+		"validation fails if no justification and not the best number",
 	);
 }
 
@@ -466,3 +509,55 @@ impl HeaderBackend<PBlock> for TestApi {
 		Ok(None)
 	}
 }
+
+/// Blockchain database header backend. Does not perform any validation.
+impl HeaderBackend<Block> for TestApi {
+	fn header(
+		&self,
+		_id: BlockId<Block>,
+	) -> std::result::Result<Option<Header>, sp_blockchain::Error> {
+		Ok(None)
+	}
+
+	fn info(&self) -> sc_client_api::blockchain::Info<Block> {
+		let best_hash = H256::from_low_u64_be(1);
+
+		sc_client_api::blockchain::Info {
+			best_hash,
+			best_number: 1,
+			finalized_hash: Default::default(),
+			finalized_number: Zero::zero(),
+			genesis_hash: Default::default(),
+			number_leaves: Default::default(),
+		}
+	}
+
+	fn status(
+		&self,
+		_id: BlockId<Block>,
+	) -> std::result::Result<sc_client_api::blockchain::BlockStatus, sp_blockchain::Error> {
+		Ok(sc_client_api::blockchain::BlockStatus::Unknown)
+	}
+
+	fn number(
+		&self,
+		hash: Hash,
+	) -> std::result::Result<Option<NumberFor<Block>>, sp_blockchain::Error> {
+		if hash == H256::zero() {
+			Ok(Some(0))
+		} else if hash == H256::from_low_u64_be(1) {
+			Ok(Some(1))
+		} else if hash == H256::from_low_u64_be(0xdead) {
+			Err(sp_blockchain::Error::Backend("dead".to_string()))
+		} else {
+			Ok(None)
+		}
+	}
+
+	fn hash(
+		&self,
+		_number: NumberFor<Block>,
+	) -> std::result::Result<Option<Hash>, sp_blockchain::Error> {
+		Ok(None)
+	}
+}