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

Fix XCMP max message size check #1250

Merged
merged 14 commits into from
Dec 4, 2023
14 changes: 12 additions & 2 deletions cumulus/pallets/xcmp-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,21 @@ impl<T: Config> Pallet<T> {
) -> Result<u32, MessageSendError> {
let encoded_fragment = fragment.encode();

// Optimization note: `max_message_size` could potentially be stored in
// `OutboundXcmpMessages` once known; that way it's only accessed when a new page is needed.

let channel_info =
T::ChannelInfo::get_channel_info(recipient).ok_or(MessageSendError::NoChannel)?;
let max_message_size = channel_info.max_message_size as usize;
// Max message size refers to aggregates, or pages. Not to individual fragments.
if encoded_fragment.len() > max_message_size {
let max_message_size = channel_info.max_message_size as usize;
let format_size = format.encoded_size();
// We check the encoded fragment length plus the format size agains the max message size
// because the format is concatenated if a new page is needed.
let size_to_check = encoded_fragment
.len()
.checked_add(format_size)
.ok_or(MessageSendError::TooBig)?;
if size_to_check > max_message_size {
return Err(MessageSendError::TooBig)
}

Expand Down
44 changes: 44 additions & 0 deletions cumulus/pallets/xcmp-queue/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,50 @@ fn xcmp_queue_send_xcm_works() {
})
}

#[test]
fn xcmp_queue_send_too_big_xcm_fails() {
new_test_ext().execute_with(|| {
let sibling_para_id = ParaId::from(12345);
let dest = (Parent, X1(Parachain(sibling_para_id.into()))).into();

let max_message_size = 100_u32;

// open HRMP channel to the sibling_para_id with a set `max_message_size`
ParachainSystem::open_custom_outbound_hrmp_channel_for_benchmarks_or_tests(
sibling_para_id,
cumulus_primitives_core::AbridgedHrmpChannel {
max_message_size,
max_capacity: 10,
max_total_size: 10_000_000_u32,
msg_count: 0,
total_size: 0,
mqc_head: None,
},
);

// Message is crafted to exceed `max_message_size`
let mut message = Xcm::builder_unsafe();
for _ in 0..97 {
message = message.clear_origin();
}
let message = message.build();
let encoded_message_size = message.encode().len();
let versioned_size = 1; // VersionedXcm enum is added by `send_xcm` and it add one additional byte
assert_eq!(encoded_message_size, max_message_size as usize - versioned_size);

// check empty outbound queue
assert!(XcmpQueue::take_outbound_messages(usize::MAX).is_empty());

// Message is too big because after adding the VersionedXcm enum, it would reach
// `max_message_size` Then, adding the format, which is the worst case scenario in which a
// new page is needed, would get it over the limit
assert_eq!(send_xcm::<XcmpQueue>(dest, message), Err(SendError::Transport("TooBig")),);

// outbound queue is still empty
assert!(XcmpQueue::take_outbound_messages(usize::MAX).is_empty());
});
}

#[test]
fn verify_fee_factor_increase_and_decrease() {
use cumulus_primitives_core::AbridgedHrmpChannel;
Expand Down