establish_channel_with_system (#3721)

Implements polkadot-fellows/RFCs#82
Allow any parachain to have bidirectional channel with any system
parachains

Looking for initial feedbacks before continue finish it

TODOs:
- [x] docs
- [x] benchmarks
- [x] tests

---------

Co-authored-by: joe petrowski <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>

polkadot/runtime/parachains/src/hrmp.rs

@@ -65,6 +65,7 @@ pub trait WeightInfo {
 	fn force_open_hrmp_channel(c: u32) -> Weight;
 	fn establish_system_channel() -> Weight;
 	fn poke_channel_deposits() -> Weight;
+	fn establish_channel_with_system() -> Weight;
 }
 
 /// A weight info that is only suitable for testing.
@@ -104,6 +105,9 @@ impl WeightInfo for TestWeightInfo {
 	fn poke_channel_deposits() -> Weight {
 		Weight::MAX
 	}
+	fn establish_channel_with_system() -> Weight {
+		Weight::MAX
+	}
 }
 
 /// A description of a request to open an HRMP channel.
@@ -270,6 +274,10 @@ pub mod pallet {
 		/// implementation should be the same as `Balance` as used in the `Configuration`.
 		type Currency: ReservableCurrency<Self::AccountId>;
 
+		/// The default channel size and capacity to use when opening a channel to a system
+		/// parachain.
+		type DefaultChannelSizeAndCapacityWithSystem: Get<(u32, u32)>;
+
 		/// Something that provides the weight of this pallet.
 		type WeightInfo: WeightInfo;
 	}
@@ -297,7 +305,7 @@ pub mod pallet {
 			proposed_max_capacity: u32,
 			proposed_max_message_size: u32,
 		},
-		/// An HRMP channel was opened between two system chains.
+		/// An HRMP channel was opened with a system chain.
 		HrmpSystemChannelOpened {
 			sender: ParaId,
 			recipient: ParaId,
@@ -836,6 +844,50 @@ pub mod pallet {
 
 			Ok(())
 		}
+
+		/// Establish a bidirectional HRMP channel between a parachain and a system chain.
+		///
+		/// Arguments:
+		///
+		/// - `target_system_chain`: A system chain, `ParaId`.
+		///
+		/// The origin needs to be the parachain origin.
+		#[pallet::call_index(10)]
+		#[pallet::weight(<T as Config>::WeightInfo::establish_channel_with_system())]
+		pub fn establish_channel_with_system(
+			origin: OriginFor<T>,
+			target_system_chain: ParaId,
+		) -> DispatchResultWithPostInfo {
+			let sender = ensure_parachain(<T as Config>::RuntimeOrigin::from(origin))?;
+
+			ensure!(target_system_chain.is_system(), Error::<T>::ChannelCreationNotAuthorized);
+
+			let (max_message_size, max_capacity) =
+				T::DefaultChannelSizeAndCapacityWithSystem::get();
+
+			// create bidirectional channel
+			Self::init_open_channel(sender, target_system_chain, max_capacity, max_message_size)?;
+			Self::accept_open_channel(target_system_chain, sender)?;
+
+			Self::init_open_channel(target_system_chain, sender, max_capacity, max_message_size)?;
+			Self::accept_open_channel(sender, target_system_chain)?;
+
+			Self::deposit_event(Event::HrmpSystemChannelOpened {
+				sender,
+				recipient: target_system_chain,
+				proposed_max_capacity: max_capacity,
+				proposed_max_message_size: max_message_size,
+			});
+
+			Self::deposit_event(Event::HrmpSystemChannelOpened {
+				sender: target_system_chain,
+				recipient: sender,
+				proposed_max_capacity: max_capacity,
+				proposed_max_message_size: max_message_size,
+			});
+
+			Ok(Pays::No.into())
+		}
 	}
 }
 

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

@@ -50,6 +50,13 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
 	assert_eq!(event, &system_event);
 }
 
+fn assert_has_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
+	let events = frame_system::Pallet::<T>::events();
+	let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();
+
+	assert!(events.iter().any(|record| record.event == system_event));
+}
+
 /// Enumerates the phase in the setup process of a channel between two parachains.
 enum ParachainSetupStep {
 	/// A channel open has been requested
@@ -513,6 +520,43 @@ mod benchmarks {
 		);
 	}
 
+	#[benchmark]
+	fn establish_channel_with_system() {
+		let sender_id = 1u32;
+		let recipient_id: ParaId = 2u32.into();
+
+		let sender_origin: crate::Origin = sender_id.into();
+
+		// make sure para is registered, and has zero balance.
+		register_parachain_with_balance::<T>(sender_id.into(), Zero::zero());
+		register_parachain_with_balance::<T>(recipient_id, Zero::zero());
+
+		#[extrinsic_call]
+		_(sender_origin, recipient_id);
+
+		let (max_message_size, max_capacity) = T::DefaultChannelSizeAndCapacityWithSystem::get();
+
+		assert_has_event::<T>(
+			Event::<T>::HrmpSystemChannelOpened {
+				sender: sender_id.into(),
+				recipient: recipient_id,
+				proposed_max_capacity: max_capacity,
+				proposed_max_message_size: max_message_size,
+			}
+			.into(),
+		);
+
+		assert_has_event::<T>(
+			Event::<T>::HrmpSystemChannelOpened {
+				sender: recipient_id,
+				recipient: sender_id.into(),
+				proposed_max_capacity: max_capacity,
+				proposed_max_message_size: max_message_size,
+			}
+			.into(),
+		);
+	}
+
 	impl_benchmark_test_suite!(
 		Hrmp,
 		crate::mock::new_test_ext(crate::hrmp::tests::GenesisConfigBuilder::default().build()),

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

@@ -24,7 +24,7 @@ use crate::mock::{
 	Configuration, Hrmp, MockGenesisConfig, Paras, ParasShared, RuntimeEvent as MockEvent,
 	RuntimeOrigin, System, Test,
 };
-use frame_support::{assert_noop, assert_ok};
+use frame_support::{assert_noop, assert_ok, error::BadOrigin};
 use primitives::BlockNumber;
 use std::collections::BTreeMap;
 
@@ -932,3 +932,72 @@ fn watermark_maxed_out_at_relay_parent() {
 		Hrmp::assert_storage_consistency_exhaustive();
 	});
 }
+
+#[test]
+fn establish_channel_with_system_works() {
+	let para_a = 2000.into();
+	let para_a_origin: crate::Origin = 2000.into();
+	let para_b = 3.into();
+
+	new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| {
+		// We need both A & B to be registered and live parachains.
+		register_parachain(para_a);
+		register_parachain(para_b);
+
+		run_to_block(5, Some(vec![4, 5]));
+		Hrmp::establish_channel_with_system(para_a_origin.into(), para_b).unwrap();
+		Hrmp::assert_storage_consistency_exhaustive();
+		assert!(System::events().iter().any(|record| record.event ==
+			MockEvent::Hrmp(Event::HrmpSystemChannelOpened {
+				sender: para_a,
+				recipient: para_b,
+				proposed_max_capacity: 1,
+				proposed_max_message_size: 4
+			})));
+
+		assert!(System::events().iter().any(|record| record.event ==
+			MockEvent::Hrmp(Event::HrmpSystemChannelOpened {
+				sender: para_b,
+				recipient: para_a,
+				proposed_max_capacity: 1,
+				proposed_max_message_size: 4
+			})));
+
+		// Advance to a block 6, but without session change. That means that the channel has
+		// not been created yet.
+		run_to_block(6, None);
+		assert!(!channel_exists(para_a, para_b));
+		assert!(!channel_exists(para_b, para_a));
+		Hrmp::assert_storage_consistency_exhaustive();
+
+		// Now let the session change happen and thus open the channel.
+		run_to_block(8, Some(vec![8]));
+		assert!(channel_exists(para_a, para_b));
+		assert!(channel_exists(para_b, para_a));
+		Hrmp::assert_storage_consistency_exhaustive();
+	});
+}
+
+#[test]
+fn establish_channel_with_system_with_invalid_args() {
+	let para_a = 2001.into();
+	let para_a_origin: crate::Origin = 2000.into();
+	let para_b = 2003.into();
+
+	new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| {
+		// We need both A & B to be registered and live parachains.
+		register_parachain(para_a);
+		register_parachain(para_b);
+
+		run_to_block(5, Some(vec![4, 5]));
+		assert_noop!(
+			Hrmp::establish_channel_with_system(RuntimeOrigin::signed(1), para_b),
+			BadOrigin
+		);
+		assert_noop!(
+			Hrmp::establish_channel_with_system(para_a_origin.into(), para_b),
+			Error::<Test>::ChannelCreationNotAuthorized
+		);
+		Hrmp::assert_storage_consistency_exhaustive();
+	});
+}

polkadot/runtime/parachains/src/mock.rs

@@ -248,13 +248,15 @@ impl crate::dmp::Config for Test {}
 
 parameter_types! {
 	pub const FirstMessageFactorPercent: u64 = 100;
+	pub const DefaultChannelSizeAndCapacityWithSystem: (u32, u32) = (4, 1);
 }
 
 impl crate::hrmp::Config for Test {
 	type RuntimeOrigin = RuntimeOrigin;
 	type RuntimeEvent = RuntimeEvent;
 	type ChannelManager = frame_system::EnsureRoot<u64>;
 	type Currency = pallet_balances::Pallet<Test>;
+	type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem;
 	type WeightInfo = crate::hrmp::TestWeightInfo;
 }
 

polkadot/runtime/rococo/src/lib.rs

@@ -996,11 +996,16 @@ impl pallet_message_queue::Config for Runtime {
 
 impl parachains_dmp::Config for Runtime {}
 
+parameter_types! {
+	pub const DefaultChannelSizeAndCapacityWithSystem: (u32, u32) = (51200, 500);
+}
+
 impl parachains_hrmp::Config for Runtime {
 	type RuntimeOrigin = RuntimeOrigin;
 	type RuntimeEvent = RuntimeEvent;
 	type ChannelManager = EnsureRoot<AccountId>;
 	type Currency = Balances;
+	type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem;
 	type WeightInfo = weights::runtime_parachains_hrmp::WeightInfo<Runtime>;
 }
 

polkadot/runtime/rococo/src/weights/runtime_parachains_hrmp.rs

@@ -331,4 +331,14 @@ impl<T: frame_system::Config> runtime_parachains::hrmp::WeightInfo for WeightInf
 			.saturating_add(T::DbWeight::get().reads(1))
 			.saturating_add(T::DbWeight::get().writes(1))
 	}
+	fn establish_channel_with_system() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `417`
+		//  Estimated: `6357`
+		// Minimum execution time: 629_674_000 picoseconds.
+		Weight::from_parts(640_174_000, 0)
+			.saturating_add(Weight::from_parts(0, 6357))
+			.saturating_add(T::DbWeight::get().reads(12))
+			.saturating_add(T::DbWeight::get().writes(8))
+	}
 }

polkadot/runtime/test-runtime/src/lib.rs

@@ -554,13 +554,15 @@ impl parachains_dmp::Config for Runtime {}
 
 parameter_types! {
 	pub const FirstMessageFactorPercent: u64 = 100;
+	pub const DefaultChannelSizeAndCapacityWithSystem: (u32, u32) = (51200, 500);
 }
 
 impl parachains_hrmp::Config for Runtime {
 	type RuntimeOrigin = RuntimeOrigin;
 	type RuntimeEvent = RuntimeEvent;
 	type ChannelManager = frame_system::EnsureRoot<AccountId>;
 	type Currency = Balances;
+	type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem;
 	type WeightInfo = parachains_hrmp::TestWeightInfo;
 }
 

polkadot/runtime/westend/src/lib.rs

@@ -1197,11 +1197,16 @@ impl pallet_message_queue::Config for Runtime {
 
 impl parachains_dmp::Config for Runtime {}
 
+parameter_types! {
+	pub const DefaultChannelSizeAndCapacityWithSystem: (u32, u32) = (4096, 4);
+}
+
 impl parachains_hrmp::Config for Runtime {
 	type RuntimeOrigin = RuntimeOrigin;
 	type RuntimeEvent = RuntimeEvent;
 	type ChannelManager = EnsureRoot<AccountId>;
 	type Currency = Balances;
+	type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem;
 	type WeightInfo = weights::runtime_parachains_hrmp::WeightInfo<Self>;
 }
 

polkadot/runtime/westend/src/weights/runtime_parachains_hrmp.rs

@@ -324,4 +324,14 @@ impl<T: frame_system::Config> runtime_parachains::hrmp::WeightInfo for WeightInf
 			.saturating_add(T::DbWeight::get().reads(1))
 			.saturating_add(T::DbWeight::get().writes(1))
 	}
+	fn establish_channel_with_system() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `417`
+		//  Estimated: `6357`
+		// Minimum execution time: 629_674_000 picoseconds.
+		Weight::from_parts(640_174_000, 0)
+			.saturating_add(Weight::from_parts(0, 6357))
+			.saturating_add(T::DbWeight::get().reads(12))
+			.saturating_add(T::DbWeight::get().writes(8))
+	}
 }

prdoc/pr_3721.prdoc

@@ -0,0 +1,13 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: New call `hrmp.establish_channel_with_system` to allow parachains to establish a channel with a system parachain
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR adds a new call `hrmp.establish_channel_with_system` that allows a parachain origin to open a bidirectional channel with a system parachain.
+
+crates: 
+- name: polkadot-runtime-parachains
+  bump: minor