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 6 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
97 changes: 66 additions & 31 deletions polkadot/runtime/parachains/src/hrmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,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 DefaultChannelSizeAndCapacityToSystem: Get<(u32, u32)>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add an integrity_test that these are below the maximum allowed by Configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

config is from storages so it is not really possible to assert it at build time.
if the value is above the max defined in config, the call will fail, and I think that's fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right 👍

Copy link
Member

Choose a reason for hiding this comment

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

We do run integrity_test in externalities, so at least the genesis values can be checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but why check with genesis values?

Copy link
Member

Choose a reason for hiding this comment

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

Because they should also be correct? but its not a must, just an idea.

xlc marked this conversation as resolved.
Show resolved Hide resolved

/// Something that provides the weight of this pallet.
type WeightInfo: WeightInfo;
}
Expand Down Expand Up @@ -564,13 +568,13 @@ pub mod pallet {
T::ChannelManager::ensure_origin(origin)?;

ensure!(
HrmpIngressChannelsIndex::<T>::decode_len(para).unwrap_or_default() <=
num_inbound as usize,
HrmpIngressChannelsIndex::<T>::decode_len(para).unwrap_or_default()
<= num_inbound as usize,
Error::<T>::WrongWitness
);
ensure!(
HrmpEgressChannelsIndex::<T>::decode_len(para).unwrap_or_default() <=
num_outbound as usize,
HrmpEgressChannelsIndex::<T>::decode_len(para).unwrap_or_default()
<= num_outbound as usize,
Error::<T>::WrongWitness
);

Expand All @@ -592,8 +596,8 @@ pub mod pallet {
T::ChannelManager::ensure_origin(origin)?;

ensure!(
HrmpOpenChannelRequestsList::<T>::decode_len().unwrap_or_default() as u32 <=
channels,
HrmpOpenChannelRequestsList::<T>::decode_len().unwrap_or_default() as u32
<= channels,
Error::<T>::WrongWitness
);

Expand All @@ -616,8 +620,8 @@ pub mod pallet {
T::ChannelManager::ensure_origin(origin)?;

ensure!(
HrmpCloseChannelRequestsList::<T>::decode_len().unwrap_or_default() as u32 <=
channels,
HrmpCloseChannelRequestsList::<T>::decode_len().unwrap_or_default() as u32
<= channels,
Error::<T>::WrongWitness
);

Expand All @@ -642,8 +646,8 @@ pub mod pallet {
) -> DispatchResult {
let origin = ensure_parachain(<T as Config>::RuntimeOrigin::from(origin))?;
ensure!(
HrmpOpenChannelRequestsList::<T>::decode_len().unwrap_or_default() as u32 <=
open_requests,
HrmpOpenChannelRequestsList::<T>::decode_len().unwrap_or_default() as u32
<= open_requests,
Error::<T>::WrongWitness
);
Self::cancel_open_request(origin, channel_id.clone())?;
Expand Down Expand Up @@ -776,10 +780,10 @@ pub mod pallet {
let current_recipient_deposit = channel.recipient_deposit;

// nothing to update
if current_sender_deposit == new_sender_deposit &&
current_recipient_deposit == new_recipient_deposit
if current_sender_deposit == new_sender_deposit
&& current_recipient_deposit == new_recipient_deposit
{
return Ok(())
return Ok(());
}

// sender
Expand Down Expand Up @@ -827,7 +831,7 @@ pub mod pallet {
channel.sender_deposit = new_sender_deposit;
channel.recipient_deposit = new_recipient_deposit;
} else {
return Err(Error::<T>::OpenHrmpChannelDoesntExist.into())
return Err(Error::<T>::OpenHrmpChannelDoesntExist.into());
}
Ok(())
})?;
Expand All @@ -836,6 +840,36 @@ pub mod pallet {

Ok(())
}

/// TODO write docs
xlc marked this conversation as resolved.
Show resolved Hide resolved
#[pallet::call_index(10)]
#[pallet::weight(<T as Config>::WeightInfo::establish_system_channel())] // TODO benchmarks
pub fn establish_channel_with_system(
origin: OriginFor<T>,
recipient: ParaId,
) -> DispatchResultWithPostInfo {
xlc marked this conversation as resolved.
Show resolved Hide resolved
let sender = ensure_parachain(<T as Config>::RuntimeOrigin::from(origin))?;

ensure!(recipient.is_system(), Error::<T>::ChannelCreationNotAuthorized);

let (max_message_size, max_capacity) = T::DefaultChannelSizeAndCapacityToSystem::get();

// create bidirectional channell
xlc marked this conversation as resolved.
Show resolved Hide resolved
Self::init_open_channel(sender, recipient, max_capacity, max_message_size)?;
Self::accept_open_channel(recipient, sender)?;
xlc marked this conversation as resolved.
Show resolved Hide resolved

Self::init_open_channel(recipient, sender, max_capacity, max_message_size)?;
Self::accept_open_channel(sender, recipient)?;

Self::deposit_event(Event::HrmpSystemChannelOpened {
xlc marked this conversation as resolved.
Show resolved Hide resolved
sender,
recipient,
proposed_max_capacity: max_capacity,
proposed_max_message_size: max_message_size,
});

Ok(Pays::No.into())
}
}
}

Expand Down Expand Up @@ -937,7 +971,7 @@ impl<T: Config> Pallet<T> {
Some(req_data) => req_data,
None => {
// Can't normally happen but no need to panic.
continue
continue;
},
};

Expand Down Expand Up @@ -995,7 +1029,7 @@ impl<T: Config> Pallet<T> {
fn process_hrmp_open_channel_requests(config: &HostConfiguration<BlockNumberFor<T>>) {
let mut open_req_channels = HrmpOpenChannelRequestsList::<T>::get();
if open_req_channels.is_empty() {
return
return;
}

// iterate the vector starting from the end making our way to the beginning. This way we
Expand All @@ -1004,7 +1038,7 @@ impl<T: Config> Pallet<T> {
loop {
// bail if we've iterated over all items.
if idx == 0 {
break
break;
xlc marked this conversation as resolved.
Show resolved Hide resolved
}

idx -= 1;
Expand All @@ -1018,8 +1052,8 @@ impl<T: Config> Pallet<T> {
let recipient_deposit = if system_channel { 0 } else { config.hrmp_recipient_deposit };

if request.confirmed {
if <paras::Pallet<T>>::is_valid_para(channel_id.sender) &&
<paras::Pallet<T>>::is_valid_para(channel_id.recipient)
if <paras::Pallet<T>>::is_valid_para(channel_id.sender)
&& <paras::Pallet<T>>::is_valid_para(channel_id.recipient)
{
HrmpChannels::<T>::insert(
&channel_id,
Expand Down Expand Up @@ -1116,22 +1150,22 @@ impl<T: Config> Pallet<T> {
// (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 new_hrmp_watermark == relay_chain_parent_number {
return Ok(())
return Ok(());
}

if new_hrmp_watermark > relay_chain_parent_number {
return Err(HrmpWatermarkAcceptanceErr::AheadRelayParent {
new_watermark: new_hrmp_watermark,
relay_chain_parent_number,
})
});
}

if let Some(last_watermark) = HrmpWatermarks::<T>::get(&recipient) {
if new_hrmp_watermark <= last_watermark {
return Err(HrmpWatermarkAcceptanceErr::AdvancementRule {
new_watermark: new_hrmp_watermark,
last_watermark,
})
});
}
}

Expand All @@ -1146,7 +1180,7 @@ impl<T: Config> Pallet<T> {
{
return Err(HrmpWatermarkAcceptanceErr::LandsOnBlockWithNoMessages {
new_watermark: new_hrmp_watermark,
})
});
}
Ok(())
}
Expand All @@ -1168,7 +1202,7 @@ impl<T: Config> Pallet<T> {
return Err(OutboundHrmpAcceptanceErr::MoreMessagesThanPermitted {
sent: out_hrmp_msgs.len() as u32,
permitted: config.hrmp_max_message_num_per_candidate,
})
});
}

let mut last_recipient = None::<ParaId>;
Expand All @@ -1180,8 +1214,9 @@ impl<T: Config> Pallet<T> {
// the messages must be sorted in ascending order and there must be no two messages
// sent to the same recipient. Thus we can check that every recipient is strictly
// greater than the previous one.
Some(last_recipient) if out_msg.recipient <= last_recipient =>
return Err(OutboundHrmpAcceptanceErr::NotSorted { idx }),
Some(last_recipient) if out_msg.recipient <= last_recipient => {
return Err(OutboundHrmpAcceptanceErr::NotSorted { idx })
},
_ => last_recipient = Some(out_msg.recipient),
}

Expand All @@ -1198,7 +1233,7 @@ impl<T: Config> Pallet<T> {
idx,
msg_size,
max_size: channel.max_message_size,
})
});
}

let new_total_size = channel.total_size + out_msg.data.len() as u32;
Expand All @@ -1207,7 +1242,7 @@ impl<T: Config> Pallet<T> {
idx,
total_size: new_total_size,
limit: channel.max_total_size,
})
});
}

let new_msg_count = channel.msg_count + 1;
Expand All @@ -1216,7 +1251,7 @@ impl<T: Config> Pallet<T> {
idx,
count: new_msg_count,
limit: channel.max_capacity,
})
});
}
}

Expand All @@ -1230,7 +1265,7 @@ impl<T: Config> Pallet<T> {

for recipient in recipients {
let Some(channel) = HrmpChannels::<T>::get(&HrmpChannelId { sender, recipient }) else {
continue
continue;
};
remaining.push((
recipient,
Expand Down Expand Up @@ -1320,7 +1355,7 @@ impl<T: Config> Pallet<T> {
None => {
// apparently, that since acceptance of this candidate the recipient was
// offboarded and the channel no longer exists.
continue
continue;
},
};

Expand Down
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 DefaultChannelSizeAndCapacityToSystem: (u32, u32) = (51200, 500);
}

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 DefaultChannelSizeAndCapacityToSystem = DefaultChannelSizeAndCapacityToSystem;
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 @@ -995,11 +995,16 @@ impl pallet_message_queue::Config for Runtime {

impl parachains_dmp::Config for Runtime {}

parameter_types! {
pub const DefaultChannelSizeAndCapacityToSystem: (u32, u32) = (51200, 500);
}

impl parachains_hrmp::Config for Runtime {
type RuntimeOrigin = RuntimeOrigin;
type RuntimeEvent = RuntimeEvent;
type ChannelManager = EnsureRoot<AccountId>;
type Currency = Balances;
type DefaultChannelSizeAndCapacityToSystem = DefaultChannelSizeAndCapacityToSystem;
type WeightInfo = weights::runtime_parachains_hrmp::WeightInfo<Runtime>;
}

Expand Down
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 @@ -554,13 +554,15 @@ impl parachains_dmp::Config for Runtime {}

parameter_types! {
pub const FirstMessageFactorPercent: u64 = 100;
pub const DefaultChannelSizeAndCapacityToSystem: (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 DefaultChannelSizeAndCapacityToSystem = DefaultChannelSizeAndCapacityToSystem;
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 @@ -1200,11 +1200,16 @@ impl pallet_message_queue::Config for Runtime {

impl parachains_dmp::Config for Runtime {}

parameter_types! {
pub const DefaultChannelSizeAndCapacityToSystem: (u32, u32) = (51200, 500);
}

impl parachains_hrmp::Config for Runtime {
type RuntimeOrigin = RuntimeOrigin;
type RuntimeEvent = RuntimeEvent;
type ChannelManager = EnsureRoot<AccountId>;
type Currency = Balances;
type DefaultChannelSizeAndCapacityToSystem = DefaultChannelSizeAndCapacityToSystem;
type WeightInfo = weights::runtime_parachains_hrmp::WeightInfo<Self>;
}

Expand Down
Loading