From 1f54ef65d912af3cba948fdc8d6902a694a83364 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 3 Apr 2024 13:10:50 +0200 Subject: [PATCH] Added tests for XCM barriers: `AllowSubscriptions`, `WithUniqueTopic` and `TrailingSetTopicAsId` (#3955) Closes: https://github.com/paritytech/polkadot-sdk/issues/1756 --- .../xcm/xcm-builder/src/tests/barriers.rs | 59 ++++++++++++ polkadot/xcm/xcm-builder/src/tests/mod.rs | 1 + polkadot/xcm/xcm-builder/src/tests/routing.rs | 95 +++++++++++++++++++ .../src/tests/version_subscriptions.rs | 32 +++++-- .../xcm-executor/src/traits/should_execute.rs | 4 +- 5 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 polkadot/xcm/xcm-builder/src/tests/routing.rs diff --git a/polkadot/xcm/xcm-builder/src/tests/barriers.rs b/polkadot/xcm/xcm-builder/src/tests/barriers.rs index 99a9dd5a6609f..6516263f57a09 100644 --- a/polkadot/xcm/xcm-builder/src/tests/barriers.rs +++ b/polkadot/xcm/xcm-builder/src/tests/barriers.rs @@ -309,3 +309,62 @@ fn suspension_should_work() { ); assert_eq!(r, Ok(())); } + +#[test] +fn allow_subscriptions_from_should_work() { + // allow only parent + AllowSubsFrom::set(vec![Location::parent()]); + + let valid_xcm_1 = Xcm::(vec![SubscribeVersion { + query_id: 42, + max_response_weight: Weight::from_parts(5000, 5000), + }]); + let valid_xcm_2 = Xcm::(vec![UnsubscribeVersion]); + let invalid_xcm_1 = Xcm::(vec![ + SetAppendix(Xcm(vec![])), + SubscribeVersion { query_id: 42, max_response_weight: Weight::from_parts(5000, 5000) }, + ]); + let invalid_xcm_2 = Xcm::(vec![ + SubscribeVersion { query_id: 42, max_response_weight: Weight::from_parts(5000, 5000) }, + SetTopic([0; 32]), + ]); + + let test_data = vec![ + ( + valid_xcm_1.clone(), + Parachain(1).into_location(), + // not allowed origin + Err(ProcessMessageError::Unsupported), + ), + (valid_xcm_1, Location::parent(), Ok(())), + ( + valid_xcm_2.clone(), + Parachain(1).into_location(), + // not allowed origin + Err(ProcessMessageError::Unsupported), + ), + (valid_xcm_2, Location::parent(), Ok(())), + ( + invalid_xcm_1, + Location::parent(), + // invalid XCM + Err(ProcessMessageError::BadFormat), + ), + ( + invalid_xcm_2, + Location::parent(), + // invalid XCM + Err(ProcessMessageError::BadFormat), + ), + ]; + + for (mut message, origin, expected_result) in test_data { + let r = AllowSubscriptionsFrom::>::should_execute( + &origin, + message.inner_mut(), + Weight::from_parts(10, 10), + &mut props(Weight::zero()), + ); + assert_eq!(r, expected_result, "Failed for origin: {origin:?} and message: {message:?}"); + } +} diff --git a/polkadot/xcm/xcm-builder/src/tests/mod.rs b/polkadot/xcm/xcm-builder/src/tests/mod.rs index e11caf6282be7..63d254a106758 100644 --- a/polkadot/xcm/xcm-builder/src/tests/mod.rs +++ b/polkadot/xcm/xcm-builder/src/tests/mod.rs @@ -36,6 +36,7 @@ mod locking; mod origins; mod pay; mod querying; +mod routing; mod transacting; mod version_subscriptions; mod weight; diff --git a/polkadot/xcm/xcm-builder/src/tests/routing.rs b/polkadot/xcm/xcm-builder/src/tests/routing.rs new file mode 100644 index 0000000000000..28117d647a086 --- /dev/null +++ b/polkadot/xcm/xcm-builder/src/tests/routing.rs @@ -0,0 +1,95 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +use super::*; +use frame_support::{assert_ok, traits::Everything}; +use xcm_executor::traits::Properties; + +fn props() -> Properties { + Properties { weight_credit: Weight::zero(), message_id: None } +} + +#[test] +fn trailing_set_topic_as_id_with_unique_topic_should_work() { + type AllowSubscriptions = AllowSubscriptionsFrom; + + // check the validity of XCM for the `AllowSubscriptions` barrier + let valid_xcm = Xcm::<()>(vec![SubscribeVersion { + query_id: 42, + max_response_weight: Weight::from_parts(5000, 5000), + }]); + assert_eq!( + AllowSubscriptions::should_execute( + &Location::parent(), + valid_xcm.clone().inner_mut(), + Weight::from_parts(10, 10), + &mut props(), + ), + Ok(()) + ); + + // simulate sending `valid_xcm` with the `WithUniqueTopic` router + let mut sent_xcm = sp_io::TestExternalities::default().execute_with(|| { + assert_ok!(send_xcm::>(Location::parent(), valid_xcm,)); + sent_xcm() + }); + assert_eq!(1, sent_xcm.len()); + + // `sent_xcm` should contain `SubscribeVersion` and have `SetTopic` added + let mut sent_xcm = sent_xcm.remove(0).1; + let _ = sent_xcm + .0 + .matcher() + .assert_remaining_insts(2) + .expect("two instructions") + .match_next_inst(|instr| match instr { + SubscribeVersion { .. } => Ok(()), + _ => Err(ProcessMessageError::BadFormat), + }) + .expect("expected instruction `SubscribeVersion`") + .match_next_inst(|instr| match instr { + SetTopic(..) => Ok(()), + _ => Err(ProcessMessageError::BadFormat), + }) + .expect("expected instruction `SetTopic`"); + + // `sent_xcm` contains `SetTopic` and is now invalid for `AllowSubscriptions` + assert_eq!( + AllowSubscriptions::should_execute( + &Location::parent(), + sent_xcm.clone().inner_mut(), + Weight::from_parts(10, 10), + &mut props(), + ), + Err(ProcessMessageError::BadFormat) + ); + + // let's apply `TrailingSetTopicAsId` before `AllowSubscriptions` + let mut props = props(); + assert!(props.message_id.is_none()); + + // should pass, and the `message_id` is set + assert_eq!( + TrailingSetTopicAsId::::should_execute( + &Location::parent(), + sent_xcm.clone().inner_mut(), + Weight::from_parts(10, 10), + &mut props, + ), + Ok(()) + ); + assert!(props.message_id.is_some()); +} diff --git a/polkadot/xcm/xcm-builder/src/tests/version_subscriptions.rs b/polkadot/xcm/xcm-builder/src/tests/version_subscriptions.rs index e29e3a546615b..01047fde989f9 100644 --- a/polkadot/xcm/xcm-builder/src/tests/version_subscriptions.rs +++ b/polkadot/xcm/xcm-builder/src/tests/version_subscriptions.rs @@ -27,16 +27,32 @@ fn simple_version_subscriptions_should_work() { ]); let mut hash = fake_message_hash(&message); let weight_limit = Weight::from_parts(20, 20); - let r = XcmExecutor::::prepare_and_execute( - origin, - message, - &mut hash, - weight_limit, - Weight::zero(), + + // this case fails because the origin is not allowed + assert_eq!( + XcmExecutor::::prepare_and_execute( + origin, + message.clone(), + &mut hash, + weight_limit, + Weight::zero(), + ), + Outcome::Error { error: XcmError::Barrier } + ); + + // this case fails because the additional `SetAppendix` instruction is not allowed in the + // `AllowSubscriptionsFrom` + assert_eq!( + XcmExecutor::::prepare_and_execute( + Parent, + message, + &mut hash, + weight_limit, + Weight::zero(), + ), + Outcome::Error { error: XcmError::Barrier } ); - assert_eq!(r, Outcome::Error { error: XcmError::Barrier }); - let origin = Parachain(1000); let message = Xcm::(vec![SubscribeVersion { query_id: 42, max_response_weight: Weight::from_parts(5000, 5000), diff --git a/polkadot/xcm/xcm-executor/src/traits/should_execute.rs b/polkadot/xcm/xcm-executor/src/traits/should_execute.rs index 12e8fd6b87f10..e76d56bfe6164 100644 --- a/polkadot/xcm/xcm-executor/src/traits/should_execute.rs +++ b/polkadot/xcm/xcm-executor/src/traits/should_execute.rs @@ -33,9 +33,9 @@ pub struct Properties { /// Trait to determine whether the execution engine should actually execute a given XCM. /// /// Can be amalgamated into a tuple to have multiple trials. If any of the tuple elements returns -/// `Ok()`, the execution stops. Else, `Err(_)` is returned if all elements reject the message. +/// `Ok(())`, the execution stops. Else, `Err(_)` is returned if all elements reject the message. pub trait ShouldExecute { - /// Returns `true` if the given `message` may be executed. + /// Returns `Ok(())` if the given `message` may be executed. /// /// - `origin`: The origin (sender) of the message. /// - `instructions`: The message itself.