Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

establish_channel_with_system #3721

Merged
merged 29 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion polkadot/runtime/parachains/src/hrmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
type DefaultChannelSizeAndCapacityWithSystem: Get<(u32, u32)>;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved

/// Something that provides the weight of this pallet.
type WeightInfo: WeightInfo;
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
xlc marked this conversation as resolved.
Show resolved Hide resolved
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())
}
}
}

Expand Down
54 changes: 50 additions & 4 deletions polkadot/runtime/parachains/src/hrmp/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ 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();

println!("events: {:?}", events);

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
Expand Down Expand Up @@ -101,24 +110,24 @@ where
));

if matches!(until, ParachainSetupStep::Requested) {
return output
return output;
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
}

assert_ok!(Hrmp::<T>::hrmp_accept_open_channel(recipient_origin.into(), sender));
if matches!(until, ParachainSetupStep::Accepted) {
return output
return output;
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
}

Hrmp::<T>::process_hrmp_open_channel_requests(&Configuration::<T>::config());
if matches!(until, ParachainSetupStep::Established) {
return output
return output;
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
}

let channel_id = HrmpChannelId { sender, recipient };
assert_ok!(Hrmp::<T>::hrmp_close_channel(sender_origin.clone().into(), channel_id));
if matches!(until, ParachainSetupStep::CloseRequested) {
// NOTE: this is just for expressiveness, otherwise the if-statement is 100% useless.
return output
return output;
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
}

output
Expand Down Expand Up @@ -513,6 +522,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);
xlc marked this conversation as resolved.
Show resolved Hide resolved

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()),
Expand Down
71 changes: 70 additions & 1 deletion polkadot/runtime/parachains/src/hrmp/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
});
}
2 changes: 2 additions & 0 deletions polkadot/runtime/parachains/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,11 +1000,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>;
}

Expand Down
10 changes: 10 additions & 0 deletions polkadot/runtime/rococo/src/weights/runtime_parachains_hrmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from establish_system_channel

.saturating_add(Weight::from_parts(0, 6357))
.saturating_add(T::DbWeight::get().reads(12))
.saturating_add(T::DbWeight::get().writes(8))
}
}
2 changes: 2 additions & 0 deletions polkadot/runtime/test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,13 +555,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;
}

Expand Down
5 changes: 5 additions & 0 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1203,11 +1203,16 @@ impl pallet_message_queue::Config for Runtime {

impl parachains_dmp::Config for Runtime {}

parameter_types! {
pub const DefaultChannelSizeAndCapacityWithSystem: (u32, u32) = (4096, 4);
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
}

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>;
}

Expand Down
10 changes: 10 additions & 0 deletions polkadot/runtime/westend/src/weights/runtime_parachains_hrmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
12 changes: 12 additions & 0 deletions prdoc/pr_3721.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# 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
xlc marked this conversation as resolved.
Show resolved Hide resolved
Loading