Clear other messages before dry-run to get only the ones produced dur…

…ing (#5581) The dry-run shows in `forwarded_xcms` all the messages in the queues at the time of calling the API. Each time the API is called, the result could be different. You could get messages even if you dry-run something that doesn't send a message, like a `System::remark`. This PR fixes this by clearing the message queues before doing the dry-run, so the only messages left are the ones the users of the API actually care about. --------- Co-authored-by: Adrian Catangiu <[email protected]> (cherry picked from commit 8d9ebcd)
paritytech · Sep 5, 2024 · 7dd97e4 · 7dd97e4
1 parent d6f482d
commit 7dd97e4
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 1 deletion.
diff --git a/bridges/modules/xcm-bridge-hub-router/src/lib.rs b/bridges/modules/xcm-bridge-hub-router/src/lib.rs
@@ -398,6 +398,10 @@ impl<T: Config<I>, I: 'static> SendXcm for Pallet<T, I> {
 }
 
 impl<T: Config<I>, I: 'static> InspectMessageQueues for Pallet<T, I> {
+	fn clear_messages() {
+		ViaBridgeHubExporter::<T, I>::clear_messages()
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		ViaBridgeHubExporter::<T, I>::get_messages()
 	}

diff --git a/bridges/modules/xcm-bridge-hub-router/src/mock.rs b/bridges/modules/xcm-bridge-hub-router/src/mock.rs
@@ -131,6 +131,10 @@ impl SendXcm for TestToBridgeHubSender {
 }
 
 impl InspectMessageQueues for TestToBridgeHubSender {
+	fn clear_messages() {
+		SENT_XCM.with(|q| q.borrow_mut().clear());
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		SENT_XCM.with(|q| {
 			(*q.borrow())

diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs
@@ -1611,6 +1611,10 @@ impl<T: Config> UpwardMessageSender for Pallet<T> {
 }
 
 impl<T: Config> InspectMessageQueues for Pallet<T> {
+	fn clear_messages() {
+		PendingUpwardMessages::<T>::kill();
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		use xcm::prelude::*;
 

diff --git a/cumulus/pallets/xcmp-queue/src/lib.rs b/cumulus/pallets/xcmp-queue/src/lib.rs
@@ -1008,6 +1008,11 @@ impl<T: Config> SendXcm for Pallet<T> {
 }
 
 impl<T: Config> InspectMessageQueues for Pallet<T> {
+	fn clear_messages() {
+		// Best effort.
+		let _ = OutboundXcmpMessages::<T>::clear(u32::MAX, None);
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		use xcm::prelude::*;
 

diff --git a/cumulus/primitives/utility/src/lib.rs b/cumulus/primitives/utility/src/lib.rs
@@ -99,6 +99,10 @@ where
 impl<T: UpwardMessageSender + InspectMessageQueues, W, P> InspectMessageQueues
 	for ParentAsUmp<T, W, P>
 {
+	fn clear_messages() {
+		T::clear_messages();
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		T::get_messages()
 	}

diff --git a/polkadot/runtime/common/src/xcm_sender.rs b/polkadot/runtime/common/src/xcm_sender.rs
@@ -141,6 +141,11 @@ where
 }
 
 impl<T: dmp::Config, W, P> InspectMessageQueues for ChildParachainRouter<T, W, P> {
+	fn clear_messages() {
+		// Best effort.
+		let _ = dmp::DownwardMessageQueues::<T>::clear(u32::MAX, None);
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		dmp::DownwardMessageQueues::<T>::iter()
 			.map(|(para_id, messages)| {

diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs
@@ -2457,10 +2457,14 @@ impl<T: Config> Pallet<T> {
 		<RuntimeCall as Dispatchable>::RuntimeOrigin: From<OriginCaller>,
 	{
 		crate::Pallet::<Runtime>::set_record_xcm(true);
-		frame_system::Pallet::<Runtime>::reset_events(); // To make sure we only record events from current call.
+		// Clear other messages in queues...
+		Router::clear_messages();
+		// ...and reset events to make sure we only record events from current call.
+		frame_system::Pallet::<Runtime>::reset_events();
 		let result = call.dispatch(origin.into());
 		crate::Pallet::<Runtime>::set_record_xcm(false);
 		let local_xcm = crate::Pallet::<Runtime>::recorded_xcm();
+		// Should only get messages from this call since we cleared previous ones.
 		let forwarded_xcms = Router::get_messages();
 		let events: Vec<<Runtime as frame_system::Config>::RuntimeEvent> =
 			frame_system::Pallet::<Runtime>::read_events_no_consensus()

diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs
@@ -62,6 +62,10 @@ impl<Inner: SendXcm> SendXcm for WithUniqueTopic<Inner> {
 	}
 }
 impl<Inner: InspectMessageQueues> InspectMessageQueues for WithUniqueTopic<Inner> {
+	fn clear_messages() {
+		Inner::clear_messages()
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		Inner::get_messages()
 	}
@@ -149,12 +153,21 @@ impl EnsureDelivery for Tuple {
 /// Inspects messages in queues.
 /// Meant to be used in runtime APIs, not in runtimes.
 pub trait InspectMessageQueues {
+	/// Clear the queues at the beginning of Runtime API call, so that subsequent
+	/// `Self::get_messages()` will return only messages generated by said Runtime API.
+	fn clear_messages();
 	/// Get queued messages and their destinations.
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)>;
 }
 
 #[impl_trait_for_tuples::impl_for_tuples(30)]
 impl InspectMessageQueues for Tuple {
+	fn clear_messages() {
+		for_tuples!( #(
+			Tuple::clear_messages();
+		)* );
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		let mut messages = Vec::new();
 

diff --git a/polkadot/xcm/xcm-builder/src/universal_exports.rs b/polkadot/xcm/xcm-builder/src/universal_exports.rs
@@ -340,6 +340,10 @@ impl<Bridges: ExporterFor, Router: SendXcm, UniversalLocation: Get<InteriorLocat
 impl<Bridges, Router: InspectMessageQueues, UniversalLocation> InspectMessageQueues
 	for SovereignPaidRemoteExporter<Bridges, Router, UniversalLocation>
 {
+	fn clear_messages() {
+		Router::clear_messages()
+	}
+
 	fn get_messages() -> Vec<(VersionedLocation, Vec<VersionedXcm<()>>)> {
 		Router::get_messages()
 	}

diff --git a/prdoc/pr_5581.prdoc b/prdoc/pr_5581.prdoc
@@ -0,0 +1,30 @@
+# 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: Clear other messages before dry-running
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      The DryRunApi.dry_run_call and DryRunApi.dry_run_xcm functions used to populate
+      `forwarded_xcms` with all the existing messages in the queues at the time.
+      Now, existing (irrelevant) messages are cleared when dry-running, meaning only the
+      messages produced by the dry-run call (or xcm) will be returned in `forwarded_xcms`.
+
+crates:
+  - name: pallet-xcm
+    bump: minor
+  - name: staging-xcm-builder
+    bump: major
+  - name: pallet-xcm-bridge-hub-router
+    bump: minor
+  - name: cumulus-pallet-parachain-system
+    bump: minor
+  - name: cumulus-pallet-xcmp-queue
+    bump: minor
+  - name: cumulus-primitives-utility
+    bump: minor
+  - name: polkadot-runtime-common
+    bump: minor
+  - name: pallet-xcm-bridge-hub
+    bump: minor