Add try_state check to Pallet MessageQueue (#13502)

* feat: Add try_state for message_queue

* Update frame/message-queue/src/lib.rs

change if let to while let and modify bump_service_head function

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* feat: update try_state, add checks for storage

* fix: add try_state builder for remaining tests

* Update frame/message-queue/src/mock.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* chore: assert statement to ensure

* Fix tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use ensure instead of assert

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix function signature and feature gate

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Cleanup code

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Juan <juangirini@gmail.com>

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

@@ -22,8 +22,8 @@
 
 use crate::{
 	mock::{
-		new_test_ext, CountingMessageProcessor, IntoWeight, MockedWeightInfo, NumMessagesProcessed,
-		YieldingQueues,
+		build_and_execute, CountingMessageProcessor, IntoWeight, MockedWeightInfo,
+		NumMessagesProcessed, YieldingQueues,
 	},
 	mock_helpers::MessageOrigin,
 	*,
@@ -123,7 +123,7 @@ fn stress_test_enqueue_and_service() {
 	let max_msg_len = MaxMessageLenOf::<Test>::get();
 	let mut rng = StdRng::seed_from_u64(42);
 
-	new_test_ext::<Test>().execute_with(|| {
+	build_and_execute::<Test>(|| {
 		let mut msgs_remaining = 0;
 		for _ in 0..blocks {
 			// Start by enqueuing a large number of messages.
@@ -171,7 +171,7 @@ fn stress_test_queue_suspension() {
 	let max_msg_len = MaxMessageLenOf::<Test>::get();
 	let mut rng = StdRng::seed_from_u64(41);
 
-	new_test_ext::<Test>().execute_with(|| {
+	build_and_execute::<Test>(|| {
 		let mut suspended = BTreeSet::<u32>::new();
 		let mut msgs_remaining = 0;
 

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

@@ -578,7 +578,12 @@ pub mod pallet {
 			}
 		}
 
-		/// Check all assumptions about [`crate::Config`].
+		#[cfg(feature = "try-runtime")]
+		fn try_state(_: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
+			Self::do_try_state()
+		}
+
+		/// Check all compile-time assumptions about [`crate::Config`].
 		fn integrity_test() {
 			assert!(!MaxMessageLenOf::<T>::get().is_zero(), "HeapSize too low");
 		}
@@ -1105,6 +1110,106 @@ impl<T: Config> Pallet<T> {
 		ItemExecutionStatus::Executed(is_processed)
 	}
 
+	/// Ensure the correctness of state of this pallet.
+	///
+	/// # Assumptions-
+	///
+	/// If `serviceHead` points to a ready Queue, then BookState of that Queue has:
+	///
+	/// * `message_count` > 0
+	/// * `size` > 0
+	/// * `end` > `begin`
+	/// * Some(ready_neighbours)
+	/// * If `ready_neighbours.next` == self.origin, then `ready_neighbours.prev` == self.origin
+	///   (only queue in ring)
+	///
+	/// For Pages(begin to end-1) in BookState:
+	///
+	/// * `remaining` > 0
+	/// * `remaining_size` > 0
+	/// * `first` <= `last`
+	/// * Every page can be decoded into peek_* functions
+	#[cfg(any(test, feature = "try-runtime"))]
+	pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> {
+		// Checking memory corruption for BookStateFor
+		ensure!(
+			BookStateFor::<T>::iter_keys().count() == BookStateFor::<T>::iter_values().count(),
+			"Memory Corruption in BookStateFor"
+		);
+		// Checking memory corruption for Pages
+		ensure!(
+			Pages::<T>::iter_keys().count() == Pages::<T>::iter_values().count(),
+			"Memory Corruption in Pages"
+		);
+
+		// No state to check
+		if ServiceHead::<T>::get().is_none() {
+			return Ok(())
+		}
+
+		//loop around this origin
+		let starting_origin = ServiceHead::<T>::get().unwrap();
+
+		while let Some(head) = Self::bump_service_head(&mut WeightMeter::max_limit()) {
+			ensure!(
+				BookStateFor::<T>::contains_key(&head),
+				"Service head must point to an existing book"
+			);
+
+			let head_book_state = BookStateFor::<T>::get(&head);
+			ensure!(
+				head_book_state.message_count > 0,
+				"There must be some messages if in ReadyRing"
+			);
+			ensure!(head_book_state.size > 0, "There must be some message size if in ReadyRing");
+			ensure!(
+				head_book_state.end > head_book_state.begin,
+				"End > Begin if unprocessed messages exists"
+			);
+			ensure!(
+				head_book_state.ready_neighbours.is_some(),
+				"There must be neighbours if in ReadyRing"
+			);
+
+			if head_book_state.ready_neighbours.as_ref().unwrap().next == head {
+				ensure!(
+					head_book_state.ready_neighbours.as_ref().unwrap().prev == head,
+					"Can only happen if only queue in ReadyRing"
+				);
+			}
+
+			for page_index in head_book_state.begin..head_book_state.end {
+				let page = Pages::<T>::get(&head, page_index).unwrap();
+				let remaining_messages = page.remaining;
+				let mut counted_remaining_messages = 0;
+				ensure!(
+					remaining_messages > 0.into(),
+					"These must be some messages that have not been processed yet!"
+				);
+
+				for i in 0..u32::MAX {
+					if let Some((_, processed, _)) = page.peek_index(i as usize) {
+						if !processed {
+							counted_remaining_messages += 1;
+						}
+					} else {
+						break
+					}
+				}
+
+				ensure!(
+					remaining_messages == counted_remaining_messages.into(),
+					"Memory Corruption"
+				);
+			}
+
+			if head_book_state.ready_neighbours.as_ref().unwrap().next == starting_origin {
+				break
+			}
+		}
+		Ok(())
+	}
+
 	/// Print the pages in each queue and the messages in each page.
 	///
 	/// Processed messages are prefixed with a `*` and the current `begin`ning page with a `>`.

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

@@ -295,10 +295,15 @@ where
 	ext
 }
 
-/// Run this closure in test externalities.
-pub fn test_closure<R>(f: impl FnOnce() -> R) -> R {
-	let mut ext = new_test_ext::<Test>();
-	ext.execute_with(f)
+/// Run the function pointer inside externalities and asserts the try_state hook at the end.
+pub fn build_and_execute<T: Config>(test: impl FnOnce() -> ())
+where
+	BlockNumberFor<T>: From<u32>,
+{
+	new_test_ext::<T>().execute_with(|| {
+		test();
+		MessageQueue::do_try_state().expect("All invariants must hold after a test");
+	});
 }
 
 /// Set the weight of a specific weight function.

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

@@ -89,7 +89,7 @@ pub fn page<T: Config>(msg: &[u8]) -> PageOf<T> {
 }
 
 pub fn single_page_book<T: Config>() -> BookStateOf<T> {
-	BookState { begin: 0, end: 1, count: 1, ..Default::default() }
+	BookState { begin: 0, end: 1, count: 1, message_count: 1, size: 1, ..Default::default() }
 }
 
 pub fn empty_book<T: Config>() -> BookStateOf<T> {
@@ -139,10 +139,8 @@ pub fn setup_bump_service_head<T: Config>(
 	current: <<T as Config>::MessageProcessor as ProcessMessage>::Origin,
 	next: <<T as Config>::MessageProcessor as ProcessMessage>::Origin,
 ) {
-	let mut book = single_page_book::<T>();
-	book.ready_neighbours = Some(Neighbours::<MessageOriginOf<T>> { prev: next.clone(), next });
-	ServiceHead::<T>::put(&current);
-	BookStateFor::<T>::insert(&current, &book);
+	crate::Pallet::<T>::enqueue_message(msg("1"), current);
+	crate::Pallet::<T>::enqueue_message(msg("1"), next);
 }
 
 /// Knit a queue into the ready-ring and write it back to storage.
@@ -164,11 +162,8 @@ pub fn unknit<T: Config>(o: &<<T as Config>::MessageProcessor as ProcessMessage>
 pub fn build_ring<T: Config>(
 	queues: &[<<T as Config>::MessageProcessor as ProcessMessage>::Origin],
 ) {
-	for queue in queues {
-		BookStateFor::<T>::insert(queue, empty_book::<T>());
-	}
-	for queue in queues {
-		knit::<T>(queue);
+	for queue in queues.iter() {
+		crate::Pallet::<T>::enqueue_message(msg("1"), queue.clone());
 	}
 	assert_ring::<T>(queues);
 }

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

@@ -27,22 +27,22 @@ use sp_core::blake2_256;
 
 #[test]
 fn mocked_weight_works() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		assert!(<Test as Config>::WeightInfo::service_queue_base().is_zero());
 	});
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("service_queue_base", Weight::MAX);
 		assert_eq!(<Test as Config>::WeightInfo::service_queue_base(), Weight::MAX);
 	});
 	// The externalities reset it.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		assert!(<Test as Config>::WeightInfo::service_queue_base().is_zero());
 	});
 }
 
 #[test]
 fn enqueue_within_one_page_works() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		use MessageOrigin::*;
 		MessageQueue::enqueue_message(msg("a"), Here);
 		MessageQueue::enqueue_message(msg("b"), Here);
@@ -77,7 +77,7 @@ fn enqueue_within_one_page_works() {
 
 #[test]
 fn queue_priority_retains() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		use MessageOrigin::*;
 		assert_ring(&[]);
 		MessageQueue::enqueue_message(msg("a"), Everywhere(1));
@@ -108,11 +108,13 @@ fn queue_priority_retains() {
 
 #[test]
 fn queue_priority_reset_once_serviced() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		use MessageOrigin::*;
 		MessageQueue::enqueue_message(msg("a"), Everywhere(1));
 		MessageQueue::enqueue_message(msg("b"), Everywhere(2));
 		MessageQueue::enqueue_message(msg("c"), Everywhere(3));
+		MessageQueue::do_try_state().unwrap();
+		println!("{}", MessageQueue::debug_info());
 		// service head is 1, it will process a, leaving service head at 2. it also processes b and
 		// empties queue 2, so service head will end at 3.
 		assert_eq!(MessageQueue::service_queues(2.into_weight()), 2.into_weight());
@@ -135,7 +137,7 @@ fn queue_priority_reset_once_serviced() {
 #[test]
 fn service_queues_basic_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		MessageQueue::enqueue_messages(vec![msg("a"), msg("ab"), msg("abc")].into_iter(), Here);
 		MessageQueue::enqueue_messages(vec![msg("x"), msg("xy"), msg("xyz")].into_iter(), There);
 		assert_eq!(QueueChanges::take(), vec![(Here, 3, 6), (There, 3, 6)]);
@@ -159,13 +161,14 @@ fn service_queues_basic_works() {
 		assert_eq!(MessageQueue::service_queues(Weight::MAX), 2.into_weight());
 		assert_eq!(MessagesProcessed::take(), vec![(vmsg("xy"), There), (vmsg("xyz"), There)]);
 		assert_eq!(QueueChanges::take(), vec![(There, 0, 0)]);
+		MessageQueue::do_try_state().unwrap();
 	});
 }
 
 #[test]
 fn service_queues_failing_messages_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("service_page_item", 1.into_weight());
 		MessageQueue::enqueue_message(msg("badformat"), Here);
 		MessageQueue::enqueue_message(msg("corrupt"), Here);
@@ -211,7 +214,7 @@ fn service_queues_failing_messages_works() {
 #[test]
 fn service_queues_suspension_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		MessageQueue::enqueue_messages(vec![msg("a"), msg("b"), msg("c")].into_iter(), Here);
 		MessageQueue::enqueue_messages(vec![msg("x"), msg("y"), msg("z")].into_iter(), There);
 		MessageQueue::enqueue_messages(
@@ -266,7 +269,7 @@ fn service_queues_suspension_works() {
 #[test]
 fn reap_page_permanent_overweight_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		// Create 10 pages more than the stale limit.
 		let n = (MaxStale::get() + 10) as usize;
 		for _ in 0..n {
@@ -306,7 +309,7 @@ fn reaping_overweight_fails_properly() {
 	use MessageOrigin::*;
 	assert_eq!(MaxStale::get(), 2, "The stale limit is two");
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		// page 0
 		MessageQueue::enqueue_message(msg("weight=4"), Here);
 		MessageQueue::enqueue_message(msg("a"), Here);
@@ -376,7 +379,7 @@ fn reaping_overweight_fails_properly() {
 #[test]
 fn service_queue_bails() {
 	// Not enough weight for `service_queue_base`.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("service_queue_base", 2.into_weight());
 		let mut meter = WeightMeter::from_limit(1.into_weight());
 
@@ -384,7 +387,7 @@ fn service_queue_bails() {
 		assert!(meter.consumed().is_zero());
 	});
 	// Not enough weight for `ready_ring_unknit`.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("ready_ring_unknit", 2.into_weight());
 		let mut meter = WeightMeter::from_limit(1.into_weight());
 
@@ -392,7 +395,7 @@ fn service_queue_bails() {
 		assert!(meter.consumed().is_zero());
 	});
 	// Not enough weight for `service_queue_base` and `ready_ring_unknit`.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("service_queue_base", 2.into_weight());
 		set_weight("ready_ring_unknit", 2.into_weight());
 
@@ -407,7 +410,7 @@ fn service_page_works() {
 	use super::integration_test::Test; // Run with larger page size.
 	use MessageOrigin::*;
 	use PageExecutionStatus::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("service_page_base_completion", 2.into_weight());
 		set_weight("service_page_item", 3.into_weight());
 
@@ -444,7 +447,7 @@ fn service_page_works() {
 #[test]
 fn service_page_bails() {
 	// Not enough weight for `service_page_base_completion`.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("service_page_base_completion", 2.into_weight());
 		let mut meter = WeightMeter::from_limit(1.into_weight());
 
@@ -461,7 +464,7 @@ fn service_page_bails() {
 		assert!(meter.consumed().is_zero());
 	});
 	// Not enough weight for `service_page_base_no_completion`.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("service_page_base_no_completion", 2.into_weight());
 		let mut meter = WeightMeter::from_limit(1.into_weight());
 
@@ -481,7 +484,7 @@ fn service_page_bails() {
 
 #[test]
 fn service_page_item_bails() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let _guard = StorageNoopGuard::default();
 		let (mut page, _) = full_page::<Test>();
 		let mut weight = WeightMeter::from_limit(10.into_weight());
@@ -508,7 +511,7 @@ fn service_page_suspension_works() {
 	use MessageOrigin::*;
 	use PageExecutionStatus::*;
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let (page, mut msgs) = full_page::<Test>();
 		assert!(msgs >= 10, "pre-condition: need at least 10 msgs per page");
 		let mut book = book_for::<Test>(&page);
@@ -556,14 +559,8 @@ fn service_page_suspension_works() {
 #[test]
 fn bump_service_head_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
-		// Create a ready ring with three queues.
-		BookStateFor::<Test>::insert(Here, empty_book::<Test>());
-		knit(&Here);
-		BookStateFor::<Test>::insert(There, empty_book::<Test>());
-		knit(&There);
-		BookStateFor::<Test>::insert(Everywhere(0), empty_book::<Test>());
-		knit(&Everywhere(0));
+	build_and_execute::<Test>(|| {
+		build_triple_ring();
 
 		// Bump 99 times.
 		for i in 0..99 {
@@ -579,9 +576,9 @@ fn bump_service_head_works() {
 /// `bump_service_head` does nothing when called with an insufficient weight limit.
 #[test]
 fn bump_service_head_bails() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("bump_service_head", 2.into_weight());
-		setup_bump_service_head::<Test>(0.into(), 10.into());
+		setup_bump_service_head::<Test>(0.into(), 1.into());
 
 		let _guard = StorageNoopGuard::default();
 		let mut meter = WeightMeter::from_limit(1.into_weight());
@@ -592,7 +589,7 @@ fn bump_service_head_bails() {
 
 #[test]
 fn bump_service_head_trivial_works() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("bump_service_head", 2.into_weight());
 		let mut meter = WeightMeter::max_limit();
 
@@ -605,22 +602,15 @@ fn bump_service_head_trivial_works() {
 		assert_eq!(ServiceHead::<Test>::get().unwrap(), 1.into(), "Bumped the head");
 		assert_eq!(meter.consumed(), 4.into_weight());
 
-		assert_eq!(MessageQueue::bump_service_head(&mut meter), None, "Cannot bump");
+		assert_eq!(MessageQueue::bump_service_head(&mut meter), Some(1.into()), "Its a ring");
 		assert_eq!(meter.consumed(), 6.into_weight());
 	});
 }
 
 #[test]
 fn bump_service_head_no_head_noops() {
-	use MessageOrigin::*;
-	test_closure(|| {
-		// Create a ready ring with three queues.
-		BookStateFor::<Test>::insert(Here, empty_book::<Test>());
-		knit(&Here);
-		BookStateFor::<Test>::insert(There, empty_book::<Test>());
-		knit(&There);
-		BookStateFor::<Test>::insert(Everywhere(0), empty_book::<Test>());
-		knit(&Everywhere(0));
+	build_and_execute::<Test>(|| {
+		build_triple_ring();
 
 		// But remove the service head.
 		ServiceHead::<Test>::kill();
@@ -632,7 +622,7 @@ fn bump_service_head_no_head_noops() {
 
 #[test]
 fn service_page_item_consumes_correct_weight() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let mut page = page::<Test>(b"weight=3");
 		let mut weight = WeightMeter::from_limit(10.into_weight());
 		let overweight_limit = 0.into_weight();
@@ -656,7 +646,7 @@ fn service_page_item_consumes_correct_weight() {
 /// `service_page_item` skips a permanently `Overweight` message and marks it as `unprocessed`.
 #[test]
 fn service_page_item_skips_perm_overweight_message() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let mut page = page::<Test>(b"TooMuch");
 		let mut weight = WeightMeter::from_limit(2.into_weight());
 		let overweight_limit = 0.into_weight();
@@ -695,7 +685,7 @@ fn service_page_item_skips_perm_overweight_message() {
 #[test]
 fn peek_index_works() {
 	use super::integration_test::Test; // Run with larger page size.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		// Fill a page with messages.
 		let (mut page, msgs) = full_page::<Test>();
 		let msg_enc_len = ItemHeader::<<Test as Config>::Size>::max_encoded_len() + 4;
@@ -716,7 +706,7 @@ fn peek_index_works() {
 #[test]
 fn peek_first_and_skip_first_works() {
 	use super::integration_test::Test; // Run with larger page size.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		// Fill a page with messages.
 		let (mut page, msgs) = full_page::<Test>();
 
@@ -739,7 +729,7 @@ fn peek_first_and_skip_first_works() {
 #[test]
 fn note_processed_at_pos_works() {
 	use super::integration_test::Test; // Run with larger page size.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let (mut page, msgs) = full_page::<Test>();
 
 		for i in 0..msgs {
@@ -775,7 +765,7 @@ fn note_processed_at_pos_idempotent() {
 #[test]
 fn is_complete_works() {
 	use super::integration_test::Test; // Run with larger page size.
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let (mut page, msgs) = full_page::<Test>();
 		assert!(msgs > 3, "Boring");
 		let msg_enc_len = ItemHeader::<<Test as Config>::Size>::max_encoded_len() + 4;
@@ -931,8 +921,9 @@ fn page_from_message_max_len_works() {
 #[test]
 fn sweep_queue_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		build_triple_ring();
+		QueueChanges::take();
 
 		let book = BookStateFor::<Test>::get(Here);
 		assert!(book.begin != book.end);
@@ -967,9 +958,8 @@ fn sweep_queue_works() {
 #[test]
 fn sweep_queue_wraps_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
-		BookStateFor::<Test>::insert(Here, empty_book::<Test>());
-		knit(&Here);
+	build_and_execute::<Test>(|| {
+		build_ring::<Test>(&[Here]);
 
 		MessageQueue::sweep_queue(Here);
 		let book = BookStateFor::<Test>::get(Here);
@@ -980,14 +970,14 @@ fn sweep_queue_wraps_works() {
 #[test]
 fn sweep_queue_invalid_noops() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		assert_storage_noop!(MessageQueue::sweep_queue(Here));
 	});
 }
 
 #[test]
 fn footprint_works() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let origin = MessageOrigin::Here;
 		let (page, msgs) = full_page::<Test>();
 		let book = book_for::<Test>(&page);
@@ -1005,7 +995,7 @@ fn footprint_works() {
 /// The footprint of an invalid queue is the default footprint.
 #[test]
 fn footprint_invalid_works() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let origin = MessageOrigin::Here;
 		assert_eq!(MessageQueue::footprint(origin), Default::default());
 	})
@@ -1015,7 +1005,7 @@ fn footprint_invalid_works() {
 #[test]
 fn footprint_on_swept_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let mut book = empty_book::<Test>();
 		book.message_count = 3;
 		book.size = 10;
@@ -1031,7 +1021,7 @@ fn footprint_on_swept_works() {
 
 #[test]
 fn execute_overweight_works() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("bump_service_head", 1.into_weight());
 		set_weight("service_queue_base", 1.into_weight());
 		set_weight("service_page_base_completion", 1.into_weight());
@@ -1091,7 +1081,7 @@ fn execute_overweight_works() {
 fn permanently_overweight_book_unknits() {
 	use MessageOrigin::*;
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("bump_service_head", 1.into_weight());
 		set_weight("service_queue_base", 1.into_weight());
 		set_weight("service_page_base_completion", 1.into_weight());
@@ -1128,7 +1118,7 @@ fn permanently_overweight_book_unknits() {
 fn permanently_overweight_book_unknits_multiple() {
 	use MessageOrigin::*;
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		set_weight("bump_service_head", 1.into_weight());
 		set_weight("service_queue_base", 1.into_weight());
 		set_weight("service_page_base_completion", 1.into_weight());
@@ -1167,7 +1157,7 @@ fn permanently_overweight_book_unknits_multiple() {
 fn ready_but_empty_does_not_panic() {
 	use MessageOrigin::*;
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		BookStateFor::<Test>::insert(Here, empty_book::<Test>());
 		BookStateFor::<Test>::insert(There, empty_book::<Test>());
 
@@ -1187,7 +1177,7 @@ fn ready_but_empty_does_not_panic() {
 fn ready_but_perm_overweight_does_not_panic() {
 	use MessageOrigin::*;
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		MessageQueue::enqueue_message(msg("weight=9"), Here);
 		assert_eq!(MessageQueue::service_queues(8.into_weight()), 0.into_weight());
 		assert_ring(&[]);
@@ -1207,7 +1197,7 @@ fn ready_but_perm_overweight_does_not_panic() {
 fn ready_ring_knit_basic_works() {
 	use MessageOrigin::*;
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		BookStateFor::<Test>::insert(Here, empty_book::<Test>());
 
 		for i in 0..10 {
@@ -1227,7 +1217,7 @@ fn ready_ring_knit_basic_works() {
 fn ready_ring_knit_and_unknit_works() {
 	use MessageOrigin::*;
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		// Place three queues into the storage.
 		BookStateFor::<Test>::insert(Here, empty_book::<Test>());
 		BookStateFor::<Test>::insert(There, empty_book::<Test>());
@@ -1261,7 +1251,7 @@ fn enqueue_message_works() {
 	let max_msg_per_page = <Test as Config>::HeapSize::get() as u64 /
 		(ItemHeader::<<Test as Config>::Size>::max_encoded_len() as u64 + 1);
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		// Enqueue messages which should fill three pages.
 		let n = max_msg_per_page * 3;
 		for i in 1..=n {
@@ -1291,7 +1281,7 @@ fn enqueue_messages_works() {
 	let max_msg_per_page = <Test as Config>::HeapSize::get() as u64 /
 		(ItemHeader::<<Test as Config>::Size>::max_encoded_len() as u64 + 1);
 
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		// Enqueue messages which should fill three pages.
 		let n = max_msg_per_page * 3;
 		let msgs = vec![msg("a"); n as usize];
@@ -1320,7 +1310,7 @@ fn enqueue_messages_works() {
 #[test]
 fn service_queues_suspend_works() {
 	use MessageOrigin::*;
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		MessageQueue::enqueue_messages(vec![msg("a"), msg("ab"), msg("abc")].into_iter(), Here);
 		MessageQueue::enqueue_messages(vec![msg("x"), msg("xy"), msg("xyz")].into_iter(), There);
 		assert_eq!(QueueChanges::take(), vec![(Here, 3, 6), (There, 3, 6)]);
@@ -1387,7 +1377,7 @@ fn service_queues_suspend_works() {
 /// Tests that manual overweight execution on a suspended queue errors with `QueueSuspended`.
 #[test]
 fn execute_overweight_respects_suspension() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let origin = MessageOrigin::Here;
 		MessageQueue::enqueue_message(msg("weight=5"), origin);
 		// Mark the message as permanently overweight.
@@ -1433,7 +1423,7 @@ fn execute_overweight_respects_suspension() {
 
 #[test]
 fn service_queue_suspension_ready_ring_works() {
-	test_closure(|| {
+	build_and_execute::<Test>(|| {
 		let origin = MessageOrigin::Here;
 		PausedQueues::set(vec![origin]);
 		MessageQueue::enqueue_message(msg("weight=5"), origin);