Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

xcm query handler #7198

Merged
merged 4 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 17 additions & 33 deletions xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
QueryStatus as OuterQueryStatus, VersionChangeNotifier, WeightBounds,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you alias it to OuterQueryStatus?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is already a QueryStatus type inside the pallet. and I've seen in Frame we use Outer suffix (not exactly same case).

},
Assets,
};
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1153,41 +1153,25 @@ impl<T: Config> XcmQueryHandler for Pallet<T> {
Ok(query_id)
}

fn take_response(query_id: Self::QueryId) -> QueryResponseStatus<Self::BlockNumber> {
/// Removes response when ready and emits [Event::ResponseTaken] event.
fn take_response(query_id: Self::QueryId) -> OuterQueryStatus<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));
OuterQueryStatus::Ready { response, at }
},
Err(_) => OuterQueryStatus::UnexpectedVersion,
},
Some(QueryStatus::Pending { .. }) => QueryResponseStatus::Pending,
None => QueryResponseStatus::NotFound,
Some(QueryStatus::Pending { timeout, .. }) => OuterQueryStatus::Pending { timeout },
Some(_) | None => OuterQueryStatus::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);
}
}

Expand Down
3 changes: 2 additions & 1 deletion xcm/pallet-xcm/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(_);
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 3 additions & 9 deletions xcm/pallet-xcm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, QueryStatus as OuterQueryStatus, ShouldExecute},
XcmExecutor,
};

Expand Down Expand Up @@ -166,10 +166,7 @@ fn report_outcome_works() {
))
);

let response = QueryResponseStatus::Finished(FinishedQuery::Response {
response: Response::ExecutionResult(None),
at: 1,
});
let response = OuterQueryStatus::Ready { response: Response::ExecutionResult(None), at: 1 };
assert_eq!(XcmPallet::take_response(0), response);
});
}
Expand Down Expand Up @@ -269,10 +266,7 @@ fn custom_querier_works() {
))
);

let response = QueryResponseStatus::Finished(FinishedQuery::Response {
response: Response::ExecutionResult(None),
at: 1,
});
let response = OuterQueryStatus::Ready { response: Response::ExecutionResult(None), at: 1 };
assert_eq!(XcmPallet::take_response(0), response);
});
}
Expand Down
27 changes: 13 additions & 14 deletions xcm/xcm-builder/src/pay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, QueryStatus};

/// 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
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -107,23 +107,22 @@ impl<
}

fn check_payment(id: Self::Id) -> PaymentStatus {
use QueryStatus::*;
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) {
Expand Down
4 changes: 1 addition & 3 deletions xcm/xcm-executor/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, QueryStatus, VersionChangeNotifier};
mod should_execute;
pub use should_execute::{CheckSuspension, ShouldExecute};
mod transact_asset;
Expand Down
28 changes: 9 additions & 19 deletions xcm/xcm-executor/src/traits/on_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
pub enum QueryStatus<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
Expand Down Expand Up @@ -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>;
fn take_response(id: Self::QueryId) -> QueryStatus<Self::BlockNumber>;

/// Makes sure to expect a response with the given id.
#[cfg(feature = "runtime-benchmarks")]
fn expect_response(id: Self::QueryId);
}