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 for Issue 4762 #4803

Merged
merged 12 commits into from
Oct 16, 2024
23 changes: 19 additions & 4 deletions substrate/frame/message-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ pub mod pallet {
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_initialize(_n: BlockNumberFor<T>) -> Weight {
if let Some(weight_limit) = T::ServiceWeight::get() {
Self::service_queues(weight_limit)
Self::service_queues(weight_limit, ServiceQueuesContext::OnInitialize)
} else {
Weight::zero()
}
Expand All @@ -655,7 +655,7 @@ pub mod pallet {
fn on_idle(_n: BlockNumberFor<T>, remaining_weight: Weight) -> Weight {
if let Some(weight_limit) = T::IdleMaxServiceWeight::get() {
// Make use of the remaining weight to process enqueued messages.
Self::service_queues(weight_limit.min(remaining_weight))
Self::service_queues(weight_limit.min(remaining_weight), ServiceQueuesContext::OnIdle)
} else {
Weight::zero()
}
Expand Down Expand Up @@ -774,6 +774,16 @@ enum MessageExecutionStatus {
StackLimitReached,
}

/// The context to pass to service_queues through on_idle and on_initialize hooks
/// We don't want to throw the defensive message if called from on_idle hook
#[derive(PartialEq)]
enum ServiceQueuesContext {
/// Context of on_idle hook
OnIdle,
/// Context of on_initialize hook
OnInitialize,
}

impl<T: Config> Pallet<T> {
/// Knit `origin` into the ready ring right at the end.
///
Expand Down Expand Up @@ -1554,12 +1564,17 @@ impl<T: Get<O>, O: Into<u32>> Get<u32> for IntoU32<T, O> {
impl<T: Config> ServiceQueues for Pallet<T> {
type OverweightMessageAddress = (MessageOriginOf<T>, PageIndex, T::Size);

fn service_queues(weight_limit: Weight) -> Weight {
fn service_queues(weight_limit: Weight, context: ServiceQueuesContext) -> Weight {
Copy link
Member

Choose a reason for hiding this comment

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

I just noticed that this is implementing a trait function. Can you maybe put the inner logic into a new function and call that from here?

Having the ServiceQueuesContext in the interface would be ugly otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I pushed a new commit last night (PST). Now that I'm re-reading this I might not have implemented the way you requested.

Copy link
Member

Choose a reason for hiding this comment

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

I mean, the code does not compile? You are implementing a trait function. Please try to run the pallet tests locally.

let mut weight = WeightMeter::with_limit(weight_limit);

// Get the maximum weight that processing a single message may take:
let max_weight = Self::max_message_weight(weight_limit).unwrap_or_else(|| {
defensive!("Not enough weight to service a single message.");
// throw defensive message when service_queues is called from on_initialize
// it doesn't matter if there is not enough weight when called in the context of on_idle
// therefore, don't throw message when service_queues is called from on_idle
if matches(context, ServiceQueuesContext::OnInitialize) {
defensive!("Not enough weight to service a single message.");
}
Weight::zero()
});

Expand Down
Loading