Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// limitations under the License.

use crate::imports::*;
use frame_support::traits::ProcessMessageError;

use codec::Encode;
use frame_support::sp_runtime::traits::Dispatchable;
Expand Down Expand Up @@ -141,7 +140,7 @@ fn relay_commands_add_registrar_wrong_origin() {
assert_expected_events!(
PeopleWestend,
vec![
RuntimeEvent::MessageQueue(pallet_message_queue::Event::ProcessingFailed { error: ProcessMessageError::Unsupported, .. }) => {},
RuntimeEvent::MessageQueue(pallet_message_queue::Event::Processed { success: false, .. }) => {},
]
);
} else {
Expand Down
5 changes: 4 additions & 1 deletion polkadot/xcm/xcm-builder/src/tests/origins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ fn unpaid_execution_should_work() {
Weight::from_parts(50, 50),
Weight::zero(),
);
assert_eq!(r, Outcome::Error { error: XcmError::Barrier });
assert_eq!(
r,
Outcome::Incomplete { used: Weight::from_parts(10, 10), error: XcmError::Barrier }
);

let message = Xcm(vec![UnpaidExecution {
weight_limit: Limited(Weight::from_parts(10, 10)),
Expand Down
5 changes: 4 additions & 1 deletion polkadot/xcm/xcm-builder/src/tests/querying.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,8 @@ fn prepaid_result_of_query_should_get_free_execution() {
weight_limit,
Weight::zero(),
);
assert_eq!(r, Outcome::Error { error: XcmError::Barrier });
assert_eq!(
r,
Outcome::Incomplete { used: Weight::from_parts(10, 10), error: XcmError::Barrier }
);
}
19 changes: 14 additions & 5 deletions polkadot/xcm/xcm-builder/src/tests/version_subscriptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn simple_version_subscriptions_should_work() {
weight_limit,
Weight::zero(),
),
Outcome::Error { error: XcmError::Barrier }
Outcome::Incomplete { used: Weight::from_parts(20, 20), error: XcmError::Barrier }
);

// this case fails because the additional `SetAppendix` instruction is not allowed in the
Expand All @@ -50,7 +50,7 @@ fn simple_version_subscriptions_should_work() {
weight_limit,
Weight::zero(),
),
Outcome::Error { error: XcmError::Barrier }
Outcome::Incomplete { used: Weight::from_parts(20, 20), error: XcmError::Barrier }
);

let message = Xcm::<TestCall>(vec![SubscribeVersion {
Expand All @@ -66,7 +66,10 @@ fn simple_version_subscriptions_should_work() {
weight_limit,
Weight::zero(),
);
assert_eq!(r, Outcome::Error { error: XcmError::Barrier });
assert_eq!(
r,
Outcome::Incomplete { used: Weight::from_parts(10, 10), error: XcmError::Barrier }
);

let r = XcmExecutor::<TestConfig>::prepare_and_execute(
Parent,
Expand Down Expand Up @@ -139,7 +142,10 @@ fn simple_version_unsubscriptions_should_work() {
weight_limit,
Weight::zero(),
);
assert_eq!(r, Outcome::Error { error: XcmError::Barrier });
assert_eq!(
r,
Outcome::Incomplete { used: Weight::from_parts(20, 20), error: XcmError::Barrier }
);

let origin = Parachain(1000);
let message = Xcm::<TestCall>(vec![UnsubscribeVersion]);
Expand All @@ -152,7 +158,10 @@ fn simple_version_unsubscriptions_should_work() {
weight_limit,
Weight::zero(),
);
assert_eq!(r, Outcome::Error { error: XcmError::Barrier });
assert_eq!(
r,
Outcome::Incomplete { used: Weight::from_parts(10, 10), error: XcmError::Barrier }
);

let r = XcmExecutor::<TestConfig>::prepare_and_execute(
Parent,
Expand Down
8 changes: 7 additions & 1 deletion polkadot/xcm/xcm-builder/tests/scenarios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,12 @@ fn recursive_xcm_execution_fail() {
Weight::zero(),
);

assert_eq!(outcome, Outcome::Error { error: XcmError::Barrier });
assert_eq!(
outcome,
Outcome::Incomplete {
used: Weight::from_parts(3000000000, 3072),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do these values come from? some benchmarking or something else?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error: XcmError::Barrier
}
);
});
}
6 changes: 5 additions & 1 deletion polkadot/xcm/xcm-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,11 @@ impl<Config: config::Config> ExecuteXcm<Config::RuntimeCall> for XcmExecutor<Con
error = ?e,
"Barrier blocked execution",
);
return Outcome::Error { error: XcmError::Barrier }

return Outcome::Incomplete {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, I think this change make sense according to the:

    /// Execution started, but did not complete successfully due to the given error; given weight
	/// was used.
	Incomplete { used: Weight, error: Error },
	/// Execution did not start due to the given error.
	Error { error: Error },

because Barrier check fails in the Self::execute(origin, pre, id, weight_credit) - so it started.

I am just wondering where else we are using Outcome::Error instead of Incomplete { used: Weight, error: Error }

impl From<Error> for Outcome {
	fn from(error: Error) -> Self {
		Self::Error { error }
	}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created the follow-up issue #7965

used: xcm_weight, // Weight consumed before the error
error: XcmError::Barrier, // The error that occurred
};
}

*id = properties.message_id.unwrap_or(*id);
Expand Down
15 changes: 15 additions & 0 deletions prdoc/pr_7843.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
title: Fix XCM Barrier Rejection Handling to Return Incomplete with Weight
doc:
- audience: Runtime Dev
description: "This PR addresses an issue with the handling of message execution\
\ when blocked by the barrier. Instead of returning an `Outcome::Error`, we modify\
\ the behaviour to return `Outcome::Incomplete`, which includes the weight consumed\
\ up to the point of rejection and the error that caused the blockage.\n\nThis\
\ change ensures more accurate weight tracking during message execution, even\
\ when interrupted. It improves resource management and aligns the XCM executor\u2019\
s behaviour with better error handling practices."
crates:
- name: staging-xcm-executor
bump: patch
- name: staging-xcm-builder
bump: patch
Loading