Skip to content

Commit a50858c

Browse files
[stable2409] Backport #4803 (#6098)
Backport #4803 into `stable2409` from gotnoshoeson. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Co-authored-by: Miles Patterson <[email protected]>
1 parent 0e02592 commit a50858c

File tree

3 files changed

+80
-45
lines changed

3 files changed

+80
-45
lines changed

prdoc/pr_4803.prdoc

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
2+
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
3+
4+
title: Fix for issue #4762
5+
6+
doc:
7+
- audience: Runtime Dev
8+
description: |
9+
When the status of the queue is on_initialize, throw a defensive message and return weight of 0,
10+
however when status is on_idle, do not throw a defensive message, only return weight of 0
11+
12+
crates:
13+
- name: pallet-message-queue
14+
bump: patch

substrate/frame/message-queue/src/lib.rs

+65-44
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ pub mod pallet {
649649
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
650650
fn on_initialize(_n: BlockNumberFor<T>) -> Weight {
651651
if let Some(weight_limit) = T::ServiceWeight::get() {
652-
Self::service_queues(weight_limit)
652+
Self::service_queues_impl(weight_limit, ServiceQueuesContext::OnInitialize)
653653
} else {
654654
Weight::zero()
655655
}
@@ -658,7 +658,10 @@ pub mod pallet {
658658
fn on_idle(_n: BlockNumberFor<T>, remaining_weight: Weight) -> Weight {
659659
if let Some(weight_limit) = T::IdleMaxServiceWeight::get() {
660660
// Make use of the remaining weight to process enqueued messages.
661-
Self::service_queues(weight_limit.min(remaining_weight))
661+
Self::service_queues_impl(
662+
weight_limit.min(remaining_weight),
663+
ServiceQueuesContext::OnIdle,
664+
)
662665
} else {
663666
Weight::zero()
664667
}
@@ -777,6 +780,18 @@ enum MessageExecutionStatus {
777780
StackLimitReached,
778781
}
779782

783+
/// The context to pass to [`Pallet::service_queues_impl`] through on_idle and on_initialize hooks
784+
/// We don't want to throw the defensive message if called from on_idle hook
785+
#[derive(PartialEq)]
786+
enum ServiceQueuesContext {
787+
/// Context of on_idle hook.
788+
OnIdle,
789+
/// Context of on_initialize hook.
790+
OnInitialize,
791+
/// Context `service_queues` trait function.
792+
ServiceQueues,
793+
}
794+
780795
impl<T: Config> Pallet<T> {
781796
/// Knit `origin` into the ready ring right at the end.
782797
///
@@ -1489,6 +1504,53 @@ impl<T: Config> Pallet<T> {
14891504
},
14901505
}
14911506
}
1507+
1508+
fn service_queues_impl(weight_limit: Weight, context: ServiceQueuesContext) -> Weight {
1509+
let mut weight = WeightMeter::with_limit(weight_limit);
1510+
1511+
// Get the maximum weight that processing a single message may take:
1512+
let max_weight = Self::max_message_weight(weight_limit).unwrap_or_else(|| {
1513+
if matches!(context, ServiceQueuesContext::OnInitialize) {
1514+
defensive!("Not enough weight to service a single message.");
1515+
}
1516+
Weight::zero()
1517+
});
1518+
1519+
match with_service_mutex(|| {
1520+
let mut next = match Self::bump_service_head(&mut weight) {
1521+
Some(h) => h,
1522+
None => return weight.consumed(),
1523+
};
1524+
// The last queue that did not make any progress.
1525+
// The loop aborts as soon as it arrives at this queue again without making any progress
1526+
// on other queues in between.
1527+
let mut last_no_progress = None;
1528+
1529+
loop {
1530+
let (progressed, n) = Self::service_queue(next.clone(), &mut weight, max_weight);
1531+
next = match n {
1532+
Some(n) =>
1533+
if !progressed {
1534+
if last_no_progress == Some(n.clone()) {
1535+
break
1536+
}
1537+
if last_no_progress.is_none() {
1538+
last_no_progress = Some(next.clone())
1539+
}
1540+
n
1541+
} else {
1542+
last_no_progress = None;
1543+
n
1544+
},
1545+
None => break,
1546+
}
1547+
}
1548+
weight.consumed()
1549+
}) {
1550+
Err(()) => weight.consumed(),
1551+
Ok(w) => w,
1552+
}
1553+
}
14921554
}
14931555

14941556
/// Run a closure that errors on re-entrance. Meant to be used by anything that services queues.
@@ -1558,48 +1620,7 @@ impl<T: Config> ServiceQueues for Pallet<T> {
15581620
type OverweightMessageAddress = (MessageOriginOf<T>, PageIndex, T::Size);
15591621

15601622
fn service_queues(weight_limit: Weight) -> Weight {
1561-
let mut weight = WeightMeter::with_limit(weight_limit);
1562-
1563-
// Get the maximum weight that processing a single message may take:
1564-
let max_weight = Self::max_message_weight(weight_limit).unwrap_or_else(|| {
1565-
defensive!("Not enough weight to service a single message.");
1566-
Weight::zero()
1567-
});
1568-
1569-
match with_service_mutex(|| {
1570-
let mut next = match Self::bump_service_head(&mut weight) {
1571-
Some(h) => h,
1572-
None => return weight.consumed(),
1573-
};
1574-
// The last queue that did not make any progress.
1575-
// The loop aborts as soon as it arrives at this queue again without making any progress
1576-
// on other queues in between.
1577-
let mut last_no_progress = None;
1578-
1579-
loop {
1580-
let (progressed, n) = Self::service_queue(next.clone(), &mut weight, max_weight);
1581-
next = match n {
1582-
Some(n) =>
1583-
if !progressed {
1584-
if last_no_progress == Some(n.clone()) {
1585-
break
1586-
}
1587-
if last_no_progress.is_none() {
1588-
last_no_progress = Some(next.clone())
1589-
}
1590-
n
1591-
} else {
1592-
last_no_progress = None;
1593-
n
1594-
},
1595-
None => break,
1596-
}
1597-
}
1598-
weight.consumed()
1599-
}) {
1600-
Err(()) => weight.consumed(),
1601-
Ok(w) => w,
1602-
}
1623+
Self::service_queues_impl(weight_limit, ServiceQueuesContext::ServiceQueues)
16031624
}
16041625

16051626
/// Execute a single overweight message.

substrate/frame/message-queue/src/tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ fn service_queues_low_weight_defensive() {
279279
assert!(MessageQueue::do_integrity_test().is_err());
280280

281281
MessageQueue::enqueue_message(msg("weight=0"), Here);
282-
MessageQueue::service_queues(104.into_weight());
282+
MessageQueue::service_queues_impl(104.into_weight(), ServiceQueuesContext::OnInitialize);
283283
});
284284
}
285285

0 commit comments

Comments
 (0)