cherry pick #137 (#140)

* cherry pick #137

* pick #141

Co-authored-by: bear <[email protected]>

Author: 2 people

bin/runtime-common/src/messages.rs

@@ -523,7 +523,7 @@ pub mod target {
 	///
 	/// Our Call is opaque (`Vec<u8>`) for Bridged chain. So it is encoded, prefixed with
 	/// vector length. Custom decode implementation here is exactly to deal with this.
-	#[derive(Decode, Encode, RuntimeDebug, PartialEq)]
+	#[derive(Decode, Encode, Clone, RuntimeDebug, PartialEq)]
 	pub struct FromBridgedChainEncodedMessageCall<DecodedCall> {
 		encoded_call: Vec<u8>,
 		_marker: PhantomData<DecodedCall>,
@@ -573,6 +573,16 @@ pub mod target {
 			message.data.payload.as_ref().map(|payload| payload.weight).unwrap_or(0)
 		}
 
+		fn pre_dispatch(
+			relayer_account: &AccountIdOf<ThisChain<B>>,
+			message: &DispatchMessage<Self::DispatchPayload, BalanceOf<BridgedChain<B>>>,
+		) -> Result<(), &'static str> {
+			pallet_bridge_dispatch::Pallet::<ThisRuntime, ThisDispatchInstance>::pre_dispatch(
+				relayer_account,
+				message.data.payload.as_ref().map_err(drop),
+			)
+		}
+
 		fn dispatch(
 			relayer_account: &AccountIdOf<ThisChain<B>>,
 			message: DispatchMessage<Self::DispatchPayload, BalanceOf<BridgedChain<B>>>,

modules/dispatch/src/lib.rs

@@ -92,7 +92,7 @@ pub mod pallet {
 		/// that all other stuff (like `spec_version`) is ok. If we would try to decode
 		/// `Call` which has been encoded using previous `spec_version`, then we might end
 		/// up with decoding error, instead of `MessageVersionSpecMismatch`.
-		type EncodedCall: Decode + Encode + Into<Result<<Self as Config<I>>::Call, ()>>;
+		type EncodedCall: Decode + Encode + Into<Result<<Self as Config<I>>::Call, ()>> + Clone;
 		/// A type which can be turned into an AccountId from a 256-bit hash.
 		///
 		/// Used when deriving target chain AccountIds from source chain AccountIds.
@@ -160,6 +160,16 @@ impl<T: Config<I>, I: 'static> MessageDispatch<T::AccountId, T::BridgeMessageId>
 		message.weight
 	}
 
+	fn pre_dispatch(
+		relayer_account: &T::AccountId,
+		message: Result<&Self::Message, ()>,
+	) -> Result<(), &'static str> {
+		let raw_message = message.map_err(|_| "Invalid Message")?;
+		let call = raw_message.clone().call.into().map_err(|_| "Invalid Call")?;
+
+		T::CallValidator::check_receiving_before_dispatch(relayer_account, &call)
+	}
+
 	fn dispatch<P: FnOnce(&T::AccountId, bp_message_dispatch::Weight) -> Result<(), ()>>(
 		source_chain: ChainId,
 		target_chain: ChainId,
@@ -275,8 +285,8 @@ impl<T: Config<I>, I: 'static> MessageDispatch<T::AccountId, T::BridgeMessageId>
 		let dispatch_origin =
 			T::IntoDispatchOrigin::into_dispatch_origin(&origin_derived_account, &call);
 
-		// filter the call
-		if let Err(_) = T::CallValidator::pre_dispatch(relayer_account, &dispatch_origin, &call) {
+		// validate the call
+		if let Err(_e) = T::CallValidator::call_validate(relayer_account, &dispatch_origin, &call) {
 			log::trace!(
 				target: "runtime::bridge-dispatch",
 				"Message {:?}/{:?}: the call ({:?}) is rejected by filter",
@@ -560,7 +570,7 @@ mod tests {
 		type TargetChainSignature = TestSignature;
 	}
 
-	#[derive(Decode, Encode)]
+	#[derive(Decode, Encode, Clone)]
 	pub struct EncodedCall(Vec<u8>);
 
 	impl From<EncodedCall> for Result<Call, ()> {
@@ -571,7 +581,14 @@ mod tests {
 
 	pub struct CallValidator;
 	impl CallValidate<AccountId, Origin, Call> for CallValidator {
-		fn pre_dispatch(
+		fn check_receiving_before_dispatch(
+			_relayer_account: &AccountId,
+			_call: &Call,
+		) -> Result<(), &'static str> {
+			Ok(())
+		}
+
+		fn call_validate(
 			_relayer_account: &AccountId,
 			_origin: &Origin,
 			call: &Call,

modules/fee-market/src/tests.rs

@@ -344,6 +344,13 @@ impl MessageDispatch<AccountId, TestMessageFee> for TestMessageDispatch {
 		}
 	}
 
+	fn pre_dispatch(
+		_relayer_account: &AccountId,
+		_message: &DispatchMessage<TestPayload, TestMessageFee>,
+	) -> Result<(), &'static str> {
+		Ok(())
+	}
+
 	fn dispatch(
 		_relayer_account: &AccountId,
 		message: DispatchMessage<TestPayload, TestMessageFee>,

modules/messages/src/inbound_lane.rs

@@ -58,6 +58,8 @@ pub enum ReceivalResult {
 	TooManyUnrewardedRelayers,
 	/// There are too many unconfirmed messages at the lane.
 	TooManyUnconfirmedMessages,
+	/// Pre-dispatch validation failed before message dispatch.
+	PreDispatchValidateFailed,
 }
 
 /// Inbound messages lane.
@@ -141,14 +143,17 @@ impl<S: InboundLaneStorage> InboundLane<S> {
 			return ReceivalResult::TooManyUnconfirmedMessages;
 		}
 
+		let dispatch_message = DispatchMessage {
+			key: MessageKey { lane_id: self.storage.id(), nonce },
+			data: message_data,
+		};
+		// if there are some extra pre-dispatch validation errors, reject this message.
+		if P::pre_dispatch(relayer_at_this_chain, &dispatch_message).is_err() {
+			return ReceivalResult::PreDispatchValidateFailed
+		}
+
 		// then, dispatch message
-		let dispatch_result = P::dispatch(
-			relayer_at_this_chain,
-			DispatchMessage {
-				key: MessageKey { lane_id: self.storage.id(), nonce },
-				data: message_data,
-			},
-		);
+		let dispatch_result = P::dispatch(relayer_at_this_chain, dispatch_message);
 
 		// now let's update inbound lane storage
 		let push_new = match data.relayers.back_mut() {

modules/messages/src/lib.rs

@@ -466,9 +466,10 @@ pub mod pallet {
 								!dispatch_result.dispatch_fee_paid_during_dispatch,
 							)
 						},
-						ReceivalResult::InvalidNonce
-						| ReceivalResult::TooManyUnrewardedRelayers
-						| ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true),
+						ReceivalResult::InvalidNonce |
+						ReceivalResult::TooManyUnrewardedRelayers |
+						ReceivalResult::PreDispatchValidateFailed |
+						ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true),
 					};
 
 					let unspent_weight = sp_std::cmp::min(unspent_weight, dispatch_weight);

modules/messages/src/mock.rs

@@ -489,6 +489,13 @@ impl MessageDispatch<AccountId, TestMessageFee> for TestMessageDispatch {
 		}
 	}
 
+	fn pre_dispatch(
+		_relayer_account: &AccountId,
+		_message: &DispatchMessage<TestPayload, TestMessageFee>,
+	) -> Result<(), &'static str> {
+		Ok(())
+	}
+
 	fn dispatch(
 		_relayer_account: &AccountId,
 		message: DispatchMessage<TestPayload, TestMessageFee>,

primitives/message-dispatch/src/lib.rs

@@ -46,6 +46,15 @@ pub trait MessageDispatch<AccountId, BridgeMessageId> {
 	/// of dispatch weight.
 	fn dispatch_weight(message: &Self::Message) -> Weight;
 
+	/// Checking in message receiving step before dispatch
+	///
+	/// This will be called before the call enter dispatch phase. If failed, the message(call) will
+	/// be not be processed by this relayer, latter relayers can still continue process it.
+	fn pre_dispatch(
+		relayer_account: &AccountId,
+		message: Result<&Self::Message, ()>,
+	) -> Result<(), &'static str>;
+
 	/// Dispatches the message internally.
 	///
 	/// `source_chain` indicates the chain where the message came from.
@@ -154,8 +163,15 @@ pub trait IntoDispatchOrigin<AccountId, Call, Origin> {
 
 /// A generic trait to validate message before dispatch.
 pub trait CallValidate<AccountId, Origin, Call> {
-	/// call validation
-	fn pre_dispatch(
+	/// Checking in message receiving step before dispatch
+	///
+	/// This will be called before the call enter dispatch phase. If failed, the message(call) will
+	/// be not be processed by this relayer, latter relayers can still continue process it.
+	fn check_receiving_before_dispatch(relayer_account: &AccountId, call: &Call) -> Result<(), &'static str>;
+	/// In-dispatch call validation
+	///
+	/// This will be called in the dispatch process, If failed, return message dispatch errors.
+	fn call_validate(
 		relayer_account: &AccountId,
 		origin: &Origin,
 		call: &Call,

primitives/messages/src/target_chain.rs

@@ -97,6 +97,15 @@ pub trait MessageDispatch<AccountId, Fee> {
 	/// of dispatch weight.
 	fn dispatch_weight(message: &DispatchMessage<Self::DispatchPayload, Fee>) -> Weight;
 
+	/// Checking in message receiving step before dispatch
+	///
+	/// This will be called before the call enter dispatch phase. If failed, the message(call) will
+	/// be not be processed by this relayer, latter relayers can still continue process it.
+	fn pre_dispatch(
+		relayer_account: &AccountId,
+		message: &DispatchMessage<Self::DispatchPayload, Fee>,
+	) -> Result<(), &'static str>;
+
 	/// Called when inbound message is received.
 	///
 	/// It is up to the implementers of this trait to determine whether the message
@@ -160,6 +169,13 @@ impl<AccountId, Fee> MessageDispatch<AccountId, Fee> for ForbidInboundMessages {
 		Weight::MAX
 	}
 
+	fn pre_dispatch(
+		_: &AccountId,
+		_message: &DispatchMessage<Self::DispatchPayload, Fee>,
+	) -> Result<(), &'static str> {
+		Ok(())
+	}
+
 	fn dispatch(
 		_: &AccountId,
 		_: DispatchMessage<Self::DispatchPayload, Fee>,