[polkadot-staging] Separate send_message() validations (#2795)

* Fix xcm-bridge-hub build

* Move send_message definition to MessagesBridge impl

* Make send_message infallible

* Split `MessagesBridge` into 2 phases

Split `MessagesBridge` into 2 phases:
- message validation
- message sending

bridges/modules/messages/src/benchmarking.rs

@@ -31,7 +31,7 @@ use codec::Decode;
 use frame_benchmarking::{account, benchmarks_instance_pallet};
 use frame_support::weights::Weight;
 use frame_system::RawOrigin;
-use sp_runtime::traits::TrailingZeroInput;
+use sp_runtime::{traits::TrailingZeroInput, BoundedVec};
 use sp_std::{ops::RangeInclusive, prelude::*};
 
 const SEED: u32 = 0;
@@ -443,7 +443,7 @@ benchmarks_instance_pallet! {
 
 fn send_regular_message<T: Config<I>, I: 'static>() {
 	let mut outbound_lane = outbound_lane::<T, I>(T::bench_lane_id());
-	outbound_lane.send_message(vec![]).expect("We craft valid messages");
+	outbound_lane.send_message(BoundedVec::try_from(vec![]).expect("We craft valid messages"));
 }
 
 fn receive_messages<T: Config<I>, I: 'static>(nonce: MessageNonce) {

bridges/modules/messages/src/lib.rs

@@ -534,6 +534,7 @@ pub mod pallet {
 	}
 
 	#[pallet::error]
+	#[derive(PartialEq, Eq)]
 	pub enum Error<T, I = ()> {
 		/// Pallet is not in Normal operating mode.
 		NotOperatingNormally,
@@ -543,8 +544,6 @@ pub mod pallet {
 		MessageDispatchInactive,
 		/// Message has been treated as invalid by chain verifier.
 		MessageRejectedByChainVerifier(VerificationError),
-		/// Message has been treated as invalid by lane verifier.
-		MessageRejectedByLaneVerifier(VerificationError),
 		/// Message has been treated as invalid by the pallet logic.
 		MessageRejectedByPallet(VerificationError),
 		/// Submitter has failed to pay fee for delivering and dispatching messages.
@@ -690,53 +689,56 @@ pub mod pallet {
 	}
 }
 
+/// Structure, containing a validated message payload and all the info required
+/// to send it on the bridge.
+#[derive(Debug, PartialEq, Eq)]
+pub struct SendMessageArgs<T: Config<I>, I: 'static> {
+	lane_id: LaneId,
+	payload: StoredMessagePayload<T, I>,
+}
+
 impl<T, I> bp_messages::source_chain::MessagesBridge<T::OutboundPayload> for Pallet<T, I>
 where
 	T: Config<I>,
 	I: 'static,
 {
-	type Error = sp_runtime::DispatchErrorWithPostInfo<PostDispatchInfo>;
+	type Error = Error<T, I>;
+	type SendMessageArgs = SendMessageArgs<T, I>;
 
-	fn send_message(
+	fn validate_message(
 		lane: LaneId,
-		message: T::OutboundPayload,
+		message: &T::OutboundPayload,
-	) -> Result<SendMessageArtifacts, Self::Error> {
+	) -> Result<SendMessageArgs<T, I>, Self::Error> {
-		crate::send_message::<T, I>(lane, message)
-	}
-}
-
-/// Function that actually sends message.
-fn send_message<T: Config<I>, I: 'static>(
-	lane_id: LaneId,
-	payload: T::OutboundPayload,
-) -> sp_std::result::Result<
-	SendMessageArtifacts,
-	sp_runtime::DispatchErrorWithPostInfo<PostDispatchInfo>,
-> {
 		ensure_normal_operating_mode::<T, I>()?;
 
 		// let's check if outbound lane is active
-	ensure!(T::ActiveOutboundLanes::get().contains(&lane_id), Error::<T, I>::InactiveOutboundLane,);
+		ensure!(T::ActiveOutboundLanes::get().contains(&lane), Error::<T, I>::InactiveOutboundLane);
 
 		// let's first check if message can be delivered to target chain
-	T::TargetHeaderChain::verify_message(&payload).map_err(|err| {
+		T::TargetHeaderChain::verify_message(message).map_err(|err| {
 			log::trace!(
 				target: LOG_TARGET,
 				"Message to lane {:?} is rejected by target chain: {:?}",
-			lane_id,
+				lane,
 				err,
 			);
 
 			Error::<T, I>::MessageRejectedByChainVerifier(err)
 		})?;
 
-	// finally, save message in outbound storage and emit event
+		Ok(SendMessageArgs {
-	let mut lane = outbound_lane::<T, I>(lane_id);
+			lane_id: lane,
-	let encoded_payload = payload.encode();
+			payload: StoredMessagePayload::<T, I>::try_from(message.encode()).map_err(|_| {
-	let encoded_payload_len = encoded_payload.len();
+				Error::<T, I>::MessageRejectedByPallet(VerificationError::MessageTooLarge)
-	let nonce = lane
+			})?,
-		.send_message(encoded_payload)
+		})
-		.map_err(Error::<T, I>::MessageRejectedByPallet)?;
+	}
+
+	fn send_message(args: SendMessageArgs<T, I>) -> SendMessageArtifacts {
+		// save message in outbound storage and emit event
+		let mut lane = outbound_lane::<T, I>(args.lane_id);
+		let message_len = args.payload.len();
+		let nonce = lane.send_message(args.payload);
 
 		// return number of messages in the queue to let sender know about its state
 		let enqueued_messages = lane.data().queued_messages().saturating_len();
@@ -745,13 +747,14 @@ fn send_message<T: Config<I>, I: 'static>(
 			target: LOG_TARGET,
 			"Accepted message {} to lane {:?}. Message size: {:?}",
 			nonce,
-		lane_id,
+			args.lane_id,
-		encoded_payload_len,
+			message_len,
 		);
 
-	Pallet::<T, I>::deposit_event(Event::MessageAccepted { lane_id, nonce });
+		Pallet::<T, I>::deposit_event(Event::MessageAccepted { lane_id: args.lane_id, nonce });
 
-	Ok(SendMessageArtifacts { nonce, enqueued_messages })
+		SendMessageArtifacts { nonce, enqueued_messages }
+	}
 }
 
 /// Ensure that the pallet is in normal operational mode.
@@ -852,6 +855,8 @@ struct RuntimeOutboundLaneStorage<T, I = ()> {
 }
 
 impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorage<T, I> {
+	type StoredMessagePayload = StoredMessagePayload<T, I>;
+
 	fn id(&self) -> LaneId {
 		self.lane_id
 	}
@@ -865,22 +870,15 @@ impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag
 	}
 
 	#[cfg(test)]
-	fn message(&self, nonce: &MessageNonce) -> Option<MessagePayload> {
+	fn message(&self, nonce: &MessageNonce) -> Option<Self::StoredMessagePayload> {
 		OutboundMessages::<T, I>::get(MessageKey { lane_id: self.lane_id, nonce: *nonce })
-			.map(Into::into)
 	}
 
-	fn save_message(
+	fn save_message(&mut self, nonce: MessageNonce, message_payload: Self::StoredMessagePayload) {
-		&mut self,
-		nonce: MessageNonce,
-		message_payload: MessagePayload,
-	) -> Result<(), VerificationError> {
 		OutboundMessages::<T, I>::insert(
 			MessageKey { lane_id: self.lane_id, nonce },
-			StoredMessagePayload::<T, I>::try_from(message_payload)
+			message_payload,
-				.map_err(|_| VerificationError::MessageTooLarge)?,
 		);
-		Ok(())
 	}
 
 	fn remove_message(&mut self, nonce: &MessageNonce) {
@@ -927,7 +925,10 @@ mod tests {
 		},
 		outbound_lane::ReceivalConfirmationError,
 	};
-	use bp_messages::{BridgeMessagesCall, UnrewardedRelayer, UnrewardedRelayersState};
+	use bp_messages::{
+		source_chain::MessagesBridge, BridgeMessagesCall, UnrewardedRelayer,
+		UnrewardedRelayersState,
+	};
 	use bp_test_utils::generate_owned_bridge_module_tests;
 	use frame_support::{
 		assert_noop, assert_ok,
@@ -944,14 +945,15 @@ mod tests {
 		System::<TestRuntime>::reset_events();
 	}
 
-	fn send_regular_message() {
+	fn send_regular_message(lane_id: LaneId) {
 		get_ready_for_events();
 
-		let outbound_lane = outbound_lane::<TestRuntime, ()>(TEST_LANE_ID);
+		let outbound_lane = outbound_lane::<TestRuntime, ()>(lane_id);
 		let message_nonce = outbound_lane.data().latest_generated_nonce + 1;
 		let prev_enqueud_messages = outbound_lane.data().queued_messages().saturating_len();
-		let artifacts = send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD)
+		let valid_message = Pallet::<TestRuntime, ()>::validate_message(lane_id, &REGULAR_PAYLOAD)
-			.expect("send_message has failed");
+			.expect("validate_message has failed");
+		let artifacts = Pallet::<TestRuntime, ()>::send_message(valid_message);
 		assert_eq!(artifacts.enqueued_messages, prev_enqueud_messages + 1);
 
 		// check event with assigned nonce
@@ -960,7 +962,7 @@ mod tests {
 			vec![EventRecord {
 				phase: Phase::Initialization,
 				event: TestEvent::Messages(Event::MessageAccepted {
-					lane_id: TEST_LANE_ID,
+					lane_id,
 					nonce: message_nonce
 				}),
 				topics: vec![],
@@ -1011,14 +1013,14 @@ mod tests {
 	fn pallet_rejects_transactions_if_halted() {
 		run_test(|| {
 			// send message first to be able to check that delivery_proof fails later
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 
 			PalletOperatingMode::<TestRuntime, ()>::put(MessagesOperatingMode::Basic(
 				BasicOperatingMode::Halted,
 			));
 
 			assert_noop!(
-				send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,),
+				Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, &REGULAR_PAYLOAD),
 				Error::<TestRuntime, ()>::NotOperatingNormally,
 			);
 
@@ -1061,14 +1063,14 @@ mod tests {
 	fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() {
 		run_test(|| {
 			// send message first to be able to check that delivery_proof fails later
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 
 			PalletOperatingMode::<TestRuntime, ()>::put(
 				MessagesOperatingMode::RejectingOutboundMessages,
 			);
 
 			assert_noop!(
-				send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,),
+				Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, &REGULAR_PAYLOAD),
 				Error::<TestRuntime, ()>::NotOperatingNormally,
 			);
 
@@ -1104,7 +1106,7 @@ mod tests {
 	#[test]
 	fn send_message_works() {
 		run_test(|| {
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 		});
 	}
 
@@ -1118,7 +1120,7 @@ mod tests {
 				.extra
 				.extend_from_slice(&[0u8; MAX_OUTBOUND_PAYLOAD_SIZE as usize]);
 			assert_noop!(
-				send_message::<TestRuntime, ()>(TEST_LANE_ID, message_payload.clone(),),
+				Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, &message_payload.clone(),),
 				Error::<TestRuntime, ()>::MessageRejectedByPallet(
 					VerificationError::MessageTooLarge
 				),
@@ -1129,7 +1131,11 @@ mod tests {
 				message_payload.extra.pop();
 			}
 			assert_eq!(message_payload.encoded_size() as u32, MAX_OUTBOUND_PAYLOAD_SIZE);
-			assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID, message_payload,),);
+
+			let valid_message =
+				Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID, &message_payload)
+					.expect("validate_message has failed");
+			Pallet::<TestRuntime, ()>::send_message(valid_message);
 		})
 	}
 
@@ -1138,7 +1144,10 @@ mod tests {
 		run_test(|| {
 			// messages with this payload are rejected by target chain verifier
 			assert_noop!(
-				send_message::<TestRuntime, ()>(TEST_LANE_ID, PAYLOAD_REJECTED_BY_TARGET_CHAIN,),
+				Pallet::<TestRuntime, ()>::validate_message(
+					TEST_LANE_ID,
+					&PAYLOAD_REJECTED_BY_TARGET_CHAIN,
+				),
 				Error::<TestRuntime, ()>::MessageRejectedByChainVerifier(VerificationError::Other(
 					mock::TEST_ERROR
 				)),
@@ -1298,7 +1307,7 @@ mod tests {
 	#[test]
 	fn receive_messages_delivery_proof_works() {
 		run_test(|| {
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 			receive_messages_delivery_proof();
 
 			assert_eq!(
@@ -1311,8 +1320,8 @@ mod tests {
 	#[test]
 	fn receive_messages_delivery_proof_rewards_relayers() {
 		run_test(|| {
-			assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,));
+			send_regular_message(TEST_LANE_ID);
-			assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID, REGULAR_PAYLOAD,));
+			send_regular_message(TEST_LANE_ID);
 
 			// this reports delivery of message 1 => reward is paid to TEST_RELAYER_A
 			let single_message_delivery_proof = TestMessagesDeliveryProof(Ok((
@@ -1698,9 +1707,9 @@ mod tests {
 	#[test]
 	fn messages_delivered_callbacks_are_called() {
 		run_test(|| {
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 
 			// messages 1+2 are confirmed in 1 tx, message 3 in a separate tx
 			// dispatch of message 2 has failed
@@ -1759,7 +1768,7 @@ mod tests {
 	) {
 		run_test(|| {
 			// send message first to be able to check that delivery_proof fails later
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 
 			// 1) InboundLaneData declares that the `last_confirmed_nonce` is 1;
 			// 2) InboundLaneData has no entries => `InboundLaneData::last_delivered_nonce()`
@@ -1826,10 +1835,10 @@ mod tests {
 	#[test]
 	fn on_idle_callback_respects_remaining_weight() {
 		run_test(|| {
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 
 			assert_ok!(Pallet::<TestRuntime>::receive_messages_delivery_proof(
 				RuntimeOrigin::signed(1),
@@ -1908,10 +1917,10 @@ mod tests {
 	fn on_idle_callback_is_rotating_lanes_to_prune() {
 		run_test(|| {
 			// send + receive confirmation for lane 1
-			send_regular_message();
+			send_regular_message(TEST_LANE_ID);
 			receive_messages_delivery_proof();
 			// send + receive confirmation for lane 2
-			assert_ok!(send_message::<TestRuntime, ()>(TEST_LANE_ID_2, REGULAR_PAYLOAD,));
+			send_regular_message(TEST_LANE_ID_2);
 			assert_ok!(Pallet::<TestRuntime>::receive_messages_delivery_proof(
 				RuntimeOrigin::signed(1),
 				TestMessagesDeliveryProof(Ok((
@@ -1987,7 +1996,7 @@ mod tests {
 	fn outbound_message_from_unconfigured_lane_is_rejected() {
 		run_test(|| {
 			assert_noop!(
-				send_message::<TestRuntime, ()>(TEST_LANE_ID_3, REGULAR_PAYLOAD,),
+				Pallet::<TestRuntime, ()>::validate_message(TEST_LANE_ID_3, &REGULAR_PAYLOAD,),
 				Error::<TestRuntime, ()>::InactiveOutboundLane,
 			);
 		});

bridges/modules/messages/src/mock.rs

@@ -17,7 +17,7 @@
 // From construct_runtime macro
 #![allow(clippy::from_over_into)]
 
-use crate::Config;
+use crate::{Config, StoredMessagePayload};
 
 use bp_messages::{
 	calc_relayers_rewards,
@@ -26,7 +26,7 @@ use bp_messages::{
 		DeliveryPayments, DispatchMessage, DispatchMessageData, MessageDispatch,
 		ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
 	},
-	DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload,
+	DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce,
 	UnrewardedRelayer, UnrewardedRelayersState, VerificationError,
 };
 use bp_runtime::{messages::MessageDispatchResult, Size};
@@ -402,8 +402,8 @@ pub fn message(nonce: MessageNonce, payload: TestPayload) -> Message {
 }
 
 /// Return valid outbound message data, constructed from given payload.
-pub fn outbound_message_data(payload: TestPayload) -> MessagePayload {
+pub fn outbound_message_data(payload: TestPayload) -> StoredMessagePayload<TestRuntime, ()> {
-	payload.encode()
+	StoredMessagePayload::<TestRuntime, ()>::try_from(payload.encode()).expect("payload too large")
 }
 
 /// Return valid inbound (dispatch) message data, constructed from given payload.

bridges/modules/messages/src/outbound_lane.rs

@@ -18,10 +18,7 @@
 
 use crate::{Config, LOG_TARGET};
 
-use bp_messages::{
+use bp_messages::{DeliveredMessages, LaneId, MessageNonce, OutboundLaneData, UnrewardedRelayer};
-	DeliveredMessages, LaneId, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer,
-	VerificationError,
-};
 use codec::{Decode, Encode};
 use frame_support::{
 	weights::{RuntimeDbWeight, Weight},
@@ -34,6 +31,8 @@ use sp_std::collections::vec_deque::VecDeque;
 
 /// Outbound lane storage.
 pub trait OutboundLaneStorage {
+	type StoredMessagePayload;
+
 	/// Lane id.
 	fn id(&self) -> LaneId;
 	/// Get lane data from the storage.
@@ -42,13 +41,9 @@ pub trait OutboundLaneStorage {
 	fn set_data(&mut self, data: OutboundLaneData);
 	/// Returns saved outbound message payload.
 	#[cfg(test)]
-	fn message(&self, nonce: &MessageNonce) -> Option<MessagePayload>;
+	fn message(&self, nonce: &MessageNonce) -> Option<Self::StoredMessagePayload>;
 	/// Save outbound message in the storage.
-	fn save_message(
+	fn save_message(&mut self, nonce: MessageNonce, message_payload: Self::StoredMessagePayload);
-		&mut self,
-		nonce: MessageNonce,
-		message_payload: MessagePayload,
-	) -> Result<(), VerificationError>;
 	/// Remove outbound message from the storage.
 	fn remove_message(&mut self, nonce: &MessageNonce);
 }
@@ -91,18 +86,15 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
 	/// Send message over lane.
 	///
 	/// Returns new message nonce.
-	pub fn send_message(
+	pub fn send_message(&mut self, message_payload: S::StoredMessagePayload) -> MessageNonce {
-		&mut self,
-		message_payload: MessagePayload,
-	) -> Result<MessageNonce, VerificationError> {
 		let mut data = self.storage.data();
 		let nonce = data.latest_generated_nonce + 1;
 		data.latest_generated_nonce = nonce;
 
-		self.storage.save_message(nonce, message_payload)?;
+		self.storage.save_message(nonce, message_payload);
 		self.storage.set_data(data);
 
-		Ok(nonce)
+		nonce
 	}
 
 	/// Confirm messages delivery.
@@ -218,7 +210,7 @@ mod tests {
 		},
 		outbound_lane,
 	};
-	use frame_support::{assert_ok, weights::constants::RocksDbWeight};
+	use frame_support::weights::constants::RocksDbWeight;
 	use sp_std::ops::RangeInclusive;
 
 	fn unrewarded_relayers(
@@ -239,9 +231,9 @@ mod tests {
 	) -> Result<Option<DeliveredMessages>, ReceivalConfirmationError> {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 0);
 			let result = lane.confirm_delivery(3, latest_received_nonce, relayers);
@@ -256,7 +248,7 @@ mod tests {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
 			assert_eq!(lane.storage.data().latest_generated_nonce, 0);
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(1));
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
 			assert!(lane.storage.message(&1).is_some());
 			assert_eq!(lane.storage.data().latest_generated_nonce, 1);
 		});
@@ -266,9 +258,9 @@ mod tests {
 	fn confirm_delivery_works() {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(1));
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(2));
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 2);
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(3));
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 3);
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 0);
 			assert_eq!(
@@ -284,9 +276,9 @@ mod tests {
 	fn confirm_delivery_rejects_nonce_lesser_than_latest_received() {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 0);
 			assert_eq!(
@@ -368,9 +360,9 @@ mod tests {
 			);
 			assert_eq!(lane.storage.data().oldest_unpruned_nonce, 1);
 			// when nothing is confirmed, nothing is pruned
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
 			assert!(lane.storage.message(&1).is_some());
 			assert!(lane.storage.message(&2).is_some());
 			assert!(lane.storage.message(&3).is_some());
@@ -412,9 +404,9 @@ mod tests {
 	fn confirm_delivery_detects_when_more_than_expected_messages_are_confirmed() {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
-			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
+			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
 			assert_eq!(
 				lane.confirm_delivery(0, 3, &unrewarded_relayers(1..=3)),
 				Err(ReceivalConfirmationError::TryingToConfirmMoreMessagesThanExpected),

bridges/modules/xcm-bridge-hub/src/exporter.rs

@@ -42,12 +42,13 @@ type MessagesPallet<T, I> = BridgeMessagesPallet<T, <T as Config<I>>::BridgeMess
 
 impl<T: Config<I>, I: 'static> ExportXcm for Pallet<T, I>
 where
-	T: BridgeMessagesConfig<
+	T: BridgeMessagesConfig<T::BridgeMessagesPalletInstance, OutboundPayload = XcmAsPlainPayload>,
-		<T as Config<I>>::BridgeMessagesPalletInstance,
-		OutboundPayload = XcmAsPlainPayload,
-	>,
 {
-	type Ticket = (SenderAndLane, XcmAsPlainPayload, XcmHash);
+	type Ticket = (
+		SenderAndLane,
+		<MessagesPallet<T, I> as MessagesBridge<T::OutboundPayload>>::SendMessageArgs,
+		XcmHash,
+	);
 
 	fn validate(
 		network: NetworkId,
@@ -74,17 +75,25 @@ where
 			message,
 		)?;
 
-		Ok(((sender_and_lane, blob, id), price))
+		let bridge_message = MessagesPallet::<T, I>::validate_message(sender_and_lane.lane, &blob)
+			.map_err(|e| {
+				log::debug!(
+					target: LOG_TARGET,
+					"XCM message {:?} cannot be exported because of bridge error {:?} on bridge {:?}",
+					id,
+					e,
+					sender_and_lane.lane,
+				);
+				SendError::Transport("BridgeValidateError")
+			})?;
+
+		Ok(((sender_and_lane, bridge_message, id), price))
 	}
 
-	fn deliver(
+	fn deliver((sender_and_lane, bridge_message, id): Self::Ticket) -> Result<XcmHash, SendError> {
-		(sender_and_lane, blob, id): (SenderAndLane, XcmAsPlainPayload, XcmHash),
-	) -> Result<XcmHash, SendError> {
 		let lane_id = sender_and_lane.lane;
-		let send_result = MessagesPallet::<T, I>::send_message(lane_id, blob);
+		let artifacts = MessagesPallet::<T, I>::send_message(bridge_message);
 
-		match send_result {
-			Ok(artifacts) => {
 		log::info!(
 			target: LOG_TARGET,
 			"XCM message {:?} has been enqueued at bridge {:?} with nonce {}",
@@ -98,18 +107,6 @@ where
 			&sender_and_lane,
 			artifacts.enqueued_messages,
 		);
-			},
-			Err(error) => {
-				log::debug!(
-					target: LOG_TARGET,
-					"XCM message {:?} has been dropped because of bridge error {:?} on bridge {:?}",
-					id,
-					error,
-					lane_id,
-				);
-				return Err(SendError::Transport("BridgeSendError"))
-			},
-		}
 
 		Ok(id)
 	}

bridges/modules/xcm-bridge-hub/src/mock.rs

@@ -19,11 +19,10 @@
 use crate as pallet_xcm_bridge_hub;
 
 use bp_messages::{
-	source_chain::LaneMessageVerifier,
 	target_chain::{DispatchMessage, MessageDispatch},
-	LaneId, OutboundLaneData, VerificationError,
+	LaneId,
 };
-use bp_runtime::{messages::MessageDispatchResult, Chain, UnderlyingChainProvider};
+use bp_runtime::{messages::MessageDispatchResult, Chain, ChainId, UnderlyingChainProvider};
 use bridge_runtime_common::{
 	messages::{
 		source::TargetHeaderChainAdapter, target::SourceHeaderChainAdapter,
@@ -78,20 +77,6 @@ impl pallet_balances::Config for TestRuntime {
 	type AccountStore = System;
 }
 
-/// Lane message verifier that is used in tests.
-#[derive(Debug, Default)]
-pub struct TestLaneMessageVerifier;
-
-impl LaneMessageVerifier<Vec<u8>> for TestLaneMessageVerifier {
-	fn verify_message(
-		_lane: &LaneId,
-		_lane_outbound_data: &OutboundLaneData,
-		_payload: &Vec<u8>,
-	) -> Result<(), VerificationError> {
-		Ok(())
-	}
-}
-
 parameter_types! {
 	pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID];
 }
@@ -110,7 +95,6 @@ impl pallet_bridge_messages::Config for TestRuntime {
 	type InboundRelayer = ();
 	type DeliveryPayments = ();
 	type TargetHeaderChain = TargetHeaderChainAdapter<OnThisChainBridge>;
-	type LaneMessageVerifier = TestLaneMessageVerifier;
 	type DeliveryConfirmationPayments = ();
 	type OnMessagesDelivered = ();
 	type SourceHeaderChain = SourceHeaderChainAdapter<OnThisChainBridge>;
@@ -220,6 +204,7 @@ impl XcmBlobHauler for TestXcmBlobHauler {
 pub struct ThisChain;
 
 impl Chain for ThisChain {
+	const ID: ChainId = *b"tuch";
 	type BlockNumber = u64;
 	type Hash = H256;
 	type Hasher = BlakeTwo256;
@@ -243,6 +228,7 @@ pub type BridgedHeaderHash = H256;
 pub type BridgedChainHeader = SubstrateHeader;
 
 impl Chain for BridgedChain {
+	const ID: ChainId = *b"tuch";
 	type BlockNumber = u64;
 	type Hash = BridgedHeaderHash;
 	type Hasher = BlakeTwo256;

bridges/primitives/messages/src/source_chain.rs

@@ -125,22 +125,22 @@ pub trait MessagesBridge<Payload> {
 	/// Error type.
 	type Error: Debug;
 
+	/// Intermediary structure returned by `validate_message()`.
+	///
+	/// It can than be passed to `send_message()` in order to actually send the message
+	/// on the bridge.
+	type SendMessageArgs;
+
+	/// Check if the message can be sent over the bridge.
+	fn validate_message(
+		lane: LaneId,
+		message: &Payload,
+	) -> Result<Self::SendMessageArgs, Self::Error>;
+
 	/// Send message over the bridge.
 	///
 	/// Returns unique message nonce or error if send has failed.
-	fn send_message(lane: LaneId, message: Payload) -> Result<SendMessageArtifacts, Self::Error>;
+	fn send_message(message: Self::SendMessageArgs) -> SendMessageArtifacts;
-}
-
-/// Bridge that does nothing when message is being sent.
-#[derive(Eq, RuntimeDebug, PartialEq)]
-pub struct NoopMessagesBridge;
-
-impl<Payload> MessagesBridge<Payload> for NoopMessagesBridge {
-	type Error = &'static str;
-
-	fn send_message(_lane: LaneId, _message: Payload) -> Result<SendMessageArtifacts, Self::Error> {
-		Ok(SendMessageArtifacts { nonce: 0, enqueued_messages: 0 })
-	}
 }
 
 /// Structure that may be used in place of `TargetHeaderChain` and

bridges/primitives/runtime/src/lib.rs

@@ -313,7 +313,7 @@ pub trait StorageDoubleMapKeyProvider {
 }
 
 /// Error generated by the `OwnedBridgeModule` trait.
-#[derive(Encode, Decode, TypeInfo, PalletError)]
+#[derive(Encode, Decode, PartialEq, Eq, TypeInfo, PalletError)]
 pub enum OwnedBridgeModuleError {
 	/// All pallet operations are halted.
 	Halted,