diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 2980e45e529e..ad1315716715 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -608,6 +608,7 @@ pub mod pallet_test_notifier { let qid = pallet_xcm::Pallet::::new_query( Junction::AccountId32 { network: Any, id }.into(), 100u32.into(), + Here, ); Self::deposit_event(Event::::QueryPrepared(qid)); Ok(()) @@ -625,6 +626,7 @@ pub mod pallet_test_notifier { Junction::AccountId32 { network: Any, id }.into(), ::Call::from(call), 100u32.into(), + Here, ); Self::deposit_event(Event::::NotifyQueryPrepared(qid)); Ok(()) diff --git a/scripts/gitlab/lingua.dic b/scripts/gitlab/lingua.dic index bf70b0512e40..b4621cec88a3 100644 --- a/scripts/gitlab/lingua.dic +++ b/scripts/gitlab/lingua.dic @@ -215,6 +215,7 @@ proxy/G proxying PRs PVF/S +querier README/MS redhat/M register/CD diff --git a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index aee21000afde..472ee6b55ac3 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -96,7 +96,8 @@ benchmarks! { let mut executor = new_executor::(Default::default()); let (query_id, response) = T::worst_case_response(); let max_weight = u64::MAX; - let instruction = Instruction::QueryResponse { query_id, response, max_weight }; + let querier: Option = Some(Here.into()); + let instruction = Instruction::QueryResponse { query_id, response, max_weight, querier }; let xcm = Xcm(vec![instruction]); }: { executor.execute(xcm)?; diff --git a/xcm/pallet-xcm-benchmarks/src/mock.rs b/xcm/pallet-xcm-benchmarks/src/mock.rs index d59cf3387268..6390d251010e 100644 --- a/xcm/pallet-xcm-benchmarks/src/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/mock.rs @@ -27,10 +27,16 @@ impl xcm::opaque::latest::SendXcm for DevNull { } impl xcm_executor::traits::OnResponse for DevNull { - fn expecting_response(_: &MultiLocation, _: u64) -> bool { + fn expecting_response(_: &MultiLocation, _: u64, _: Option<&MultiLocation>) -> bool { false } - fn on_response(_: &MultiLocation, _: u64, _: Response, _: Weight) -> Weight { + fn on_response( + _: &MultiLocation, + _: u64, + _: Option<&MultiLocation>, + _: Response, + _: Weight, + ) -> Weight { 0 } } diff --git a/xcm/pallet-xcm/src/lib.rs b/xcm/pallet-xcm/src/lib.rs index 5df4aade835a..9275e4ea9999 100644 --- a/xcm/pallet-xcm/src/lib.rs +++ b/xcm/pallet-xcm/src/lib.rs @@ -215,6 +215,22 @@ pub mod pallet { /// /// \[ location, query ID \] NotifyTargetMigrationFail(VersionedMultiLocation, 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. + /// + /// This is unexpected (since a location placed in storage in a previously executing + /// 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), + /// 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), } #[pallet::origin] @@ -269,7 +285,12 @@ pub mod pallet { pub enum QueryStatus { /// The query was sent but no response has yet been received. Pending { + /// The `QueryResponse` XCM must have this origin to be considered a reply for this + /// query. responder: VersionedMultiLocation, + /// The `QueryResponse` XCM must have this value as the `querier` field to be + /// considered a reply for this query. If `None` then the querier is ignored. + maybe_match_querier: Option, maybe_notify: Option<(u8, u8)>, timeout: BlockNumber, }, @@ -449,6 +470,77 @@ pub mod pallet { } } + pub mod migrations { + use super::*; + use frame_support::traits::{PalletInfoAccess, StorageVersion}; + + #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] + enum QueryStatusV0 { + Pending { + responder: VersionedMultiLocation, + maybe_notify: Option<(u8, u8)>, + timeout: BlockNumber, + }, + VersionNotifier { + origin: VersionedMultiLocation, + is_active: bool, + }, + Ready { + response: VersionedResponse, + at: BlockNumber, + }, + } + impl From> for QueryStatus { + fn from(old: QueryStatusV0) -> Self { + use QueryStatusV0::*; + match old { + Pending { responder, maybe_notify, timeout } => QueryStatus::Pending { + responder, + maybe_notify, + timeout, + maybe_match_querier: Some(MultiLocation::here().into()), + }, + VersionNotifier { origin, is_active } => + QueryStatus::VersionNotifier { origin, is_active }, + Ready { response, at } => QueryStatus::Ready { response, at }, + } + } + } + + pub fn migrate_to_v1( + ) -> frame_support::weights::Weight { + let on_chain_storage_version =

::on_chain_storage_version(); + log::info!( + target: "runtime::xcm", + "Running migration storage v1 for xcm with storage version {:?}", + on_chain_storage_version, + ); + + if on_chain_storage_version < 1 { + let mut count = 0; + Queries::::translate::, _>(|_key, value| { + count += 1; + Some(value.into()) + }); + StorageVersion::new(1).put::

(); + log::info!( + target: "runtime::xcm", + "Running migration storage v1 for xcm with storage version {:?} was complete", + on_chain_storage_version, + ); + // calculate and return migration weights + T::DbWeight::get().reads_writes(count as Weight + 1, count as Weight + 1) + } else { + log::warn!( + target: "runtime::xcm", + "Attempted to apply migration to v1 but failed because storage version is {:?}", + on_chain_storage_version, + ); + T::DbWeight::get().reads(1) + } + } + } + #[pallet::call] impl Pallet { #[pallet::weight(100_000_000)] @@ -952,7 +1044,8 @@ pub mod pallet { }, }; let response = Response::Version(xcm_version); - let message = Xcm(vec![QueryResponse { query_id, response, max_weight }]); + let message = + Xcm(vec![QueryResponse { query_id, response, max_weight, querier: None }]); let event = match T::XcmRouter::send_xcm(new_key.clone(), message) { Ok(()) => { let value = (query_id, max_weight, xcm_version); @@ -998,8 +1091,12 @@ pub mod pallet { } else { // Need to notify target. let response = Response::Version(xcm_version); - let message = - Xcm(vec![QueryResponse { query_id, response, max_weight }]); + let message = Xcm(vec![QueryResponse { + query_id, + response, + max_weight, + querier: None, + }]); let event = match T::XcmRouter::send_xcm(new_key.clone(), message) { Ok(()) => { VersionNotifyTargets::::insert( @@ -1076,10 +1173,12 @@ pub mod pallet { AccountIdConversion::::into_account(&ID) } + /// Create a new expectation of a query response with the querier being here. fn do_new_query( responder: impl Into, maybe_notify: Option<(u8, u8)>, timeout: T::BlockNumber, + match_querier: impl Into, ) -> u64 { QueryCounter::::mutate(|q| { let r = *q; @@ -1088,6 +1187,7 @@ pub mod pallet { r, QueryStatus::Pending { responder: responder.into().into(), + maybe_match_querier: Some(match_querier.into().into()), maybe_notify, timeout, }, @@ -1106,6 +1206,8 @@ pub mod pallet { /// /// `report_outcome` may return an error if the `responder` is not invertible. /// + /// It is assumed that the querier of the response will be `Here`. + /// /// To check the status of the query, use `fn query()` passing the resultant `QueryId` /// value. pub fn report_outcome( @@ -1116,7 +1218,7 @@ pub mod pallet { let responder = responder.into(); let destination = T::LocationInverter::invert_location(&responder) .map_err(|()| XcmError::MultiLocationNotInvertible)?; - let query_id = Self::new_query(responder, timeout); + let query_id = Self::new_query(responder, timeout, Here); let response_info = QueryResponseInfo { destination, query_id, max_weight: 0 }; let report_error = Xcm(vec![ReportError(response_info)]); message.0.insert(0, SetAppendix(report_error)); @@ -1138,6 +1240,8 @@ pub mod pallet { /// /// `report_outcome_notify` may return an error if the `responder` is not invertible. /// + /// It is assumed that the querier of the response will be `Here`. + /// /// NOTE: `notify` gets called as part of handling an incoming message, so it should be /// lightweight. Its weight is estimated during this function and stored ready for /// weighing `ReportOutcome` on the way back. If it turns out to be heavier once it returns @@ -1154,7 +1258,7 @@ pub mod pallet { .map_err(|()| XcmError::MultiLocationNotInvertible)?; let notify: ::Call = notify.into(); let max_weight = notify.get_dispatch_info().weight; - let query_id = Self::new_notify_query(responder, notify, timeout); + let query_id = Self::new_notify_query(responder, notify, timeout, Here); let response_info = QueryResponseInfo { destination, query_id, max_weight }; let report_error = Xcm(vec![ReportError(response_info)]); message.0.insert(0, SetAppendix(report_error)); @@ -1162,8 +1266,12 @@ pub mod pallet { } /// Attempt to create a new query ID and register it as a query that is yet to respond. - pub fn new_query(responder: impl Into, timeout: T::BlockNumber) -> u64 { - Self::do_new_query(responder, None, timeout) + pub fn new_query( + responder: impl Into, + timeout: T::BlockNumber, + match_querier: impl Into, + ) -> u64 { + Self::do_new_query(responder, None, timeout, match_querier) } /// Attempt to create a new query ID and register it as a query that is yet to respond, and @@ -1172,12 +1280,13 @@ pub mod pallet { responder: impl Into, notify: impl Into<::Call>, timeout: T::BlockNumber, + match_querier: impl Into, ) -> u64 { let notify = notify.into().using_encoded(|mut bytes| Decode::decode(&mut bytes)).expect( "decode input is output of Call encode; Call guaranteed to have two enums; qed", ); - Self::do_new_query(responder, Some(notify), timeout) + Self::do_new_query(responder, Some(notify), timeout, match_querier) } /// Attempt to remove and return the response of query with ID `query_id`. @@ -1252,7 +1361,7 @@ pub mod pallet { let xcm_version = T::AdvertisedXcmVersion::get(); let response = Response::Version(xcm_version); - let instruction = QueryResponse { query_id, response, max_weight }; + let instruction = QueryResponse { query_id, response, max_weight, querier: None }; T::XcmRouter::send_xcm(dest.clone(), Xcm(vec![instruction]))?; let value = (query_id, max_weight, xcm_version); @@ -1315,10 +1424,19 @@ pub mod pallet { } impl OnResponse for Pallet { - fn expecting_response(origin: &MultiLocation, query_id: QueryId) -> bool { + fn expecting_response( + origin: &MultiLocation, + query_id: QueryId, + querier: Option<&MultiLocation>, + ) -> bool { match Queries::::get(query_id) { - Some(QueryStatus::Pending { responder, .. }) => - MultiLocation::try_from(responder).map_or(false, |r| origin == &r), + Some(QueryStatus::Pending { responder, maybe_match_querier, .. }) => + MultiLocation::try_from(responder).map_or(false, |r| origin == &r) && + maybe_match_querier.map_or(true, |match_querier| { + MultiLocation::try_from(match_querier).map_or(false, |match_querier| { + querier.map_or(false, |q| q == &match_querier) + }) + }), Some(QueryStatus::VersionNotifier { origin: r, .. }) => MultiLocation::try_from(r).map_or(false, |r| origin == &r), _ => false, @@ -1328,6 +1446,7 @@ pub mod pallet { fn on_response( origin: &MultiLocation, query_id: QueryId, + querier: Option<&MultiLocation>, response: Response, max_weight: Weight, ) -> Weight { @@ -1375,7 +1494,33 @@ pub mod pallet { Self::deposit_event(Event::SupportedVersionChanged(origin, v)); 0 }, - (response, Some(QueryStatus::Pending { responder, maybe_notify, .. })) => { + ( + response, + Some(QueryStatus::Pending { + responder, maybe_notify, maybe_match_querier, .. + }), + ) => { + if let Some(match_querier) = maybe_match_querier { + let match_querier = match MultiLocation::try_from(match_querier) { + Ok(mq) => mq, + Err(_) => { + Self::deposit_event(Event::InvalidQuerierVersion( + origin.clone(), + query_id, + )); + return 0 + }, + }; + if querier.map_or(true, |q| q != &match_querier) { + Self::deposit_event(Event::InvalidQuerier( + origin.clone(), + query_id, + match_querier, + querier.cloned(), + )); + return 0 + } + } let responder = match MultiLocation::try_from(responder) { Ok(r) => r, Err(_) => { diff --git a/xcm/pallet-xcm/src/mock.rs b/xcm/pallet-xcm/src/mock.rs index af9ef8f98548..584dc651a906 100644 --- a/xcm/pallet-xcm/src/mock.rs +++ b/xcm/pallet-xcm/src/mock.rs @@ -74,7 +74,7 @@ pub mod pallet_test_notifier { #[pallet::call] impl Pallet { #[pallet::weight(1_000_000)] - pub fn prepare_new_query(origin: OriginFor) -> DispatchResult { + pub fn prepare_new_query(origin: OriginFor, querier: MultiLocation) -> DispatchResult { let who = ensure_signed(origin)?; let id = who .using_encoded(|mut d| <[u8; 32]>::decode(&mut d)) @@ -82,13 +82,17 @@ pub mod pallet_test_notifier { let qid = crate::Pallet::::new_query( Junction::AccountId32 { network: Any, id }.into(), 100u32.into(), + querier, ); Self::deposit_event(Event::::QueryPrepared(qid)); Ok(()) } #[pallet::weight(1_000_000)] - pub fn prepare_new_notify_query(origin: OriginFor) -> DispatchResult { + pub fn prepare_new_notify_query( + origin: OriginFor, + querier: MultiLocation, + ) -> DispatchResult { let who = ensure_signed(origin)?; let id = who .using_encoded(|mut d| <[u8; 32]>::decode(&mut d)) @@ -99,6 +103,7 @@ pub mod pallet_test_notifier { Junction::AccountId32 { network: Any, id }.into(), ::Call::from(call), 100u32.into(), + querier, ); Self::deposit_event(Event::::NotifyQueryPrepared(qid)); Ok(()) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 7b9b6ce864c6..c346026d17cf 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -63,10 +63,12 @@ fn report_outcome_notify_works() { TransferAsset { assets: (Here, SEND_AMOUNT).into(), beneficiary: sender.clone() }, ]) ); + let querier: MultiLocation = Here.into(); let status = QueryStatus::Pending { responder: MultiLocation::from(Parachain(PARA_ID)).into(), maybe_notify: Some((4, 2)), timeout: 100, + maybe_match_querier: Some(querier.clone().into()), }; assert_eq!(crate::Queries::::iter().collect::>(), vec![(0, status)]); @@ -76,6 +78,7 @@ fn report_outcome_notify_works() { query_id: 0, response: Response::ExecutionResult(None), max_weight: 1_000_000, + querier: Some(querier), }]), 1_000_000_000, ); @@ -117,10 +120,12 @@ fn report_outcome_works() { TransferAsset { assets: (Here, SEND_AMOUNT).into(), beneficiary: sender.clone() }, ]) ); + let querier: MultiLocation = Here.into(); let status = QueryStatus::Pending { responder: MultiLocation::from(Parachain(PARA_ID)).into(), maybe_notify: None, timeout: 100, + maybe_match_querier: Some(querier.clone().into()), }; assert_eq!(crate::Queries::::iter().collect::>(), vec![(0, status)]); @@ -130,6 +135,97 @@ fn report_outcome_works() { query_id: 0, response: Response::ExecutionResult(None), max_weight: 0, + querier: Some(querier), + }]), + 1_000_000_000, + ); + assert_eq!(r, Outcome::Complete(1_000)); + assert_eq!( + last_event(), + Event::XcmPallet(crate::Event::ResponseReady(0, Response::ExecutionResult(None),)) + ); + + let response = Some((Response::ExecutionResult(None), 1)); + assert_eq!(XcmPallet::take_response(0), response); + }); +} + +#[test] +fn custom_querier_works() { + let balances = + vec![(ALICE, INITIAL_BALANCE), (ParaId::from(PARA_ID).into_account(), INITIAL_BALANCE)]; + new_test_ext_with_balances(balances).execute_with(|| { + let querier: MultiLocation = + (Parent, AccountId32 { network: AnyNetwork::get(), id: ALICE.into() }).into(); + + let r = TestNotifier::prepare_new_query(Origin::signed(ALICE), querier.clone()); + assert_eq!(r, Ok(())); + let status = QueryStatus::Pending { + responder: MultiLocation::from(AccountId32 { + network: AnyNetwork::get(), + id: ALICE.into(), + }) + .into(), + maybe_notify: None, + timeout: 100, + maybe_match_querier: Some(querier.clone().into()), + }; + assert_eq!(crate::Queries::::iter().collect::>(), vec![(0, status)]); + + // Supplying no querier when one is expected will fail + let r = XcmExecutor::::execute_xcm_in_credit( + AccountId32 { network: AnyNetwork::get(), id: ALICE.into() }.into(), + Xcm(vec![QueryResponse { + query_id: 0, + response: Response::ExecutionResult(None), + max_weight: 0, + querier: None, + }]), + 1_000_000_000, + 1_000, + ); + assert_eq!(r, Outcome::Complete(1_000)); + assert_eq!( + last_event(), + Event::XcmPallet(crate::Event::InvalidQuerier( + AccountId32 { network: AnyNetwork::get(), id: ALICE.into() }.into(), + 0, + querier.clone(), + None, + )), + ); + + // Supplying the wrong querier will also fail + let r = XcmExecutor::::execute_xcm_in_credit( + AccountId32 { network: AnyNetwork::get(), id: ALICE.into() }.into(), + Xcm(vec![QueryResponse { + query_id: 0, + response: Response::ExecutionResult(None), + max_weight: 0, + querier: Some(MultiLocation::here()), + }]), + 1_000_000_000, + 1_000, + ); + assert_eq!(r, Outcome::Complete(1_000)); + assert_eq!( + last_event(), + Event::XcmPallet(crate::Event::InvalidQuerier( + AccountId32 { network: AnyNetwork::get(), id: ALICE.into() }.into(), + 0, + querier.clone(), + Some(MultiLocation::here()), + )), + ); + + // Multiple failures should not have changed the query state + let r = XcmExecutor::::execute_xcm( + AccountId32 { network: AnyNetwork::get(), id: ALICE.into() }.into(), + Xcm(vec![QueryResponse { + query_id: 0, + response: Response::ExecutionResult(None), + max_weight: 0, + querier: Some(querier), }]), 1_000_000_000, ); @@ -617,7 +713,12 @@ fn basic_subscription_works() { let weight = BaseXcmWeight::get(); let mut message = Xcm::<()>(vec![ // Remote supports XCM v1 - QueryResponse { query_id: 0, max_weight: 0, response: Response::Version(1) }, + QueryResponse { + query_id: 0, + max_weight: 0, + response: Response::Version(1), + querier: None, + }, ]); assert_ok!(AllowKnownQueryResponses::::should_execute( &remote, @@ -722,7 +823,12 @@ fn subscription_side_works() { let r = XcmExecutor::::execute_xcm(remote.clone(), message, weight); assert_eq!(r, Outcome::Complete(weight)); - let instr = QueryResponse { query_id: 0, max_weight: 0, response: Response::Version(1) }; + let instr = QueryResponse { + query_id: 0, + max_weight: 0, + response: Response::Version(1), + querier: None, + }; assert_eq!(take_sent_xcm(), vec![(remote.clone(), Xcm(vec![instr]))]); // A runtime upgrade which doesn't alter the version sends no notifications. @@ -736,7 +842,12 @@ fn subscription_side_works() { // A runtime upgrade which alters the version does send notifications. XcmPallet::on_runtime_upgrade(); XcmPallet::on_initialize(2); - let instr = QueryResponse { query_id: 0, max_weight: 0, response: Response::Version(2) }; + let instr = QueryResponse { + query_id: 0, + max_weight: 0, + response: Response::Version(2), + querier: None, + }; assert_eq!(take_sent_xcm(), vec![(remote.clone(), Xcm(vec![instr]))]); }); } @@ -762,9 +873,24 @@ fn subscription_side_upgrades_work_with_notify() { XcmPallet::on_runtime_upgrade(); XcmPallet::on_initialize(1); - let instr0 = QueryResponse { query_id: 69, max_weight: 0, response: Response::Version(2) }; - let instr1 = QueryResponse { query_id: 70, max_weight: 0, response: Response::Version(2) }; - let instr2 = QueryResponse { query_id: 71, max_weight: 0, response: Response::Version(2) }; + let instr0 = QueryResponse { + query_id: 69, + max_weight: 0, + response: Response::Version(2), + querier: None, + }; + let instr1 = QueryResponse { + query_id: 70, + max_weight: 0, + response: Response::Version(2), + querier: None, + }; + let instr2 = QueryResponse { + query_id: 71, + max_weight: 0, + response: Response::Version(2), + querier: None, + }; let mut sent = take_sent_xcm(); sent.sort_by_key(|k| match (k.1).0[0] { QueryResponse { query_id: q, .. } => q, @@ -836,7 +962,12 @@ fn subscriber_side_subscription_works() { let weight = BaseXcmWeight::get(); let message = Xcm(vec![ // Remote supports XCM v1 - QueryResponse { query_id: 0, max_weight: 0, response: Response::Version(1) }, + QueryResponse { + query_id: 0, + max_weight: 0, + response: Response::Version(1), + querier: None, + }, ]); let r = XcmExecutor::::execute_xcm(remote.clone(), message, weight); assert_eq!(r, Outcome::Complete(weight)); @@ -848,7 +979,12 @@ fn subscriber_side_subscription_works() { let message = Xcm(vec![ // Remote upgraded to XCM v2 - QueryResponse { query_id: 0, max_weight: 0, response: Response::Version(2) }, + QueryResponse { + query_id: 0, + max_weight: 0, + response: Response::Version(2), + querier: None, + }, ]); let r = XcmExecutor::::execute_xcm(remote.clone(), message, weight); assert_eq!(r, Outcome::Complete(weight)); @@ -903,7 +1039,12 @@ fn auto_subscription_works() { let weight = BaseXcmWeight::get(); let message = Xcm(vec![ // Remote supports XCM v2 - QueryResponse { query_id: 0, max_weight: 0, response: Response::Version(2) }, + QueryResponse { + query_id: 0, + max_weight: 0, + response: Response::Version(2), + querier: None, + }, ]); let r = XcmExecutor::::execute_xcm(remote0.clone(), message, weight); assert_eq!(r, Outcome::Complete(weight)); @@ -928,7 +1069,12 @@ fn auto_subscription_works() { let weight = BaseXcmWeight::get(); let message = Xcm(vec![ // Remote supports XCM v1 - QueryResponse { query_id: 1, max_weight: 0, response: Response::Version(1) }, + QueryResponse { + query_id: 1, + max_weight: 0, + response: Response::Version(1), + querier: None, + }, ]); let r = XcmExecutor::::execute_xcm(remote1.clone(), message, weight); assert_eq!(r, Outcome::Complete(weight)); @@ -967,9 +1113,24 @@ fn subscription_side_upgrades_work_with_multistage_notify() { } assert_eq!(counter, 4); - let instr0 = QueryResponse { query_id: 69, max_weight: 0, response: Response::Version(2) }; - let instr1 = QueryResponse { query_id: 70, max_weight: 0, response: Response::Version(2) }; - let instr2 = QueryResponse { query_id: 71, max_weight: 0, response: Response::Version(2) }; + let instr0 = QueryResponse { + query_id: 69, + max_weight: 0, + response: Response::Version(2), + querier: None, + }; + let instr1 = QueryResponse { + query_id: 70, + max_weight: 0, + response: Response::Version(2), + querier: None, + }; + let instr2 = QueryResponse { + query_id: 71, + max_weight: 0, + response: Response::Version(2), + querier: None, + }; let mut sent = take_sent_xcm(); sent.sort_by_key(|k| match (k.1).0[0] { QueryResponse { query_id: q, .. } => q, diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs index 12b507329215..6c8c648a9c77 100644 --- a/xcm/src/v1/multilocation.rs +++ b/xcm/src/v1/multilocation.rs @@ -324,6 +324,21 @@ impl MultiLocation { Ok(()) } + /// Consume `self` and return the value representing the same location from the point of view + /// of `target`. The context of `self` is provided as `ancestry`. + /// + /// Returns an `Err` with the unmodified `self` in the case of error. + pub fn reanchored( + mut self, + target: &MultiLocation, + ancestry: &MultiLocation, + ) -> Result { + match self.reanchor(target, ancestry) { + Ok(()) => Ok(self), + Err(()) => Err(self), + } + } + /// Mutate `self` so that it represents the same location from the point of view of `target`. /// The context of `self` is provided as `ancestry`. /// diff --git a/xcm/src/v2/mod.rs b/xcm/src/v2/mod.rs index 22b3f81af697..c76cd10c513a 100644 --- a/xcm/src/v2/mod.rs +++ b/xcm/src/v2/mod.rs @@ -932,7 +932,7 @@ impl TryFrom> for Instruction { WithdrawAsset(assets) => Self::WithdrawAsset(assets), ReserveAssetDeposited(assets) => Self::ReserveAssetDeposited(assets), ReceiveTeleportedAsset(assets) => Self::ReceiveTeleportedAsset(assets), - QueryResponse { query_id, response, max_weight } => + QueryResponse { query_id, response, max_weight, .. } => Self::QueryResponse { query_id, response: response.try_into()?, max_weight }, TransferAsset { assets, beneficiary } => Self::TransferAsset { assets, beneficiary }, TransferReserveAsset { assets, dest, xcm } => diff --git a/xcm/src/v3/mod.rs b/xcm/src/v3/mod.rs index 29429a6ceb03..bb73f549240f 100644 --- a/xcm/src/v3/mod.rs +++ b/xcm/src/v3/mod.rs @@ -265,8 +265,15 @@ pub enum Instruction { /// - `query_id`: The identifier of the query that resulted in this message being sent. /// - `response`: The message content. /// - `max_weight`: The maximum weight that handling this response should take. + /// - `querier`: The location responsible for the initiation of the response, if there is one. + /// In general this will tend to be the same location as the receiver of this message. + /// NOTE: As usual, this is interpreted from the perspective of the receiving consensus + /// system. /// - /// Safety: No concerns. + /// Safety: Since this is information only, there are no immediate concerns. However, it should + /// be remembered that even if the Origin behaves reasonably, it can always be asked to make + /// a response to a third-party chain who may or may not be expecting the response. Therefore + /// the `querier` should be checked to match the expected value. /// /// Kind: *Information*. /// @@ -277,6 +284,7 @@ pub enum Instruction { response: Response, #[codec(compact)] max_weight: Weight, + querier: Option, }, /// Withdraw asset(s) (`assets`) from the ownership of `origin` and place equivalent assets @@ -731,8 +739,8 @@ impl Instruction { WithdrawAsset(assets) => WithdrawAsset(assets), ReserveAssetDeposited(assets) => ReserveAssetDeposited(assets), ReceiveTeleportedAsset(assets) => ReceiveTeleportedAsset(assets), - QueryResponse { query_id, response, max_weight } => - QueryResponse { query_id, response, max_weight }, + QueryResponse { query_id, response, max_weight, querier } => + QueryResponse { query_id, response, max_weight, querier }, TransferAsset { assets, beneficiary } => TransferAsset { assets, beneficiary }, TransferReserveAsset { assets, dest, xcm } => TransferReserveAsset { assets, dest, xcm }, @@ -785,7 +793,7 @@ impl> GetWeight for Instruction { WithdrawAsset(assets) => W::withdraw_asset(assets), ReserveAssetDeposited(assets) => W::reserve_asset_deposited(assets), ReceiveTeleportedAsset(assets) => W::receive_teleported_asset(assets), - QueryResponse { query_id, response, max_weight } => + QueryResponse { query_id, response, max_weight, .. } => W::query_response(query_id, response, max_weight), TransferAsset { assets, beneficiary } => W::transfer_asset(assets, beneficiary), TransferReserveAsset { assets, dest, xcm } => @@ -873,8 +881,12 @@ impl TryFrom> for Instruction { WithdrawAsset(assets) => Self::WithdrawAsset(assets), ReserveAssetDeposited(assets) => Self::ReserveAssetDeposited(assets), ReceiveTeleportedAsset(assets) => Self::ReceiveTeleportedAsset(assets), - QueryResponse { query_id, response, max_weight } => - Self::QueryResponse { query_id, response: response.try_into()?, max_weight }, + QueryResponse { query_id, response, max_weight } => Self::QueryResponse { + query_id, + response: response.try_into()?, + max_weight, + querier: None, + }, TransferAsset { assets, beneficiary } => Self::TransferAsset { assets, beneficiary }, TransferReserveAsset { assets, dest, xcm } => Self::TransferReserveAsset { assets, dest, xcm: xcm.try_into()? }, diff --git a/xcm/src/v3/traits.rs b/xcm/src/v3/traits.rs index cd9bed5fa489..bcb8e66c7324 100644 --- a/xcm/src/v3/traits.rs +++ b/xcm/src/v3/traits.rs @@ -108,6 +108,9 @@ pub enum Error { /// The given operation would lead to an overflow of the Holding Register. #[codec(index = 26)] HoldingWouldOverflow, + /// `MultiLocation` value failed to be reanchored. + #[codec(index = 27)] + ReanchorFailed, // Errors that happen prior to instructions being executed. These fall outside of the XCM spec. /// XCM version not able to be handled. diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index ef7333e6d140..be4a6291c61d 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -138,8 +138,8 @@ impl ShouldExecute for AllowKnownQueryResponses + Some(QueryResponse { query_id, querier, .. }) + if ResponseHandler::expecting_response(origin, *query_id, querier.as_ref()) => Ok(()), _ => Err(()), } diff --git a/xcm/xcm-builder/src/mock.rs b/xcm/xcm-builder/src/mock.rs index d88df21050b6..74ba7f1fbd74 100644 --- a/xcm/xcm-builder/src/mock.rs +++ b/xcm/xcm-builder/src/mock.rs @@ -216,7 +216,11 @@ thread_local! { } pub struct TestResponseHandler; impl OnResponse for TestResponseHandler { - fn expecting_response(origin: &MultiLocation, query_id: u64) -> bool { + fn expecting_response( + origin: &MultiLocation, + query_id: u64, + _querier: Option<&MultiLocation>, + ) -> bool { QUERIES.with(|q| match q.borrow().get(&query_id) { Some(ResponseSlot::Expecting(ref l)) => l == origin, _ => false, @@ -225,6 +229,7 @@ impl OnResponse for TestResponseHandler { fn on_response( _origin: &MultiLocation, query_id: u64, + _querier: Option<&MultiLocation>, response: xcm::latest::Response, _max_weight: Weight, ) -> Weight { diff --git a/xcm/xcm-builder/src/tests.rs b/xcm/xcm-builder/src/tests.rs index bbcc4fe14bbe..81513e267511 100644 --- a/xcm/xcm-builder/src/tests.rs +++ b/xcm/xcm-builder/src/tests.rs @@ -646,6 +646,7 @@ fn prepaid_result_of_query_should_get_free_execution() { query_id, response: the_response.clone(), max_weight: 10, + querier: Some(Here.into().into()), }]); let weight_limit = 10; @@ -735,6 +736,7 @@ fn pallet_query_should_work() { query_id: 1, max_weight: 50, response: Response::PalletsInfo(vec![]), + querier: Some(Here.into()), }]), )] ); @@ -774,6 +776,7 @@ fn pallet_query_with_results_should_work() { minor: 42, patch: 69, },]), + querier: Some(Here.into()), }]), )] ); @@ -806,6 +809,7 @@ fn report_successful_transact_status_should_work() { response: Response::DispatchResult(MaybeErrorCode::Success), query_id: 42, max_weight: 5000, + querier: Some(Here.into()), }]) )] ); @@ -838,6 +842,7 @@ fn report_failed_transact_status_should_work() { response: Response::DispatchResult(MaybeErrorCode::Error(vec![2])), query_id: 42, max_weight: 5000, + querier: Some(Here.into()), }]) )] ); @@ -871,6 +876,7 @@ fn clear_transact_status_should_work() { response: Response::DispatchResult(MaybeErrorCode::Success), query_id: 42, max_weight: 5000, + querier: Some(Here.into()), }]) )] ); diff --git a/xcm/xcm-builder/tests/scenarios.rs b/xcm/xcm-builder/tests/scenarios.rs index b45b9b96c6cf..e3c0469df55c 100644 --- a/xcm/xcm-builder/tests/scenarios.rs +++ b/xcm/xcm-builder/tests/scenarios.rs @@ -142,6 +142,7 @@ fn report_holding_works() { query_id: response_info.query_id, response: Response::Assets(vec![].into()), max_weight: response_info.max_weight, + querier: Some(Here.into().into()), }]), )] ); diff --git a/xcm/xcm-executor/integration-tests/src/lib.rs b/xcm/xcm-executor/integration-tests/src/lib.rs index 5f49aa2384f0..3da4867515a8 100644 --- a/xcm/xcm-executor/integration-tests/src/lib.rs +++ b/xcm/xcm-executor/integration-tests/src/lib.rs @@ -119,7 +119,8 @@ fn query_response_fires() { let response = Response::ExecutionResult(None); let max_weight = 1_000_000; - let msg = Xcm(vec![QueryResponse { query_id, response, max_weight }]); + let querier = Some(Here.into()); + let msg = Xcm(vec![QueryResponse { query_id, response, max_weight, querier }]); let msg = Box::new(VersionedXcm::from(msg)); let execute = construct_extrinsic( @@ -208,7 +209,8 @@ fn query_response_elicits_handler() { let response = Response::ExecutionResult(None); let max_weight = 1_000_000; - let msg = Xcm(vec![QueryResponse { query_id, response, max_weight }]); + let querier = Some(Here.into()); + let msg = Xcm(vec![QueryResponse { query_id, response, max_weight, querier }]); let execute = construct_extrinsic( &client, diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 33fc376e39c2..af5d585750ce 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -392,9 +392,15 @@ impl XcmExecutor { self.total_surplus.saturating_accrue(surplus); Ok(()) }, - QueryResponse { query_id, response, max_weight } => { + QueryResponse { query_id, response, max_weight, querier } => { let origin = self.origin.as_ref().ok_or(XcmError::BadOrigin)?; - Config::ResponseHandler::on_response(origin, query_id, response, max_weight); + Config::ResponseHandler::on_response( + origin, + query_id, + querier.as_ref(), + response, + max_weight, + ); Ok(()) }, DescendOrigin(who) => self @@ -410,7 +416,11 @@ impl XcmExecutor { ReportError(response_info) => { // Report the given result by sending a QueryResponse XCM to a previously given outcome // destination if one was registered. - Self::respond(Response::ExecutionResult(self.error), response_info) + Self::respond( + self.origin.clone(), + Response::ExecutionResult(self.error), + response_info, + ) }, DepositAsset { assets, beneficiary } => { let deposited = self.holding.saturating_take(assets); @@ -461,7 +471,7 @@ impl XcmExecutor { // from Holding. let assets = Self::reanchored(self.holding.min(&assets), &response_info.destination, None); - Self::respond(Response::Assets(assets), response_info) + Self::respond(self.origin.clone(), Response::Assets(assets), response_info) }, BuyExecution { fees, weight_limit } => { // There is no need to buy any weight is `weight_limit` is `Unlimited` since it @@ -549,7 +559,8 @@ impl XcmExecutor { .collect::>(); let QueryResponseInfo { destination, query_id, max_weight } = response_info; let response = Response::PalletsInfo(pallets); - let instruction = QueryResponse { query_id, response, max_weight }; + let querier = Self::to_querier(self.origin.clone(), &destination)?; + let instruction = QueryResponse { query_id, response, max_weight, querier }; let message = Xcm(vec![instruction]); Config::XcmSender::send_xcm(destination, message).map_err(Into::into) }, @@ -566,8 +577,11 @@ impl XcmExecutor { ensure!(minor >= min_crate_minor, XcmError::VersionIncompatible); Ok(()) }, - ReportTransactStatus(response_info) => - Self::respond(Response::DispatchResult(self.transact_status.clone()), response_info), + ReportTransactStatus(response_info) => Self::respond( + self.origin.clone(), + Response::DispatchResult(self.transact_status.clone()), + response_info, + ), ClearTransactStatus => { self.transact_status = Default::default(); Ok(()) @@ -579,10 +593,31 @@ impl XcmExecutor { } } + /// Calculates what `local_querier` would be from the perspective of `destination`. + fn to_querier( + local_querier: Option, + destination: &MultiLocation, + ) -> Result, XcmError> { + Ok(match local_querier { + None => None, + Some(q) => Some( + q.reanchored(&destination, &Config::LocationInverter::ancestry()) + .map_err(|_| XcmError::ReanchorFailed)?, + ), + }) + } + /// Send a bare `QueryResponse` message containing `response` informed by the given `info`. - fn respond(response: Response, info: QueryResponseInfo) -> Result<(), XcmError> { + /// + /// The `local_querier` argument is the querier (if any) specified from the *local* perspective. + fn respond( + local_querier: Option, + response: Response, + info: QueryResponseInfo, + ) -> Result<(), XcmError> { + let querier = Self::to_querier(local_querier, &info.destination)?; let QueryResponseInfo { destination, query_id, max_weight } = info; - let instruction = QueryResponse { query_id, response, max_weight }; + let instruction = QueryResponse { query_id, response, max_weight, querier }; let message = Xcm(vec![instruction]); Config::XcmSender::send_xcm(destination, message).map_err(Into::into) } diff --git a/xcm/xcm-executor/src/traits/on_response.rs b/xcm/xcm-executor/src/traits/on_response.rs index 2c5ef4f0c3ad..016976453511 100644 --- a/xcm/xcm-executor/src/traits/on_response.rs +++ b/xcm/xcm-executor/src/traits/on_response.rs @@ -19,23 +19,35 @@ use xcm::latest::{Error as XcmError, MultiLocation, QueryId, Response, Result as /// Define what needs to be done upon receiving a query response. pub trait OnResponse { - /// Returns `true` if we are expecting a response from `origin` for query `query_id`. - fn expecting_response(origin: &MultiLocation, query_id: u64) -> bool; - /// Handler for receiving a `response` from `origin` relating to `query_id`. + /// Returns `true` if we are expecting a response from `origin` for query `query_id` that was + /// queried by `querier`. + fn expecting_response( + origin: &MultiLocation, + query_id: u64, + querier: Option<&MultiLocation>, + ) -> bool; + /// Handler for receiving a `response` from `origin` relating to `query_id` initiated by + /// `querier`. fn on_response( origin: &MultiLocation, query_id: u64, + querier: Option<&MultiLocation>, response: Response, max_weight: Weight, ) -> Weight; } impl OnResponse for () { - fn expecting_response(_origin: &MultiLocation, _query_id: u64) -> bool { + fn expecting_response( + _origin: &MultiLocation, + _query_id: u64, + _querier: Option<&MultiLocation>, + ) -> bool { false } fn on_response( _origin: &MultiLocation, _query_id: u64, + _querier: Option<&MultiLocation>, _response: Response, _max_weight: Weight, ) -> Weight { diff --git a/xcm/xcm-simulator/example/src/lib.rs b/xcm/xcm-simulator/example/src/lib.rs index e0e87899da99..0deb3a7f2227 100644 --- a/xcm/xcm-simulator/example/src/lib.rs +++ b/xcm/xcm-simulator/example/src/lib.rs @@ -305,6 +305,7 @@ mod tests { query_id: query_id_set, response: Response::Assets(MultiAssets::new()), max_weight: 1_000_000_000, + querier: Some(Here.into()), }])], ); });