diff --git a/polkadot/xcm/xcm-builder/src/barriers.rs b/polkadot/xcm/xcm-builder/src/barriers.rs index c0b328f38e96..11e9122f9a12 100644 --- a/polkadot/xcm/xcm-builder/src/barriers.rs +++ b/polkadot/xcm/xcm-builder/src/barriers.rs @@ -399,6 +399,41 @@ impl> ShouldExecute for AllowSubscriptionsFrom { } } +/// Allows execution for the Relay Chain origin (represented as `Location::parent()`) if it is just +/// a straight `HrmpNewChannelOpenRequest`, `HrmpChannelAccepted`, or `HrmpChannelClosing` +/// instruction. +/// +/// Note: This barrier fulfills safety recommendations for the mentioned instructions - see their +/// documentation. +pub struct AllowHrmpNotificationsFromRelayChain; +impl ShouldExecute for AllowHrmpNotificationsFromRelayChain { + fn should_execute( + origin: &Location, + instructions: &mut [Instruction], + _max_weight: Weight, + _properties: &mut Properties, + ) -> Result<(), ProcessMessageError> { + log::trace!( + target: "xcm::barriers", + "AllowHrmpNotificationsFromRelayChain origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, _max_weight, _properties, + ); + // accept only the Relay Chain + ensure!(matches!(origin.unpack(), (1, [])), ProcessMessageError::Unsupported); + // accept only HRMP notifications and nothing else + instructions + .matcher() + .assert_remaining_insts(1)? + .match_next_inst(|inst| match inst { + HrmpNewChannelOpenRequest { .. } | + HrmpChannelAccepted { .. } | + HrmpChannelClosing { .. } => Ok(()), + _ => Err(ProcessMessageError::BadFormat), + })?; + Ok(()) + } +} + /// Deny executing the XCM if it matches any of the Deny filter regardless of anything else. /// If it passes the Deny, and matches one of the Allow cases then it is let through. pub struct DenyThenTry(PhantomData, PhantomData) diff --git a/polkadot/xcm/xcm-builder/src/lib.rs b/polkadot/xcm/xcm-builder/src/lib.rs index bd4a4c941c91..ea158572105f 100644 --- a/polkadot/xcm/xcm-builder/src/lib.rs +++ b/polkadot/xcm/xcm-builder/src/lib.rs @@ -35,10 +35,11 @@ pub use asset_conversion::{ mod barriers; pub use barriers::{ - AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses, AllowSubscriptionsFrom, - AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, DenyReserveTransferToRelayChain, - DenyThenTry, IsChildSystemParachain, IsParentsOnly, IsSiblingSystemParachain, - RespectSuspension, TakeWeightCredit, TrailingSetTopicAsId, WithComputedOrigin, + AllowExplicitUnpaidExecutionFrom, AllowHrmpNotificationsFromRelayChain, + AllowKnownQueryResponses, AllowSubscriptionsFrom, AllowTopLevelPaidExecutionFrom, + AllowUnpaidExecutionFrom, DenyReserveTransferToRelayChain, DenyThenTry, IsChildSystemParachain, + IsParentsOnly, IsSiblingSystemParachain, RespectSuspension, TakeWeightCredit, + TrailingSetTopicAsId, WithComputedOrigin, }; mod controller; diff --git a/polkadot/xcm/xcm-builder/src/tests/barriers.rs b/polkadot/xcm/xcm-builder/src/tests/barriers.rs index 6516263f57a0..665b5febc61f 100644 --- a/polkadot/xcm/xcm-builder/src/tests/barriers.rs +++ b/polkadot/xcm/xcm-builder/src/tests/barriers.rs @@ -315,56 +315,150 @@ 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]), - ]); + // closure for (xcm, origin) testing with `AllowSubscriptionsFrom` + let assert_should_execute = |mut xcm: Vec>, origin, expected_result| { + assert_eq!( + AllowSubscriptionsFrom::>::should_execute( + &origin, + &mut xcm, + Weight::from_parts(10, 10), + &mut props(Weight::zero()), + ), + expected_result + ); + }; + + // invalid origin + assert_should_execute( + vec![SubscribeVersion { + query_id: Default::default(), + max_response_weight: Default::default(), + }], + Parachain(1).into_location(), + Err(ProcessMessageError::Unsupported), + ); + assert_should_execute( + vec![UnsubscribeVersion], + Parachain(1).into_location(), + Err(ProcessMessageError::Unsupported), + ); - 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()), + // invalid XCM (unexpected instruction before) + assert_should_execute( + vec![ + SetAppendix(Xcm(vec![])), + SubscribeVersion { + query_id: Default::default(), + max_response_weight: Default::default(), + }, + ], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + assert_should_execute( + vec![SetAppendix(Xcm(vec![])), UnsubscribeVersion], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + // invalid XCM (unexpected instruction after) + assert_should_execute( + vec![ + SubscribeVersion { + query_id: Default::default(), + max_response_weight: Default::default(), + }, + SetTopic([0; 32]), + ], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + assert_should_execute( + vec![UnsubscribeVersion, SetTopic([0; 32])], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + // invalid XCM (unexpected instruction) + assert_should_execute( + vec![SetAppendix(Xcm(vec![]))], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + + // ok + assert_should_execute( + vec![SubscribeVersion { + query_id: Default::default(), + max_response_weight: Default::default(), + }], + Location::parent(), + Ok(()), + ); + assert_should_execute(vec![UnsubscribeVersion], Location::parent(), Ok(())); +} + +#[test] +fn allow_hrmp_notifications_from_relay_chain_should_work() { + // closure for (xcm, origin) testing with `AllowHrmpNotificationsFromRelayChain` + let assert_should_execute = |mut xcm: Vec>, origin, expected_result| { + assert_eq!( + AllowHrmpNotificationsFromRelayChain::should_execute( + &origin, + &mut xcm, + Weight::from_parts(10, 10), + &mut props(Weight::zero()), + ), + expected_result ); - assert_eq!(r, expected_result, "Failed for origin: {origin:?} and message: {message:?}"); - } + }; + + // invalid origin + assert_should_execute( + vec![HrmpChannelAccepted { recipient: Default::default() }], + Location::new(1, [Parachain(1)]), + Err(ProcessMessageError::Unsupported), + ); + + // invalid XCM (unexpected instruction before) + assert_should_execute( + vec![SetAppendix(Xcm(vec![])), HrmpChannelAccepted { recipient: Default::default() }], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + // invalid XCM (unexpected instruction after) + assert_should_execute( + vec![HrmpChannelAccepted { recipient: Default::default() }, SetTopic([0; 32])], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + // invalid XCM (unexpected instruction) + assert_should_execute( + vec![SetAppendix(Xcm(vec![]))], + Location::parent(), + Err(ProcessMessageError::BadFormat), + ); + + // ok + assert_should_execute( + vec![HrmpChannelAccepted { recipient: Default::default() }], + Location::parent(), + Ok(()), + ); + assert_should_execute( + vec![HrmpNewChannelOpenRequest { + max_capacity: Default::default(), + sender: Default::default(), + max_message_size: Default::default(), + }], + Location::parent(), + Ok(()), + ); + assert_should_execute( + vec![HrmpChannelClosing { + recipient: Default::default(), + sender: Default::default(), + initiator: Default::default(), + }], + Location::parent(), + Ok(()), + ); }