From c28ec0fb32876fbe9bd635c20baed96fcb69ea5e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 5 Sep 2024 18:03:54 +0300 Subject: [PATCH] [stable2407] Backport #5581 (#5603) Backport #5581 into `stable2407` (cc @franciscoaguirre). 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 commit 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: Francisco Aguirre --- .../modules/xcm-bridge-hub-router/src/lib.rs | 4 +++ .../modules/xcm-bridge-hub-router/src/mock.rs | 4 +++ cumulus/pallets/parachain-system/src/lib.rs | 4 +++ cumulus/pallets/xcmp-queue/src/lib.rs | 5 ++++ cumulus/primitives/utility/src/lib.rs | 4 +++ polkadot/runtime/common/src/xcm_sender.rs | 5 ++++ polkadot/xcm/pallet-xcm/src/lib.rs | 6 +++- polkadot/xcm/xcm-builder/src/routing.rs | 13 ++++++++ .../xcm/xcm-builder/src/universal_exports.rs | 4 +++ prdoc/pr_5603.prdoc | 30 +++++++++++++++++++ 10 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 prdoc/pr_5603.prdoc diff --git a/bridges/modules/xcm-bridge-hub-router/src/lib.rs b/bridges/modules/xcm-bridge-hub-router/src/lib.rs index 607394603466..8b86029ab8ff 100644 --- 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, I: 'static> SendXcm for Pallet { } impl, I: 'static> InspectMessageQueues for Pallet { + fn clear_messages() { + ViaBridgeHubExporter::::clear_messages() + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { ViaBridgeHubExporter::::get_messages() } diff --git a/bridges/modules/xcm-bridge-hub-router/src/mock.rs b/bridges/modules/xcm-bridge-hub-router/src/mock.rs index 3e2c1bb369cb..276b9be5f743 100644 --- 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>)> { SENT_XCM.with(|q| { (*q.borrow()) diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 9e0a68d09a14..7f0bfceff252 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -1611,6 +1611,10 @@ impl UpwardMessageSender for Pallet { } impl InspectMessageQueues for Pallet { + fn clear_messages() { + PendingUpwardMessages::::kill(); + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { use xcm::prelude::*; diff --git a/cumulus/pallets/xcmp-queue/src/lib.rs b/cumulus/pallets/xcmp-queue/src/lib.rs index 8c4446a925d4..732ee94f3e10 100644 --- a/cumulus/pallets/xcmp-queue/src/lib.rs +++ b/cumulus/pallets/xcmp-queue/src/lib.rs @@ -1008,6 +1008,11 @@ impl SendXcm for Pallet { } impl InspectMessageQueues for Pallet { + fn clear_messages() { + // Best effort. + let _ = OutboundXcmpMessages::::clear(u32::MAX, None); + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { use xcm::prelude::*; diff --git a/cumulus/primitives/utility/src/lib.rs b/cumulus/primitives/utility/src/lib.rs index 9d5bf4e231eb..760a0c41493e 100644 --- a/cumulus/primitives/utility/src/lib.rs +++ b/cumulus/primitives/utility/src/lib.rs @@ -99,6 +99,10 @@ where impl InspectMessageQueues for ParentAsUmp { + fn clear_messages() { + T::clear_messages(); + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { T::get_messages() } diff --git a/polkadot/runtime/common/src/xcm_sender.rs b/polkadot/runtime/common/src/xcm_sender.rs index dace785a535b..37fe7f0b59e9 100644 --- a/polkadot/runtime/common/src/xcm_sender.rs +++ b/polkadot/runtime/common/src/xcm_sender.rs @@ -141,6 +141,11 @@ where } impl InspectMessageQueues for ChildParachainRouter { + fn clear_messages() { + // Best effort. + let _ = dmp::DownwardMessageQueues::::clear(u32::MAX, None); + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { dmp::DownwardMessageQueues::::iter() .map(|(para_id, messages)| { diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 6451901279b1..05d9046ab192 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -2457,10 +2457,14 @@ impl Pallet { ::RuntimeOrigin: From, { crate::Pallet::::set_record_xcm(true); - frame_system::Pallet::::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::::reset_events(); let result = call.dispatch(origin.into()); crate::Pallet::::set_record_xcm(false); let local_xcm = crate::Pallet::::recorded_xcm(); + // Should only get messages from this call since we cleared previous ones. let forwarded_xcms = Router::get_messages(); let events: Vec<::RuntimeEvent> = frame_system::Pallet::::read_events_no_consensus() diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs index 03ef780ef032..d82cc0641ec4 100644 --- a/polkadot/xcm/xcm-builder/src/routing.rs +++ b/polkadot/xcm/xcm-builder/src/routing.rs @@ -62,6 +62,10 @@ impl SendXcm for WithUniqueTopic { } } impl InspectMessageQueues for WithUniqueTopic { + fn clear_messages() { + Inner::clear_messages() + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { 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>)>; } #[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>)> { 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 index 8aa9602fcc29..30e0b7c72b03 100644 --- a/polkadot/xcm/xcm-builder/src/universal_exports.rs +++ b/polkadot/xcm/xcm-builder/src/universal_exports.rs @@ -340,6 +340,10 @@ impl InspectMessageQueues for SovereignPaidRemoteExporter { + fn clear_messages() { + Router::clear_messages() + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { Router::get_messages() } diff --git a/prdoc/pr_5603.prdoc b/prdoc/pr_5603.prdoc new file mode 100644 index 000000000000..17db32db0ebc --- /dev/null +++ b/prdoc/pr_5603.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: patch + - name: staging-xcm-builder + bump: patch + - name: pallet-xcm-bridge-hub-router + bump: patch + - name: cumulus-pallet-parachain-system + bump: patch + - name: cumulus-pallet-xcmp-queue + bump: patch + - name: cumulus-primitives-utility + bump: patch + - name: polkadot-runtime-common + bump: patch + - name: pallet-xcm-bridge-hub + bump: patch