Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
36 changes: 36 additions & 0 deletions cumulus/pallets/parachain-system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,42 @@ impl<T: Config> UpwardMessageSender for Pallet<T> {
fn send_upward_message(message: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError> {
Self::send_upward_message(message)
}

fn can_send_upward_message(message: &UpwardMessage) -> Result<(), MessageSendError> {
let max_upward_message_size = HostConfiguration::<T>::get()
.map(|cfg| cfg.max_upward_message_size)
.ok_or(MessageSendError::Other)?;
if message.len() > max_upward_message_size as usize {
Err(MessageSendError::TooBig)
} else {
Ok(())
}
}

#[cfg(any(feature = "std", feature = "runtime-benchmarks", test))]
fn ensure_successful_delivery() {
const MAX_UPWARD_MESSAGE_SIZE: u32 = 65_531 * 3;
const MAX_CODE_SIZE: u32 = 3 * 1024 * 1024;
HostConfiguration::<T>::mutate(|cfg| match cfg {
Some(cfg) => cfg.max_upward_message_size = MAX_UPWARD_MESSAGE_SIZE,
None =>
*cfg = Some(AbridgedHostConfiguration {
max_code_size: MAX_CODE_SIZE,
max_head_data_size: 32 * 1024,
max_upward_queue_count: 8,
max_upward_queue_size: 1024 * 1024,
max_upward_message_size: MAX_UPWARD_MESSAGE_SIZE,
max_upward_message_num_per_candidate: 2,
hrmp_max_message_num_per_candidate: 2,
validation_upgrade_cooldown: 2,
validation_upgrade_delay: 2,
async_backing_params: relay_chain::AsyncBackingParams {
allowed_ancestry_len: 0,
max_candidate_depth: 0,
},
}),
})
}
}

impl<T: Config> InspectMessageQueues for Pallet<T> {
Expand Down
30 changes: 29 additions & 1 deletion cumulus/pallets/parachain-system/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ fn aborted_upgrade() {
}

#[test]
fn checks_size() {
fn checks_code_size() {
BlockTests::new()
.with_relay_sproof_builder(|_, _, builder| {
builder.host_config.max_code_size = 8;
Expand Down Expand Up @@ -706,6 +706,34 @@ fn send_upward_message_relay_bottleneck() {
);
}

#[test]
fn send_upwards_message_checks_size_on_validate() {
BlockTests::new()
.with_relay_sproof_builder(|_, _, sproof| {
sproof.host_config.max_upward_message_size = 128;
})
.add(1, || {
assert_eq!(
ParachainSystem::can_send_upward_message(vec![0u8; 129].as_ref()),
Err(MessageSendError::TooBig)
);
});
}

#[test]
fn send_upward_message_check_size() {
BlockTests::new()
.with_relay_sproof_builder(|_, _, sproof| {
sproof.host_config.max_upward_message_size = 128;
})
.add(1, || {
assert_eq!(
ParachainSystem::send_upward_message(vec![0u8; 129]),
Err(MessageSendError::TooBig)
);
});
}

#[test]
fn send_hrmp_message_buffer_channel_close() {
BlockTests::new()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use super::xcm_helpers;
use crate::{assert_matches_reserve_asset_deposited_instructions, get_fungible_delivery_fees};
use codec::Encode;
use core::ops::Mul;
use cumulus_primitives_core::XcmpMessageSource;
use cumulus_primitives_core::{UpwardMessageSender, XcmpMessageSource};
use frame_support::{
assert_noop, assert_ok,
traits::{
Expand Down Expand Up @@ -171,6 +171,8 @@ pub fn teleports_for_native_asset_works<

// 2. try to teleport asset back to the relaychain
{
<cumulus_pallet_parachain_system::Pallet<Runtime> as UpwardMessageSender>::ensure_successful_delivery();

let dest = Location::parent();
let mut dest_beneficiary = Location::parent()
.appended_with(AccountId32 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ fn teleport_assets_dummy_message(

/// Given a message, a sender, and a destination, it returns the delivery fees
fn get_fungible_delivery_fees<S: SendXcm>(destination: Location, message: Xcm<()>) -> u128 {
let Ok((_, delivery_fees)) = validate_send::<S>(destination, message) else {
unreachable!("message can be sent; qed")
let delivery_fees = match validate_send::<S>(destination, message) {
Ok((_, delivery_fees)) => delivery_fees,
Err(e) => unreachable!("message can be sent - {:?}; qed", e),
};
if let Some(delivery_fee) = delivery_fees.inner().first() {
let Fungible(delivery_fee_amount) = delivery_fee.fun else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ mod bridge_hub_westend_tests {
BridgeHubWestendLocation, WestendGlobalConsensusNetwork,
WithBridgeHubWestendMessagesInstance, XcmOverBridgeHubWestendInstance,
};
use cumulus_primitives_core::UpwardMessageSender;

// Random para id of sibling chain used in tests.
pub const SIBLING_PARACHAIN_ID: u32 = 2053;
Expand Down Expand Up @@ -386,7 +387,7 @@ mod bridge_hub_westend_tests {
_ => None,
}
}),
|| (),
|| <ParachainSystem as UpwardMessageSender>::ensure_successful_delivery(),
)
}

Expand Down Expand Up @@ -533,6 +534,7 @@ mod bridge_hub_bulletin_tests {
RococoBulletinGlobalConsensusNetwork, RococoBulletinGlobalConsensusNetworkLocation,
WithRococoBulletinMessagesInstance, XcmOverPolkadotBulletinInstance,
};
use cumulus_primitives_core::UpwardMessageSender;

// Random para id of sibling chain used in tests.
pub const SIBLING_PEOPLE_PARACHAIN_ID: u32 =
Expand Down Expand Up @@ -668,7 +670,7 @@ mod bridge_hub_bulletin_tests {
_ => None,
}
}),
|| (),
|| <ParachainSystem as UpwardMessageSender>::ensure_successful_delivery(),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use bridge_to_rococo_config::{
DeliveryRewardInBalance, WithBridgeHubRococoMessagesInstance, XcmOverBridgeHubRococoInstance,
};
use codec::{Decode, Encode};
use cumulus_primitives_core::UpwardMessageSender;
use frame_support::{
assert_err, assert_ok,
dispatch::GetDispatchInfo,
Expand Down Expand Up @@ -315,7 +316,7 @@ fn message_dispatch_routing_works() {
_ => None,
}
}),
|| (),
|| <ParachainSystem as UpwardMessageSender>::ensure_successful_delivery(),
)
}

Expand Down
16 changes: 14 additions & 2 deletions cumulus/primitives/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,24 @@ pub trait UpwardMessageSender {
/// Send the given UMP message; return the expected number of blocks before the message will
/// be dispatched or an error if the message cannot be sent.
/// return the hash of the message sent
fn send_upward_message(msg: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError>;
fn send_upward_message(message: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError>;

/// Pre-check the given UMP message.
fn can_send_upward_message(message: &UpwardMessage) -> Result<(), MessageSendError>;

/// Ensure `[Self::send_upward_message]` is successful when called in benchmarks/tests.
#[cfg(any(feature = "std", feature = "runtime-benchmarks", test))]
fn ensure_successful_delivery() {}
}

impl UpwardMessageSender for () {
fn send_upward_message(_msg: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError> {
fn send_upward_message(_message: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError> {
Err(MessageSendError::NoChannel)
}

fn can_send_upward_message(_message: &UpwardMessage) -> Result<(), MessageSendError> {
Err(MessageSendError::Other)
}
}

/// The status of a channel.
Expand Down
42 changes: 32 additions & 10 deletions cumulus/primitives/utility/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ where
.map_err(|()| SendError::ExceedsMaxMessageSize)?;
let data = versioned_xcm.encode();

// Pre-check with our message sender if everything else is okay.
T::can_send_upward_message(&data).map_err(Self::map_upward_sender_err)?;

Ok((data, price))
} else {
// Anything else is unhandled. This includes a message that is not meant for us.
Expand All @@ -87,12 +90,24 @@ where
}

fn deliver(data: Vec<u8>) -> Result<XcmHash, SendError> {
let (_, hash) = T::send_upward_message(data).map_err(|e| match e {
let (_, hash) = T::send_upward_message(data).map_err(Self::map_upward_sender_err)?;
Ok(hash)
}

#[cfg(feature = "runtime-benchmarks")]
fn ensure_successful_delivery(location: Option<Location>) {
if location.as_ref().map_or(false, |l| l.contains_parents_only(1)) {
T::ensure_successful_delivery();
}
}
}

impl<T, W, P> ParentAsUmp<T, W, P> {
fn map_upward_sender_err(message_send_error: MessageSendError) -> SendError {
match message_send_error {
MessageSendError::TooBig => SendError::ExceedsMaxMessageSize,
e => SendError::Transport(e.into()),
})?;

Ok(hash)
}
}
}

Expand Down Expand Up @@ -596,12 +611,16 @@ mod test_xcm_router {
}
}

/// Impl [`UpwardMessageSender`] that return `Other` error
struct OtherErrorUpwardMessageSender;
impl UpwardMessageSender for OtherErrorUpwardMessageSender {
/// Impl [`UpwardMessageSender`] that return `Ok` for `can_send_upward_message`.
struct CanSendUpwardMessageSender;
impl UpwardMessageSender for CanSendUpwardMessageSender {
fn send_upward_message(_: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError> {
Err(MessageSendError::Other)
}

fn can_send_upward_message(_: &UpwardMessage) -> Result<(), MessageSendError> {
Ok(())
}
}

#[test]
Expand Down Expand Up @@ -641,7 +660,7 @@ mod test_xcm_router {
let dest = (Parent, Here);
let mut dest_wrapper = Some(dest.clone().into());
let mut msg_wrapper = Some(message.clone());
assert!(<ParentAsUmp<(), (), ()> as SendXcm>::validate(
assert!(<ParentAsUmp<CanSendUpwardMessageSender, (), ()> as SendXcm>::validate(
&mut dest_wrapper,
&mut msg_wrapper
)
Expand All @@ -655,7 +674,7 @@ mod test_xcm_router {
assert_eq!(
Err(SendError::Transport("Other")),
send_xcm::<(
ParentAsUmp<OtherErrorUpwardMessageSender, (), ()>,
ParentAsUmp<CanSendUpwardMessageSender, (), ()>,
OkFixedXcmHashWithAssertingRequiredInputsSender
)>(dest.into(), message)
);
Expand All @@ -665,7 +684,7 @@ mod test_xcm_router {
fn parent_as_ump_validate_nested_xcm_works() {
let dest = Parent;

type Router = ParentAsUmp<(), (), ()>;
type Router = ParentAsUmp<CanSendUpwardMessageSender, (), ()>;

// Message that is not too deeply nested:
let mut good = Xcm(vec![ClearOrigin]);
Expand Down Expand Up @@ -853,6 +872,9 @@ impl<
return (None, None);
}

// Ensure routers
XcmConfig::XcmSender::ensure_successful_delivery(Some(Location::parent()));

let mut fees_mode = None;
if !XcmConfig::FeeManager::is_waived(Some(origin_ref), fee_reason) {
// if not waived, we need to set up accounts for paying and receiving fees
Expand Down
14 changes: 14 additions & 0 deletions prdoc/pr_8409.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
title: check XCM size in VMP routing
doc:
- audience: Runtime Dev
description: |-
This PR adds the ability for a UMP (para -> relay), when using `UpwardMessageSender` to also
check the size of the message within validate. This is exposed into the existing `validate` of
`ParentAsUmp`.
crates:
- name: cumulus-pallet-parachain-system
bump: minor
- name: cumulus-primitives-core
bump: minor
- name: cumulus-primitives-utility
bump: minor
Loading