From d52d02e9b40e0f9ea92914935ff353f880e5b610 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 09:36:19 +0200 Subject: [PATCH 01/12] fix(dry-run-api): clear messages before dry-run --- bridges/modules/xcm-bridge-hub-router/src/lib.rs | 4 ++++ bridges/modules/xcm-bridge-hub-router/src/mock.rs | 4 ++++ bridges/modules/xcm-bridge-hub/src/mock.rs | 4 ++++ cumulus/pallets/parachain-system/src/lib.rs | 4 ++++ cumulus/pallets/xcmp-queue/src/lib.rs | 8 +++++++- cumulus/primitives/utility/src/lib.rs | 4 ++++ polkadot/runtime/common/src/xcm_sender.rs | 8 +++++++- polkadot/xcm/pallet-xcm/src/lib.rs | 2 ++ polkadot/xcm/xcm-builder/src/routing.rs | 12 ++++++++++++ polkadot/xcm/xcm-builder/src/universal_exports.rs | 4 ++++ 10 files changed, 52 insertions(+), 2 deletions(-) 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..4939b1524ef7 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/bridges/modules/xcm-bridge-hub/src/mock.rs b/bridges/modules/xcm-bridge-hub/src/mock.rs index 0cca32ba9e5f..53dc777d67ba 100644 --- a/bridges/modules/xcm-bridge-hub/src/mock.rs +++ b/bridges/modules/xcm-bridge-hub/src/mock.rs @@ -281,6 +281,10 @@ impl SendXcm for TestExportXcmWithXcmOverBridge { } } impl InspectMessageQueues for TestExportXcmWithXcmOverBridge { + fn clear_messages() { + todo!() + } + fn get_messages() -> Vec<(VersionedLocation, Vec>)> { todo!() } diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index bf136dc0644c..882dcb68fbbe 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -1540,6 +1540,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..3748ce9d9bb6 100644 --- a/cumulus/pallets/xcmp-queue/src/lib.rs +++ b/cumulus/pallets/xcmp-queue/src/lib.rs @@ -64,7 +64,7 @@ use frame_support::{ defensive, defensive_assert, traits::{Defensive, EnqueueMessage, EnsureOrigin, Get, QueueFootprint, QueuePausedQuery}, weights::{Weight, WeightMeter}, - BoundedVec, + BoundedVec, StoragePrefixedMap, }; use pallet_message_queue::OnQueueChanged; use polkadot_runtime_common::xcm_sender::PriceForMessageDelivery; @@ -1008,6 +1008,12 @@ impl SendXcm for Pallet { } impl InspectMessageQueues for Pallet { + fn clear_messages() { + let prefix = OutboundXcmpMessages::::final_prefix(); + // Best effort. + let _ = frame_support::storage::unhashed::clear_prefix(&prefix, None, 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 3ebcb44fa439..e568c79bb6a0 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..ca3b99e99acd 100644 --- a/polkadot/runtime/common/src/xcm_sender.rs +++ b/polkadot/runtime/common/src/xcm_sender.rs @@ -19,7 +19,7 @@ use alloc::vec::Vec; use codec::{Decode, Encode}; use core::marker::PhantomData; -use frame_support::traits::Get; +use frame_support::{traits::Get, StoragePrefixedMap}; use frame_system::pallet_prelude::BlockNumberFor; use polkadot_primitives::Id as ParaId; use polkadot_runtime_parachains::{ @@ -141,6 +141,12 @@ where } impl InspectMessageQueues for ChildParachainRouter { + fn clear_messages() { + let prefix = dmp::DownwardMessageQueues::::final_prefix(); + // Best effort. + let _ = frame_support::storage::unhashed::clear_prefix(&prefix, None, 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..6ddaa777e37a 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -2457,10 +2457,12 @@ impl Pallet { ::RuntimeOrigin: From, { crate::Pallet::::set_record_xcm(true); + Router::clear_messages(); // Clear other messages in queues. frame_system::Pallet::::reset_events(); // To make sure we only record events from current call. 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..fff83d1d60de 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,20 @@ impl EnsureDelivery for Tuple { /// Inspects messages in queues. /// Meant to be used in runtime APIs, not in runtimes. pub trait InspectMessageQueues { + /// Clear the queues, so `Self::get_messages()` can return only the desired events. + 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() } From 99f6b6149bb03dfb8f17a96430c5dcc1a6df29a2 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 10:49:42 +0200 Subject: [PATCH 02/12] chore(pallet-xcm): style --- polkadot/xcm/pallet-xcm/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 6ddaa777e37a..05d9046ab192 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -2457,8 +2457,10 @@ impl Pallet { ::RuntimeOrigin: From, { crate::Pallet::::set_record_xcm(true); - Router::clear_messages(); // Clear other messages in queues. - 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(); From d1db39a1ce19aface8a88ef9fc21aa907de798bd Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 10:50:04 +0200 Subject: [PATCH 03/12] chore(xcm-builder): add default implementation for clear_messages --- polkadot/xcm/xcm-builder/src/routing.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs index fff83d1d60de..59861d350ada 100644 --- a/polkadot/xcm/xcm-builder/src/routing.rs +++ b/polkadot/xcm/xcm-builder/src/routing.rs @@ -153,8 +153,9 @@ impl EnsureDelivery for Tuple { /// Inspects messages in queues. /// Meant to be used in runtime APIs, not in runtimes. pub trait InspectMessageQueues { - /// Clear the queues, so `Self::get_messages()` can return only the desired events. - fn clear_messages(); + /// 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>)>; } From c634a15c879c8eecc2d27e20d260a8f5fcd4674e Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 10:50:09 +0200 Subject: [PATCH 04/12] doc: prdoc --- prdoc/pr_5581.prdoc | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 prdoc/pr_5581.prdoc diff --git a/prdoc/pr_5581.prdoc b/prdoc/pr_5581.prdoc new file mode 100644 index 000000000000..af12a9adb040 --- /dev/null +++ b/prdoc/pr_5581.prdoc @@ -0,0 +1,28 @@ +# 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 just returned + in forwarded_xcms all the existing messages in the queues at the time. + Now messages are cleared before dry-running, meaning only the messages produced + by the call (or xcm) will be returned. + +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 From 3aab9965c5766457d1c2a1e0df0a0e2faa0e0519 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 10:51:04 +0200 Subject: [PATCH 05/12] fix(xcm-builder): remove semicolon --- polkadot/xcm/xcm-builder/src/routing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs index 59861d350ada..d82cc0641ec4 100644 --- a/polkadot/xcm/xcm-builder/src/routing.rs +++ b/polkadot/xcm/xcm-builder/src/routing.rs @@ -155,7 +155,7 @@ impl EnsureDelivery for Tuple { 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() {}; + fn clear_messages() {} /// Get queued messages and their destinations. fn get_messages() -> Vec<(VersionedLocation, Vec>)>; } From 16ab9c5e7ce8acc94598f277f1afc742bf974f93 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 11:14:33 +0200 Subject: [PATCH 06/12] fix(pallet-xcm-bridge-hub-router): remove dereference --- bridges/modules/xcm-bridge-hub-router/src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/modules/xcm-bridge-hub-router/src/mock.rs b/bridges/modules/xcm-bridge-hub-router/src/mock.rs index 4d2a16f55db1..bb265e1925a2 100644 --- a/bridges/modules/xcm-bridge-hub-router/src/mock.rs +++ b/bridges/modules/xcm-bridge-hub-router/src/mock.rs @@ -131,7 +131,7 @@ impl SendXcm for TestToBridgeHubSender { impl InspectMessageQueues for TestToBridgeHubSender { fn clear_messages() { - SENT_XCM.with(|q| *q.borrow_mut().clear()); + SENT_XCM.with(|q| q.borrow_mut().clear()); } fn get_messages() -> Vec<(VersionedLocation, Vec>)> { From 7143410f841177632f00911b449c414a4aec778f Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 18:12:25 +0200 Subject: [PATCH 07/12] revert(xcm-builder): remove clear_messages default implementation --- polkadot/xcm/xcm-builder/src/routing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs index d82cc0641ec4..fc2de89d2128 100644 --- a/polkadot/xcm/xcm-builder/src/routing.rs +++ b/polkadot/xcm/xcm-builder/src/routing.rs @@ -155,7 +155,7 @@ impl EnsureDelivery for Tuple { 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() {} + fn clear_messages(); /// Get queued messages and their destinations. fn get_messages() -> Vec<(VersionedLocation, Vec>)>; } From 4a3f852e3327162be8a39f54356fcb80efd2109a Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 18:14:06 +0200 Subject: [PATCH 08/12] doc: update prdoc --- prdoc/pr_5581.prdoc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/prdoc/pr_5581.prdoc b/prdoc/pr_5581.prdoc index af12a9adb040..5b941242801b 100644 --- a/prdoc/pr_5581.prdoc +++ b/prdoc/pr_5581.prdoc @@ -13,16 +13,16 @@ doc: crates: - name: pallet-xcm - bump: patch + bump: minor - name: staging-xcm-builder - bump: patch + bump: major - name: pallet-xcm-bridge-hub-router - bump: patch + bump: minor - name: cumulus-pallet-parachain-system - bump: patch + bump: minor - name: cumulus-pallet-xcmp-queue - bump: patch + bump: minor - name: cumulus-primitives-utility - bump: patch + bump: minor - name: polkadot-runtime-common - bump: patch + bump: minor From 4954486c9986756fbfb1ded97fe1c8a7a9ac44a5 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 18:23:59 +0200 Subject: [PATCH 09/12] chore: use StorageMap::clear() instead of raw storage APIs --- cumulus/pallets/xcmp-queue/src/lib.rs | 3 +-- polkadot/runtime/common/src/xcm_sender.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cumulus/pallets/xcmp-queue/src/lib.rs b/cumulus/pallets/xcmp-queue/src/lib.rs index 3748ce9d9bb6..cbf3feac0e6f 100644 --- a/cumulus/pallets/xcmp-queue/src/lib.rs +++ b/cumulus/pallets/xcmp-queue/src/lib.rs @@ -1009,9 +1009,8 @@ impl SendXcm for Pallet { impl InspectMessageQueues for Pallet { fn clear_messages() { - let prefix = OutboundXcmpMessages::::final_prefix(); // Best effort. - let _ = frame_support::storage::unhashed::clear_prefix(&prefix, None, None); + let _ = OutboundXcmpMessages::::clear(u32::MAX, None); } fn get_messages() -> Vec<(VersionedLocation, Vec>)> { diff --git a/polkadot/runtime/common/src/xcm_sender.rs b/polkadot/runtime/common/src/xcm_sender.rs index ca3b99e99acd..383a914e98f5 100644 --- a/polkadot/runtime/common/src/xcm_sender.rs +++ b/polkadot/runtime/common/src/xcm_sender.rs @@ -142,9 +142,8 @@ where impl InspectMessageQueues for ChildParachainRouter { fn clear_messages() { - let prefix = dmp::DownwardMessageQueues::::final_prefix(); // Best effort. - let _ = frame_support::storage::unhashed::clear_prefix(&prefix, None, None); + let _ = dmp::DownwardMessageQueues::::clear(u32::MAX, None); } fn get_messages() -> Vec<(VersionedLocation, Vec>)> { From 2d8930f1e83ffb4e66f4239f19bc2f694ef856f0 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 4 Sep 2024 20:00:28 +0200 Subject: [PATCH 10/12] fix: remove unused imports --- cumulus/pallets/xcmp-queue/src/lib.rs | 2 +- polkadot/runtime/common/src/xcm_sender.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/pallets/xcmp-queue/src/lib.rs b/cumulus/pallets/xcmp-queue/src/lib.rs index cbf3feac0e6f..732ee94f3e10 100644 --- a/cumulus/pallets/xcmp-queue/src/lib.rs +++ b/cumulus/pallets/xcmp-queue/src/lib.rs @@ -64,7 +64,7 @@ use frame_support::{ defensive, defensive_assert, traits::{Defensive, EnqueueMessage, EnsureOrigin, Get, QueueFootprint, QueuePausedQuery}, weights::{Weight, WeightMeter}, - BoundedVec, StoragePrefixedMap, + BoundedVec, }; use pallet_message_queue::OnQueueChanged; use polkadot_runtime_common::xcm_sender::PriceForMessageDelivery; diff --git a/polkadot/runtime/common/src/xcm_sender.rs b/polkadot/runtime/common/src/xcm_sender.rs index 383a914e98f5..37fe7f0b59e9 100644 --- a/polkadot/runtime/common/src/xcm_sender.rs +++ b/polkadot/runtime/common/src/xcm_sender.rs @@ -19,7 +19,7 @@ use alloc::vec::Vec; use codec::{Decode, Encode}; use core::marker::PhantomData; -use frame_support::{traits::Get, StoragePrefixedMap}; +use frame_support::traits::Get; use frame_system::pallet_prelude::BlockNumberFor; use polkadot_primitives::Id as ParaId; use polkadot_runtime_parachains::{ From f78451f5032708c16d7577b6a234b82c5d37df69 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Thu, 5 Sep 2024 09:47:50 +0200 Subject: [PATCH 11/12] fix(prdoc): add missing crate --- prdoc/pr_5581.prdoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prdoc/pr_5581.prdoc b/prdoc/pr_5581.prdoc index 5b941242801b..fd9ab07a57ea 100644 --- a/prdoc/pr_5581.prdoc +++ b/prdoc/pr_5581.prdoc @@ -26,3 +26,5 @@ crates: bump: minor - name: polkadot-runtime-common bump: minor + - name: pallet-xcm-bridge-hub + bump: minor From 3e1e723cd12aaca6e65b496a7e0f4983f4373c7c Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Thu, 5 Sep 2024 10:54:20 +0200 Subject: [PATCH 12/12] Update prdoc/pr_5581.prdoc Co-authored-by: Adrian Catangiu --- prdoc/pr_5581.prdoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/prdoc/pr_5581.prdoc b/prdoc/pr_5581.prdoc index fd9ab07a57ea..e33690939d8b 100644 --- a/prdoc/pr_5581.prdoc +++ b/prdoc/pr_5581.prdoc @@ -6,10 +6,10 @@ title: Clear other messages before dry-running doc: - audience: Runtime Dev description: | - The DryRunApi.dry_run_call and DryRunApi.dry_run_xcm functions just returned - in forwarded_xcms all the existing messages in the queues at the time. - Now messages are cleared before dry-running, meaning only the messages produced - by the call (or xcm) will be returned. + 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