[stable2506] Backport #8860 relay chain logic (runtime + client)

polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs

@@ -274,7 +274,7 @@ impl Constraints {
 	) -> Result<(), ModificationError> {
 		if let Some(HrmpWatermarkUpdate::Trunk(hrmp_watermark)) = modifications.hrmp_watermark {
 			// head updates are always valid.
-			if self.hrmp_inbound.valid_watermarks.iter().all(|w| w != &hrmp_watermark) {
+			if !self.hrmp_inbound.valid_watermarks.contains(&hrmp_watermark) {
 				return Err(ModificationError::DisallowedHrmpWatermark(hrmp_watermark))
 			}
 		}
@@ -347,7 +347,7 @@ impl Constraints {
 			match new.hrmp_inbound.valid_watermarks.binary_search(&hrmp_watermark.watermark()) {
 				Ok(pos) => {
 					// Exact match, so this is OK in all cases.
-					let _ = new.hrmp_inbound.valid_watermarks.drain(..pos + 1);
+					let _ = new.hrmp_inbound.valid_watermarks.drain(..pos);
 				},
 				Err(pos) => match hrmp_watermark {
 					HrmpWatermarkUpdate::Head(_) => {
@@ -1038,30 +1038,44 @@ mod tests {
 	}
 
 	#[test]
-	fn constraints_disallowed_trunk_watermark() {
+	fn constraints_check_trunk_watermark() {
 		let constraints = make_constraints();
 		let mut modifications = ConstraintModifications::identity();
-		modifications.hrmp_watermark = Some(HrmpWatermarkUpdate::Trunk(7));
 
+		// The current hrmp watermark is kept
+		modifications.hrmp_watermark = Some(HrmpWatermarkUpdate::Trunk(6));
+		assert!(constraints.check_modifications(&modifications).is_ok());
+		let new_constraints = constraints.apply_modifications(&modifications).unwrap();
+		assert_eq!(new_constraints.hrmp_inbound.valid_watermarks, vec![6, 8]);
+
+		modifications.hrmp_watermark = Some(HrmpWatermarkUpdate::Trunk(7));
 		assert_eq!(
 			constraints.check_modifications(&modifications),
 			Err(ModificationError::DisallowedHrmpWatermark(7)),
 		);
-
 		assert_eq!(
 			constraints.apply_modifications(&modifications),
 			Err(ModificationError::DisallowedHrmpWatermark(7)),
 		);
+
+		modifications.hrmp_watermark = Some(HrmpWatermarkUpdate::Trunk(8));
+		assert!(constraints.check_modifications(&modifications).is_ok());
+		let new_constraints = constraints.apply_modifications(&modifications).unwrap();
+		assert_eq!(new_constraints.hrmp_inbound.valid_watermarks, vec![8]);
 	}
 
 	#[test]
-	fn constraints_always_allow_head_watermark() {
+	fn constraints_check_head_watermark() {
 		let constraints = make_constraints();
 		let mut modifications = ConstraintModifications::identity();
-		modifications.hrmp_watermark = Some(HrmpWatermarkUpdate::Head(7));
 
+		modifications.hrmp_watermark = Some(HrmpWatermarkUpdate::Head(5));
 		assert!(constraints.check_modifications(&modifications).is_ok());
+		let new_constraints = constraints.apply_modifications(&modifications).unwrap();
+		assert_eq!(new_constraints.hrmp_inbound.valid_watermarks, vec![6, 8]);
 
+		modifications.hrmp_watermark = Some(HrmpWatermarkUpdate::Head(7));
+		assert!(constraints.check_modifications(&modifications).is_ok());
 		let new_constraints = constraints.apply_modifications(&modifications).unwrap();
 		assert_eq!(new_constraints.hrmp_inbound.valid_watermarks, vec![8]);
 	}

polkadot/runtime/parachains/src/hrmp.rs

@@ -1189,12 +1189,16 @@ impl<T: Config> Pallet<T> {
 		}
 
 		if let Some(last_watermark) = HrmpWatermarks::<T>::get(&recipient) {
-			if new_hrmp_watermark <= last_watermark {
+			if new_hrmp_watermark < last_watermark {
 				return Err(HrmpWatermarkAcceptanceErr::AdvancementRule {
 					new_watermark: new_hrmp_watermark,
 					last_watermark,
 				})
 			}
+
+			if new_hrmp_watermark == last_watermark {
+				return Ok(())
+			}
 		}
 
 		// Second, check where the watermark CAN land. It's one of the following:
@@ -1215,10 +1219,19 @@ impl<T: Config> Pallet<T> {
 
 	/// Returns HRMP watermarks of previously sent messages to a given para.
 	pub(crate) fn valid_watermarks(recipient: ParaId) -> Vec<BlockNumberFor<T>> {
-		HrmpChannelDigests::<T>::get(&recipient)
+		let mut valid_watermarks: Vec<_> = HrmpChannelDigests::<T>::get(&recipient)
 			.into_iter()
 			.map(|(block_no, _)| block_no)
-			.collect()
+			.collect();
+
+		// The current watermark will remain valid until updated.
+		if let Some(last_watermark) = HrmpWatermarks::<T>::get(&recipient) {
+			if valid_watermarks.first().map_or(false, |w| w > &last_watermark) {
+				valid_watermarks.insert(0, last_watermark);
+			}
+		}
+
+		valid_watermarks
 	}
 
 	pub(crate) fn check_outbound_hrmp(

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

@@ -892,6 +892,86 @@ fn cancel_pending_open_channel_request() {
 	});
 }
 
+#[test]
+fn watermark_can_remain_the_same() {
+	let para_a = 2032.into();
+	let para_b = 2064.into();
+	let para_c = 3000.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);
+		register_parachain(para_c);
+
+		run_to_block(3, Some(vec![2, 3]));
+		Hrmp::init_open_channel(para_a, para_b, 2, 20).unwrap();
+		Hrmp::accept_open_channel(para_b, para_a).unwrap();
+		Hrmp::init_open_channel(para_c, para_b, 2, 20).unwrap();
+		Hrmp::accept_open_channel(para_b, para_c).unwrap();
+
+		// Update watermark
+		assert!(Hrmp::check_hrmp_watermark(para_b, 3, 3).is_ok());
+		let _ = Hrmp::prune_hrmp(para_b, 3);
+
+		// On Block 6:
+		// A sends 1 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"HRMP message from A".to_vec() }]
+				.try_into()
+				.unwrap();
+		let config = configuration::ActiveConfig::<Test>::get();
+		assert!(Hrmp::check_outbound_hrmp(&config, para_a, &msgs).is_ok());
+		let _ = Hrmp::queue_outbound_hrmp(para_a, msgs);
+		Hrmp::assert_storage_consistency_exhaustive();
+		// C sends 1 message to B
+		assert!(channel_exists(para_c, para_b));
+		let msgs: HorizontalMessages =
+			vec![OutboundHrmpMessage { recipient: para_b, data: b"HRMP message from C".to_vec() }]
+				.try_into()
+				.unwrap();
+		let config = configuration::ActiveConfig::<Test>::get();
+		assert!(Hrmp::check_outbound_hrmp(&config, para_c, &msgs).is_ok());
+		let _ = Hrmp::queue_outbound_hrmp(para_c, msgs);
+		Hrmp::assert_storage_consistency_exhaustive();
+
+		// Check that a smaller HRMP watermark is not accepted
+		assert!(matches!(
+			Hrmp::check_hrmp_watermark(para_b, 6, 2),
+			Err(HrmpWatermarkAcceptanceErr::AdvancementRule {
+				new_watermark: 2,
+				last_watermark: 3
+			})
+		));
+
+		// Check that an HRMP watermark representing a relay chain block that doesn't contain
+		// any message is not accepted
+		assert!(matches!(
+			Hrmp::check_hrmp_watermark(para_b, 6, 5),
+			Err(HrmpWatermarkAcceptanceErr::LandsOnBlockWithNoMessages { new_watermark: 5 })
+		));
+
+		// On block 7:
+		// B receives the messages, but can process only the one sent by A.
+		// B keeps the old watermark.
+		run_to_block(7, None);
+		assert!(Hrmp::check_hrmp_watermark(para_b, 7, 3).is_ok());
+		let _ = Hrmp::prune_hrmp(para_b, 5);
+		Hrmp::assert_storage_consistency_exhaustive();
+
+		// On block 8:
+		// B can also process the message sent by C. B sets the watermark to 6.
+		run_to_block(8, None);
+		assert!(Hrmp::check_hrmp_watermark(para_b, 7, 6).is_ok());
+		let _ = Hrmp::prune_hrmp(para_b, 6);
+		Hrmp::assert_storage_consistency_exhaustive();
+	});
+}
+
 #[test]
 fn watermark_maxed_out_at_relay_parent() {
 	let para_a = 2032.into();
@@ -921,6 +1001,15 @@ fn watermark_maxed_out_at_relay_parent() {
 		let _ = Hrmp::queue_outbound_hrmp(para_a, msgs);
 		Hrmp::assert_storage_consistency_exhaustive();
 
+		// Check that an HRMP watermark greater than the relay parent is not accepted
+		assert!(matches!(
+			Hrmp::check_hrmp_watermark(para_b, 6, 7),
+			Err(HrmpWatermarkAcceptanceErr::AheadRelayParent {
+				new_watermark: 7,
+				relay_chain_parent_number: 6,
+			})
+		));
+
 		// On block 8:
 		// B receives the message sent by A. B sets the watermark to 7.
 		run_to_block(8, None);

prdoc/pr_8860.prdoc

@@ -0,0 +1,10 @@
+title: '[stable2506] Backport #8860 relay chain logic (runtime + client) '
+doc:
+- audience: Node Operator
+  description: |-
+    This PR relaxes the advancement rule for HRMP messages, making it possible to keep the old watermark.
+crates:
+- name: polkadot-node-subsystem-util
+  bump: patch
+- name: polkadot-runtime-parachains
+  bump: patch