diff --git a/runtime/parachains/src/hrmp.rs b/runtime/parachains/src/hrmp.rs index 034fac9ccf85..f33e753f62e5 100644 --- a/runtime/parachains/src/hrmp.rs +++ b/runtime/parachains/src/hrmp.rs @@ -889,19 +889,16 @@ impl Pallet { ) -> Result<(), HrmpWatermarkAcceptanceErr> { // 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 let Some(last_watermark) = HrmpWatermarks::::get(&recipient) { - if new_hrmp_watermark <= last_watermark { - return Err(HrmpWatermarkAcceptanceErr::AdvancementRule { - new_watermark: new_hrmp_watermark, - last_watermark, - }) - } + 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, @@ -909,24 +906,29 @@ impl Pallet { }) } - // 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::::get(&recipient); - if !digest - .binary_search_by_key(&new_hrmp_watermark, |(block_no, _)| *block_no) - .is_ok() - { - return Err(HrmpWatermarkAcceptanceErr::LandsOnBlockWithNoMessages { + if let Some(last_watermark) = HrmpWatermarks::::get(&recipient) { + if new_hrmp_watermark <= last_watermark { + return Err(HrmpWatermarkAcceptanceErr::AdvancementRule { new_watermark: new_hrmp_watermark, + last_watermark, }) } - Ok(()) } + + // Second, check where the watermark CAN land. It's one of the following: + // + // (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::::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( diff --git a/runtime/parachains/src/hrmp/tests.rs b/runtime/parachains/src/hrmp/tests.rs index c396260d2c30..7bffdc818f7c 100644 --- a/runtime/parachains/src/hrmp/tests.rs +++ b/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(); + }); +}