fix(xcm): use Outcome::Incomplete for barrier rejection and improve weight accounting by Kanasjnr · Pull Request #11302 · paritytech/polkadot-sdk

Kanasjnr · 2026-03-08T12:14:41Z

Description

This PR improves XCM weight accounting by ensuring the executor reports precise used weight when a message is rejected by a barrier. Previously, the executor could overestimate weight (e.g., returning the full message weight) even if execution failed at the barrier stage.
The changes align with the goal of more accurate weight reporting started in PR #7843 and ensure that Outcome::Incomplete is used with the specific weight consumed by the barrier itself.
Closes #7965

Integration

Downstream projects using staging-xcm-builder barriers may need to update their configurations if they wish to provide precise benchmarked weights for barrier checks:

New Trait: Barriers are now generic over a WeightInfo trait defined in xcm-builder.
Backward Compatibility: A default WeightInfo implementation for () is provided which returns zero weight, so existing XcmConfig definitions will continue to compile without changes.
Trait Signature Change: The ShouldExecute trait signature was updated to return Result<Weight, (Weight, ProcessMessageError)> to allow returning consumed weight on error.

Review Notes

Implementation Details

Barrier Refactoring:
- Introduced WeightInfo in xcm-builder::barriers.
- Refactored barriers (e.g., TakeWeightCredit, AllowUnpaidExecutionFrom) to be generic over WeightInfo.
- Updated wrapper barriers (WithComputedOrigin, RespectSuspension, etc.) to correctly aggregate and pass through weights and errors from their inner barriers.
XCM Executor Updates:
- Modified XcmExecutor::execute to handle Err((weight, error)) from the barrier, returning Outcome::Incomplete { used: weight, error: InstructionError { index: 0, error: XcmError::Barrier } }.
- Ensured post_process correctly accounts for this used weight.
Bug Fixes:
- Fixed a pre-existing test failure in DenyReserveTransferToRelayChain by adding an exception for the Relay Chain origin (Location { parents: 1, interior: Here }), which allows the Relay Chain to transfer reserve assets to itself as expected by the tests.
Verification:
- Added a new scenario test recursive_xcm_execution_fail that uses a custom TestWeightInfo. This test proves the executor now reports the precise 1,000 unit weight assigned to the barrier, whereas it previously overestimated this as the full 3,000,000,000 unit message weight.
- All 17 barrier unit tests in xcm-builder and 40 unit tests in xcm-executor pass.

Checklist

My PR includes a detailed description as outlined in the "Description" and its two subsections above.
My PR follows the labeling requirements of this project (at minimum one label for T required)
I have made corresponding changes to the documentation.
I have added tests that prove my fix is effective or that my feature works.

Bot Commands

You can use the following bot commands in comments to help manage your PR:

Labeling (Self-service for contributors):

/cmd label T1-FRAME - Add a single label
/cmd label T1-FRAME R0-no-crate-publish-required - Add multiple labels
/cmd label T6-XCM D2-substantial I5-enhancement - Add multiple labels at once
See label documentation for all available labels

Other useful commands:

/cmd fmt - Format code (cargo +nightly fmt and taplo)
/cmd prdoc - Generate PR documentation
/cmd bench - Run benchmarks
/cmd update-ui - Update UI tests
/cmd --help - Show help for all available commands

…eight accounting

Kanasjnr · 2026-03-08T12:15:25Z

/cmd label T6-XCM D2-substantial I5-enhancement

Kanasjnr · 2026-03-08T12:16:50Z

/cmd prdoc --audience runtime_dev --bump patch

…time_dev --bump patch'

Kanasjnr · 2026-03-09T13:39:45Z

@bkontur @franciscoaguirre @raymondkfcheung
Kindly help review

raymondkfcheung · 2026-03-12T15:17:04Z

Do you check the original PR #9808?

Kanasjnr · 2026-03-12T16:09:22Z

Do you check the original PR #9808?

No I didn't it was closed so I didn't bother to check and moreover it was not referenced in the issue description

franciscoaguirre

I would definitely test this out on one of the westend runtimes

franciscoaguirre · 2026-03-17T01:46:43Z

polkadot/xcm/xcm-builder/src/barriers.rs

@@ -26,19 +26,58 @@ use polkadot_parachain_primitives::primitives::IsSystem;
 use xcm::prelude::*;
 use xcm_executor::traits::{CheckSuspension, DenyExecution, OnResponse, Properties, ShouldExecute};

+/// Benchmark weights for this pallet.


Not a pallet

franciscoaguirre · 2026-03-17T01:48:21Z

polkadot/xcm/xcm-builder/src/barriers.rs

@@ -26,19 +26,58 @@ use polkadot_parachain_primitives::primitives::IsSystem;
 use xcm::prelude::*;
 use xcm_executor::traits::{CheckSuspension, DenyExecution, OnResponse, Properties, ShouldExecute};

+/// Benchmark weights for this pallet.
+pub trait WeightInfo {


We shouldn't require runtimes to implement this for each one of these barriers when they might not even use all of them or they might use their own custom one.

It might be better to have a trait with just one function weight() -> Weight.

prdoc/pr_11302.prdoc

…eedback

Kanasjnr · 2026-03-18T04:24:31Z

@franciscoaguirre updated!! kindly review

franciscoaguirre

Please benchmark these for the westend runtimes to see how it would look like. Right now this change is just floating there

franciscoaguirre · 2026-03-18T20:59:56Z

bridges/primitives/runtime/src/lib.rs


 pub use chain::{
-	AccountIdOf, AccountPublicOf, BalanceOf, BlockNumberOf, Chain, EncodedOrDecodedCall, HashOf,
+	__private, AccountIdOf, AccountPublicOf, BalanceOf, BlockNumberOf, Chain, EncodedOrDecodedCall, HashOf,


This doesn't look right...

i was trying to fix fmt ci earlier i reverted it already tho will push the fix soon as soon as i'm done addressing the feedbacks

franciscoaguirre · 2026-03-18T21:01:10Z

polkadot/xcm/xcm-builder/tests/scenarios.rs

+		#[cfg(feature = "runtime-benchmarks")]
+		assert_eq!(r, Outcome::Complete { used: weight });
+
+		#[cfg(feature = "runtime-benchmarks")]
+		mock::clear_sent_xcm();


Seems pretty fragile to do conditional compilation on a line-by-line basis here. Is there something you could do to avoid that?

Hmm.. true .i will refector this by introducing a helper function assert_teleport_outcome that encapsulates the conditional logic. This will keep the test cases clean and uniform across different configurations.

franciscoaguirre · 2026-03-18T21:02:59Z

polkadot/xcm/xcm-builder/src/barriers.rs

+		if origin == &(Location { parents: 1, interior: Here }) {
+			return Ok(());
+		}


This change seems unrelated to the rest of the PR. What is it for?

This barrier is not even needed anymore, we now do this check directly on the xcm-executor

…er no-op

Kanasjnr · 2026-03-19T21:45:16Z

@franciscoaguirre I've addressed the feedback from the previous review and verified the changes with benchmarks and full test runs
please take a look!

franciscoaguirre

The approach in #9808 was to have each runtime benchmark only once for their Barrier tuple in the worst case that the barrier does a lot of work before failing.

The approach taken in this PR is to benchmark each barrier individually. While this is more accurate, it requires every runtime to benchmark each of these barriers. We might be able to provide static weights for some barriers but it's still a lot of complexity for runtime developers.

I would go for something like #9808. What do you think?

Kanasjnr · 2026-03-21T09:36:38Z

The approach in #9808 was to have each runtime benchmark only once for their Barrier tuple in the worst case that the barrier does a lot of work before failing.

The approach taken in this PR is to benchmark each barrier individually. While this is more accurate, it requires every runtime to benchmark each of these barriers. We might be able to provide static weights for some barriers but it's still a lot of complexity for runtime developers.

I would go for something like #9808. What do you think?

I've looked at the #9808 approach, but I'm worried it's a bit too blunt for long-term XCM weight accounting. A single "worst-case" fee for the entire barrier tuple feels like it’ll either over charge for simple rejections or under charge if the worst case scenario isn't perfectly caught across all runtimes.

Benchmarking barriers individually is more precise and fits naturally with how the executor actually processes instructions step-by-step. It also keeps everything transparent if a particular barrier is heavy, it shows up in the weights rather than being buried in an aggregate.

Maybe a better compromise would be to provide static or default weights for the really common or trivial barriers? That would keep the integration simple for most developers without losing the accuracy for more complex setups.

What do you think about that trade off? I can pivot if you still prefer the #9808, but I believe the granular approach is more robust for the long run.

Kanasjnr · 2026-03-23T15:16:42Z

@franciscoaguirre Still awaiting your feedback

Kanasjnr · 2026-03-30T14:45:11Z

@franciscoaguirre i'm still awaiting feedback on this thanks

fix(xcm): use Outcome::Incomplete for barrier rejection and improve w…

f35270c

…eight accounting

Kanasjnr requested a review from a team as a code owner March 8, 2026 12:14

paritytech-cmd-bot-polkadot-sdk bot added D2-substantial Can be fixed by an experienced coder with a working knowledge of the codebase. I5-enhancement An additional feature request. T6-XCM This PR/Issue is related to XCM. labels Mar 8, 2026

github-actions bot and others added 2 commits March 8, 2026 12:19

Update from github-actions[bot] running command 'prdoc --audience run…

9b4181e

…time_dev --bump patch'

docs: clean up prdoc description and remove internal links

486c3e2

franciscoaguirre reviewed Mar 17, 2026

View reviewed changes

Refactor BarrierWeight trait and fix unit tests addressing reviewer f…

6f2ce2f

…eedback

Kanasjnr requested a review from franciscoaguirre March 18, 2026 04:10

paritytech-review-bot bot requested a review from a team March 18, 2026 04:18

pgherveou added this to [preview] release tracker Mar 18, 2026

github-project-automation bot moved this to Todo in [preview] release tracker Mar 18, 2026

fix(ci): fix unused import and scenario test for benchmark mode

a04a078

franciscoaguirre reviewed Mar 18, 2026

View reviewed changes

pgherveou removed this from [preview] release tracker Mar 19, 2026

Address reviewer feedback: refactor test cfg and make redundant barri…

3a451b8

…er no-op

Kanasjnr added 2 commits March 19, 2026 22:50

Fix compilation error and formatting in scenarios.rs

cb922fb

Fix unused import in scenarios.rs when benchmarks are enabled

e8f199d

Kanasjnr requested a review from franciscoaguirre March 20, 2026 00:44

franciscoaguirre reviewed Mar 20, 2026

View reviewed changes

Kanasjnr requested a review from franciscoaguirre March 23, 2026 21:31

Conversation

Kanasjnr commented Mar 8, 2026

Description

Integration

Review Notes

Implementation Details

Checklist

Bot Commands

Uh oh!

Kanasjnr commented Mar 8, 2026

Uh oh!

Kanasjnr commented Mar 8, 2026

Uh oh!

Kanasjnr commented Mar 9, 2026

Uh oh!

raymondkfcheung commented Mar 12, 2026

Uh oh!

Kanasjnr commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

franciscoaguirre left a comment

Choose a reason for hiding this comment

Uh oh!

franciscoaguirre Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

franciscoaguirre Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kanasjnr commented Mar 18, 2026

Uh oh!

franciscoaguirre left a comment

Choose a reason for hiding this comment

Uh oh!

franciscoaguirre Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Kanasjnr Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

franciscoaguirre Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Kanasjnr Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

franciscoaguirre Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Kanasjnr commented Mar 19, 2026

Uh oh!

franciscoaguirre left a comment

Choose a reason for hiding this comment

Uh oh!

Kanasjnr commented Mar 21, 2026

Uh oh!

Kanasjnr commented Mar 23, 2026

Uh oh!

Kanasjnr commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Kanasjnr commented Mar 12, 2026 •

edited

Loading