From b1db5f3a90ab639c9fb9bb95d6aa620b1dea6598 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Fri, 12 Apr 2024 22:14:20 +1200 Subject: [PATCH] establish_channel_with_system (#3721) Implements https://github.com/polkadot-fellows/RFCs/issues/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 <25483142+joepetrowski@users.noreply.github.com> Co-authored-by: Adrian Catangiu --- polkadot/runtime/parachains/src/hrmp.rs | 54 +++++++++++++- .../parachains/src/hrmp/benchmarking.rs | 44 ++++++++++++ polkadot/runtime/parachains/src/hrmp/tests.rs | 71 ++++++++++++++++++- polkadot/runtime/parachains/src/mock.rs | 2 + polkadot/runtime/rococo/src/lib.rs | 5 ++ .../src/weights/runtime_parachains_hrmp.rs | 10 +++ polkadot/runtime/test-runtime/src/lib.rs | 2 + polkadot/runtime/westend/src/lib.rs | 5 ++ .../src/weights/runtime_parachains_hrmp.rs | 10 +++ prdoc/pr_3721.prdoc | 13 ++++ 10 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 prdoc/pr_3721.prdoc diff --git a/polkadot/runtime/parachains/src/hrmp.rs b/polkadot/runtime/parachains/src/hrmp.rs index 05a540aef828..65652b38577b 100644 --- a/polkadot/runtime/parachains/src/hrmp.rs +++ b/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; + /// 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(::WeightInfo::establish_channel_with_system())] + pub fn establish_channel_with_system( + origin: OriginFor, + target_system_chain: ParaId, + ) -> DispatchResultWithPostInfo { + let sender = ensure_parachain(::RuntimeOrigin::from(origin))?; + + ensure!(target_system_chain.is_system(), Error::::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()) + } } } diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index 13f4cdfe3eea..8bf444e647e2 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -50,6 +50,13 @@ fn assert_last_event(generic_event: ::RuntimeEvent) { assert_eq!(event, &system_event); } +fn assert_has_event(generic_event: ::RuntimeEvent) { + let events = frame_system::Pallet::::events(); + let system_event: ::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 @@ -517,6 +524,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::(sender_id.into(), Zero::zero()); + register_parachain_with_balance::(recipient_id, Zero::zero()); + + #[extrinsic_call] + _(sender_origin, recipient_id); + + let (max_message_size, max_capacity) = T::DefaultChannelSizeAndCapacityWithSystem::get(); + + assert_has_event::( + Event::::HrmpSystemChannelOpened { + sender: sender_id.into(), + recipient: recipient_id, + proposed_max_capacity: max_capacity, + proposed_max_message_size: max_message_size, + } + .into(), + ); + + assert_has_event::( + Event::::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()), diff --git a/polkadot/runtime/parachains/src/hrmp/tests.rs b/polkadot/runtime/parachains/src/hrmp/tests.rs index 8d43b866bc19..2f767ab7e1b1 100644 --- a/polkadot/runtime/parachains/src/hrmp/tests.rs +++ b/polkadot/runtime/parachains/src/hrmp/tests.rs @@ -27,7 +27,7 @@ use crate::{ }, shared, }; -use frame_support::{assert_noop, assert_ok}; +use frame_support::{assert_noop, assert_ok, error::BadOrigin}; use primitives::BlockNumber; use std::collections::BTreeMap; @@ -935,3 +935,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::::ChannelCreationNotAuthorized + ); + Hrmp::assert_storage_consistency_exhaustive(); + }); +} diff --git a/polkadot/runtime/parachains/src/mock.rs b/polkadot/runtime/parachains/src/mock.rs index 461b9f4b431a..c91f5be127cd 100644 --- a/polkadot/runtime/parachains/src/mock.rs +++ b/polkadot/runtime/parachains/src/mock.rs @@ -248,6 +248,7 @@ 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 { @@ -255,6 +256,7 @@ impl crate::hrmp::Config for Test { type RuntimeEvent = RuntimeEvent; type ChannelManager = frame_system::EnsureRoot; type Currency = pallet_balances::Pallet; + type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem; type WeightInfo = crate::hrmp::TestWeightInfo; } diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 894d7fac2f0a..91b95790a4f6 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -950,11 +950,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; type Currency = Balances; + type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem; type WeightInfo = weights::runtime_parachains_hrmp::WeightInfo; } diff --git a/polkadot/runtime/rococo/src/weights/runtime_parachains_hrmp.rs b/polkadot/runtime/rococo/src/weights/runtime_parachains_hrmp.rs index 417820e6627f..97b84155b36a 100644 --- a/polkadot/runtime/rococo/src/weights/runtime_parachains_hrmp.rs +++ b/polkadot/runtime/rococo/src/weights/runtime_parachains_hrmp.rs @@ -331,4 +331,14 @@ impl 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)) + } } diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index f01382509501..d457de0b24cb 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -554,6 +554,7 @@ 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 { @@ -561,6 +562,7 @@ impl parachains_hrmp::Config for Runtime { type RuntimeEvent = RuntimeEvent; type ChannelManager = frame_system::EnsureRoot; type Currency = Balances; + type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem; type WeightInfo = parachains_hrmp::TestWeightInfo; } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 4930610c1d80..4cc7421fbd55 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1153,11 +1153,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; type Currency = Balances; + type DefaultChannelSizeAndCapacityWithSystem = DefaultChannelSizeAndCapacityWithSystem; type WeightInfo = weights::runtime_parachains_hrmp::WeightInfo; } diff --git a/polkadot/runtime/westend/src/weights/runtime_parachains_hrmp.rs b/polkadot/runtime/westend/src/weights/runtime_parachains_hrmp.rs index 9beb15303d87..3d2ab827b8fd 100644 --- a/polkadot/runtime/westend/src/weights/runtime_parachains_hrmp.rs +++ b/polkadot/runtime/westend/src/weights/runtime_parachains_hrmp.rs @@ -324,4 +324,14 @@ impl 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)) + } } diff --git a/prdoc/pr_3721.prdoc b/prdoc/pr_3721.prdoc new file mode 100644 index 000000000000..be36103c4742 --- /dev/null +++ b/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