limit number of pending messages at outbound lane (paritytech#715)

bridges/bin/millau/runtime/src/rialto_messages.rs

@@ -167,6 +167,16 @@ impl messages::ChainWithMessageLanes for Millau {
 	type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 }
 
+impl messages::ThisChainWithMessageLanes for Millau {
+	fn is_outbound_lane_enabled(lane: &LaneId) -> bool {
+		*lane == LaneId::default()
+	}
+
+	fn maximal_pending_messages_at_outbound_lane() -> MessageNonce {
+		MessageNonce::MAX
+	}
+}
+
 /// Rialto chain from message lane point of view.
 #[derive(RuntimeDebug, Clone, Copy)]
 pub struct Rialto;

bridges/bin/rialto/runtime/src/millau_messages.rs

@@ -167,6 +167,16 @@ impl messages::ChainWithMessageLanes for Rialto {
 	type MessageLaneInstance = crate::WithMillauMessageLaneInstance;
 }
 
+impl messages::ThisChainWithMessageLanes for Rialto {
+	fn is_outbound_lane_enabled(lane: &LaneId) -> bool {
+		*lane == LaneId::default()
+	}
+
+	fn maximal_pending_messages_at_outbound_lane() -> MessageNonce {
+		MessageNonce::MAX
+	}
+}
+
 /// Millau chain from message lane point of view.
 #[derive(RuntimeDebug, Clone, Copy)]
 pub struct Millau;

bridges/bin/runtime-common/src/messages.rs

@@ -44,7 +44,7 @@ pub trait MessageBridge {
 	const RELAYER_FEE_PERCENT: u32;
 
 	/// This chain in context of message bridge.
-	type ThisChain: ChainWithMessageLanes;
+	type ThisChain: ThisChainWithMessageLanes;
 	/// Bridged chain in context of message bridge.
 	type BridgedChain: ChainWithMessageLanes;
 
@@ -104,6 +104,17 @@ pub trait ChainWithMessageLanes {
 	type MessageLaneInstance: Instance;
 }
 
+/// This chain that has `message-lane` and `call-dispatch` modules.
+pub trait ThisChainWithMessageLanes: ChainWithMessageLanes {
+	/// Are we accepting any messages to the given lane?
+	fn is_outbound_lane_enabled(lane: &LaneId) -> bool;
+
+	/// Maximal number of pending (not yet delivered) messages at this chain.
+	///
+	/// Any messages over this limit, will be rejected.
+	fn maximal_pending_messages_at_outbound_lane() -> MessageNonce;
+}
+
 pub(crate) type ThisChain<B> = <B as MessageBridge>::ThisChain;
 pub(crate) type BridgedChain<B> = <B as MessageBridge>::BridgedChain;
 pub(crate) type HashOf<C> = <C as ChainWithMessageLanes>::Hash;
@@ -187,10 +198,24 @@ pub mod source {
 	/// 'Parsed' message delivery proof - inbound lane id and its state.
 	pub type ParsedMessagesDeliveryProofFromBridgedChain<B> = (LaneId, InboundLaneData<AccountIdOf<ThisChain<B>>>);
 
-	/// Message verifier that requires submitter to pay minimal delivery and dispatch fee.
+	/// Message verifier that is doing all basic checks.
+	///
+	/// This verifier assumes following:
+	///
+	/// - all message lanes are equivalent, so all checks are the same;
+	/// - messages are being dispatched using `pallet-bridge-call-dispatch` pallet on the target chain.
+	///
+	/// Following checks are made:
+	///
+	/// - message is rejected if its lane is currently blocked;
+	/// - message is rejected if there are too many pending (undelivered) messages at the outbound lane;
+	/// - check that the sender has rights to dispatch the call on target chain using provided dispatch origin;
+	/// - check that the sender has paid enough funds for both message delivery and dispatch.
 	#[derive(RuntimeDebug)]
 	pub struct FromThisChainMessageVerifier<B>(PhantomData<B>);
 
+	pub(crate) const OUTBOUND_LANE_DISABLED: &str = "The outbound message lane is disabled.";
+	pub(crate) const TOO_MANY_PENDING_MESSAGES: &str = "Too many pending messages at the lane.";
 	pub(crate) const BAD_ORIGIN: &str = "Unable to match the source origin to expected target origin.";
 	pub(crate) const TOO_LOW_FEE: &str = "Provided fee is below minimal threshold required by the lane.";
 
@@ -205,9 +230,24 @@ pub mod source {
 		fn verify_message(
 			submitter: &Sender<AccountIdOf<ThisChain<B>>>,
 			delivery_and_dispatch_fee: &BalanceOf<ThisChain<B>>,
-			_lane: &LaneId,
+			lane: &LaneId,
+			lane_outbound_data: &OutboundLaneData,
 			payload: &FromThisChainMessagePayload<B>,
 		) -> Result<(), Self::Error> {
+			// reject message if lane is blocked
+			if !ThisChain::<B>::is_outbound_lane_enabled(lane) {
+				return Err(OUTBOUND_LANE_DISABLED);
+			}
+
+			// reject message if there are too many pending messages at this lane
+			let max_pending_messages = ThisChain::<B>::maximal_pending_messages_at_outbound_lane();
+			let pending_messages = lane_outbound_data
+				.latest_generated_nonce
+				.saturating_sub(lane_outbound_data.latest_received_nonce);
+			if pending_messages > max_pending_messages {
+				return Err(TOO_MANY_PENDING_MESSAGES);
+			}
+
 			// Do the dispatch-specific check. We assume that the target chain uses
 			// `CallDispatch`, so we verify the message accordingly.
 			pallet_bridge_call_dispatch::verify_message_origin(submitter, payload).map_err(|_| BAD_ORIGIN)?;
@@ -812,6 +852,16 @@ mod tests {
 		type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 	}
 
+	impl ThisChainWithMessageLanes for ThisChain {
+		fn is_outbound_lane_enabled(lane: &LaneId) -> bool {
+			lane == TEST_LANE_ID
+		}
+
+		fn maximal_pending_messages_at_outbound_lane() -> MessageNonce {
+			MAXIMAL_PENDING_MESSAGES_AT_TEST_LANE
+		}
+	}
+
 	struct BridgedChain;
 
 	impl ChainWithMessageLanes for BridgedChain {
@@ -826,6 +876,20 @@ mod tests {
 		type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 	}
 
+	impl ThisChainWithMessageLanes for BridgedChain {
+		fn is_outbound_lane_enabled(_lane: &LaneId) -> bool {
+			unreachable!()
+		}
+
+		fn maximal_pending_messages_at_outbound_lane() -> MessageNonce {
+			unreachable!()
+		}
+	}
+
+	fn test_lane_outbound_data() -> OutboundLaneData {
+		OutboundLaneData::default()
+	}
+
 	#[test]
 	fn message_from_bridged_chain_is_decoded() {
 		// the message is encoded on the bridged chain
@@ -857,18 +921,23 @@ mod tests {
 	}
 
 	const TEST_LANE_ID: &LaneId = b"test";
+	const MAXIMAL_PENDING_MESSAGES_AT_TEST_LANE: MessageNonce = 32;
+
+	fn regular_outbound_message_payload() -> source::FromThisChainMessagePayload<OnThisChainBridge> {
+		source::FromThisChainMessagePayload::<OnThisChainBridge> {
+			spec_version: 1,
+			weight: 100,
+			origin: pallet_bridge_call_dispatch::CallOrigin::SourceRoot,
+			call: vec![42],
+		}
+	}
 
 	#[test]
 	fn message_fee_is_checked_by_verifier() {
 		const EXPECTED_MINIMAL_FEE: u32 = 5500;
 
 		// payload of the This -> Bridged chain message
-		let payload = source::FromThisChainMessagePayload::<OnThisChainBridge> {
-			spec_version: 1,
-			weight: 100,
-			origin: pallet_bridge_call_dispatch::CallOrigin::SourceRoot,
-			call: vec![42],
-		};
+		let payload = regular_outbound_message_payload();
 
 		// let's check if estimation matching hardcoded value
 		assert_eq!(
@@ -885,6 +954,7 @@ mod tests {
 				&Sender::Root,
 				&ThisChainBalance(1),
 				&TEST_LANE_ID,
+				&test_lane_outbound_data(),
 				&payload,
 			),
 			Err(source::TOO_LOW_FEE)
@@ -894,6 +964,7 @@ mod tests {
 				&Sender::Root,
 				&ThisChainBalance(1_000_000),
 				&TEST_LANE_ID,
+				&test_lane_outbound_data(),
 				&payload,
 			)
 			.is_ok(),
@@ -916,6 +987,7 @@ mod tests {
 				&Sender::Signed(ThisChainAccountId(0)),
 				&ThisChainBalance(1_000_000),
 				&TEST_LANE_ID,
+				&test_lane_outbound_data(),
 				&payload,
 			),
 			Err(source::BAD_ORIGIN)
@@ -925,6 +997,7 @@ mod tests {
 				&Sender::None,
 				&ThisChainBalance(1_000_000),
 				&TEST_LANE_ID,
+				&test_lane_outbound_data(),
 				&payload,
 			),
 			Err(source::BAD_ORIGIN)
@@ -934,6 +1007,7 @@ mod tests {
 				&Sender::Root,
 				&ThisChainBalance(1_000_000),
 				&TEST_LANE_ID,
+				&test_lane_outbound_data(),
 				&payload,
 			)
 			.is_ok(),
@@ -956,6 +1030,7 @@ mod tests {
 				&Sender::Signed(ThisChainAccountId(0)),
 				&ThisChainBalance(1_000_000),
 				&TEST_LANE_ID,
+				&test_lane_outbound_data(),
 				&payload,
 			),
 			Err(source::BAD_ORIGIN)
@@ -965,12 +1040,45 @@ mod tests {
 				&Sender::Signed(ThisChainAccountId(1)),
 				&ThisChainBalance(1_000_000),
 				&TEST_LANE_ID,
+				&test_lane_outbound_data(),
 				&payload,
 			)
 			.is_ok(),
 		);
 	}
 
+	#[test]
+	fn message_is_rejected_when_sent_using_disabled_lane() {
+		assert_eq!(
+			source::FromThisChainMessageVerifier::<OnThisChainBridge>::verify_message(
+				&Sender::Root,
+				&ThisChainBalance(1_000_000),
+				b"dsbl",
+				&test_lane_outbound_data(),
+				&regular_outbound_message_payload(),
+			),
+			Err(source::OUTBOUND_LANE_DISABLED)
+		);
+	}
+
+	#[test]
+	fn message_is_rejected_when_there_are_too_many_pending_messages_at_outbound_lane() {
+		assert_eq!(
+			source::FromThisChainMessageVerifier::<OnThisChainBridge>::verify_message(
+				&Sender::Root,
+				&ThisChainBalance(1_000_000),
+				&TEST_LANE_ID,
+				&OutboundLaneData {
+					latest_received_nonce: 100,
+					latest_generated_nonce: 100 + MAXIMAL_PENDING_MESSAGES_AT_TEST_LANE + 1,
+					..Default::default()
+				},
+				&regular_outbound_message_payload(),
+			),
+			Err(source::TOO_MANY_PENDING_MESSAGES)
+		);
+	}
+
 	#[test]
 	fn verify_chain_message_rejects_message_with_too_small_declared_weight() {
 		assert!(

bridges/modules/message-lane/src/lib.rs

@@ -293,10 +293,12 @@ decl_module! {
 				})?;
 
 			// now let's enforce any additional lane rules
+			let mut lane = outbound_lane::<T, I>(lane_id);
 			T::LaneMessageVerifier::verify_message(
 				&submitter,
 				&delivery_and_dispatch_fee,
 				&lane_id,
+				&lane.data(),
 				&payload,
 			).map_err(|err| {
 				frame_support::debug::trace!(
@@ -326,7 +328,6 @@ decl_module! {
 			})?;
 
 			// finally, save message in outbound storage and emit event
-			let mut lane = outbound_lane::<T, I>(lane_id);
 			let encoded_payload = payload.encode();
 			let encoded_payload_len = encoded_payload.len();
 			let nonce = lane.send_message(MessageData {

bridges/modules/message-lane/src/mock.rs

@@ -21,7 +21,7 @@ use bp_message_lane::{
 		LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, Sender, TargetHeaderChain,
 	},
 	target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain},
-	InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce,
+	InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData,
 };
 use bp_runtime::Size;
 use codec::{Decode, Encode};
@@ -253,6 +253,7 @@ impl LaneMessageVerifier<AccountId, TestPayload, TestMessageFee> for TestLaneMes
 		_submitter: &Sender<AccountId>,
 		delivery_and_dispatch_fee: &TestMessageFee,
 		_lane: &LaneId,
+		_lane_outbound_data: &OutboundLaneData,
 		_payload: &TestPayload,
 	) -> Result<(), Self::Error> {
 		if *delivery_and_dispatch_fee != 0 {

bridges/modules/message-lane/src/outbound_lane.rs

@@ -49,6 +49,11 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
 		OutboundLane { storage }
 	}
 
+	/// Get this lane data.
+	pub fn data(&self) -> OutboundLaneData {
+		self.storage.data()
+	}
+
 	/// Send message over lane.
 	///
 	/// Returns new message nonce.

bridges/primitives/message-lane/src/source_chain.rs

@@ -16,7 +16,7 @@
 
 //! Primitives of message lane module, that are used on the source chain.
 
-use crate::{InboundLaneData, LaneId, MessageNonce};
+use crate::{InboundLaneData, LaneId, MessageNonce, OutboundLaneData};
 
 use bp_runtime::Size;
 use frame_support::{Parameter, RuntimeDebug};
@@ -86,6 +86,7 @@ pub trait LaneMessageVerifier<Submitter, Payload, Fee> {
 		submitter: &Sender<Submitter>,
 		delivery_and_dispatch_fee: &Fee,
 		lane: &LaneId,
+		outbound_data: &OutboundLaneData,
 		payload: &Payload,
 	) -> Result<(), Self::Error>;
 }