Deduplicate parachains validation

Deduplicate the logic that validates the updated parachain heads.

Signed-off-by: Serban Iorga <serban@parity.io>

bridges/modules/parachains/src/extension.rs

@@ -17,7 +17,7 @@
 use crate::{Config, Pallet, RelayBlockHash, RelayBlockHasher, RelayBlockNumber};
 use bp_runtime::FilterCall;
 use frame_support::{dispatch::CallableCallFor, traits::IsSubType};
-use sp_runtime::transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction};
+use sp_runtime::transaction_validity::{TransactionValidity, ValidTransaction};
 
 /// Validate parachain heads in order to avoid "mining" transactions that provide
 /// outdated bridged parachain heads. Without this validation, even honest relayers
@@ -43,45 +43,27 @@ where
 		>,
 {
 	fn validate(call: &Call) -> TransactionValidity {
-		let (bundled_relay_block_number, parachains) = match call.is_sub_type() {
+		let (updated_at_relay_block_number, parachains) = match call.is_sub_type() {
 			Some(crate::Call::<T, I>::submit_parachain_heads {
 				ref at_relay_block,
 				ref parachains,
 				..
-			}) if parachains.len() == 1 => (at_relay_block.0, parachains),
+			}) => (at_relay_block.0, parachains),
+			_ => return Ok(ValidTransaction::default()),
+		};
+		let (parachain, parachain_head_hash) = match parachains.as_slice() {
+			&[(parachain, parachain_head_hash)] => (parachain, parachain_head_hash),
 			_ => return Ok(ValidTransaction::default()),
 		};
 
-		let (parachain, parachain_head_hash) =
+		let maybe_stored_best_head = crate::BestParaHeads::<T, I>::get(parachain);
-			parachains.get(0).expect("verified by match condition; qed");
+		Self::validate_updated_parachain_head(
-		let best_parachain_head = crate::BestParaHeads::<T, I>::get(parachain);
-
-		match best_parachain_head {
-			Some(best_parachain_head)
-				if best_parachain_head.at_relay_block_number >= bundled_relay_block_number =>
-			{
-				log::trace!(
-					target: crate::LOG_TARGET,
-					"Rejecting obsolete parachain-head {:?} transaction: \
-										bundled relay block number: {:?} \
-										best relay block number: {:?}",
 			parachain,
-					bundled_relay_block_number,
+			&maybe_stored_best_head,
-					best_parachain_head.at_relay_block_number,
+			updated_at_relay_block_number,
-				);
+			parachain_head_hash,
-				InvalidTransaction::Stale.into()
+			"Rejecting obsolete parachain-head transaction",
-			},
+		)
-			Some(best_parachain_head) if best_parachain_head.head_hash == *parachain_head_hash => {
-				log::trace!(
-					target: crate::LOG_TARGET,
-					"Rejecting obsolete parachain-head {:?} transaction: head hash {:?}",
-					parachain,
-					best_parachain_head.head_hash,
-				);
-				InvalidTransaction::Stale.into()
-			},
-			_ => Ok(ValidTransaction::default()),
-		}
 	}
 }
 

bridges/modules/parachains/src/lib.rs

@@ -373,6 +373,48 @@ pub mod pallet {
 			storage.read_and_decode_value(parachain_head_key.0.as_ref())
 		}
 
+		/// Check if para head has been already updated at better relay chain block.
+		/// Without this check, we may import heads in random order.
+		pub fn validate_updated_parachain_head(
+			parachain: ParaId,
+			maybe_stored_best_head: &Option<BestParaHead>,
+			updated_at_relay_block_number: RelayBlockNumber,
+			updated_head_hash: ParaHash,
+			err_log_prefix: &str,
+		) -> TransactionValidity {
+			let stored_best_head = match maybe_stored_best_head {
+				Some(stored_best_head) => stored_best_head,
+				None => return Ok(ValidTransaction::default()),
+			};
+
+			if stored_best_head.at_relay_block_number >= updated_at_relay_block_number {
+				log::trace!(
+					target: LOG_TARGET,
+					"{}. The parachain head for {:?} was already updated at better relay chain block {} >= {}.",
+					err_log_prefix,
+					parachain,
+					stored_best_head.at_relay_block_number,
+					updated_at_relay_block_number
+				);
+				return InvalidTransaction::Stale.into()
+			}
+
+			if stored_best_head.head_hash == updated_head_hash {
+				log::trace!(
+					target: LOG_TARGET,
+					"{}. The parachain head hash for {:?} was already updated to {} at block {} < {}.",
+					err_log_prefix,
+					parachain,
+					updated_head_hash,
+					stored_best_head.at_relay_block_number,
+					updated_at_relay_block_number
+				);
+				return InvalidTransaction::Stale.into()
+			}
+
+			Ok(ValidTransaction::default())
+		}
+
 		/// Try to update parachain head.
 		pub(super) fn update_parachain_head(
 			parachain: ParaId,
@@ -383,41 +425,16 @@ pub mod pallet {
 		) -> Result<UpdateParachainHeadArtifacts, ()> {
 			// check if head has been already updated at better relay chain block. Without this
 			// check, we may import heads in random order
-			let next_imported_hash_position = match stored_best_head {
+			Self::validate_updated_parachain_head(
-				Some(stored_best_head)
-					if stored_best_head.at_relay_block_number <= updated_at_relay_block_number =>
-				{
-					// check if this head has already been imported before
-					if updated_head_hash == stored_best_head.head_hash {
-						log::trace!(
-							target: LOG_TARGET,
-							"The head of parachain {:?} can't be updated to {}, because it has been already updated \
-							to the same value at previous relay chain block: {} < {}",
 				parachain,
-							updated_head_hash,
+				&stored_best_head,
-							stored_best_head.at_relay_block_number,
 				updated_at_relay_block_number,
-						);
-						return Err(())
-					}
-
-					stored_best_head.next_imported_hash_position
-				},
-				None => 0,
-				Some(stored_best_head) => {
-					log::trace!(
-						target: LOG_TARGET,
-						"The head of parachain {:?} can't be updated to {}, because it has been already updated \
-						to {} at better relay chain block: {} > {}",
-						parachain,
 				updated_head_hash,
-						stored_best_head.head_hash,
+				"The parachain head can't be updated",
-						stored_best_head.at_relay_block_number,
+			)
-						updated_at_relay_block_number,
+			.map_err(|_| ())?;
-					);
+			let next_imported_hash_position = stored_best_head
-					return Err(())
+				.map_or(0, |stored_best_head| stored_best_head.next_imported_hash_position);
-				},
-			};
 
 			// insert updated best parachain head
 			let head_hash_to_prune =

bridges/relays/lib-substrate-relay/src/parachains/source.rs

@@ -161,11 +161,11 @@ where
 		at_block: HeaderIdOf<P::SourceRelayChain>,
 		parachains: &[ParaId],
 	) -> Result<(ParaHeadsProof, Vec<ParaHash>), Self::Error> {
-		if parachains.len() != 1 || parachains[0].0 != P::SOURCE_PARACHAIN_PARA_ID {
+		let parachain = ParaId(P::SOURCE_PARACHAIN_PARA_ID);
+		if parachains != [parachain] {
 			return Err(SubstrateError::Custom(format!(
 				"Trying to prove unexpected parachains {:?}. Expected {:?}",
-				parachains,
+				parachains, parachain,
-				P::SOURCE_PARACHAIN_PARA_ID,
 			)))
 		}