Skip to content

Commit

Permalink
[stable2407] Backport #5581 (#5603)
Browse files Browse the repository at this point in the history
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 <franciscoaguirreperez@gmail.com>
  • Loading branch information
github-actions[bot] and franciscoaguirre authored Sep 5, 2024
1 parent d6f482d commit c28ec0f
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 1 deletion.
4 changes: 4 additions & 0 deletions bridges/modules/xcm-bridge-hub-router/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
4 changes: 4 additions & 0 deletions bridges/modules/xcm-bridge-hub-router/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
4 changes: 4 additions & 0 deletions cumulus/pallets/parachain-system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down
5 changes: 5 additions & 0 deletions cumulus/pallets/xcmp-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down
4 changes: 4 additions & 0 deletions cumulus/primitives/utility/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
5 changes: 5 additions & 0 deletions polkadot/runtime/common/src/xcm_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)| {
Expand Down
6 changes: 5 additions & 1 deletion polkadot/xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
13 changes: 13 additions & 0 deletions polkadot/xcm/xcm-builder/src/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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();

Expand Down
4 changes: 4 additions & 0 deletions polkadot/xcm/xcm-builder/src/universal_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
30 changes: 30 additions & 0 deletions prdoc/pr_5603.prdoc
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit c28ec0f

Please sign in to comment.