diff --git a/runtime/kusama/src/xcm_config.rs b/runtime/kusama/src/xcm_config.rs index 1806a06066dd..5d3b48a0aab1 100644 --- a/runtime/kusama/src/xcm_config.rs +++ b/runtime/kusama/src/xcm_config.rs @@ -41,8 +41,8 @@ use xcm_builder::{ ChildParachainConvertsVia, ChildSystemParachainAsSuperuser, CurrencyAdapter as XcmCurrencyAdapter, FixedWeightBounds, IsChildSystemParachain, IsConcrete, MintLocation, OriginToPluralityVoice, SignedAccountId32AsNative, SignedToAccountId32, - SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, WeightInfoBounds, - WithComputedOrigin, + SovereignSignedViaLocation, TakeWeightCredit, TrailingSetTopicAsId, UsingComponents, + WeightInfoBounds, WithComputedOrigin, WithUniqueTopic, }; use xcm_executor::traits::WithOriginFilter; @@ -115,14 +115,14 @@ parameter_types! { /// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our /// individual routers. -pub type XcmRouter = ( +pub type XcmRouter = WithUniqueTopic<( // Only one router so far - use DMP to communicate with child parachains. ChildParachainRouter< Runtime, XcmPallet, ExponentialPrice, >, -); +)>; parameter_types! { pub const Ksm: MultiAssetFilter = Wild(AllOf { fun: WildFungible, id: Concrete(TokenLocation::get()) }); @@ -142,7 +142,7 @@ match_types! { } /// The barriers one of which must be passed for an XCM message to be executed. -pub type Barrier = ( +pub type Barrier = TrailingSetTopicAsId<( // Weight that is paid for may be consumed. TakeWeightCredit, // Expected responses are OK. @@ -159,7 +159,7 @@ pub type Barrier = ( UniversalLocation, ConstU32<8>, >, -); +)>; /// A call filter for the XCM Transact instruction. This is a temporary measure until we properly /// account for proof size weights. diff --git a/runtime/rococo/src/xcm_config.rs b/runtime/rococo/src/xcm_config.rs index 4bf3cb3181ea..e0e73bf3c4a3 100644 --- a/runtime/rococo/src/xcm_config.rs +++ b/runtime/rococo/src/xcm_config.rs @@ -40,7 +40,8 @@ use xcm_builder::{ ChildParachainAsNative, ChildParachainConvertsVia, ChildSystemParachainAsSuperuser, CurrencyAdapter as XcmCurrencyAdapter, FixedWeightBounds, IsChildSystemParachain, IsConcrete, MintLocation, SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, - TakeWeightCredit, UsingComponents, WeightInfoBounds, WithComputedOrigin, + TakeWeightCredit, TrailingSetTopicAsId, UsingComponents, WeightInfoBounds, WithComputedOrigin, + WithUniqueTopic, }; use xcm_executor::{traits::WithOriginFilter, XcmExecutor}; @@ -95,14 +96,14 @@ parameter_types! { /// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our /// individual routers. -pub type XcmRouter = ( +pub type XcmRouter = WithUniqueTopic<( // Only one router so far - use DMP to communicate with child parachains. ChildParachainRouter< Runtime, XcmPallet, ExponentialPrice, >, -); +)>; parameter_types! { pub const Roc: MultiAssetFilter = Wild(AllOf { fun: WildFungible, id: Concrete(TokenLocation::get()) }); @@ -137,7 +138,7 @@ match_types! { } /// The barriers one of which must be passed for an XCM message to be executed. -pub type Barrier = ( +pub type Barrier = TrailingSetTopicAsId<( // Weight that is paid for may be consumed. TakeWeightCredit, // Expected responses are OK. @@ -154,7 +155,7 @@ pub type Barrier = ( UniversalLocation, ConstU32<8>, >, -); +)>; /// A call filter for the XCM Transact instruction. This is a temporary measure until we /// properly account for proof size weights. diff --git a/runtime/westend/src/xcm_config.rs b/runtime/westend/src/xcm_config.rs index 7e37d227a677..c3c28e8c3fe7 100644 --- a/runtime/westend/src/xcm_config.rs +++ b/runtime/westend/src/xcm_config.rs @@ -39,7 +39,7 @@ use xcm_builder::{ ChildParachainConvertsVia, ChildSystemParachainAsSuperuser, CurrencyAdapter as XcmCurrencyAdapter, IsChildSystemParachain, IsConcrete, MintLocation, SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, - UsingComponents, WeightInfoBounds, WithComputedOrigin, + TrailingSetTopicAsId, UsingComponents, WeightInfoBounds, WithComputedOrigin, WithUniqueTopic, }; use xcm_executor::{traits::WithOriginFilter, XcmExecutor}; @@ -80,14 +80,14 @@ type LocalOriginConverter = ( /// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our /// individual routers. -pub type XcmRouter = ( +pub type XcmRouter = WithUniqueTopic<( // Only one router so far - use DMP to communicate with child parachains. ChildParachainRouter< Runtime, XcmPallet, ExponentialPrice, >, -); +)>; parameter_types! { pub const Westmint: MultiLocation = Parachain(1000).into_location(); @@ -108,7 +108,7 @@ pub type TrustedTeleporters = (xcm_builder::Case, xcm_builder::Case); /// The barriers one of which must be passed for an XCM message to be executed. -pub type Barrier = ( +pub type Barrier = TrailingSetTopicAsId<( // Weight that is paid for may be consumed. TakeWeightCredit, // Expected responses are OK. @@ -125,7 +125,7 @@ pub type Barrier = ( UniversalLocation, ConstU32<8>, >, -); +)>; /// A call filter for the XCM Transact instruction. This is a temporary measure until we /// properly account for proof size weights. diff --git a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs index 0a316a3f0aca..844b2f68795e 100644 --- a/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs @@ -49,7 +49,7 @@ benchmarks_instance_pallet! { &sender_location, &XcmContext { origin: Some(sender_location.clone()), - message_hash: [0; 32], + message_id: [0; 32], topic: None, }, ).unwrap(); @@ -82,7 +82,7 @@ benchmarks_instance_pallet! { &sender_location, &XcmContext { origin: Some(sender_location.clone()), - message_hash: [0; 32], + message_id: [0; 32], topic: None, }, ).unwrap(); @@ -109,7 +109,7 @@ benchmarks_instance_pallet! { &sender_location, &XcmContext { origin: Some(sender_location.clone()), - message_hash: [0; 32], + message_id: [0; 32], topic: None, }, ).unwrap(); diff --git a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index b10c924529ba..9a4519937aea 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -209,7 +209,7 @@ benchmarks! { assets.clone().into(), &XcmContext { origin: Some(origin.clone()), - message_hash: [0; 32], + message_id: [0; 32], topic: None, }, ); @@ -266,7 +266,7 @@ benchmarks! { max_response_weight, &XcmContext { origin: Some(origin.clone()), - message_hash: [0; 32], + message_id: [0; 32], topic: None, }, ).map_err(|_| "Could not start subscription")?; diff --git a/xcm/pallet-xcm/src/lib.rs b/xcm/pallet-xcm/src/lib.rs index 58d363665f28..041e76b2f69f 100644 --- a/xcm/pallet-xcm/src/lib.rs +++ b/xcm/pallet-xcm/src/lib.rs @@ -40,7 +40,7 @@ use sp_runtime::{ }; use sp_std::{boxed::Box, marker::PhantomData, prelude::*, result::Result, vec}; use xcm::{latest::QueryResponseInfo, prelude::*}; -use xcm_executor::traits::{Convert, ConvertOrigin}; +use xcm_executor::traits::{Convert, ConvertOrigin, Properties}; use frame_support::{ dispatch::{Dispatchable, GetDispatchInfo}, @@ -272,52 +272,49 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// Execution of an XCM message was attempted. - /// - /// \[ outcome \] - Attempted(xcm::latest::Outcome), + Attempted { outcome: xcm::latest::Outcome }, /// A XCM message was sent. - /// - /// \[ origin, destination, message \] - Sent(MultiLocation, MultiLocation, Xcm<()>), + Sent { + origin: MultiLocation, + destination: MultiLocation, + message: Xcm<()>, + message_id: XcmHash, + }, /// Query response received which does not match a registered query. This may be because a /// matching query was never registered, it may be because it is a duplicate response, or /// because the query timed out. - /// - /// \[ origin location, id \] - UnexpectedResponse(MultiLocation, QueryId), + UnexpectedResponse { origin: MultiLocation, query_id: QueryId }, /// Query response has been received and is ready for taking with `take_response`. There is /// no registered notification call. - /// - /// \[ id, response \] - ResponseReady(QueryId, Response), + ResponseReady { query_id: QueryId, response: Response }, /// Query response has been received and query is removed. The registered notification has /// been dispatched and executed successfully. - /// - /// \[ id, pallet index, call index \] - Notified(QueryId, u8, u8), + Notified { query_id: QueryId, pallet_index: u8, call_index: u8 }, /// Query response has been received and query is removed. The registered notification could /// not be dispatched because the dispatch weight is greater than the maximum weight /// originally budgeted by this runtime for the query result. - /// - /// \[ id, pallet index, call index, actual weight, max budgeted weight \] - NotifyOverweight(QueryId, u8, u8, Weight, Weight), + NotifyOverweight { + query_id: QueryId, + pallet_index: u8, + call_index: u8, + actual_weight: Weight, + max_budgeted_weight: Weight, + }, /// Query response has been received and query is removed. There was a general error with /// dispatching the notification call. - /// - /// \[ id, pallet index, call index \] - NotifyDispatchError(QueryId, u8, u8), + NotifyDispatchError { query_id: QueryId, pallet_index: u8, call_index: u8 }, /// Query response has been received and query is removed. The dispatch was unable to be /// decoded into a `Call`; this might be due to dispatch function having a signature which /// is not `(origin, QueryId, Response)`. - /// - /// \[ id, pallet index, call index \] - NotifyDecodeFailed(QueryId, u8, u8), + NotifyDecodeFailed { query_id: QueryId, pallet_index: u8, call_index: u8 }, /// Expected query response has been received but the origin location of the response does /// not match that expected. The query remains registered for a later, valid, response to /// be received and acted upon. - /// - /// \[ origin location, id, expected location \] - InvalidResponder(MultiLocation, QueryId, Option), + InvalidResponder { + origin: MultiLocation, + query_id: QueryId, + expected_location: Option, + }, /// Expected query response has been received but the expected origin location placed in /// storage by this runtime previously cannot be decoded. The query remains registered. /// @@ -325,38 +322,29 @@ pub mod pallet { /// runtime should be readable prior to query timeout) and dangerous since the possibly /// valid response will be dropped. Manual governance intervention is probably going to be /// needed. - /// - /// \[ origin location, id \] - InvalidResponderVersion(MultiLocation, QueryId), + InvalidResponderVersion { origin: MultiLocation, query_id: QueryId }, /// Received query response has been read and removed. - /// - /// \[ id \] - ResponseTaken(QueryId), + ResponseTaken { query_id: QueryId }, /// Some assets have been placed in an asset trap. - /// - /// \[ hash, origin, assets \] - AssetsTrapped(H256, MultiLocation, VersionedMultiAssets), + AssetsTrapped { hash: H256, origin: MultiLocation, assets: VersionedMultiAssets }, /// An XCM version change notification message has been attempted to be sent. /// /// The cost of sending it (borne by the chain) is included. - /// - /// \[ destination, result, cost \] - VersionChangeNotified(MultiLocation, XcmVersion, MultiAssets), + VersionChangeNotified { + destination: MultiLocation, + result: XcmVersion, + cost: MultiAssets, + message_id: XcmHash, + }, /// The supported version of a location has been changed. This might be through an /// automatic notification or a manual intervention. - /// - /// \[ location, XCM version \] - SupportedVersionChanged(MultiLocation, XcmVersion), + SupportedVersionChanged { location: MultiLocation, version: XcmVersion }, /// A given location which had a version change subscription was dropped owing to an error /// sending the notification to it. - /// - /// \[ location, query ID, error \] - NotifyTargetSendFail(MultiLocation, QueryId, XcmError), + NotifyTargetSendFail { location: MultiLocation, query_id: QueryId, error: XcmError }, /// A given location which had a version change subscription was dropped owing to an error /// migrating the location to our new XCM format. - /// - /// \[ location, query ID \] - NotifyTargetMigrationFail(VersionedMultiLocation, QueryId), + NotifyTargetMigrationFail { location: VersionedMultiLocation, query_id: QueryId }, /// Expected query response has been received but the expected querier location placed in /// storage by this runtime previously cannot be decoded. The query remains registered. /// @@ -364,36 +352,35 @@ pub mod pallet { /// runtime should be readable prior to query timeout) and dangerous since the possibly /// valid response will be dropped. Manual governance intervention is probably going to be /// needed. - /// - /// \[ origin location, id \] - InvalidQuerierVersion(MultiLocation, QueryId), + InvalidQuerierVersion { origin: MultiLocation, query_id: QueryId }, /// Expected query response has been received but the querier location of the response does /// not match the expected. The query remains registered for a later, valid, response to /// be received and acted upon. - /// - /// \[ origin location, id, expected querier, maybe actual querier \] - InvalidQuerier(MultiLocation, QueryId, MultiLocation, Option), + InvalidQuerier { + origin: MultiLocation, + query_id: QueryId, + expected_querier: MultiLocation, + maybe_actual_querier: Option, + }, /// A remote has requested XCM version change notification from us and we have honored it. /// A version information message is sent to them and its cost is included. - /// - /// \[ destination location, cost \] - VersionNotifyStarted(MultiLocation, MultiAssets), - /// We have requested that a remote chain sends us XCM version change notifications. - /// - /// \[ destination location, cost \] - VersionNotifyRequested(MultiLocation, MultiAssets), + VersionNotifyStarted { destination: MultiLocation, cost: MultiAssets, message_id: XcmHash }, + /// We have requested that a remote chain send us XCM version change notifications. + VersionNotifyRequested { + destination: MultiLocation, + cost: MultiAssets, + message_id: XcmHash, + }, /// We have requested that a remote chain stops sending us XCM version change notifications. - /// - /// \[ destination location, cost \] - VersionNotifyUnrequested(MultiLocation, MultiAssets), + VersionNotifyUnrequested { + destination: MultiLocation, + cost: MultiAssets, + message_id: XcmHash, + }, /// Fees were paid from a location for an operation (often for using `SendXcm`). - /// - /// \[ paying location, fees \] - FeesPaid(MultiLocation, MultiAssets), + FeesPaid { paying: MultiLocation, fees: MultiAssets }, /// Some assets have been claimed from an asset trap - /// - /// \[ hash, origin, assets \] - AssetsClaimed(H256, MultiLocation, VersionedMultiAssets), + AssetsClaimed { hash: H256, origin: MultiLocation, assets: VersionedMultiAssets }, } #[pallet::origin] @@ -793,8 +780,10 @@ pub mod pallet { let dest = MultiLocation::try_from(*dest).map_err(|()| Error::::BadVersion)?; let message: Xcm<()> = (*message).try_into().map_err(|()| Error::::BadVersion)?; - Self::send_xcm(interior, dest, message.clone()).map_err(Error::::from)?; - Self::deposit_event(Event::Sent(origin_location, dest, message)); + let message_id = + Self::send_xcm(interior, dest, message.clone()).map_err(Error::::from)?; + let e = Event::Sent { origin: origin_location, destination: dest, message, message_id }; + Self::deposit_event(e); Ok(()) } @@ -927,7 +916,7 @@ pub mod pallet { ); let result = Ok(Some(outcome.weight_used().saturating_add(T::WeightInfo::execute())).into()); - Self::deposit_event(Event::Attempted(outcome)); + Self::deposit_event(Event::Attempted { outcome }); result } @@ -942,16 +931,16 @@ pub mod pallet { pub fn force_xcm_version( origin: OriginFor, location: Box, - xcm_version: XcmVersion, + version: XcmVersion, ) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; let location = *location; SupportedVersion::::insert( XCM_VERSION, LatestVersionedMultiLocation(&location), - xcm_version, + version, ); - Self::deposit_event(Event::SupportedVersionChanged(location, xcm_version)); + Self::deposit_event(Event::SupportedVersionChanged { location, version }); Ok(()) } @@ -1195,7 +1184,7 @@ impl Pallet { let hash = message.using_encoded(sp_io::hashing::blake2_256); let outcome = T::XcmExecutor::execute_xcm_in_credit(origin_location, message, hash, weight, weight); - Self::deposit_event(Event::Attempted(outcome)); + Self::deposit_event(Event::Attempted { outcome }); Ok(()) } @@ -1256,7 +1245,7 @@ impl Pallet { let hash = message.using_encoded(sp_io::hashing::blake2_256); let outcome = T::XcmExecutor::execute_xcm_in_credit(origin_location, message, hash, weight, weight); - Self::deposit_event(Event::Attempted(outcome)); + Self::deposit_event(Event::Attempted { outcome }); Ok(()) } @@ -1331,14 +1320,19 @@ impl Pallet { let message = Xcm(vec![QueryResponse { query_id, response, max_weight, querier: None }]); let event = match send_xcm::(new_key, message) { - Ok((_hash, cost)) => { + Ok((message_id, cost)) => { let value = (query_id, max_weight, xcm_version); VersionNotifyTargets::::insert(XCM_VERSION, key, value); - Event::VersionChangeNotified(new_key, xcm_version, cost) + Event::VersionChangeNotified { + destination: new_key, + result: xcm_version, + cost, + message_id, + } }, Err(e) => { VersionNotifyTargets::::remove(XCM_VERSION, key); - Event::NotifyTargetSendFail(new_key, query_id, e.into()) + Event::NotifyTargetSendFail { location: new_key, query_id, error: e.into() } }, }; Self::deposit_event(event); @@ -1357,7 +1351,10 @@ impl Pallet { let new_key = match MultiLocation::try_from(old_key.clone()) { Ok(k) => k, Err(()) => { - Self::deposit_event(Event::NotifyTargetMigrationFail(old_key, value.0)); + Self::deposit_event(Event::NotifyTargetMigrationFail { + location: old_key, + query_id: value.0, + }); weight_used.saturating_accrue(vnt_migrate_fail_weight); if weight_used.any_gte(weight_cutoff) { return (weight_used, Some(stage)) @@ -1380,15 +1377,24 @@ impl Pallet { querier: None, }]); let event = match send_xcm::(new_key, message) { - Ok((_hash, cost)) => { + Ok((message_id, cost)) => { VersionNotifyTargets::::insert( XCM_VERSION, versioned_key, (query_id, max_weight, xcm_version), ); - Event::VersionChangeNotified(new_key, xcm_version, cost) + Event::VersionChangeNotified { + destination: new_key, + result: xcm_version, + cost, + message_id, + } + }, + Err(e) => Event::NotifyTargetSendFail { + location: new_key, + query_id, + error: e.into(), }, - Err(e) => Event::NotifyTargetSendFail(new_key, query_id, e.into()), }; Self::deposit_event(event); weight_used.saturating_accrue(vnt_notify_migrate_weight); @@ -1415,8 +1421,8 @@ impl Pallet { }); // TODO #3735: Correct weight. let instruction = SubscribeVersion { query_id, max_response_weight: Weight::zero() }; - let (_hash, cost) = send_xcm::(dest, Xcm(vec![instruction]))?; - Self::deposit_event(Event::VersionNotifyRequested(dest, cost)); + let (message_id, cost) = send_xcm::(dest, Xcm(vec![instruction]))?; + Self::deposit_event(Event::VersionNotifyRequested { destination: dest, cost, message_id }); VersionNotifiers::::insert(XCM_VERSION, &versioned_dest, query_id); let query_status = QueryStatus::VersionNotifier { origin: versioned_dest, is_active: false }; @@ -1430,8 +1436,12 @@ impl Pallet { let versioned_dest = LatestVersionedMultiLocation(&dest); let query_id = VersionNotifiers::::take(XCM_VERSION, versioned_dest) .ok_or(XcmError::InvalidLocation)?; - let (_hash, cost) = send_xcm::(dest, Xcm(vec![UnsubscribeVersion]))?; - Self::deposit_event(Event::VersionNotifyUnrequested(dest, cost)); + let (message_id, cost) = send_xcm::(dest, Xcm(vec![UnsubscribeVersion]))?; + Self::deposit_event(Event::VersionNotifyUnrequested { + destination: dest, + cost, + message_id, + }); Queries::::remove(query_id); Ok(()) } @@ -1589,7 +1599,7 @@ impl Pallet { if let Some(QueryStatus::Ready { response, at }) = Queries::::get(query_id) { let response = response.try_into().ok()?; Queries::::remove(query_id); - Self::deposit_event(Event::ResponseTaken(query_id)); + Self::deposit_event(Event::ResponseTaken { query_id }); Some((response, at)) } else { None @@ -1623,7 +1633,7 @@ impl Pallet { fn charge_fees(location: MultiLocation, assets: MultiAssets) -> DispatchResult { T::XcmExecutor::charge_fees(location, assets.clone()) .map_err(|_| Error::::FeesNotMet)?; - Self::deposit_event(Event::FeesPaid(location, assets)); + Self::deposit_event(Event::FeesPaid { paying: location, fees: assets }); Ok(()) } } @@ -1861,8 +1871,12 @@ impl VersionChangeNotifier for Pallet { let xcm_version = T::AdvertisedXcmVersion::get(); let response = Response::Version(xcm_version); let instruction = QueryResponse { query_id, response, max_weight, querier: None }; - let (_hash, cost) = send_xcm::(*dest, Xcm(vec![instruction]))?; - Self::deposit_event(Event::::VersionNotifyStarted(*dest, cost)); + let (message_id, cost) = send_xcm::(*dest, Xcm(vec![instruction]))?; + Self::deposit_event(Event::::VersionNotifyStarted { + destination: *dest, + cost, + message_id, + }); let value = (query_id, max_weight, xcm_version); VersionNotifyTargets::::insert(XCM_VERSION, versioned_dest, value); @@ -1891,7 +1905,7 @@ impl DropAssets for Pallet { let versioned = VersionedMultiAssets::from(MultiAssets::from(assets)); let hash = BlakeTwo256::hash_of(&(&origin, &versioned)); AssetTraps::::mutate(hash, |n| *n += 1); - Self::deposit_event(Event::AssetsTrapped(hash, *origin, versioned)); + Self::deposit_event(Event::AssetsTrapped { hash, origin: *origin, assets: versioned }); // TODO #3735: Put the real weight in there. Weight::zero() } @@ -1920,7 +1934,7 @@ impl ClaimAssets for Pallet { 1 => AssetTraps::::remove(hash), n => AssetTraps::::insert(hash, n - 1), } - Self::deposit_event(Event::AssetsClaimed(hash, *origin, versioned)); + Self::deposit_event(Event::AssetsClaimed { hash, origin: *origin, assets: versioned }); return true } } @@ -1953,19 +1967,28 @@ impl OnResponse for Pallet { max_weight: Weight, _context: &XcmContext, ) -> Weight { + let origin = *origin; match (response, Queries::::get(query_id)) { ( Response::Version(v), Some(QueryStatus::VersionNotifier { origin: expected_origin, is_active }), ) => { let origin: MultiLocation = match expected_origin.try_into() { - Ok(o) if &o == origin => o, + Ok(o) if o == origin => o, Ok(o) => { - Self::deposit_event(Event::InvalidResponder(*origin, query_id, Some(o))); + Self::deposit_event(Event::InvalidResponder { + origin, + query_id, + expected_location: Some(o), + }); return Weight::zero() }, _ => { - Self::deposit_event(Event::InvalidResponder(*origin, query_id, None)); + Self::deposit_event(Event::InvalidResponder { + origin, + query_id, + expected_location: None, + }); // TODO #3735: Correct weight for this. return Weight::zero() }, @@ -1983,7 +2006,10 @@ impl OnResponse for Pallet { LatestVersionedMultiLocation(&origin), v, ); - Self::deposit_event(Event::SupportedVersionChanged(origin, v)); + Self::deposit_event(Event::SupportedVersionChanged { + location: origin, + version: v, + }); Weight::zero() }, ( @@ -1994,33 +2020,33 @@ impl OnResponse for Pallet { let match_querier = match MultiLocation::try_from(match_querier) { Ok(mq) => mq, Err(_) => { - Self::deposit_event(Event::InvalidQuerierVersion(*origin, query_id)); + Self::deposit_event(Event::InvalidQuerierVersion { origin, query_id }); return Weight::zero() }, }; if querier.map_or(true, |q| q != &match_querier) { - Self::deposit_event(Event::InvalidQuerier( - *origin, + Self::deposit_event(Event::InvalidQuerier { + origin, query_id, - match_querier, - querier.cloned(), - )); + expected_querier: match_querier, + maybe_actual_querier: querier.cloned(), + }); return Weight::zero() } } let responder = match MultiLocation::try_from(responder) { Ok(r) => r, Err(_) => { - Self::deposit_event(Event::InvalidResponderVersion(*origin, query_id)); + Self::deposit_event(Event::InvalidResponderVersion { origin, query_id }); return Weight::zero() }, }; - if origin != &responder { - Self::deposit_event(Event::InvalidResponder( - *origin, + if origin != responder { + Self::deposit_event(Event::InvalidResponder { + origin, query_id, - Some(responder), - )); + expected_location: Some(responder), + }); return Weight::zero() } return match maybe_notify { @@ -2035,29 +2061,29 @@ impl OnResponse for Pallet { Queries::::remove(query_id); let weight = call.get_dispatch_info().weight; if weight.any_gt(max_weight) { - let e = Event::NotifyOverweight( + let e = Event::NotifyOverweight { query_id, pallet_index, call_index, - weight, - max_weight, - ); + actual_weight: weight, + max_budgeted_weight: max_weight, + }; Self::deposit_event(e); return Weight::zero() } - let dispatch_origin = Origin::Response(*origin).into(); + let dispatch_origin = Origin::Response(origin).into(); match call.dispatch(dispatch_origin) { Ok(post_info) => { - let e = Event::Notified(query_id, pallet_index, call_index); + let e = Event::Notified { query_id, pallet_index, call_index }; Self::deposit_event(e); post_info.actual_weight }, Err(error_and_info) => { - let e = Event::NotifyDispatchError( + let e = Event::NotifyDispatchError { query_id, pallet_index, call_index, - ); + }; Self::deposit_event(e); // Not much to do with the result as it is. It's up to the parachain to ensure that the // message makes sense. @@ -2066,13 +2092,14 @@ impl OnResponse for Pallet { } .unwrap_or(weight) } else { - let e = Event::NotifyDecodeFailed(query_id, pallet_index, call_index); + let e = + Event::NotifyDecodeFailed { query_id, pallet_index, call_index }; Self::deposit_event(e); Weight::zero() } }, None => { - let e = Event::ResponseReady(query_id, response.clone()); + let e = Event::ResponseReady { query_id, response: response.clone() }; Self::deposit_event(e); let at = frame_system::Pallet::::current_block_number(); let response = response.into(); @@ -2082,7 +2109,8 @@ impl OnResponse for Pallet { } }, _ => { - Self::deposit_event(Event::UnexpectedResponse(*origin, query_id)); + let e = Event::UnexpectedResponse { origin, query_id }; + Self::deposit_event(e); Weight::zero() }, } @@ -2094,7 +2122,7 @@ impl CheckSuspension for Pallet { _origin: &MultiLocation, _instructions: &mut [Instruction], _max_weight: Weight, - _weight_credit: &mut Weight, + _properties: &mut Properties, ) -> bool { XcmExecutionSuspended::::get() } diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index ae359116e023..6415fe03d895 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -27,7 +27,10 @@ use polkadot_parachain::primitives::Id as ParaId; use sp_runtime::traits::{AccountIdConversion, BlakeTwo256, Hash}; use xcm::{latest::QueryResponseInfo, prelude::*}; use xcm_builder::AllowKnownQueryResponses; -use xcm_executor::{traits::ShouldExecute, XcmExecutor}; +use xcm_executor::{ + traits::{Properties, ShouldExecute}, + XcmExecutor, +}; const ALICE: AccountId = AccountId::new([0u8; 32]); const BOB: AccountId = AccountId::new([1u8; 32]); @@ -101,7 +104,11 @@ fn report_outcome_notify_works() { 0, Response::ExecutionResult(None), )), - RuntimeEvent::XcmPallet(crate::Event::Notified(0, 4, 2)), + RuntimeEvent::XcmPallet(crate::Event::Notified { + query_id: 0, + pallet_index: 4, + call_index: 2 + }), ] ); assert_eq!(crate::Queries::::iter().collect::>(), vec![]); @@ -157,10 +164,10 @@ fn report_outcome_works() { assert_eq!(r, Outcome::Complete(Weight::from_parts(1_000, 1_000))); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::ResponseReady( - 0, - Response::ExecutionResult(None), - )) + RuntimeEvent::XcmPallet(crate::Event::ResponseReady { + query_id: 0, + response: Response::ExecutionResult(None), + }) ); let response = Some((Response::ExecutionResult(None), 1)); @@ -206,12 +213,12 @@ fn custom_querier_works() { assert_eq!(r, Outcome::Complete(Weight::from_parts(1_000, 1_000))); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::InvalidQuerier( - AccountId32 { network: None, id: ALICE.into() }.into(), - 0, - querier.clone(), - None, - )), + RuntimeEvent::XcmPallet(crate::Event::InvalidQuerier { + origin: AccountId32 { network: None, id: ALICE.into() }.into(), + query_id: 0, + expected_querier: querier.clone(), + maybe_actual_querier: None, + }), ); // Supplying the wrong querier will also fail @@ -232,12 +239,12 @@ fn custom_querier_works() { assert_eq!(r, Outcome::Complete(Weight::from_parts(1_000, 1_000))); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::InvalidQuerier( - AccountId32 { network: None, id: ALICE.into() }.into(), - 0, - querier.clone(), - Some(MultiLocation::here()), - )), + RuntimeEvent::XcmPallet(crate::Event::InvalidQuerier { + origin: AccountId32 { network: None, id: ALICE.into() }.into(), + query_id: 0, + expected_querier: querier.clone(), + maybe_actual_querier: Some(MultiLocation::here()), + }), ); // Multiple failures should not have changed the query state @@ -257,10 +264,10 @@ fn custom_querier_works() { assert_eq!(r, Outcome::Complete(Weight::from_parts(1_000, 1_000))); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::ResponseReady( - 0, - Response::ExecutionResult(None), - )) + RuntimeEvent::XcmPallet(crate::Event::ResponseReady { + query_id: 0, + response: Response::ExecutionResult(None), + }) ); let response = Some((Response::ExecutionResult(None), 1)); @@ -285,6 +292,7 @@ fn send_works() { buy_execution((Parent, SEND_AMOUNT)), DepositAsset { assets: AllCounted(1).into(), beneficiary: sender.clone() }, ]); + let versioned_dest = Box::new(RelayLocation::get().into()); let versioned_message = Box::new(VersionedXcm::from(message.clone())); assert_ok!(XcmPallet::send( @@ -292,19 +300,20 @@ fn send_works() { versioned_dest, versioned_message )); - assert_eq!( - sent_xcm(), - vec![( - Here.into(), - Xcm(Some(DescendOrigin(sender.clone().try_into().unwrap())) - .into_iter() - .chain(message.0.clone().into_iter()) - .collect()) - )], - ); + let sent_message = Xcm(Some(DescendOrigin(sender.clone().try_into().unwrap())) + .into_iter() + .chain(message.0.clone().into_iter()) + .collect()); + let id = fake_message_hash(&sent_message); + assert_eq!(sent_xcm(), vec![(Here.into(), sent_message)]); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Sent(sender, RelayLocation::get(), message)) + RuntimeEvent::XcmPallet(crate::Event::Sent { + origin: sender, + destination: RelayLocation::get(), + message, + message_id: id, + }) ); }); } @@ -376,7 +385,7 @@ fn teleport_assets_works() { let _check_v2_ok: xcm::v2::Xcm<()> = versioned_sent.try_into().unwrap(); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete(weight))) + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) ); }); } @@ -420,7 +429,7 @@ fn limited_teleport_assets_works() { let _check_v2_ok: xcm::v2::Xcm<()> = versioned_sent.try_into().unwrap(); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete(weight))) + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) ); }); } @@ -462,7 +471,7 @@ fn unlimited_teleport_assets_works() { ); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete(weight))) + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) ); }); } @@ -509,7 +518,7 @@ fn reserve_transfer_assets_works() { let _check_v2_ok: xcm::v2::Xcm<()> = versioned_sent.try_into().unwrap(); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete(weight))) + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) ); }); } @@ -557,7 +566,7 @@ fn limited_reserve_transfer_assets_works() { let _check_v2_ok: xcm::v2::Xcm<()> = versioned_sent.try_into().unwrap(); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete(weight))) + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) ); }); } @@ -603,7 +612,7 @@ fn unlimited_reserve_transfer_assets_works() { ); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete(weight))) + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) ); }); } @@ -635,7 +644,7 @@ fn execute_withdraw_to_deposit_works() { assert_eq!(Balances::total_balance(&BOB), SEND_AMOUNT); assert_eq!( last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete(weight))) + RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome: Outcome::Complete(weight) }) ); }); } @@ -670,10 +679,14 @@ fn trapped_assets_can_be_claimed() { assert_eq!( last_events(2), vec![ - RuntimeEvent::XcmPallet(crate::Event::AssetsTrapped(hash.clone(), source, vma)), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Complete( - BaseXcmWeight::get() * 5 - ))) + RuntimeEvent::XcmPallet(crate::Event::AssetsTrapped { + hash: hash.clone(), + origin: source, + assets: vma + }), + RuntimeEvent::XcmPallet(crate::Event::Attempted { + outcome: Outcome::Complete(BaseXcmWeight::get() * 5) + }), ] ); assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE - SEND_AMOUNT); @@ -707,13 +720,8 @@ fn trapped_assets_can_be_claimed() { ]))), weight )); - assert_eq!( - last_event(), - RuntimeEvent::XcmPallet(crate::Event::Attempted(Outcome::Incomplete( - BaseXcmWeight::get(), - XcmError::UnknownClaim - ))) - ); + let outcome = Outcome::Incomplete(BaseXcmWeight::get(), XcmError::UnknownClaim); + assert_eq!(last_event(), RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome })); }); } @@ -768,7 +776,7 @@ fn basic_subscription_works() { &remote, message.inner_mut(), weight, - &mut Weight::zero(), + &mut Properties { weight_credit: Weight::zero(), message_id: None }, )); }); } diff --git a/xcm/src/v3/mod.rs b/xcm/src/v3/mod.rs index 435998255a9c..aac0b15d33c9 100644 --- a/xcm/src/v3/mod.rs +++ b/xcm/src/v3/mod.rs @@ -184,7 +184,7 @@ pub mod prelude { NetworkId::{self, *}, OriginKind, Outcome, PalletInfo, Parent, ParentThen, PreparedMessage, QueryId, QueryResponseInfo, Response, Result as XcmResult, SendError, SendResult, SendXcm, - Unwrappable, + Unwrappable, Weight, WeightLimit::{self, *}, WildFungibility::{self, Fungible as WildFungible, NonFungible as WildNonFungible}, WildMultiAsset::{self, *}, @@ -337,19 +337,27 @@ impl TryFrom for WeightLimit { /// Contextual data pertaining to a specific list of XCM instructions. #[derive(Clone, Eq, PartialEq, Encode, Decode, Debug)] pub struct XcmContext { - /// The `MultiLocation` origin of the corresponding XCM. + /// The current value of the Origin register of the XCVM. pub origin: Option, - /// The hash of the XCM. - pub message_hash: XcmHash, - /// The topic of the XCM. + /// The identity of the XCM; this may be a hash of its versioned encoding but could also be + /// a high-level identity set by an appropriate barrier. + pub message_id: XcmHash, + /// The current value of the Topic register of the XCVM. pub topic: Option<[u8; 32]>, } impl XcmContext { - /// Constructor which sets the message hash to the supplied parameter and leaves the origin and + /// Constructor which sets the message ID to the supplied parameter and leaves the origin and /// topic unset. - pub fn with_message_hash(message_hash: XcmHash) -> XcmContext { - XcmContext { origin: None, message_hash, topic: None } + #[deprecated = "Use `with_message_id` instead."] + pub fn with_message_hash(message_id: XcmHash) -> XcmContext { + XcmContext { origin: None, message_id, topic: None } + } + + /// Constructor which sets the message ID to the supplied parameter and leaves the origin and + /// topic unset. + pub fn with_message_id(message_id: XcmHash) -> XcmContext { + XcmContext { origin: None, message_id, topic: None } } } @@ -933,7 +941,7 @@ pub enum Instruction { UnlockAsset { asset: MultiAsset, target: MultiLocation }, /// Asset (`asset`) has been locked on the `origin` system and may not be transferred. It may - /// only be unlocked with the receipt of the `UnlockAsset` instruction from this chain. + /// only be unlocked with the receipt of the `UnlockAsset` instruction from this chain. /// /// - `asset`: The asset(s) which are now unlockable from this origin. /// - `owner`: The owner of the asset on the chain in which it was locked. This may be a diff --git a/xcm/src/v3/traits.rs b/xcm/src/v3/traits.rs index b752647b0819..0482c030ee64 100644 --- a/xcm/src/v3/traits.rs +++ b/xcm/src/v3/traits.rs @@ -212,6 +212,52 @@ impl From for Error { pub type Result = result::Result<(), Error>; +/* +TODO: XCMv4 +/// Outcome of an XCM execution. +#[derive(Clone, Encode, Decode, Eq, PartialEq, Debug, TypeInfo)] +pub enum Outcome { + /// Execution completed successfully; given weight was used. + Complete { used: Weight }, + /// 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 }, +} + +impl Outcome { + pub fn ensure_complete(self) -> Result { + match self { + Outcome::Complete { .. } => Ok(()), + Outcome::Incomplete { error, .. } => Err(error), + Outcome::Error { error, .. } => Err(error), + } + } + pub fn ensure_execution(self) -> result::Result { + match self { + Outcome::Complete { used, .. } => Ok(used), + Outcome::Incomplete { used, .. } => Ok(used), + Outcome::Error { error, .. } => Err(error), + } + } + /// How much weight was used by the XCM execution attempt. + pub fn weight_used(&self) -> Weight { + match self { + Outcome::Complete { used, .. } => *used, + Outcome::Incomplete { used, .. } => *used, + Outcome::Error { .. } => Weight::zero(), + } + } +} + +impl From for Outcome { + fn from(error: Error) -> Self { + Self::Error { error, maybe_id: None } + } +} +*/ + /// Outcome of an XCM execution. #[derive(Clone, Encode, Decode, Eq, PartialEq, Debug, TypeInfo)] pub enum Outcome { @@ -259,13 +305,34 @@ pub trait ExecuteXcm { fn execute( origin: impl Into, pre: Self::Prepared, - hash: XcmHash, + id: &mut XcmHash, weight_credit: Weight, ) -> Outcome; + fn prepare_and_execute( + origin: impl Into, + message: Xcm, + id: &mut XcmHash, + weight_limit: Weight, + weight_credit: Weight, + ) -> Outcome { + let pre = match Self::prepare(message) { + Ok(x) => x, + Err(_) => return Outcome::Error(Error::WeightNotComputable), + }; + let xcm_weight = pre.weight_of(); + if xcm_weight.any_gt(weight_limit) { + return Outcome::Error(Error::WeightLimitReached(xcm_weight)) + } + Self::execute(origin, pre, id, weight_credit) + } - /// Execute some XCM `message` with the message `hash` from `origin` using no more than `weight_limit` weight. - /// The weight limit is a basic hard-limit and the implementation may place further restrictions or requirements - /// on weight and other aspects. + /// Execute some XCM `message` with the message `hash` from `origin` using no more than + /// `weight_limit` weight. + /// + /// The weight limit is a basic hard-limit and the implementation may place further + /// restrictions or requirements on weight and other aspects. + // TODO: XCMv4 + // #[deprecated = "Use `prepare_and_execute` instead"] fn execute_xcm( origin: impl Into, message: Xcm, @@ -283,14 +350,17 @@ pub trait ExecuteXcm { Self::execute_xcm_in_credit(origin, message, hash, weight_limit, Weight::zero()) } - /// Execute some XCM `message` with the message `hash` from `origin` using no more than `weight_limit` weight. + /// Execute some XCM `message` with the message `hash` from `origin` using no more than + /// `weight_limit` weight. /// - /// Some amount of `weight_credit` may be provided which, depending on the implementation, may allow - /// execution without associated payment. + /// Some amount of `weight_credit` may be provided which, depending on the implementation, may + /// allow execution without associated payment. + // TODO: XCMv4 + // #[deprecated = "Use `prepare_and_execute` instead"] fn execute_xcm_in_credit( origin: impl Into, message: Xcm, - hash: XcmHash, + mut hash: XcmHash, weight_limit: Weight, weight_credit: Weight, ) -> Outcome { @@ -302,7 +372,7 @@ pub trait ExecuteXcm { if xcm_weight.any_gt(weight_limit) { return Outcome::Error(Error::WeightLimitReached(xcm_weight)) } - Self::execute(origin, pre, hash, weight_credit) + Self::execute(origin, pre, &mut hash, weight_credit) } /// Deduct some `fees` to the sovereign account of the given `location` and place them as per @@ -322,7 +392,12 @@ impl ExecuteXcm for () { fn prepare(message: Xcm) -> result::Result> { Err(message) } - fn execute(_: impl Into, _: Self::Prepared, _: XcmHash, _: Weight) -> Outcome { + fn execute( + _: impl Into, + _: Self::Prepared, + _: &mut XcmHash, + _: Weight, + ) -> Outcome { unreachable!() } fn charge_fees(_location: impl Into, _fees: MultiAssets) -> Result { diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index 489f91d6b659..c29dd6c685ae 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -23,16 +23,10 @@ use frame_support::{ }; use polkadot_parachain::primitives::IsSystem; use sp_std::{cell::Cell, marker::PhantomData, ops::ControlFlow, result::Result}; -use xcm::latest::{ - Instruction::{self, *}, - InteriorMultiLocation, Junction, Junctions, - Junctions::X1, - MultiLocation, Weight, - WeightLimit::*, -}; -use xcm_executor::traits::{CheckSuspension, OnResponse, ShouldExecute}; +use xcm::prelude::*; +use xcm_executor::traits::{CheckSuspension, OnResponse, Properties, ShouldExecute}; -/// Execution barrier that just takes `max_weight` from `weight_credit`. +/// Execution barrier that just takes `max_weight` from `properties.weight_credit`. /// /// Useful to allow XCM execution by local chain users via extrinsics. /// E.g. `pallet_xcm::reserve_asset_transfer` to transfer a reserve asset @@ -43,14 +37,15 @@ impl ShouldExecute for TakeWeightCredit { _origin: &MultiLocation, _instructions: &mut [Instruction], max_weight: Weight, - weight_credit: &mut Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { log::trace!( target: "xcm::barriers", - "TakeWeightCredit origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", - _origin, _instructions, max_weight, weight_credit, + "TakeWeightCredit origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + _origin, _instructions, max_weight, properties, ); - *weight_credit = weight_credit + properties.weight_credit = properties + .weight_credit .checked_sub(&max_weight) .ok_or(ProcessMessageError::Overweight(max_weight))?; Ok(()) @@ -68,12 +63,12 @@ impl> ShouldExecute for AllowTopLevelPaidExecutionFro origin: &MultiLocation, instructions: &mut [Instruction], max_weight: Weight, - _weight_credit: &mut Weight, + _properties: &mut Properties, ) -> Result<(), ProcessMessageError> { log::trace!( target: "xcm::barriers", - "AllowTopLevelPaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", - origin, instructions, max_weight, _weight_credit, + "AllowTopLevelPaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, max_weight, _properties, ); ensure!(T::contains(origin), ProcessMessageError::Unsupported); @@ -166,12 +161,12 @@ impl< origin: &MultiLocation, instructions: &mut [Instruction], max_weight: Weight, - weight_credit: &mut Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { log::trace!( target: "xcm::barriers", - "WithComputedOrigin origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", - origin, instructions, max_weight, weight_credit, + "WithComputedOrigin origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, max_weight, properties, ); let mut actual_origin = *origin; let skipped = Cell::new(0usize); @@ -205,11 +200,37 @@ impl< &actual_origin, &mut instructions[skipped.get()..], max_weight, - weight_credit, + properties, ) } } +/// Sets the message ID to `t` using a `SetTopic(t)` in the last position if present. +/// +/// Requires some inner barrier to pass on the rest of the message. +pub struct TrailingSetTopicAsId(PhantomData); +impl ShouldExecute for TrailingSetTopicAsId { + fn should_execute( + origin: &MultiLocation, + instructions: &mut [Instruction], + max_weight: Weight, + properties: &mut Properties, + ) -> Result<(), ProcessMessageError> { + log::trace!( + target: "xcm::barriers", + "TrailingSetTopicAsId origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, max_weight, properties, + ); + let until = if let Some(SetTopic(t)) = instructions.last() { + properties.message_id = Some(*t); + instructions.len() - 1 + } else { + instructions.len() + }; + InnerBarrier::should_execute(&origin, &mut instructions[..until], max_weight, properties) + } +} + /// Barrier condition that allows for a `SuspensionChecker` that controls whether or not the XCM /// executor will be suspended from executing the given XCM. pub struct RespectSuspension(PhantomData<(Inner, SuspensionChecker)>); @@ -222,12 +243,12 @@ where origin: &MultiLocation, instructions: &mut [Instruction], max_weight: Weight, - weight_credit: &mut Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - if SuspensionChecker::is_suspended(origin, instructions, max_weight, weight_credit) { + if SuspensionChecker::is_suspended(origin, instructions, max_weight, properties) { Err(ProcessMessageError::Yield) } else { - Inner::should_execute(origin, instructions, max_weight, weight_credit) + Inner::should_execute(origin, instructions, max_weight, properties) } } } @@ -242,12 +263,12 @@ impl> ShouldExecute for AllowUnpaidExecutionFrom { origin: &MultiLocation, instructions: &mut [Instruction], _max_weight: Weight, - _weight_credit: &mut Weight, + _properties: &mut Properties, ) -> Result<(), ProcessMessageError> { log::trace!( target: "xcm::barriers", - "AllowUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", - origin, instructions, _max_weight, _weight_credit, + "AllowUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, _max_weight, _properties, ); ensure!(T::contains(origin), ProcessMessageError::Unsupported); Ok(()) @@ -264,12 +285,12 @@ impl> ShouldExecute for AllowExplicitUnpaidExecutionF origin: &MultiLocation, instructions: &mut [Instruction], max_weight: Weight, - _weight_credit: &mut Weight, + _properties: &mut Properties, ) -> Result<(), ProcessMessageError> { log::trace!( target: "xcm::barriers", - "AllowExplicitUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", - origin, instructions, max_weight, _weight_credit, + "AllowExplicitUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, max_weight, _properties, ); ensure!(T::contains(origin), ProcessMessageError::Unsupported); instructions.matcher().match_next_inst(|inst| match inst { @@ -300,12 +321,12 @@ impl ShouldExecute for AllowKnownQueryResponses], _max_weight: Weight, - _weight_credit: &mut Weight, + _properties: &mut Properties, ) -> Result<(), ProcessMessageError> { log::trace!( target: "xcm::barriers", - "AllowKnownQueryResponses origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", - origin, instructions, _max_weight, _weight_credit, + "AllowKnownQueryResponses origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, _max_weight, _properties, ); instructions .matcher() @@ -328,12 +349,12 @@ impl> ShouldExecute for AllowSubscriptionsFrom { origin: &MultiLocation, instructions: &mut [Instruction], _max_weight: Weight, - _weight_credit: &mut Weight, + _properties: &mut Properties, ) -> Result<(), ProcessMessageError> { log::trace!( target: "xcm::barriers", - "AllowSubscriptionsFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", - origin, instructions, _max_weight, _weight_credit, + "AllowSubscriptionsFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, _max_weight, _properties, ); ensure!(T::contains(origin), ProcessMessageError::Unsupported); instructions @@ -346,3 +367,72 @@ impl> ShouldExecute for AllowSubscriptionsFrom { 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) +where + Deny: ShouldExecute, + Allow: ShouldExecute; + +impl ShouldExecute for DenyThenTry +where + Deny: ShouldExecute, + Allow: ShouldExecute, +{ + fn should_execute( + origin: &MultiLocation, + message: &mut [Instruction], + max_weight: Weight, + properties: &mut Properties, + ) -> Result<(), ProcessMessageError> { + Deny::should_execute(origin, message, max_weight, properties)?; + Allow::should_execute(origin, message, max_weight, properties) + } +} + +// See issue +pub struct DenyReserveTransferToRelayChain; +impl ShouldExecute for DenyReserveTransferToRelayChain { + fn should_execute( + origin: &MultiLocation, + message: &mut [Instruction], + _max_weight: Weight, + _properties: &mut Properties, + ) -> Result<(), ProcessMessageError> { + message.matcher().match_next_inst_while( + |_| true, + |inst| match inst { + InitiateReserveWithdraw { + reserve: MultiLocation { parents: 1, interior: Here }, + .. + } | + DepositReserveAsset { + dest: MultiLocation { parents: 1, interior: Here }, .. + } | + TransferReserveAsset { + dest: MultiLocation { parents: 1, interior: Here }, .. + } => { + Err(ProcessMessageError::Unsupported) // Deny + }, + + // An unexpected reserve transfer has arrived from the Relay Chain. Generally, + // `IsReserve` should not allow this, but we just log it here. + ReserveAssetDeposited { .. } + if matches!(origin, MultiLocation { parents: 1, interior: Here }) => + { + log::warn!( + target: "xcm::barrier", + "Unexpected ReserveAssetDeposited from the Relay Chain", + ); + Ok(ControlFlow::Continue(())) + }, + + _ => Ok(ControlFlow::Continue(())), + }, + )?; + + // Permit everything else + Ok(()) + } +} diff --git a/xcm/xcm-builder/src/lib.rs b/xcm/xcm-builder/src/lib.rs index 0f1753474624..9ff37209c015 100644 --- a/xcm/xcm-builder/src/lib.rs +++ b/xcm/xcm-builder/src/lib.rs @@ -50,8 +50,9 @@ pub use asset_conversion::{ConvertedAbstractAssetId, ConvertedConcreteAssetId}; mod barriers; pub use barriers::{ AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses, AllowSubscriptionsFrom, - AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, IsChildSystemParachain, - RespectSuspension, TakeWeightCredit, WithComputedOrigin, + AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, DenyReserveTransferToRelayChain, + DenyThenTry, IsChildSystemParachain, RespectSuspension, TakeWeightCredit, TrailingSetTopicAsId, + WithComputedOrigin, }; mod process_xcm_message; @@ -85,6 +86,9 @@ pub use matcher::{CreateMatcher, MatchXcm, Matcher}; mod filter_asset_location; pub use filter_asset_location::{Case, NativeAsset}; +mod routing; +pub use routing::{WithTopicSource, WithUniqueTopic}; + mod universal_exports; pub use universal_exports::{ ensure_is_remote, BridgeBlobDispatcher, BridgeMessage, DispatchBlob, DispatchBlobError, diff --git a/xcm/xcm-builder/src/process_xcm_message.rs b/xcm/xcm-builder/src/process_xcm_message.rs index 4e73b11c3422..23fbb9d0cb6b 100644 --- a/xcm/xcm-builder/src/process_xcm_message.rs +++ b/xcm/xcm-builder/src/process_xcm_message.rs @@ -53,7 +53,7 @@ impl< let required = pre.weight_of(); ensure!(meter.can_accrue(required), ProcessMessageError::Overweight(required)); - let (consumed, result) = match XcmExecutor::execute(origin.into(), pre, *id, Weight::zero()) + let (consumed, result) = match XcmExecutor::execute(origin.into(), pre, id, Weight::zero()) { Outcome::Complete(w) => (w, Ok(true)), Outcome::Incomplete(w, _) => (w, Ok(false)), diff --git a/xcm/xcm-builder/src/routing.rs b/xcm/xcm-builder/src/routing.rs new file mode 100644 index 000000000000..b2d94b788628 --- /dev/null +++ b/xcm/xcm-builder/src/routing.rs @@ -0,0 +1,102 @@ +// 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 . + +//! Various implementations for `SendXcm`. + +use frame_system::unique; +use parity_scale_codec::Encode; +use sp_std::{marker::PhantomData, result::Result}; +use xcm::prelude::*; + +/// Wrapper router which, if the message does not already end with a `SetTopic` instruction, +/// appends one to the message filled with a universally unique ID. This ID is returned from a +/// successful `deliver`. +/// +/// This is designed to be at the top-level of any routers, since it will always mutate the +/// passed `message` reference into a `None`. Don't try to combine it within a tuple except as the +/// last element. +pub struct WithUniqueTopic(PhantomData); +impl SendXcm for WithUniqueTopic { + type Ticket = (Inner::Ticket, [u8; 32]); + + fn validate( + destination: &mut Option, + message: &mut Option>, + ) -> SendResult { + let mut message = message.take().ok_or(SendError::MissingArgument)?; + let unique_id = if let Some(SetTopic(id)) = message.last() { + *id + } else { + let unique_id = unique(&message); + message.0.push(SetTopic(unique_id.clone())); + unique_id + }; + let (ticket, assets) = Inner::validate(destination, &mut Some(message)) + .map_err(|_| SendError::NotApplicable)?; + Ok(((ticket, unique_id), assets)) + } + + fn deliver(ticket: Self::Ticket) -> Result { + let (ticket, unique_id) = ticket; + Inner::deliver(ticket)?; + Ok(unique_id) + } +} + +pub trait SourceTopic { + fn source_topic(entropy: impl Encode) -> XcmHash; +} + +impl SourceTopic for () { + fn source_topic(_: impl Encode) -> XcmHash { + [0u8; 32] + } +} + +/// Wrapper router which, if the message does not already end with a `SetTopic` instruction, +/// prepends one to the message filled with an ID from `TopicSource`. This ID is returned from a +/// successful `deliver`. +/// +/// This is designed to be at the top-level of any routers, since it will always mutate the +/// passed `message` reference into a `None`. Don't try to combine it within a tuple except as the +/// last element. +pub struct WithTopicSource(PhantomData<(Inner, TopicSource)>); +impl SendXcm for WithTopicSource { + type Ticket = (Inner::Ticket, [u8; 32]); + + fn validate( + destination: &mut Option, + message: &mut Option>, + ) -> SendResult { + let mut message = message.take().ok_or(SendError::MissingArgument)?; + let unique_id = if let Some(SetTopic(id)) = message.last() { + *id + } else { + let unique_id = TopicSource::source_topic(&message); + message.0.push(SetTopic(unique_id.clone())); + unique_id + }; + let (ticket, assets) = Inner::validate(destination, &mut Some(message)) + .map_err(|_| SendError::NotApplicable)?; + Ok(((ticket, unique_id), assets)) + } + + fn deliver(ticket: Self::Ticket) -> Result { + let (ticket, unique_id) = ticket; + Inner::deliver(ticket)?; + Ok(unique_id) + } +} diff --git a/xcm/xcm-builder/src/tests/barriers.rs b/xcm/xcm-builder/src/tests/barriers.rs index 241a1458fe6c..99a9dd5a6609 100644 --- a/xcm/xcm-builder/src/tests/barriers.rs +++ b/xcm/xcm-builder/src/tests/barriers.rs @@ -14,30 +14,36 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +use xcm_executor::traits::Properties; + use super::*; +fn props(weight_credit: Weight) -> Properties { + Properties { weight_credit, message_id: None } +} + #[test] fn take_weight_credit_barrier_should_work() { let mut message = Xcm::<()>(vec![TransferAsset { assets: (Parent, 100).into(), beneficiary: Here.into() }]); - let mut weight_credit = Weight::from_parts(10, 10); + let mut properties = props(Weight::from_parts(10, 10)); let r = TakeWeightCredit::should_execute( &Parent.into(), message.inner_mut(), Weight::from_parts(10, 10), - &mut weight_credit, + &mut properties, ); assert_eq!(r, Ok(())); - assert_eq!(weight_credit, Weight::zero()); + assert_eq!(properties.weight_credit, Weight::zero()); let r = TakeWeightCredit::should_execute( &Parent.into(), message.inner_mut(), Weight::from_parts(10, 10), - &mut weight_credit, + &mut properties, ); assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(10, 10)))); - assert_eq!(weight_credit, Weight::zero()); + assert_eq!(properties.weight_credit, Weight::zero()); } #[test] @@ -67,7 +73,7 @@ fn computed_origin_should_work() { &Parent.into(), message.inner_mut(), Weight::from_parts(100, 100), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Unsupported)); @@ -79,7 +85,7 @@ fn computed_origin_should_work() { &Parent.into(), message.inner_mut(), Weight::from_parts(100, 100), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Unsupported)); @@ -91,7 +97,7 @@ fn computed_origin_should_work() { &Parent.into(), message.inner_mut(), Weight::from_parts(100, 100), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Ok(())); } @@ -107,7 +113,7 @@ fn allow_unpaid_should_work() { &Parachain(1).into(), message.inner_mut(), Weight::from_parts(10, 10), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Unsupported)); @@ -115,7 +121,7 @@ fn allow_unpaid_should_work() { &Parent.into(), message.inner_mut(), Weight::from_parts(10, 10), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Ok(())); } @@ -147,7 +153,7 @@ fn allow_explicit_unpaid_should_work() { &Parachain(1).into(), good_message.inner_mut(), Weight::from_parts(20, 20), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Unsupported)); @@ -155,7 +161,7 @@ fn allow_explicit_unpaid_should_work() { &Parent.into(), bad_message1.inner_mut(), Weight::from_parts(20, 20), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20)))); @@ -163,7 +169,7 @@ fn allow_explicit_unpaid_should_work() { &Parent.into(), bad_message2.inner_mut(), Weight::from_parts(20, 20), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20)))); @@ -171,7 +177,7 @@ fn allow_explicit_unpaid_should_work() { &Parent.into(), good_message.inner_mut(), Weight::from_parts(20, 20), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Ok(())); @@ -187,7 +193,7 @@ fn allow_explicit_unpaid_should_work() { &Parent.into(), message_with_different_weight_parts.inner_mut(), Weight::from_parts(20, 20), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20)))); @@ -195,7 +201,7 @@ fn allow_explicit_unpaid_should_work() { &Parent.into(), message_with_different_weight_parts.inner_mut(), Weight::from_parts(10, 10), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Ok(())); } @@ -211,7 +217,7 @@ fn allow_paid_should_work() { &Parachain(1).into(), message.inner_mut(), Weight::from_parts(10, 10), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Unsupported)); @@ -226,7 +232,7 @@ fn allow_paid_should_work() { &Parent.into(), underpaying_message.inner_mut(), Weight::from_parts(30, 30), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(30, 30)))); @@ -241,7 +247,7 @@ fn allow_paid_should_work() { &Parachain(1).into(), paying_message.inner_mut(), Weight::from_parts(30, 30), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Unsupported)); @@ -249,7 +255,7 @@ fn allow_paid_should_work() { &Parent.into(), paying_message.inner_mut(), Weight::from_parts(30, 30), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Ok(())); @@ -264,7 +270,7 @@ fn allow_paid_should_work() { &Parent.into(), paying_message_with_different_weight_parts.inner_mut(), Weight::from_parts(20, 20), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20)))); @@ -272,7 +278,7 @@ fn allow_paid_should_work() { &Parent.into(), paying_message_with_different_weight_parts.inner_mut(), Weight::from_parts(10, 10), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Ok(())) } @@ -288,7 +294,7 @@ fn suspension_should_work() { &Parent.into(), message.inner_mut(), Weight::from_parts(10, 10), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Err(ProcessMessageError::Yield)); @@ -299,7 +305,7 @@ fn suspension_should_work() { &Parent.into(), message.inner_mut(), Weight::from_parts(10, 10), - &mut Weight::zero(), + &mut props(Weight::zero()), ); assert_eq!(r, Ok(())); } diff --git a/xcm/xcm-builder/src/tests/bridging/local_para_para.rs b/xcm/xcm-builder/src/tests/bridging/local_para_para.rs index 117909725b76..77248e72ed19 100644 --- a/xcm/xcm-builder/src/tests/bridging/local_para_para.rs +++ b/xcm/xcm-builder/src/tests/bridging/local_para_para.rs @@ -26,7 +26,8 @@ parameter_types! { } type TheBridge = TestBridge>; -type Router = UnpaidLocalExporter, UniversalLocation>; +type Router = + TestTopic, UniversalLocation>>; /// ```nocompile /// local | remote @@ -40,21 +41,26 @@ type Router = UnpaidLocalExporter, Un /// ``` #[test] fn sending_to_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Parent, Remote::get(), Parachain(1)).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); - assert_eq!(TheBridge::service(), 1); - assert_eq!( - take_received_remote_messages(), - vec![( - Here.into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1).into()), - Trap(1), - ]) - )] - ); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Parent, Remote::get(), Parachain(1)).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); + assert_eq!(TheBridge::service(), 1); + assert_eq!( + take_received_remote_messages(), + vec![( + Here.into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1).into()), + Trap(1), + ] + ) + )] + ); + }); } /// ```nocompile @@ -69,19 +75,24 @@ fn sending_to_bridged_chain_works() { /// ``` #[test] fn sending_to_parachain_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Parent, Remote::get(), Parachain(1000)).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![( - (Parent, Parachain(1000)).into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1).into()), - Trap(1), - ]), - )]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Parent, Remote::get(), Parachain(1000)).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + (Parent, Parachain(1000)).into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1).into()), + Trap(1), + ], + ), + )]; + assert_eq!(take_received_remote_messages(), expected); + }); } /// ```nocompile @@ -96,17 +107,22 @@ fn sending_to_parachain_of_bridged_chain_works() { /// ``` #[test] fn sending_to_relay_chain_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Parent, Remote::get()).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![( - Parent.into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1).into()), - Trap(1), - ]), - )]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Parent, Remote::get()).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Parent.into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1).into()), + Trap(1), + ], + ), + )]; + assert_eq!(take_received_remote_messages(), expected); + }); } diff --git a/xcm/xcm-builder/src/tests/bridging/local_relay_relay.rs b/xcm/xcm-builder/src/tests/bridging/local_relay_relay.rs index 66099bb823d4..ff2b714718e2 100644 --- a/xcm/xcm-builder/src/tests/bridging/local_relay_relay.rs +++ b/xcm/xcm-builder/src/tests/bridging/local_relay_relay.rs @@ -26,7 +26,8 @@ parameter_types! { } type TheBridge = TestBridge>; -type Router = UnpaidLocalExporter, UniversalLocation>; +type Router = + TestTopic, UniversalLocation>>; /// ```nocompile /// local | remote @@ -36,16 +37,20 @@ type Router = UnpaidLocalExporter, Un /// ``` #[test] fn sending_to_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - assert_eq!( - send_xcm::((Parent, Remote::get()).into(), msg).unwrap().1, - (Here, 100).into() - ); - assert_eq!(TheBridge::service(), 1); - assert_eq!( - take_received_remote_messages(), - vec![(Here.into(), Xcm(vec![UniversalOrigin(Local::get().into()), Trap(1)]))] - ); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + assert_eq!( + send_xcm::((Parent, Remote::get()).into(), msg).unwrap().1, + (Here, 100).into() + ); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Here.into(), + xcm_with_topic([0; 32], vec![UniversalOrigin(Local::get().into()), Trap(1)]), + )]; + assert_eq!(take_received_remote_messages(), expected); + assert_eq!(RoutingLog::take(), vec![]); + }); } /// ```nocompile @@ -60,11 +65,16 @@ fn sending_to_bridged_chain_works() { /// ``` #[test] fn sending_to_parachain_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Remote::get(), Parachain(1000)).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); - assert_eq!(TheBridge::service(), 1); - let expected = - vec![(Parachain(1000).into(), Xcm(vec![UniversalOrigin(Local::get().into()), Trap(1)]))]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Remote::get(), Parachain(1000)).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, (Here, 100).into()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Parachain(1000).into(), + xcm_with_topic([0; 32], vec![UniversalOrigin(Local::get().into()), Trap(1)]), + )]; + assert_eq!(take_received_remote_messages(), expected); + assert_eq!(RoutingLog::take(), vec![]); + }); } diff --git a/xcm/xcm-builder/src/tests/bridging/mod.rs b/xcm/xcm-builder/src/tests/bridging/mod.rs index 194e171cb620..2b5de62975ee 100644 --- a/xcm/xcm-builder/src/tests/bridging/mod.rs +++ b/xcm/xcm-builder/src/tests/bridging/mod.rs @@ -17,7 +17,7 @@ //! Tests specific to the bridging primitives use super::mock::*; -use crate::universal_exports::*; +use crate::{universal_exports::*, WithTopicSource}; use frame_support::{parameter_types, traits::Get}; use std::{cell::RefCell, marker::PhantomData}; use xcm_executor::{ @@ -37,12 +37,73 @@ parameter_types! { pub Local: NetworkId = ByGenesis([0; 32]); pub Remote: NetworkId = ByGenesis([1; 32]); pub Price: MultiAssets = MultiAssets::from((Here, 100u128)); + pub static UsingTopic: bool = false; } std::thread_local! { static BRIDGE_TRAFFIC: RefCell>> = RefCell::new(Vec::new()); } +fn maybe_with_topic(f: impl Fn()) { + UsingTopic::set(false); + f(); + UsingTopic::set(true); + f(); +} + +fn xcm_with_topic(topic: XcmHash, mut xcm: Vec>) -> Xcm { + if UsingTopic::get() { + xcm.push(SetTopic(topic)); + } + Xcm(xcm) +} + +fn fake_id() -> XcmHash { + [255; 32] +} + +fn test_weight(mut count: u64) -> Weight { + if UsingTopic::get() { + count += 1; + } + Weight::from_parts(count * 10, count * 10) +} + +fn maybe_forward_id_for(topic: &XcmHash) -> XcmHash { + match UsingTopic::get() { + true => forward_id_for(topic), + false => fake_id(), + } +} + +enum TestTicket { + Basic(T::Ticket), + Topic( as SendXcm>::Ticket), +} + +struct TestTopic(PhantomData); +impl SendXcm for TestTopic { + type Ticket = TestTicket; + fn deliver(ticket: Self::Ticket) -> core::result::Result { + match ticket { + TestTicket::Basic(t) => R::deliver(t), + TestTicket::Topic(t) => WithTopicSource::::deliver(t), + } + } + fn validate( + destination: &mut Option, + message: &mut Option>, + ) -> SendResult { + Ok(if UsingTopic::get() { + let (t, a) = WithTopicSource::::validate(destination, message)?; + (TestTicket::Topic(t), a) + } else { + let (t, a) = R::validate(destination, message)?; + (TestTicket::Basic(t), a) + }) + } +} + struct TestBridge(PhantomData); impl TestBridge { fn service() -> u64 { @@ -71,7 +132,7 @@ impl SendXcm for TestRemoteIncomingRouter { Ok((pair, MultiAssets::new())) } fn deliver(pair: (MultiLocation, Xcm<()>)) -> Result { - let hash = fake_message_hash(&pair.1); + let hash = fake_id(); REMOTE_INCOMING_XCM.with(|q| q.borrow_mut().push(pair)); Ok(hash) } @@ -107,6 +168,20 @@ fn deliver( export_xcm::(n, c, s, d, m).map(|(hash, _)| hash) } +#[derive(Eq, PartialEq, Clone, Debug)] +pub struct LogEntry { + local: Junctions, + remote: Junctions, + id: XcmHash, + message: Xcm<()>, + outcome: Outcome, + paid: bool, +} + +parameter_types! { + pub static RoutingLog: Vec = vec![]; +} + impl, Remote: Get, RemoteExporter: ExportXcm> SendXcm for UnpaidExecutingRouter { @@ -133,15 +208,20 @@ impl, Remote: Get, RemoteExporter: ExportXcm> S AllowUnpaidFrom::set(vec![origin.clone()]); set_exporter_override(price::, deliver::); // The we execute it: - let hash = fake_message_hash(&message); - let outcome = XcmExecutor::::execute_xcm( + let mut id = fake_id(); + let outcome = XcmExecutor::::prepare_and_execute( origin, - message.into(), - hash, + message.clone().into(), + &mut id, Weight::from_parts(2_000_000_000_000, 2_000_000_000_000), + Weight::zero(), ); + let local = Local::get(); + let remote = Remote::get(); + let entry = LogEntry { local, remote, id, message, outcome: outcome.clone(), paid: false }; + RoutingLog::mutate(|l| l.push(entry)); match outcome { - Outcome::Complete(..) => Ok(hash), + Outcome::Complete(..) => Ok(id), Outcome::Incomplete(..) => Err(Transport("Error executing")), Outcome::Error(..) => Err(Transport("Unable to execute")), } @@ -178,15 +258,20 @@ impl, Remote: Get, RemoteExporter: ExportXcm> S AllowPaidFrom::set(vec![origin.clone()]); set_exporter_override(price::, deliver::); // Then we execute it: - let hash = fake_message_hash(&message); - let outcome = XcmExecutor::::execute_xcm( + let mut id = fake_id(); + let outcome = XcmExecutor::::prepare_and_execute( origin, - message.into(), - hash, + message.clone().into(), + &mut id, Weight::from_parts(2_000_000_000_000, 2_000_000_000_000), + Weight::zero(), ); + let local = Local::get(); + let remote = Remote::get(); + let entry = LogEntry { local, remote, id, message, outcome: outcome.clone(), paid: true }; + RoutingLog::mutate(|l| l.push(entry)); match outcome { - Outcome::Complete(..) => Ok(hash), + Outcome::Complete(..) => Ok(id), Outcome::Incomplete(..) => Err(Transport("Error executing")), Outcome::Error(..) => Err(Transport("Unable to execute")), } diff --git a/xcm/xcm-builder/src/tests/bridging/paid_remote_relay_relay.rs b/xcm/xcm-builder/src/tests/bridging/paid_remote_relay_relay.rs index a618193d6e30..5472a3bd541d 100644 --- a/xcm/xcm-builder/src/tests/bridging/paid_remote_relay_relay.rs +++ b/xcm/xcm-builder/src/tests/bridging/paid_remote_relay_relay.rs @@ -27,8 +27,8 @@ parameter_types! { pub UniversalLocation: Junctions = X2(GlobalConsensus(Local::get()), Parachain(100)); pub RelayUniversalLocation: Junctions = X1(GlobalConsensus(Local::get())); pub RemoteUniversalLocation: Junctions = X1(GlobalConsensus(Remote::get())); - pub static BridgeTable: Vec<(NetworkId, MultiLocation, Option)> - = vec![(Remote::get(), MultiLocation::parent(), Some((Parent, 200u128).into()))]; + pub BridgeTable: Vec<(NetworkId, MultiLocation, Option)> + = vec![(Remote::get(), MultiLocation::parent(), Some((Parent, 200u128 + if UsingTopic::get() { 20 } else { 0 }).into()))]; // ^^^ 100 to use the bridge (export) and 100 for the remote execution weight (5 instructions // x (10 + 10) weight each). } @@ -41,7 +41,7 @@ type LocalBridgeRouter = SovereignPaidRemoteExporter< LocalInnerRouter, UniversalLocation, >; -type LocalRouter = (LocalInnerRouter, LocalBridgeRouter); +type LocalRouter = TestTopic<(LocalInnerRouter, LocalBridgeRouter)>; /// ```nocompile /// local | remote @@ -55,33 +55,66 @@ type LocalRouter = (LocalInnerRouter, LocalBridgeRouter); /// ``` #[test] fn sending_to_bridged_chain_works() { + maybe_with_topic(|| { + let dest: MultiLocation = (Parent, Parent, Remote::get()).into(); + + // Initialize the local relay so that our parachain has funds to pay for export. + clear_assets(Parachain(100)); + add_asset(Parachain(100), (Here, 1000u128)); + + let price = 200u128 + if UsingTopic::get() { 20 } else { 0 }; + + let msg = Xcm(vec![Trap(1)]); + assert_eq!(send_xcm::(dest, msg).unwrap().1, (Parent, price).into()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Here.into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(100).into()), + Trap(1), + ], + ), + )]; + assert_eq!(take_received_remote_messages(), expected); + + // The export cost 50 ref time and 50 proof size weight units (and thus 100 units of balance). + assert_eq!(asset_list(Parachain(100)), vec![(Here, 1000u128 - price).into()]); + + let entry = LogEntry { + local: UniversalLocation::get(), + remote: RelayUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + WithdrawAsset(MultiAsset::from((Here, price)).into()), + BuyExecution { fees: (Here, price).into(), weight_limit: Unlimited }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Here, + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + RefundSurplus, + DepositAsset { assets: Wild(All), beneficiary: Parachain(100).into() }, + ], + ), + outcome: Outcome::Complete(test_weight(5)), + paid: true, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); +} +#[test] +fn sending_to_bridged_chain_without_funds_fails() { let dest: MultiLocation = (Parent, Parent, Remote::get()).into(); // Routing won't work if we don't have enough funds. assert_eq!( send_xcm::(dest.clone(), Xcm(vec![Trap(1)])), Err(SendError::Transport("Error executing")), ); - - // Initialize the local relay so that our parachain has funds to pay for export. - add_asset(Parachain(100), (Here, 1000u128)); - - let msg = Xcm(vec![Trap(1)]); - assert_eq!(send_xcm::(dest, msg).unwrap().1, (Parent, 200u128).into()); - assert_eq!(TheBridge::service(), 1); - assert_eq!( - take_received_remote_messages(), - vec![( - Here.into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(100).into()), - Trap(1), - ]) - )] - ); - - // The export cost 50 ref time and 50 proof size weight units (and thus 100 units of balance). - assert_eq!(asset_list(Parachain(100)), vec![(Here, 800u128).into()]); } /// ```nocompile @@ -96,29 +129,64 @@ fn sending_to_bridged_chain_works() { /// ``` #[test] fn sending_to_parachain_of_bridged_chain_works() { + maybe_with_topic(|| { + let dest: MultiLocation = (Parent, Parent, Remote::get(), Parachain(100)).into(); + + // Initialize the local relay so that our parachain has funds to pay for export. + clear_assets(Parachain(100)); + add_asset(Parachain(100), (Here, 1000u128)); + + let price = 200u128 + if UsingTopic::get() { 20 } else { 0 }; + + let msg = Xcm(vec![Trap(1)]); + assert_eq!(send_xcm::(dest, msg).unwrap().1, (Parent, price).into()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Parachain(100).into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(100).into()), + Trap(1), + ], + ), + )]; + assert_eq!(take_received_remote_messages(), expected); + + // The export cost 50 ref time and 50 proof size weight units (and thus 100 units of balance). + assert_eq!(asset_list(Parachain(100)), vec![(Here, 1000u128 - price).into()]); + + let entry = LogEntry { + local: UniversalLocation::get(), + remote: RelayUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + WithdrawAsset(MultiAsset::from((Here, price)).into()), + BuyExecution { fees: (Here, price).into(), weight_limit: Unlimited }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Parachain(100).into(), + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + RefundSurplus, + DepositAsset { assets: Wild(All), beneficiary: Parachain(100).into() }, + ], + ), + outcome: Outcome::Complete(test_weight(5)), + paid: true, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); +} +#[test] +fn sending_to_parachain_of_bridged_chain_without_funds_fails() { let dest: MultiLocation = (Parent, Parent, Remote::get(), Parachain(100)).into(); // Routing won't work if we don't have enough funds. assert_eq!( send_xcm::(dest.clone(), Xcm(vec![Trap(1)])), Err(SendError::Transport("Error executing")), ); - - // Initialize the local relay so that our parachain has funds to pay for export. - add_asset(Parachain(100), (Here, 1000u128)); - - let msg = Xcm(vec![Trap(1)]); - assert_eq!(send_xcm::(dest, msg).unwrap().1, (Parent, 200u128).into()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![( - Parachain(100).into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(100).into()), - Trap(1), - ]), - )]; - assert_eq!(take_received_remote_messages(), expected); - - // The export cost 50 ref time and 50 proof size weight units (and thus 100 units of balance). - assert_eq!(asset_list(Parachain(100)), vec![(Here, 800u128).into()]); } diff --git a/xcm/xcm-builder/src/tests/bridging/remote_para_para.rs b/xcm/xcm-builder/src/tests/bridging/remote_para_para.rs index 06df4cae2a79..b760e6829e9f 100644 --- a/xcm/xcm-builder/src/tests/bridging/remote_para_para.rs +++ b/xcm/xcm-builder/src/tests/bridging/remote_para_para.rs @@ -34,7 +34,7 @@ type LocalInnerRouter = UnpaidExecutingRouter; type LocalBridgingRouter = UnpaidRemoteExporter, LocalInnerRouter, UniversalLocation>; -type LocalRouter = (LocalInnerRouter, LocalBridgingRouter); +type LocalRouter = TestTopic<(LocalInnerRouter, LocalBridgingRouter)>; /// ```nocompile /// local | remote @@ -48,25 +48,49 @@ type LocalRouter = (LocalInnerRouter, LocalBridgingRouter); /// ``` #[test] fn sending_to_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - assert_eq!( - send_xcm::((Parent, Parent, Remote::get(), Parachain(1)).into(), msg) - .unwrap() - .1, - MultiAssets::new() - ); - assert_eq!(TheBridge::service(), 1); - assert_eq!( - take_received_remote_messages(), - vec![( - Here.into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1000).into()), - Trap(1) - ]) - )] - ); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + assert_eq!( + send_xcm::((Parent, Parent, Remote::get(), Parachain(1)).into(), msg) + .unwrap() + .1, + MultiAssets::new() + ); + assert_eq!(TheBridge::service(), 1); + assert_eq!( + take_received_remote_messages(), + vec![( + Here.into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1000).into()), + Trap(1) + ] + ) + )] + ); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: ParaBridgeUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Parachain(1).into(), + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); } /// ```nocompile @@ -81,19 +105,43 @@ fn sending_to_bridged_chain_works() { /// ``` #[test] fn sending_to_sibling_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Parent, Remote::get(), Parachain(1000)).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![( - (Parent, Parachain(1000)).into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1000).into()), - Trap(1), - ]), - )]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Parent, Remote::get(), Parachain(1000)).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + (Parent, Parachain(1000)).into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1000).into()), + Trap(1), + ], + ), + )]; + assert_eq!(take_received_remote_messages(), expected); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: ParaBridgeUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Parachain(1000).into(), + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); } /// ```nocompile @@ -108,17 +156,41 @@ fn sending_to_sibling_of_bridged_chain_works() { /// ``` #[test] fn sending_to_relay_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Parent, Remote::get()).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![( - Parent.into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1000).into()), - Trap(1), - ]), - )]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Parent, Remote::get()).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Parent.into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1000).into()), + Trap(1), + ], + ), + )]; + assert_eq!(take_received_remote_messages(), expected); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: ParaBridgeUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Here, + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); } diff --git a/xcm/xcm-builder/src/tests/bridging/remote_para_para_via_relay.rs b/xcm/xcm-builder/src/tests/bridging/remote_para_para_via_relay.rs index 56579643861d..9dc94e27e9aa 100644 --- a/xcm/xcm-builder/src/tests/bridging/remote_para_para_via_relay.rs +++ b/xcm/xcm-builder/src/tests/bridging/remote_para_para_via_relay.rs @@ -34,7 +34,7 @@ type LocalInnerRouter = UnpaidExecutingRouter; type LocalBridgingRouter = UnpaidRemoteExporter, LocalInnerRouter, UniversalLocation>; -type LocalRouter = (LocalInnerRouter, LocalBridgingRouter); +type LocalRouter = TestTopic<(LocalInnerRouter, LocalBridgingRouter)>; /// ```nocompile /// local | remote @@ -48,18 +48,40 @@ type LocalRouter = (LocalInnerRouter, LocalBridgingRouter); /// ``` #[test] fn sending_to_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - assert_eq!( - send_xcm::((Parent, Remote::get(), Parachain(1)).into(), msg) - .unwrap() - .1, - MultiAssets::new() - ); - assert_eq!(TheBridge::service(), 1); - assert_eq!( - take_received_remote_messages(), - vec![(Here.into(), Xcm(vec![UniversalOrigin(Local::get().into()), Trap(1)]))] - ); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + assert_eq!( + send_xcm::((Parent, Remote::get(), Parachain(1)).into(), msg) + .unwrap() + .1, + MultiAssets::new() + ); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Here.into(), + xcm_with_topic([0; 32], vec![UniversalOrigin(Local::get().into()), Trap(1)]), + )]; + assert_eq!(take_received_remote_messages(), expected); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: ParaBridgeUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Parachain(1).into(), + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); } /// ```nocompile @@ -74,15 +96,36 @@ fn sending_to_bridged_chain_works() { /// ``` #[test] fn sending_to_sibling_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Remote::get(), Parachain(1000)).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![( - (Parent, Parachain(1000)).into(), - Xcm(vec![UniversalOrigin(Local::get().into()), Trap(1)]), - )]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Remote::get(), Parachain(1000)).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + (Parent, Parachain(1000)).into(), + xcm_with_topic([0; 32], vec![UniversalOrigin(Local::get().into()), Trap(1)]), + )]; + assert_eq!(take_received_remote_messages(), expected); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: ParaBridgeUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Parachain(1000).into(), + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); } /// ```nocompile @@ -97,10 +140,34 @@ fn sending_to_sibling_of_bridged_chain_works() { /// ``` #[test] fn sending_to_relay_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Remote::get()).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![(Parent.into(), Xcm(vec![UniversalOrigin(Local::get().into()), Trap(1)]))]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Remote::get()).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Parent.into(), + xcm_with_topic([0; 32], vec![UniversalOrigin(Local::get().into()), Trap(1)]), + )]; + assert_eq!(take_received_remote_messages(), expected); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: ParaBridgeUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Here, + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); } diff --git a/xcm/xcm-builder/src/tests/bridging/remote_relay_relay.rs b/xcm/xcm-builder/src/tests/bridging/remote_relay_relay.rs index 4230ea27b715..2b34c93c0752 100644 --- a/xcm/xcm-builder/src/tests/bridging/remote_relay_relay.rs +++ b/xcm/xcm-builder/src/tests/bridging/remote_relay_relay.rs @@ -34,7 +34,7 @@ type LocalInnerRouter = UnpaidExecutingRouter; type LocalBridgeRouter = UnpaidRemoteExporter, LocalInnerRouter, UniversalLocation>; -type LocalRouter = (LocalInnerRouter, LocalBridgeRouter); +type LocalRouter = TestTopic<(LocalInnerRouter, LocalBridgeRouter)>; /// ```nocompile /// local | remote @@ -48,23 +48,47 @@ type LocalRouter = (LocalInnerRouter, LocalBridgeRouter); /// ``` #[test] fn sending_to_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - assert_eq!( - send_xcm::((Parent, Parent, Remote::get()).into(), msg).unwrap().1, - MultiAssets::new() - ); - assert_eq!(TheBridge::service(), 1); - assert_eq!( - take_received_remote_messages(), - vec![( - Here.into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1000).into()), - Trap(1) - ]) - )] - ); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + assert_eq!( + send_xcm::((Parent, Parent, Remote::get()).into(), msg).unwrap().1, + MultiAssets::new() + ); + assert_eq!(TheBridge::service(), 1); + assert_eq!( + take_received_remote_messages(), + vec![( + Here.into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1000).into()), + Trap(1) + ] + ) + )] + ); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: RelayUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Here, + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }); } /// ```nocompile @@ -79,17 +103,41 @@ fn sending_to_bridged_chain_works() { /// ``` #[test] fn sending_to_parachain_of_bridged_chain_works() { - let msg = Xcm(vec![Trap(1)]); - let dest = (Parent, Parent, Remote::get(), Parachain(1000)).into(); - assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); - assert_eq!(TheBridge::service(), 1); - let expected = vec![( - Parachain(1000).into(), - Xcm(vec![ - UniversalOrigin(Local::get().into()), - DescendOrigin(Parachain(1000).into()), - Trap(1), - ]), - )]; - assert_eq!(take_received_remote_messages(), expected); + maybe_with_topic(|| { + let msg = Xcm(vec![Trap(1)]); + let dest = (Parent, Parent, Remote::get(), Parachain(1000)).into(); + assert_eq!(send_xcm::(dest, msg).unwrap().1, MultiAssets::new()); + assert_eq!(TheBridge::service(), 1); + let expected = vec![( + Parachain(1000).into(), + xcm_with_topic( + [0; 32], + vec![ + UniversalOrigin(Local::get().into()), + DescendOrigin(Parachain(1000).into()), + Trap(1), + ], + ), + )]; + assert_eq!(take_received_remote_messages(), expected); + let entry = LogEntry { + local: UniversalLocation::get(), + remote: RelayUniversalLocation::get(), + id: maybe_forward_id_for(&[0; 32]), + message: xcm_with_topic( + maybe_forward_id_for(&[0; 32]), + vec![ + UnpaidExecution { weight_limit: Unlimited, check_origin: None }, + ExportMessage { + network: ByGenesis([1; 32]), + destination: Parachain(1000).into(), + xcm: xcm_with_topic([0; 32], vec![Trap(1)]), + }, + ], + ), + outcome: Outcome::Complete(test_weight(2)), + paid: false, + }; + assert_eq!(RoutingLog::take(), vec![entry]); + }) } diff --git a/xcm/xcm-builder/src/tests/mock.rs b/xcm/xcm-builder/src/tests/mock.rs index dad4e8854448..b87e78df5b12 100644 --- a/xcm/xcm-builder/src/tests/mock.rs +++ b/xcm/xcm-builder/src/tests/mock.rs @@ -15,7 +15,7 @@ // along with Polkadot. If not, see . use crate::{ - barriers::{AllowSubscriptionsFrom, RespectSuspension}, + barriers::{AllowSubscriptionsFrom, RespectSuspension, TrailingSetTopicAsId}, test_utils::*, }; pub use crate::{ @@ -41,6 +41,7 @@ pub use sp_std::{ marker::PhantomData, }; pub use xcm::latest::{prelude::*, Weight}; +use xcm_executor::traits::Properties; pub use xcm_executor::{ traits::{ AssetExchange, AssetLock, CheckSuspension, ConvertOrigin, Enact, ExportXcm, FeeManager, @@ -241,6 +242,9 @@ pub fn asset_list(who: impl Into) -> Vec { pub fn add_asset(who: impl Into, what: impl Into) { ASSETS.with(|a| a.borrow_mut().entry(who.into()).or_insert(Assets::new()).subsume(what.into())); } +pub fn clear_assets(who: impl Into) { + ASSETS.with(|a| a.borrow_mut().remove(&who.into())); +} pub struct TestAssetTransactor; impl TransactAsset for TestAssetTransactor { @@ -429,7 +433,7 @@ impl CheckSuspension for TestSuspender { _origin: &MultiLocation, _instructions: &mut [Instruction], _max_weight: Weight, - _weight_credit: &mut Weight, + _properties: &mut Properties, ) -> bool { SUSPENDED.with(|s| s.get()) } @@ -651,7 +655,7 @@ impl Config for TestConfig { type IsReserve = TestIsReserve; type IsTeleporter = TestIsTeleporter; type UniversalLocation = ExecutorUniversalLocation; - type Barrier = RespectSuspension; + type Barrier = TrailingSetTopicAsId>; type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; type ResponseHandler = TestResponseHandler; diff --git a/xcm/xcm-builder/src/universal_exports.rs b/xcm/xcm-builder/src/universal_exports.rs index 2d1c9f6d4c35..38795042d00d 100644 --- a/xcm/xcm-builder/src/universal_exports.rs +++ b/xcm/xcm-builder/src/universal_exports.rs @@ -130,6 +130,10 @@ impl)>>> ExporterFor } } +pub fn forward_id_for(original_id: &XcmHash) -> XcmHash { + (b"forward_id_for", original_id).using_encoded(sp_io::hashing::blake2_256) +} + /// Implementation of `SendXcm` which wraps the message inside an `ExportMessage` instruction /// and sends it to a destination known to be able to handle it. /// @@ -139,6 +143,16 @@ impl)>>> ExporterFor /// /// This is only useful if we have special dispensation by the remote bridges to have the /// `ExportMessage` instruction executed without payment. +/// +/// The `XcmHash` value returned by `deliver` will always be the same as that returned by the +/// message exporter (`Bridges`). Generally this should take notice of the message should it +/// end with the `SetTopic` instruction. +/// +/// In the case that the message ends with a `SetTopic(T)` (as should be the case if the top-level +/// router is `EnsureUniqueTopic`), then the forwarding message (i.e. the one carrying the +/// export instruction *to* the bridge in local consensus) will also end with a `SetTopic` whose +/// inner is `forward_id_for(T)`. If this is not the case then the onward message will not be given +/// the `SetTopic` afterword. pub struct UnpaidRemoteExporter( PhantomData<(Bridges, Router, UniversalLocation)>, ); @@ -155,21 +169,32 @@ impl Some(forward_id_for(t)), + _ => None, + }; + // We then send a normal message to the bridge asking it to export the prepended // message to the remote chain. This will only work if the bridge will do the message // export for free. Common-good chains will typically be afforded this. - let message = Xcm(vec![ + let mut message = Xcm(vec![ UnpaidExecution { weight_limit: Unlimited, check_origin: None }, ExportMessage { network: remote_network, destination: remote_location, xcm }, ]); + if let Some(forward_id) = maybe_forward_id { + message.0.push(SetTopic(forward_id)); + } validate_send::(bridge, message) } - fn deliver(validation: Router::Ticket) -> Result { + fn deliver(validation: Self::Ticket) -> Result { Router::deliver(validation) } } @@ -179,6 +204,16 @@ impl( PhantomData<(Bridges, Router, UniversalLocation)>, ); @@ -196,6 +231,14 @@ impl Some(forward_id_for(t)), + _ => None, + }; + let (bridge, maybe_payment) = Bridges::exporter_for(&remote_network, &remote_location, &xcm).ok_or(NotApplicable)?; @@ -204,7 +247,7 @@ impl, Price: Get> .ok_or(SendError::MissingArgument)? .split_global() .map_err(|()| SendError::Unroutable)?; - let mut inner: Xcm<()> = vec![UniversalOrigin(GlobalConsensus(local_net))].into(); + let mut message = message.take().ok_or(SendError::MissingArgument)?; + let maybe_id = match message.last() { + Some(SetTopic(t)) => Some(*t), + _ => None, + }; + message.0.insert(0, UniversalOrigin(GlobalConsensus(local_net))); if local_sub != Here { - inner.inner_mut().push(DescendOrigin(local_sub)); + message.0.insert(1, DescendOrigin(local_sub)); } - inner - .inner_mut() - .extend(message.take().ok_or(SendError::MissingArgument)?.into_iter()); - let message = VersionedXcm::from(inner); - let hash = message.using_encoded(sp_io::hashing::blake2_256); + let message = VersionedXcm::from(message); + let id = maybe_id.unwrap_or_else(|| message.using_encoded(sp_io::hashing::blake2_256)); let blob = BridgeMessage { universal_dest, message }.encode(); - Ok(((blob, hash), Price::get())) + Ok(((blob, id), Price::get())) } - fn deliver((blob, hash): (Vec, XcmHash)) -> Result { + fn deliver((blob, id): (Vec, XcmHash)) -> Result { Bridge::haul_blob(blob)?; - Ok(hash) + Ok(id) } } diff --git a/xcm/xcm-builder/tests/scenarios.rs b/xcm/xcm-builder/tests/scenarios.rs index 64e9b86a91cd..e587c4118e74 100644 --- a/xcm/xcm-builder/tests/scenarios.rs +++ b/xcm/xcm-builder/tests/scenarios.rs @@ -16,7 +16,6 @@ mod mock; -use frame_support::weights::Weight; use mock::{ fake_message_hash, kusama_like_with_balances, AccountId, Balance, Balances, BaseXcmWeight, System, XcmConfig, CENTS, diff --git a/xcm/xcm-executor/integration-tests/src/lib.rs b/xcm/xcm-executor/integration-tests/src/lib.rs index 1500bb836ae4..95138250cca9 100644 --- a/xcm/xcm-executor/integration-tests/src/lib.rs +++ b/xcm/xcm-executor/integration-tests/src/lib.rs @@ -65,9 +65,9 @@ fn basic_buy_fees_message_executes() { client.state_at(block_hash).expect("state should exist").inspect_state(|| { assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!( r.event, - polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::Attempted( - Outcome::Complete(_) - )), + polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::Attempted { + outcome: Outcome::Complete(_) + }), ))); }); } @@ -118,9 +118,9 @@ fn transact_recursion_limit_works() { client.state_at(block_hash).expect("state should exist").inspect_state(|| { assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!( r.event, - polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::Attempted( - Outcome::Incomplete(_, XcmError::ExceedsStackLimit) - )), + polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::Attempted { + outcome: Outcome::Incomplete(_, XcmError::ExceedsStackLimit) + }), ))); }); } @@ -195,10 +195,10 @@ fn query_response_fires() { client.state_at(block_hash).expect("state should exist").inspect_state(|| { assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!( r.event, - polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::ResponseReady( - q, - Response::ExecutionResult(None), - )) if q == query_id, + polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::ResponseReady { + query_id: q, + response: Response::ExecutionResult(None), + }) if q == query_id, ))); assert_eq!( polkadot_test_runtime::Xcm::query(query_id), diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 749a63114d95..ce9d3d4644e8 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -31,8 +31,8 @@ use xcm::latest::prelude::*; pub mod traits; use traits::{ validate_export, AssetExchange, AssetLock, CallDispatcher, ClaimAssets, ConvertOrigin, - DropAssets, Enact, ExportXcm, FeeManager, FeeReason, OnResponse, ShouldExecute, TransactAsset, - VersionChangeNotifier, WeightBounds, WeightTrader, + DropAssets, Enact, ExportXcm, FeeManager, FeeReason, OnResponse, Properties, ShouldExecute, + TransactAsset, VersionChangeNotifier, WeightBounds, WeightTrader, }; mod assets; @@ -193,8 +193,8 @@ impl ExecuteXcm for XcmExecutor, WeighedMessage(xcm_weight, mut message): WeighedMessage, - message_hash: XcmHash, - mut weight_credit: Weight, + id: &mut XcmHash, + weight_credit: Weight, ) -> Outcome { let origin = origin.into(); log::trace!( @@ -204,24 +204,27 @@ impl ExecuteXcm for XcmExecutor for frame_benchmarking::BenchmarkError { } impl XcmExecutor { - pub fn new(origin: impl Into, message_hash: XcmHash) -> Self { + pub fn new(origin: impl Into, message_id: XcmHash) -> Self { let origin = origin.into(); Self { holding: Assets::new(), holding_limit: Config::MaxAssetsIntoHolding::get() as usize, - context: XcmContext { origin: Some(origin), message_hash, topic: None }, + context: XcmContext { origin: Some(origin), message_id, topic: None }, original_origin: origin, trader: Config::Trader::new(), error: None, diff --git a/xcm/xcm-executor/src/traits/mod.rs b/xcm/xcm-executor/src/traits/mod.rs index 64c77f2abf45..3b904630d73e 100644 --- a/xcm/xcm-executor/src/traits/mod.rs +++ b/xcm/xcm-executor/src/traits/mod.rs @@ -40,7 +40,7 @@ pub use token_matching::{ mod on_response; pub use on_response::{OnResponse, VersionChangeNotifier}; mod should_execute; -pub use should_execute::{CheckSuspension, ShouldExecute}; +pub use should_execute::{CheckSuspension, Properties, ShouldExecute}; mod transact_asset; pub use transact_asset::TransactAsset; mod weight; diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index 84aa9d1364ea..2b634e375136 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -16,7 +16,19 @@ use frame_support::traits::ProcessMessageError; use sp_std::result::Result; -use xcm::latest::{Instruction, MultiLocation, Weight}; +use xcm::latest::{Instruction, MultiLocation, Weight, XcmHash}; + +/// Properyies of an XCM message and its imminent execution. +#[derive(Clone, Eq, PartialEq, Debug)] +pub struct Properties { + /// The amount of weight that the system has determined this + /// message may utilize in its execution. Typically non-zero only because of prior fee + /// payment, but could in principle be due to other factors. + pub weight_credit: Weight, + /// The identity of the message, if one is known. If left as `None`, then it will generally + /// default to the hash of the message which may be non-unique. + pub message_id: Option, +} /// Trait to determine whether the execution engine should actually execute a given XCM. /// @@ -28,14 +40,13 @@ pub trait ShouldExecute { /// - `origin`: The origin (sender) of the message. /// - `instructions`: The message itself. /// - `max_weight`: The (possibly over-) estimation of the weight of execution of the message. - /// - `weight_credit`: The pre-established amount of weight that the system has determined this - /// message may utilize in its execution. Typically non-zero only because of prior fee - /// payment, but could in principle be due to other factors. + /// - `properties`: Various pre-established properties of the message which may be mutated by + /// this API. fn should_execute( origin: &MultiLocation, instructions: &mut [Instruction], max_weight: Weight, - weight_credit: &mut Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError>; } @@ -45,21 +56,21 @@ impl ShouldExecute for Tuple { origin: &MultiLocation, instructions: &mut [Instruction], max_weight: Weight, - weight_credit: &mut Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { for_tuples!( #( - match Tuple::should_execute(origin, instructions, max_weight, weight_credit) { + match Tuple::should_execute(origin, instructions, max_weight, properties) { Ok(()) => return Ok(()), _ => (), } )* ); log::trace!( target: "xcm::should_execute", - "did not pass barrier: origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", + "did not pass barrier: origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", origin, instructions, max_weight, - weight_credit, + properties, ); Err(ProcessMessageError::Unsupported) } @@ -79,7 +90,7 @@ pub trait CheckSuspension { origin: &MultiLocation, instructions: &mut [Instruction], max_weight: Weight, - weight_credit: &mut Weight, + properties: &mut Properties, ) -> bool; } @@ -89,10 +100,10 @@ impl CheckSuspension for Tuple { origin: &MultiLocation, instruction: &mut [Instruction], max_weight: Weight, - weight_credit: &mut Weight, + properties: &mut Properties, ) -> bool { for_tuples!( #( - if Tuple::is_suspended(origin, instruction, max_weight, weight_credit) { + if Tuple::is_suspended(origin, instruction, max_weight, properties) { return true } )* ); diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index 0770228ed827..832397a0fd25 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -397,7 +397,7 @@ mod tests { MultiTransactor::deposit_asset( &(Here, 1u128).into(), &Here.into(), - &XcmContext::with_message_hash([0; 32]), + &XcmContext::with_message_id([0; 32]), ), Err(XcmError::AssetNotFound) ); @@ -411,7 +411,7 @@ mod tests { MultiTransactor::deposit_asset( &(Here, 1u128).into(), &Here.into(), - &XcmContext::with_message_hash([0; 32]), + &XcmContext::with_message_id([0; 32]), ), Ok(()) ); @@ -425,7 +425,7 @@ mod tests { MultiTransactor::deposit_asset( &(Here, 1u128).into(), &Here.into(), - &XcmContext::with_message_hash([0; 32]), + &XcmContext::with_message_id([0; 32]), ), Err(XcmError::Overflow) ); @@ -439,7 +439,7 @@ mod tests { MultiTransactor::deposit_asset( &(Here, 1u128).into(), &Here.into(), - &XcmContext::with_message_hash([0; 32]), + &XcmContext::with_message_id([0; 32]), ), Ok(()), );