Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
13 changes: 13 additions & 0 deletions prdoc/pr_4803.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Fix for issue #4762

doc:
- audience: Runtime Dev
description: |
When the status of the queue is on_initialize, throw a defensive message and return weight of 0,
however when status is on_idle, do not throw a defensive message, only return weight of 0

crates:
- name: MQ pallet
Comment thread
bkchr marked this conversation as resolved.
Outdated
29 changes: 24 additions & 5 deletions substrate/frame/message-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,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 @@ -658,7 +658,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 @@ -777,6 +777,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 @@ -1579,13 +1589,12 @@ 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.");
Weight::zero()
check_queue_context(context);
Comment thread
bkchr marked this conversation as resolved.
Outdated
});

match with_service_mutex(|| {
Expand Down Expand Up @@ -1654,6 +1663,16 @@ impl<T: Config> ServiceQueues for Pallet<T> {
},
)
}

// Check the message queue context for on_idle and on_initialize status
// throw defensive message in `on_initialize` status
// don't throw defensive message in `on_idle` status
fn check_queue_context(context: ServiceQueuesContext) -> Weight {
if matches(context, ServiceQueuesContext::OnInitialize) {
defensive!("Not enough weight to service a single message.");
}
Weight::zero()
}
Comment thread
bkchr marked this conversation as resolved.
Outdated
}

impl<T: Config> EnqueueMessage<MessageOriginOf<T>> for Pallet<T> {
Expand Down