xcm query handler (#7198)

* drop redundant query status

* rename ReportQueryStatus to OuterQueryStatus

* revert rename of QueryResponseStatus

* update mapping

xcm/pallet-xcm/src/lib.rs

@@ -52,8 +52,8 @@ use frame_system::pallet_prelude::*;
 pub use pallet::*;
 use xcm_executor::{
 	traits::{
-		CheckSuspension, ClaimAssets, DropAssets, FinishedQuery, MatchesFungible, OnResponse, QueryResponseStatus,
-		VersionChangeNotifier, WeightBounds, XcmQueryHandler,
+		CheckSuspension, ClaimAssets, DropAssets, MatchesFungible, OnResponse, QueryHandler,
+		QueryResponseStatus, VersionChangeNotifier, WeightBounds,
 	},
 	Assets,
 };
@@ -1120,7 +1120,7 @@ pub mod pallet {
 /// The maximum number of distinct assets allowed to be transferred in a single helper extrinsic.
 const MAX_ASSETS_FOR_TRANSFER: usize = 2;
 
-impl<T: Config> XcmQueryHandler for Pallet<T> {
+impl<T: Config> QueryHandler for Pallet<T> {
 	type QueryId = u64;
 	type BlockNumber = T::BlockNumber;
 	type Error = XcmError;
@@ -1153,41 +1153,26 @@ impl<T: Config> XcmQueryHandler for Pallet<T> {
 		Ok(query_id)
 	}
 
+	/// Removes response when ready and emits [Event::ResponseTaken] event.
 	fn take_response(query_id: Self::QueryId) -> QueryResponseStatus<Self::BlockNumber> {
 		match Queries::<T>::get(query_id) {
-			Some(QueryStatus::Ready { response, at }) => {
-				let response = response.try_into();
-				match response {
-					Ok(response) => {
-						Queries::<T>::remove(query_id);
-						Self::deposit_event(Event::ResponseTaken(query_id));
-						QueryResponseStatus::Finished(FinishedQuery::Response { response, at })
-					},
-					Err(_) => QueryResponseStatus::UnexpectedVersion,
-				}
-			},
-			Some(QueryStatus::VersionNotifier { origin, is_active }) => {
-				let origin = origin.try_into();
-				match origin {
-					Ok(origin) =>
-						QueryResponseStatus::Finished(FinishedQuery::VersionNotification {
-							origin,
-							is_active,
-						}),
-					Err(_) => QueryResponseStatus::UnexpectedVersion,
-				}
+			Some(QueryStatus::Ready { response, at }) => match response.try_into() {
+				Ok(response) => {
+					Queries::<T>::remove(query_id);
+					Self::deposit_event(Event::ResponseTaken(query_id));
+					QueryResponseStatus::Ready { response, at }
+				},
+				Err(_) => QueryResponseStatus::UnexpectedVersion,
 			},
-			Some(QueryStatus::Pending { .. }) => QueryResponseStatus::Pending,
+			Some(QueryStatus::Pending { timeout, .. }) => QueryResponseStatus::Pending { timeout },
+			Some(_) => QueryResponseStatus::UnexpectedVersion,
 			None => QueryResponseStatus::NotFound,
 		}
 	}
 
-	fn expect_response(id: Self::QueryId) {
-		let query_status = QueryStatus::Ready {
-			response: VersionedResponse::V3(Response::Null),
-			at: Self::BlockNumber::default(),
-		};
-		Queries::<T>::insert(id, query_status);
+	#[cfg(feature = "runtime-benchmarks")]
+	fn expect_response(id: Self::QueryId, status: QueryStatus) {
+		Queries::<T>::insert(id, status);
 	}
 }
 

xcm/pallet-xcm/src/mock.rs

@@ -50,6 +50,7 @@ pub mod pallet_test_notifier {
 	use frame_system::pallet_prelude::*;
 	use sp_runtime::DispatchResult;
 	use xcm::latest::prelude::*;
+	use xcm_executor::traits::QueryHandler;
 
 	#[pallet::pallet]
 	pub struct Pallet<T>(_);
@@ -85,7 +86,7 @@ pub mod pallet_test_notifier {
 			let id = who
 				.using_encoded(|mut d| <[u8; 32]>::decode(&mut d))
 				.map_err(|_| Error::<T>::BadAccountFormat)?;
-			let qid = <crate::Pallet<T> as XcmQueryHandler>::new_query(
+			let qid = <crate::Pallet<T> as QueryHandler>::new_query(
 				Junction::AccountId32 { network: None, id },
 				100u32.into(),
 				querier,

xcm/pallet-xcm/src/tests.rs

@@ -28,7 +28,7 @@ use sp_runtime::traits::{AccountIdConversion, BlakeTwo256, Hash};
 use xcm::{latest::QueryResponseInfo, prelude::*};
 use xcm_builder::AllowKnownQueryResponses;
 use xcm_executor::{
-	traits::{FinishedQuery, QueryResponseStatus, ShouldExecute, XcmQueryHandler},
+	traits::{QueryHandler, QueryResponseStatus, ShouldExecute},
 	XcmExecutor,
 };
 
@@ -166,10 +166,8 @@ fn report_outcome_works() {
 			))
 		);
 
-		let response = QueryResponseStatus::Finished(FinishedQuery::Response {
-			response: Response::ExecutionResult(None),
-			at: 1,
-		});
+		let response =
+			QueryResponseStatus::Ready { response: Response::ExecutionResult(None), at: 1 };
 		assert_eq!(XcmPallet::take_response(0), response);
 	});
 }
@@ -269,10 +267,8 @@ fn custom_querier_works() {
 			))
 		);
 
-		let response = QueryResponseStatus::Finished(FinishedQuery::Response {
-			response: Response::ExecutionResult(None),
-			at: 1,
-		});
+		let response =
+			QueryResponseStatus::Ready { response: Response::ExecutionResult(None), at: 1 };
 		assert_eq!(XcmPallet::take_response(0), response);
 	});
 }

xcm/xcm-builder/src/pay.rs

@@ -23,7 +23,7 @@ use frame_support::traits::{
 use polkadot_core_primitives::AccountId;
 use sp_std::{marker::PhantomData, vec};
 use xcm::{opaque::lts::Weight, prelude::*};
-use xcm_executor::traits::{FinishedQuery, QueryResponseStatus, XcmQueryHandler};
+use xcm_executor::traits::{QueryHandler, QueryResponseStatus};
 
 /// Implementation of the `frame_support_traits::tokens::Pay` trait, to allow
 /// for generic payments of a given `AssetKind` and `Balance` from an implied origin, to a
@@ -55,7 +55,7 @@ impl<
 		DestinationChain: Get<MultiLocation>,
 		SenderAccount: Get<AccountId>,
 		Router: SendXcm,
-		Querier: XcmQueryHandler,
+		Querier: QueryHandler,
 		Timeout: Get<Querier::BlockNumber>,
 		Beneficiary: Into<[u8; 32]> + Clone,
 		AssetKind: Into<AssetId>,
@@ -107,23 +107,22 @@ impl<
 	}
 
 	fn check_payment(id: Self::Id) -> PaymentStatus {
+		use QueryResponseStatus::*;
 		match Querier::take_response(id) {
-			QueryResponseStatus::Finished(FinishedQuery::Response { response, at: _ }) =>
-				match response {
-					Response::ExecutionResult(Some(_)) => PaymentStatus::Failure,
-					Response::ExecutionResult(None) => PaymentStatus::Success,
-					_ => PaymentStatus::Unknown,
-				},
-			QueryResponseStatus::Pending => PaymentStatus::InProgress,
-			QueryResponseStatus::NotFound |
-			QueryResponseStatus::UnexpectedVersion |
-			QueryResponseStatus::Finished(FinishedQuery::VersionNotification { .. }) =>
-				PaymentStatus::Unknown,
+			Ready { response, .. } => match response {
+				Response::ExecutionResult(None) => PaymentStatus::Success,
+				Response::ExecutionResult(Some(_)) => PaymentStatus::Failure,
+				_ => PaymentStatus::Unknown,
+			},
+			Pending { .. } => PaymentStatus::InProgress,
+			NotFound | UnexpectedVersion => PaymentStatus::Unknown,
 		}
 	}
 
 	#[cfg(feature = "runtime-benchmarks")]
-	fn ensure_successful(_: &Self::Beneficiary, _: Self::Balance) {}
+	fn ensure_successful(_: &Self::Beneficiary, _: Self::Balance) {
+		Querier::expect_response(id);
+	}
 
 	#[cfg(feature = "runtime-benchmarks")]
 	fn ensure_concluded(id: Self::Id) {

xcm/xcm-executor/src/traits/mod.rs

@@ -38,9 +38,7 @@ pub use token_matching::{
 	Error, MatchesFungible, MatchesFungibles, MatchesNonFungible, MatchesNonFungibles,
 };
 mod on_response;
-pub use on_response::{
-	FinishedQuery, OnResponse, QueryResponseStatus, VersionChangeNotifier, XcmQueryHandler,
-};
+pub use on_response::{OnResponse, QueryHandler, QueryResponseStatus, VersionChangeNotifier};
 mod should_execute;
 pub use should_execute::{CheckSuspension, ShouldExecute};
 mod transact_asset;

xcm/xcm-executor/src/traits/on_response.rs

@@ -103,27 +103,21 @@ impl VersionChangeNotifier for () {
 	}
 }
 
-#[derive(Debug, PartialEq, Eq)]
-pub enum FinishedQuery<BlockNumber> {
-	Response { response: Response, at: BlockNumber },
-	VersionNotification { origin: MultiLocation, is_active: bool },
-}
-
 /// The possible state of an XCM query response.
 #[derive(Debug, PartialEq, Eq)]
 pub enum QueryResponseStatus<BlockNumber> {
 	/// The response has arrived, and includes the inner Response and the block number it arrived at.
-	Finished(FinishedQuery<BlockNumber>),
+	Ready { response: Response, at: BlockNumber },
 	/// The response has not yet arrived, the XCM might still be executing or the response might be in transit.
-	Pending,
+	Pending { timeout: BlockNumber },
 	/// No response with the given `QueryId` was found, or the response was already queried and removed from local storage.
 	NotFound,
 	/// Got an unexpected XCM version.
 	UnexpectedVersion,
 }
 
 /// Provides methods to expect responses from XCMs and query their status.
-pub trait XcmQueryHandler {
+pub trait QueryHandler {
 	type QueryId: From<u64>
 		+ FullCodec
 		+ MaxEncodedLen
@@ -161,14 +155,10 @@ pub trait XcmQueryHandler {
 		timeout: Self::BlockNumber,
 	) -> result::Result<Self::QueryId, Self::Error>;
 
-	/// Makes sure to expect a response with the given id
-	/// Used for bencharks.
-	fn expect_response(id: Self::QueryId);
-
 	/// Attempt to remove and return the response of query with ID `query_id`.
-	///
-	/// Returns `Finished` if the response is available and includes inner response.
-	/// Returns `Pending` if the response is not yet available.
-	/// Returns `NotFound` if the response is not available and won't ever be.
 	fn take_response(id: Self::QueryId) -> QueryResponseStatus<Self::BlockNumber>;
+
+	/// Makes sure to expect a response with the given id.
+	#[cfg(feature = "runtime-benchmarks")]
+	fn expect_response(id: Self::QueryId);
 }