Relax the watermark rule in the runtime (#7188)

* Relax the watermark rule in the runtime

* make comment more clear

* add hrmp test

* remove TODO now comment

polkadot/runtime/parachains/src/hrmp.rs

@@ -889,11 +889,23 @@ impl<T: Config> Pallet<T> {
 	) -> Result<(), HrmpWatermarkAcceptanceErr<T::BlockNumber>> {
 		// First, check where the watermark CANNOT legally land.
 		//
-		// (a) For ensuring that messages are eventually, a rule requires each parablock new
-		//     watermark should be greater than the last one.
+		// (a) For ensuring that messages are eventually processed, we require each parablock's
+		//     watermark to be greater than the last one. The exception to this is if the previous
+		//     watermark was already equal to the current relay-parent number.
 		//
 		// (b) However, a parachain cannot read into "the future", therefore the watermark should
 		//     not be greater than the relay-chain context block which the parablock refers to.
+		if new_hrmp_watermark == relay_chain_parent_number {
+			return Ok(())
+		}
+
+		if new_hrmp_watermark > relay_chain_parent_number {
+			return Err(HrmpWatermarkAcceptanceErr::AheadRelayParent {
+				new_watermark: new_hrmp_watermark,
+				relay_chain_parent_number,
+			})
+		}
+
 		if let Some(last_watermark) = HrmpWatermarks::<T>::get(&recipient) {
 			if new_hrmp_watermark <= last_watermark {
 				return Err(HrmpWatermarkAcceptanceErr::AdvancementRule {
@@ -902,31 +914,21 @@ impl<T: Config> Pallet<T> {
 				})
 			}
 		}
-		if new_hrmp_watermark > relay_chain_parent_number {
-			return Err(HrmpWatermarkAcceptanceErr::AheadRelayParent {
-				new_watermark: new_hrmp_watermark,
-				relay_chain_parent_number,
-			})
-		}
 
 		// Second, check where the watermark CAN land. It's one of the following:
 		//
-		// (a) The relay parent block number.
-		// (b) A relay-chain block in which this para received at least one message.
-		if new_hrmp_watermark == relay_chain_parent_number {
-			Ok(())
-		} else {
-			let digest = HrmpChannelDigests::<T>::get(&recipient);
-			if !digest
-				.binary_search_by_key(&new_hrmp_watermark, |(block_no, _)| *block_no)
-				.is_ok()
-			{
-				return Err(HrmpWatermarkAcceptanceErr::LandsOnBlockWithNoMessages {
-					new_watermark: new_hrmp_watermark,
-				})
-			}
-			Ok(())
+		// (a) The relay parent block number (checked above).
+		// (b) A relay-chain block in which this para received at least one message (checked here)
+		let digest = HrmpChannelDigests::<T>::get(&recipient);
+		if !digest
+			.binary_search_by_key(&new_hrmp_watermark, |(block_no, _)| *block_no)
+			.is_ok()
+		{
+			return Err(HrmpWatermarkAcceptanceErr::LandsOnBlockWithNoMessages {
+				new_watermark: new_hrmp_watermark,
+			})
 		}
+		Ok(())
 	}
 
 	pub(crate) fn check_outbound_hrmp(

polkadot/runtime/parachains/src/hrmp/tests.rs

@@ -660,3 +660,48 @@ fn cancel_pending_open_channel_request() {
 		Hrmp::assert_storage_consistency_exhaustive();
 	});
 }
+
+#[test]
+fn watermark_maxed_out_at_relay_parent() {
+	let para_a = 32.into();
+	let para_b = 64.into();
+
+	let mut genesis = GenesisConfigBuilder::default();
+	genesis.hrmp_channel_max_message_size = 20;
+	genesis.hrmp_channel_max_total_size = 20;
+	new_test_ext(genesis.build()).execute_with(|| {
+		register_parachain(para_a);
+		register_parachain(para_b);
+
+		run_to_block(5, Some(vec![4, 5]));
+		Hrmp::init_open_channel(para_a, para_b, 2, 20).unwrap();
+		Hrmp::accept_open_channel(para_b, para_a).unwrap();
+
+		// On Block 6:
+		// A sends a message to B
+		run_to_block(6, Some(vec![6]));
+		assert!(channel_exists(para_a, para_b));
+		let msgs: HorizontalMessages =
+			vec![OutboundHrmpMessage { recipient: para_b, data: b"this is an emergency".to_vec() }]
+				.try_into()
+				.unwrap();
+		let config = Configuration::config();
+		assert!(Hrmp::check_outbound_hrmp(&config, para_a, &msgs).is_ok());
+		let _ = Hrmp::queue_outbound_hrmp(para_a, msgs);
+		Hrmp::assert_storage_consistency_exhaustive();
+
+		// On block 8:
+		// B receives the message sent by A. B sets the watermark to 7.
+		run_to_block(8, None);
+		assert!(Hrmp::check_hrmp_watermark(para_b, 7, 7).is_ok());
+		let _ = Hrmp::prune_hrmp(para_b, 7);
+		Hrmp::assert_storage_consistency_exhaustive();
+
+		// On block 9:
+		// B includes a candidate with the same relay parent as before.
+		run_to_block(9, None);
+		assert!(Hrmp::check_hrmp_watermark(para_b, 7, 7).is_ok());
+		let _ = Hrmp::prune_hrmp(para_b, 7);
+		Hrmp::assert_storage_consistency_exhaustive();
+	});
+}