diff --git a/substrate/frame/message-queue/src/lib.rs b/substrate/frame/message-queue/src/lib.rs index 04bbea121ddcf..12d289478b37c 100644 --- a/substrate/frame/message-queue/src/lib.rs +++ b/substrate/frame/message-queue/src/lib.rs @@ -195,7 +195,7 @@ use frame_support::{ pallet_prelude::*, traits::{ DefensiveTruncateFrom, EnqueueMessage, ExecuteOverweightError, Footprint, ProcessMessage, - ProcessMessageError, QueuePausedQuery, ServiceQueues, + ProcessMessageError, QueueFootprint, QueuePausedQuery, ServiceQueues, }, BoundedSlice, CloneNoBound, DefaultNoBound, }; @@ -423,14 +423,23 @@ impl Default for BookState { } } +impl From> for QueueFootprint { + fn from(book: BookState) -> Self { + QueueFootprint { + pages: book.count, + storage: Footprint { count: book.message_count, size: book.size }, + } + } +} + /// Handler code for when the items in a queue change. pub trait OnQueueChanged { /// Note that the queue `id` now has `item_count` items in it, taking up `items_size` bytes. - fn on_queue_changed(id: Id, items_count: u64, items_size: u64); + fn on_queue_changed(id: Id, fp: QueueFootprint); } impl OnQueueChanged for () { - fn on_queue_changed(_: Id, _: u64, _: u64) {} + fn on_queue_changed(_: Id, _: QueueFootprint) {} } #[frame_support::pallet] @@ -907,11 +916,7 @@ impl Pallet { T::WeightInfo::execute_overweight_page_updated() }; BookStateFor::::insert(&origin, &book_state); - T::QueueChangeHandler::on_queue_changed( - origin, - book_state.message_count, - book_state.size, - ); + T::QueueChangeHandler::on_queue_changed(origin, book_state.into()); Ok(weight_counter.consumed().saturating_add(page_weight)) }, } @@ -976,11 +981,7 @@ impl Pallet { book_state.message_count.saturating_reduce(page.remaining.into() as u64); book_state.size.saturating_reduce(page.remaining_size.into() as u64); BookStateFor::::insert(origin, &book_state); - T::QueueChangeHandler::on_queue_changed( - origin.clone(), - book_state.message_count, - book_state.size, - ); + T::QueueChangeHandler::on_queue_changed(origin.clone(), book_state.into()); Self::deposit_event(Event::PageReaped { origin: origin.clone(), index: page_index }); Ok(()) @@ -1035,11 +1036,7 @@ impl Pallet { } BookStateFor::::insert(&origin, &book_state); if total_processed > 0 { - T::QueueChangeHandler::on_queue_changed( - origin, - book_state.message_count, - book_state.size, - ); + T::QueueChangeHandler::on_queue_changed(origin, book_state.into()); } (total_processed > 0, next_ready) } @@ -1482,7 +1479,7 @@ impl EnqueueMessage> for Pallet { ) { Self::do_enqueue_message(&origin, message); let book_state = BookStateFor::::get(&origin); - T::QueueChangeHandler::on_queue_changed(origin, book_state.message_count, book_state.size); + T::QueueChangeHandler::on_queue_changed(origin, book_state.into()); } fn enqueue_messages<'a>( @@ -1493,7 +1490,7 @@ impl EnqueueMessage> for Pallet { Self::do_enqueue_message(&origin, message); } let book_state = BookStateFor::::get(&origin); - T::QueueChangeHandler::on_queue_changed(origin, book_state.message_count, book_state.size); + T::QueueChangeHandler::on_queue_changed(origin, book_state.into()); } fn sweep_queue(origin: MessageOriginOf) { @@ -1508,8 +1505,7 @@ impl EnqueueMessage> for Pallet { BookStateFor::::insert(&origin, &book_state); } - fn footprint(origin: MessageOriginOf) -> Footprint { - let book_state = BookStateFor::::get(&origin); - Footprint { count: book_state.message_count, size: book_state.size } + fn footprint(origin: MessageOriginOf) -> QueueFootprint { + BookStateFor::::get(&origin).into() } } diff --git a/substrate/frame/message-queue/src/mock.rs b/substrate/frame/message-queue/src/mock.rs index e6af0d9f1ee78..55a6457435423 100644 --- a/substrate/frame/message-queue/src/mock.rs +++ b/substrate/frame/message-queue/src/mock.rs @@ -278,8 +278,8 @@ parameter_types! { /// Records all queue changes into [`QueueChanges`]. pub struct RecordingQueueChangeHandler; impl OnQueueChanged for RecordingQueueChangeHandler { - fn on_queue_changed(id: MessageOrigin, items_count: u64, items_size: u64) { - QueueChanges::mutate(|cs| cs.push((id, items_count, items_size))); + fn on_queue_changed(id: MessageOrigin, fp: QueueFootprint) { + QueueChanges::mutate(|cs| cs.push((id, fp.storage.count, fp.storage.size))); } } @@ -366,3 +366,7 @@ pub fn num_overweight_enqueued_events() -> u32 { }) .count() as u32 } + +pub fn fp(pages: u32, count: u64, size: u64) -> QueueFootprint { + QueueFootprint { storage: Footprint { count, size }, pages } +} diff --git a/substrate/frame/message-queue/src/tests.rs b/substrate/frame/message-queue/src/tests.rs index 5a235a8750e1f..d94ad581ea0d5 100644 --- a/substrate/frame/message-queue/src/tests.rs +++ b/substrate/frame/message-queue/src/tests.rs @@ -49,6 +49,7 @@ fn enqueue_within_one_page_works() { MessageQueue::enqueue_message(msg("c"), Here); assert_eq!(MessageQueue::service_queues(2.into_weight()), 2.into_weight()); assert_eq!(MessagesProcessed::take(), vec![(b"a".to_vec(), Here), (b"b".to_vec(), Here)]); + assert_eq!(MessageQueue::footprint(Here).pages, 1); assert_eq!(MessageQueue::service_queues(2.into_weight()), 1.into_weight()); assert_eq!(MessagesProcessed::take(), vec![(b"c".to_vec(), Here)]); @@ -314,6 +315,7 @@ fn reap_page_permanent_overweight_works() { MessageQueue::enqueue_message(msg("weight=2"), Here); } assert_eq!(Pages::::iter().count(), n); + assert_eq!(MessageQueue::footprint(Here).pages, n as u32); assert_eq!(QueueChanges::take().len(), n); // Mark all pages as stale since their message is permanently overweight. MessageQueue::service_queues(1.into_weight()); @@ -339,6 +341,7 @@ fn reap_page_permanent_overweight_works() { assert_noop!(MessageQueue::do_reap_page(&o, i), Error::::NotReapable); assert!(QueueChanges::take().is_empty()); } + assert_eq!(MessageQueue::footprint(Here).pages, 3); }); } @@ -1022,8 +1025,9 @@ fn footprint_works() { BookStateFor::::insert(origin, book); let info = MessageQueue::footprint(origin); - assert_eq!(info.count as usize, msgs); - assert_eq!(info.size, page.remaining_size as u64); + assert_eq!(info.storage.count as usize, msgs); + assert_eq!(info.storage.size, page.remaining_size as u64); + assert_eq!(info.pages, 1); // Sweeping a queue never calls OnQueueChanged. assert!(QueueChanges::take().is_empty()); @@ -1044,16 +1048,44 @@ fn footprint_invalid_works() { fn footprint_on_swept_works() { use MessageOrigin::*; build_and_execute::(|| { - let mut book = empty_book::(); - book.message_count = 3; - book.size = 10; - BookStateFor::::insert(Here, &book); - knit(&Here); + build_ring::(&[Here]); MessageQueue::sweep_queue(Here); let fp = MessageQueue::footprint(Here); - assert_eq!(fp.count, 3); - assert_eq!(fp.size, 10); + assert_eq!((1, 1, 1), (fp.storage.count, fp.storage.size, fp.pages)); + }) +} + +/// The number of reported pages takes overweight pages into account. +#[test] +fn footprint_num_pages_works() { + use MessageOrigin::*; + build_and_execute::(|| { + MessageQueue::enqueue_message(msg("weight=2"), Here); + MessageQueue::enqueue_message(msg("weight=3"), Here); + + assert_eq!(MessageQueue::footprint(Here), fp(2, 2, 16)); + + // Mark the messages as overweight. + assert_eq!(MessageQueue::service_queues(1.into_weight()), 0.into_weight()); + assert_eq!(System::events().len(), 2); + // Overweight does not change the footprint. + assert_eq!(MessageQueue::footprint(Here), fp(2, 2, 16)); + + // Now execute the second message. + assert_eq!( + ::execute_overweight(3.into_weight(), (Here, 1, 0)) + .unwrap(), + 3.into_weight() + ); + assert_eq!(MessageQueue::footprint(Here), fp(1, 1, 8)); + // And the first one: + assert_eq!( + ::execute_overweight(2.into_weight(), (Here, 0, 0)) + .unwrap(), + 2.into_weight() + ); + assert_eq!(MessageQueue::footprint(Here), Default::default()); }) } @@ -1143,6 +1175,7 @@ fn permanently_overweight_book_unknits() { assert_ring(&[]); assert_eq!(MessagesProcessed::take().len(), 0); assert_eq!(BookStateFor::::get(Here).message_count, 1); + assert_eq!(MessageQueue::footprint(Here).pages, 1); // Now if we enqueue another message, it will become ready again. MessageQueue::enqueue_messages([msg("weight=1")].into_iter(), Here); assert_ring(&[Here]); diff --git a/substrate/frame/safe-mode/Cargo.toml b/substrate/frame/safe-mode/Cargo.toml index 1b8054c51a8b2..ac469bb385c93 100644 --- a/substrate/frame/safe-mode/Cargo.toml +++ b/substrate/frame/safe-mode/Cargo.toml @@ -30,6 +30,7 @@ sp-io = { path = "../../primitives/io" } pallet-balances = { path = "../balances" } pallet-utility = { path = "../utility" } pallet-proxy = { path = "../proxy" } +frame-support = { path = "../support", features = ["experimental"] } [features] default = [ "std" ] diff --git a/substrate/frame/safe-mode/src/tests.rs b/substrate/frame/safe-mode/src/tests.rs index ca1d7eb1d9340..b92c5b87a5308 100644 --- a/substrate/frame/safe-mode/src/tests.rs +++ b/substrate/frame/safe-mode/src/tests.rs @@ -22,32 +22,8 @@ use super::*; use crate::mock::{RuntimeCall, *}; -use frame_support::{assert_err, assert_noop, assert_ok, traits::Currency}; -use sp_runtime::{traits::Dispatchable, TransactionOutcome}; - -/// Do something hypothetically by rolling back any changes afterwards. -/// -/// Returns the original result of the closure. -macro_rules! hypothetically { - ( $e:expr ) => { - frame_support::storage::transactional::with_transaction( - || -> TransactionOutcome> { - sp_runtime::TransactionOutcome::Rollback(Ok($e)) - }, - ) - .expect("Always returning Ok; qed") - }; -} - -/// Assert something to be [*hypothetically*] `Ok` without actually committing it. -/// -/// Reverts any storage changes made by the closure. -macro_rules! hypothetically_ok { - ($e:expr $(, $args:expr)* $(,)?) => { - let result = hypothetically!($e); - assert_ok!(result $(, $args)*); - }; -} +use frame_support::{assert_err, assert_noop, assert_ok, hypothetically_ok, traits::Currency}; +use sp_runtime::traits::Dispatchable; #[test] fn fails_to_filter_calls_to_safe_mode_pallet() { diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 4888b8996d1ad..d495886868519 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -51,7 +51,9 @@ pub mod __private { pub use sp_metadata_ir as metadata_ir; #[cfg(feature = "std")] pub use sp_runtime::{bounded_btree_map, bounded_vec}; - pub use sp_runtime::{traits::Dispatchable, RuntimeDebug, StateVersion}; + pub use sp_runtime::{ + traits::Dispatchable, DispatchError, RuntimeDebug, StateVersion, TransactionOutcome, + }; #[cfg(feature = "std")] pub use sp_state_machine::BasicExternalities; pub use sp_std; @@ -781,6 +783,31 @@ macro_rules! assert_error_encoded_size { } => {}; } +/// Do something hypothetically by rolling back any changes afterwards. +/// +/// Returns the original result of the closure. +#[macro_export] +#[cfg(feature = "experimental")] +macro_rules! hypothetically { + ( $e:expr ) => { + $crate::storage::transactional::with_transaction(|| -> $crate::__private::TransactionOutcome> { + $crate::__private::TransactionOutcome::Rollback(Ok($e)) + }, + ).expect("Always returning Ok; qed") + }; +} + +/// Assert something to be *hypothetically* `Ok`, without actually committing it. +/// +/// Reverts any storage changes made by the closure. +#[macro_export] +#[cfg(feature = "experimental")] +macro_rules! hypothetically_ok { + ($e:expr $(, $args:expr)* $(,)?) => { + $crate::assert_ok!($crate::hypothetically!($e) $(, $args)*); + }; +} + #[doc(hidden)] pub use serde::{Deserialize, Serialize}; diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 75c43d8cc5082..4c692f848fde4 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -113,7 +113,8 @@ pub use preimages::{Bounded, BoundedInline, FetchResult, QueryPreimage, StorePre mod messages; pub use messages::{ EnqueueMessage, EnqueueWithOrigin, ExecuteOverweightError, HandleMessage, NoopServiceQueues, - ProcessMessage, ProcessMessageError, QueuePausedQuery, ServiceQueues, TransformOrigin, + ProcessMessage, ProcessMessageError, QueueFootprint, QueuePausedQuery, ServiceQueues, + TransformOrigin, }; mod safe_mode; diff --git a/substrate/frame/support/src/traits/messages.rs b/substrate/frame/support/src/traits/messages.rs index 0db163e072b17..654380dbb0a84 100644 --- a/substrate/frame/support/src/traits/messages.rs +++ b/substrate/frame/support/src/traits/messages.rs @@ -116,6 +116,15 @@ impl ServiceQueues for NoopServiceQueues { } } +/// The resource footprint of a queue. +#[derive(Default, Copy, Clone, Eq, PartialEq, RuntimeDebug)] +pub struct QueueFootprint { + /// The number of pages in the queue (including overweight pages). + pub pages: u32, + /// The storage footprint of the queue (including overweight messages). + pub storage: Footprint, +} + /// Can enqueue messages for multiple origins. pub trait EnqueueMessage { /// The maximal length any enqueued message may have. @@ -134,7 +143,7 @@ pub trait EnqueueMessage { fn sweep_queue(origin: Origin); /// Return the state footprint of the given queue. - fn footprint(origin: Origin) -> Footprint; + fn footprint(origin: Origin) -> QueueFootprint; } impl EnqueueMessage for () { @@ -146,8 +155,8 @@ impl EnqueueMessage for () { ) { } fn sweep_queue(_: Origin) {} - fn footprint(_: Origin) -> Footprint { - Footprint::default() + fn footprint(_: Origin) -> QueueFootprint { + QueueFootprint::default() } } @@ -173,7 +182,7 @@ impl, O: MaxEncodedLen, N: MaxEncodedLen, C: Convert> E::sweep_queue(C::convert(origin)); } - fn footprint(origin: N) -> Footprint { + fn footprint(origin: N) -> QueueFootprint { E::footprint(C::convert(origin)) } } @@ -195,7 +204,7 @@ pub trait HandleMessage { fn sweep_queue(); /// Return the state footprint of the queue. - fn footprint() -> Footprint; + fn footprint() -> QueueFootprint; } /// Adapter type to transform an [`EnqueueMessage`] with an origin into a [`HandleMessage`] impl. @@ -220,7 +229,7 @@ where E::sweep_queue(O::get()); } - fn footprint() -> Footprint { + fn footprint() -> QueueFootprint { E::footprint(O::get()) } } diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index 9098b6e48ed97..fe0905c3bed40 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -14,7 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = ["derive","max-encoded-len"] } -scale-info = { version = "2.10.0", default-features = false, features = ["derive"] } +scale-info = { version = "2.5.0", default-features = false, features = ["derive"] } log = { version = "0.4.17", default-features = false } serde = { version = "1.0.188", optional = true, default-features = false, features = ["derive", "alloc"] } bounded-collections = { version = "0.1.8", default-features = false }