Do not expire HRMP open channel requests (#3543)

* Do not expire HRMP open channel requests

* Fix the build and update the docs

* Implement canceling requests and do not remove them automatically

* Fix a borked merge

* Fix fmt

* Please spellchecker

* Apply suggestions from code review

Co-authored-by: Amar Singh <asinghchrony@protonmail.com>

* Use `mutate_exists` for maintaining request counts

* Apply `rustfmt`

* Move newly introduced entrypoint to end to preserve ordering

Co-authored-by: Amar Singh <asinghchrony@protonmail.com>

node/service/src/chain_spec.rs

@@ -174,7 +174,7 @@ fn default_parachains_host_configuration(
 		ump_service_total_weight: 4 * 1_000_000_000,
 		max_upward_message_size: 1024 * 1024,
 		max_upward_message_num_per_candidate: 5,
-		hrmp_open_request_ttl: 5,
+		_hrmp_open_request_ttl: 5,
 		hrmp_sender_deposit: 0,
 		hrmp_recipient_deposit: 0,
 		hrmp_channel_max_capacity: 8,

parachain/src/primitives.rs

@@ -329,7 +329,8 @@ impl<T: Encode + Decode + Default> AccountIdConversion<T> for Id {
 /// unidirectional, meaning that `(A, B)` and `(B, A)` refer to different channels. The convention is
 /// that we use the first item tuple for the sender and the second for the recipient. Only one channel
 /// is allowed between two participants in one direction, i.e. there cannot be 2 different channels
-/// identified by `(A, B)`.
+/// identified by `(A, B)`. A channel with the same para id in sender and recipient is invalid. That
+/// is, however, not enforced.
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Encode, Decode, RuntimeDebug)]
 #[cfg_attr(feature = "std", derive(Hash))]
 pub struct HrmpChannelId {
@@ -339,6 +340,13 @@ pub struct HrmpChannelId {
 	pub recipient: Id,
 }
 
+impl HrmpChannelId {
+	/// Returns true if the given id corresponds to either the sender or the recipient.
+	pub fn is_participant(&self, id: Id) -> bool {
+		id == self.sender || id == self.recipient
+	}
+}
+
 /// A message from a parachain to its Relay Chain.
 pub type UpwardMessage = Vec<u8>;
 

roadmap/implementers-guide/src/runtime/hrmp.md

@@ -11,8 +11,6 @@ HRMP related structs:
 struct HrmpOpenChannelRequest {
     /// Indicates if this request was confirmed by the recipient.
     confirmed: bool,
-    /// How many session boundaries ago this request was seen.
-    age: SessionIndex,
     /// The amount that the sender supplied at the time of creation of this request.
     sender_deposit: Balance,
     /// The maximum message size that could be put into the channel.
@@ -158,8 +156,8 @@ Candidate Enactment:
         1. Decrement `C.msg_count`
         1. Decrement `C.total_size` by `M`'s payload size.
     1. Set `HrmpWatermarks` for `P` to be equal to `new_hrmp_watermark`
-    > NOTE: That collecting digests can be inefficient and the time it takes grows very fast. Thanks to the aggresive
-    > parametrization this shouldn't be a big of a deal.
+    > NOTE: That collecting digests can be inefficient and the time it takes grows very fast. Thanks to the aggressive
+    > parameterization this shouldn't be a big of a deal.
     > If that becomes a problem consider introducing an extra dictionary which says at what block the given sender
     > sent a message to the recipient.
 
@@ -212,6 +210,13 @@ the parachain executed the message.
         - The DM is sent using `queue_downward_message`.
         - The DM is represented by the `HrmpChannelAccepted` XCM message.
             - `recipient` is set to `origin`.
+* `hrmp_cancel_open_request(ch)`:
+    1. Check that `origin` is either `ch.sender` or `ch.recipient`
+    1. Check that the open channel request `ch` exists.
+    1. Check that the open channel request for `ch` is not confirmed.
+    1. Remove `ch` from `HrmpOpenChannelRequests` and `HrmpOpenChannelRequestsList`
+    1. Decrement `HrmpAcceptedChannelRequestCount` for `ch.recipient` by 1.
+    1. Unreserve the deposit of `ch.sender`.
 * `hrmp_close_channel(ch)`:
     1. Check that `origin` is either `ch.sender` or `ch.recipient`
     1. Check that `HrmpChannels` for `ch` exists.
@@ -233,15 +238,12 @@ the parachain executed the message.
     1. Remove all outbound channels of `P`, i.e. `(P, _)`,
     1. Remove `HrmpOpenChannelRequestCount` for `P`
     1. Remove `HrmpAcceptedChannelRequestCount` for `P`.
+    1. Remove `HrmpOpenChannelRequests` and `HrmpOpenChannelRequestsList` for `(P, _)` and `(_, P)`.
+        1. For each removed channel request `C`:
+            1. Unreserve the sender's deposit if the sender is not present in `outgoing_paras`
+            1. Unreserve the recipient's deposit if `C` is confirmed and the recipient is not present in `outgoing_paras`
 1. For each channel designator `D` in `HrmpOpenChannelRequestsList` we query the request `R` from `HrmpOpenChannelRequests`:
-    1. if `R.confirmed = false`:
-        1. increment `R.age` by 1.
-        1. if `R.age` reached a preconfigured time-to-live limit `config.hrmp_open_request_ttl`, then:
-            1. refund `R.sender_deposit` to the sender
-            1. decrement `HrmpOpenChannelRequestCount` for `D.sender` by 1.
-            1. remove `R`
-            1. remove `D`
-    2. if `R.confirmed = true`,
+    1. if `R.confirmed = true`,
         1. if both `D.sender` and `D.recipient` are not offboarded.
           1. create a new channel `C` between `(D.sender, D.recipient)`.
               1. Initialize the `C.sender_deposit` with `R.sender_deposit` and `C.recipient_deposit`

roadmap/implementers-guide/src/types/runtime.md

@@ -42,7 +42,7 @@ struct HostConfiguration {
 	pub dispute_period: SessionIndex,
 	/// How long after dispute conclusion to accept statements.
 	pub dispute_post_conclusion_acceptance_period: BlockNumber,
-	/// The maximum number of dispute spam slots 
+	/// The maximum number of dispute spam slots
 	pub dispute_max_spam_slots: u32,
 	/// How long it takes for a dispute to conclude by time-out, if no supermajority is reached.
 	pub dispute_conclusion_by_time_out_period: BlockNumber,
@@ -85,8 +85,6 @@ struct HostConfiguration {
 	/// decide to do with its PoV so this value in practice will be picked as a fraction of the PoV
 	/// size.
 	pub max_downward_message_size: u32,
-	/// Number of sessions after which an HRMP open channel request expires.
-	pub hrmp_open_request_ttl: u32,
 	/// The deposit that the sender should provide for opening an HRMP channel.
 	pub hrmp_sender_deposit: u32,
 	/// The deposit that the recipient should provide for accepting opening an HRMP channel.
@@ -119,7 +117,7 @@ struct HostConfiguration {
 Inherent data passed to a runtime entry-point for the advancement of parachain consensus.
 
 This contains 3 pieces of data:
-1. [`Bitfields`](availability.md#signed-availability-bitfield) 
+1. [`Bitfields`](availability.md#signed-availability-bitfield)
 2. [`BackedCandidates`](backing.md#backed-candidate)
 3. [`MultiDisputeStatementSet`](disputes.md#multidisputestatementset)
 

runtime/parachains/src/configuration.rs

@@ -91,8 +91,10 @@ pub struct HostConfiguration<BlockNumber> {
 	pub hrmp_max_parachain_outbound_channels: u32,
 	/// The maximum number of outbound HRMP channels a parathread is allowed to open.
 	pub hrmp_max_parathread_outbound_channels: u32,
-	/// Number of sessions after which an HRMP open channel request expires.
-	pub hrmp_open_request_ttl: u32,
+	/// NOTE: this field is deprecated. Channel open requests became non-expiring. Changing this value
+	/// doesn't have any effect. This field doesn't have a `deprecated` attribute because that would
+	/// trigger warnings coming from macros.
+	pub _hrmp_open_request_ttl: u32,
 	/// The deposit that the sender should provide for opening an HRMP channel.
 	pub hrmp_sender_deposit: Balance,
 	/// The deposit that the recipient should provide for accepting opening an HRMP channel.
@@ -202,7 +204,7 @@ impl<BlockNumber: Default + From<u32>> Default for HostConfiguration<BlockNumber
 			ump_service_total_weight: Default::default(),
 			max_upward_message_size: Default::default(),
 			max_upward_message_num_per_candidate: Default::default(),
-			hrmp_open_request_ttl: Default::default(),
+			_hrmp_open_request_ttl: Default::default(),
 			hrmp_sender_deposit: Default::default(),
 			hrmp_recipient_deposit: Default::default(),
 			hrmp_channel_max_capacity: Default::default(),
@@ -641,12 +643,10 @@ pub mod pallet {
 
 		/// Sets the number of sessions after which an HRMP open channel request expires.
 		#[pallet::weight((1_000, DispatchClass::Operational))]
-		pub fn set_hrmp_open_request_ttl(origin: OriginFor<T>, new: u32) -> DispatchResult {
-			ensure_root(origin)?;
-			Self::update_config_member(|config| {
-				sp_std::mem::replace(&mut config.hrmp_open_request_ttl, new) != new
-			});
-			Ok(())
+		// Deprecated, but is not marked as such, because that would trigger warnings coming from
+		// the macro.
+		pub fn set_hrmp_open_request_ttl(_origin: OriginFor<T>, _new: u32) -> DispatchResult {
+			Err("this doesn't have any effect".into())
 		}
 
 		/// Sets the amount of funds that the sender should provide for opening an HRMP channel.
@@ -888,7 +888,7 @@ mod tests {
 				ump_service_total_weight: 20000,
 				max_upward_message_size: 448,
 				max_upward_message_num_per_candidate: 5,
-				hrmp_open_request_ttl: 1312,
+				_hrmp_open_request_ttl: 0,
 				hrmp_sender_deposit: 22,
 				hrmp_recipient_deposit: 4905,
 				hrmp_channel_max_capacity: 3921,
@@ -1013,11 +1013,6 @@ mod tests {
 				new_config.max_upward_message_num_per_candidate,
 			)
 			.unwrap();
-			Configuration::set_hrmp_open_request_ttl(
-				Origin::root(),
-				new_config.hrmp_open_request_ttl,
-			)
-			.unwrap();
 			Configuration::set_hrmp_sender_deposit(Origin::root(), new_config.hrmp_sender_deposit)
 				.unwrap();
 			Configuration::set_hrmp_recipient_deposit(