Check relayer balance before dispatch (#137)

* Add todo

* A first review

* Add `relayer_account_id` param

* Add clone trait bound

* Update returned type, add some comment

* Remove unwrap

* Rename things

* Rename error type

* Remove todo, it's time to test

* fix test (#139)

* update docs and partially distinguish naming between CallValidator and MessageDispatch

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

bin/runtime-common/src/messages.rs

@@ -601,6 +601,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

@@ -91,7 +91,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.
@@ -159,6 +159,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,7 +285,7 @@ impl<T: Config<I>, I: 'static> MessageDispatch<T::AccountId, T::BridgeMessageId>
 			T::IntoDispatchOrigin::into_dispatch_origin(&origin_derived_account, &call);
 
 		// validate the call
-		if let Err(e) = T::CallValidator::pre_dispatch(relayer_account, &dispatch_origin, &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 the validator",
@@ -546,7 +556,7 @@ mod tests {
 		type AccountIdConverter = AccountIdConverter;
 	}
 
-	#[derive(Decode, Encode)]
+	#[derive(Decode, Encode, Clone)]
 	pub struct EncodedCall(Vec<u8>);
 
 	impl From<EncodedCall> for Result<Call, ()> {
@@ -557,7 +567,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

@@ -341,6 +341,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

@@ -471,6 +471,7 @@ pub mod pallet {
 						},
 						ReceivalResult::InvalidNonce |
 						ReceivalResult::TooManyUnrewardedRelayers |
+						ReceivalResult::PreDispatchValidateFailed |
 						ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true),
 					};
 

modules/messages/src/mock.rs

@@ -482,6 +482,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>,