-
Notifications
You must be signed in to change notification settings - Fork 707
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
[HRMP] Dont partially modify pages #4710
Conversation
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
The CI pipeline was cancelled due to failure one of the required jobs. |
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
@@ -491,7 +491,7 @@ impl<T: Config> Pallet<T> { | |||
let channel_info = | |||
T::ChannelInfo::get_channel_info(recipient).ok_or(MessageSendError::NoChannel)?; | |||
// Max message size refers to aggregates, or pages. Not to individual fragments. | |||
let max_message_size = channel_info.max_message_size as usize; | |||
let max_message_size = channel_info.max_message_size.min(T::MaxPageSize::get()) as usize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let max_message_size = channel_info.max_message_size.min(T::MaxPageSize::get()) as usize; | |
let max_message_size = channel_info.max_message_size.min.saturating_add(XcmpMessageFormat::max_encoded_len())(T::MaxPageSize::get()) as usize; |
The XcmpMessageFormat
is part of the page
. So, if we don't do this, we will never be able to storage a message of max_message_size
(when this is smaller than T::MaxPageSize
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but i think this is a hard limit from the relay chain that we cannot go over. So the "max message size" is for a HRMP message - not for the XCM inside of it.
Changes: - The XCMP queue does not partially modify pages anymore by using `try_mutate` instead of `mutate`. - The XCMP queue max page size is now the min between the value that the relay reports and the local limit. Thanks to whom pointed this out to me via DM. --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Changes:
try_mutate
instead ofmutate
.Thanks to whom pointed this out to me via DM.