Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EventEmitter to XCM executor. #7234

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions bridges/modules/xcm-bridge-hub/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = ();
type XcmEventEmitter = ();
type AssetTransactor = ();
type OriginConverter = BridgeHubLocationXcmOriginAsRoot<RuntimeOrigin>;
type IsReserve = ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = AssetTransactors;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// Asset Hub trusts only particular, pre-configured bridged locations from a different consensus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = AssetTransactors;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// Asset Hub trusts only particular, pre-configured bridged locations from a different consensus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = FungibleTransactor;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// BridgeHub does not recognize a reserve location for any asset. Users must teleport Native
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = FungibleTransactor;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// BridgeHub does not recognize a reserve location for any asset. Users must teleport Native
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = FungibleTransactor;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// Collectives does not recognize a reserve location for any asset. Users must teleport WND
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = FungibleTransactor;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
type IsReserve = NativeAsset;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = AssetTransactors;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// Coretime chain does not recognize a reserve location for any asset. Users must teleport ROC
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = AssetTransactors;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// Coretime chain does not recognize a reserve location for any asset. Users must teleport ROC
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = (); // sending XCM not supported
type XcmEventEmitter = ();
type AssetTransactor = (); // balances not supported
type OriginConverter = XcmOriginToTransactDispatchOrigin;
type IsReserve = (); // balances not supported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = FungibleTransactor;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// People chain does not recognize a reserve location for any asset. Users must teleport ROC
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
type AssetTransactor = FungibleTransactor;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
// People does not recognize a reserve location for any asset. Users must teleport WND
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
// How to withdraw and deposit an asset.
type AssetTransactor = AssetTransactors;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = PolkadotXcm;
// How to withdraw and deposit an asset.
type AssetTransactor = AssetTransactors;
type OriginConverter = XcmOriginToTransactDispatchOrigin;
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/rococo/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = XcmPallet;
type AssetTransactor = LocalAssetTransactor;
type OriginConverter = LocalOriginConverter;
type IsReserve = ();
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/test-runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = super::RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = super::Xcm;
type AssetTransactor = DummyAssetTransactor;
type OriginConverter = OriginConverter;
type IsReserve = ();
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/westend/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = XcmPallet;
type AssetTransactor = LocalAssetTransactor;
type OriginConverter = LocalOriginConverter;
type IsReserve = ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = ();
type XcmEventEmitter = ();
type AssetTransactor = asset_transactor::AssetTransactor;
type OriginConverter = ();
// The declaration of which Locations are reserves for which Assets.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = ();
type XcmEventEmitter = ();
type AssetTransactor = asset_transactor::AssetTransactor;
type OriginConverter = ();
// We don't need to recognize anyone as a reserve
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = EnsureDecodableXcm<DevNull>;
type XcmEventEmitter = ();
type AssetTransactor = AssetTransactor;
type OriginConverter = ();
type IsReserve = TrustedReserves;
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = EnsureDecodableXcm<DevNull>;
type XcmEventEmitter = ();
type AssetTransactor = NoAssetTransactor;
type OriginConverter = AlwaysSignedByDefault<RuntimeOrigin>;
type IsReserve = AllAssetLocationsPass;
Expand Down
31 changes: 30 additions & 1 deletion polkadot/xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ use xcm_builder::{
use xcm_executor::{
traits::{
AssetTransferError, CheckSuspension, ClaimAssets, ConvertLocation, ConvertOrigin,
DropAssets, MatchesFungible, OnResponse, Properties, QueryHandler, QueryResponseStatus,
DropAssets, EventEmitter, MatchesFungible, OnResponse, Properties, QueryHandler, QueryResponseStatus,
RecordXcm, TransactAsset, TransferType, VersionChangeNotifier, WeightBounds,
XcmAssetTransfers,
},
Expand Down Expand Up @@ -402,13 +402,42 @@ pub mod pallet {
}
}

impl<T: Config> EventEmitter for Pallet<T> {
fn emit_sent_event(
origin: Location,
destination: Location,
message_id: XcmHash,
) {
Self::deposit_event(Event::Sent { origin, destination, message: Xcm::default(), message_id });
raymondkfcheung marked this conversation as resolved.
Show resolved Hide resolved
}

fn emit_sent_failure_event(
origin: Location,
destination: Location,
error: SendError,
) {
Self::deposit_event(Event::SentFailed { origin, destination, error });
}

fn emit_process_failure_event(
origin: Location,
error: XcmError,
) {
Self::deposit_event(Event::ProcessXcmError { origin, error });
}
Comment on lines +414 to +427

Choose a reason for hiding this comment

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

These two functions are not tested.

}

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
/// Execution of an XCM message was attempted.
Attempted { outcome: xcm::latest::Outcome },
/// A XCM message was sent.
Sent { origin: Location, destination: Location, message: Xcm<()>, message_id: XcmHash },
/// A XCM message failed to be sent.
SentFailed { origin: Location, destination: Location, error: SendError },
/// Process XCM message failed.
ProcessXcmError { origin: Location, error: XcmError },
Comment on lines +437 to +440

Choose a reason for hiding this comment

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

These two new events should include, message_id, for better message tracking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For these ones when the event is emitted we don't have the message id. This was something that I was in doubt when creating this two events. Do you have any suggestion on what we can add to these events?

Choose a reason for hiding this comment

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

Is possible to retrieve message_id from SetTopic or properties.message_id?

/// 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.
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/pallet-xcm/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = XcmRouter;
type XcmEventEmitter = XcmPallet;
type AssetTransactor = AssetTransactors;
type OriginConverter = LocalOriginConverter;
type IsReserve = (Case<TrustedForeign>, Case<TrustedUsdc>, Case<TrustedPaidParaForeign>);
Expand Down
52 changes: 52 additions & 0 deletions polkadot/xcm/pallet-xcm/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1456,3 +1456,55 @@ fn record_xcm_works() {
assert_eq!(RecordedXcm::<Test>::get(), Some(message.into()));
});
}

#[test]
fn execute_initiate_transfer_and_check_sent_event() {
let balances = vec![(ALICE, INITIAL_BALANCE)];
new_test_ext_with_balances(balances).execute_with(|| {
let dest: Location = Location::new(1, [Junction::AccountId32 { network: None, id: BOB.into() }]);
let assets: Asset = (Parent, SEND_AMOUNT).into();

let message = Xcm(vec![
InitiateReserveWithdraw {
assets: Wild(All),
reserve: Parent.into(),
xcm: Xcm(vec![
BuyExecution { fees: assets.clone(), weight_limit: Unlimited },
DepositAsset { assets: All.into(), beneficiary: dest.clone() },
]),
},
]);

assert_ok!(XcmPallet::execute(
RuntimeOrigin::signed(ALICE),
Box::new(VersionedXcm::from(message.clone())),
BaseXcmWeight::get() * 3,
));

let sender: Location = AccountId32 { network: None, id: ALICE.into() }.into();
let expected_message = Xcm(vec![
WithdrawAsset(Assets::new()),
ClearOrigin,
BuyExecution { fees: assets.clone(), weight_limit: Unlimited },
DepositAsset { assets: All.into(), beneficiary: dest.clone() },
]);
let id = fake_message_hash(&expected_message);

assert_eq!(
last_events(2),
vec![
RuntimeEvent::XcmPallet(crate::Event::Sent {
origin: sender,
destination: Parent.into(),
message: expected_message,
message_id: id,
}),
RuntimeEvent::XcmPallet(crate::Event::Attempted {
outcome: Outcome::Complete {
used: Weight::from_parts(1000, 1000)
}
})
]
);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = ();
type XcmEventEmitter = ();
type AssetTransactor = FungibleTransactor;
type OriginConverter = ();
type IsReserve = ();
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-builder/src/tests/pay/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = TestMessageSender;
type XcmEventEmitter = XcmPallet;
type AssetTransactor = LocalAssetsTransactor;
type OriginConverter = OriginConverter;
type IsReserve = ();
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-builder/tests/mock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
type XcmSender = TestXcmRouter;
type XcmEventEmitter = XcmPallet;
type AssetTransactor = LocalAssetTransactor;
type OriginConverter = LocalOriginConverter;
type IsReserve = ();
Expand Down
5 changes: 4 additions & 1 deletion polkadot/xcm/xcm-executor/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use crate::traits::{
AssetExchange, AssetLock, CallDispatcher, ClaimAssets, ConvertOrigin, DropAssets, ExportXcm,
AssetExchange, AssetLock, CallDispatcher, ClaimAssets, ConvertOrigin, DropAssets, EventEmitter, ExportXcm,
FeeManager, HandleHrmpChannelAccepted, HandleHrmpChannelClosing,
HandleHrmpNewChannelOpenRequest, OnResponse, ProcessTransaction, RecordXcm, ShouldExecute,
TransactAsset, VersionChangeNotifier, WeightBounds, WeightTrader,
Expand All @@ -39,6 +39,9 @@ pub trait Config {
/// the executor.
type XcmSender: SendXcm;

/// How to emit XCM events.
type XcmEventEmitter: EventEmitter;

/// How to withdraw and deposit an asset.
type AssetTransactor: TransactAsset;

Expand Down
25 changes: 22 additions & 3 deletions polkadot/xcm/xcm-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use xcm::latest::{prelude::*, AssetTransferFilter};
pub mod traits;
use traits::{
validate_export, AssetExchange, AssetLock, CallDispatcher, ClaimAssets, ConvertOrigin,
DropAssets, Enact, ExportXcm, FeeManager, FeeReason, HandleHrmpChannelAccepted,
DropAssets, Enact, ExportXcm, EventEmitter, FeeManager, FeeReason, HandleHrmpChannelAccepted,
HandleHrmpChannelClosing, HandleHrmpNewChannelOpenRequest, OnResponse, ProcessTransaction,
Properties, ShouldExecute, TransactAsset, VersionChangeNotifier, WeightBounds, WeightTrader,
XcmAssetTransfers,
Expand Down Expand Up @@ -427,9 +427,27 @@ impl<Config: config::Config> XcmExecutor<Config> {
reason = ?reason,
"Sending msg",
);
let (ticket, fee) = validate_send::<Config::XcmSender>(dest, msg)?;
let (ticket, fee) = validate_send::<Config::XcmSender>(dest.clone(), msg)?;
self.take_fee(fee, reason)?;
Config::XcmSender::deliver(ticket).map_err(Into::into)
match Config::XcmSender::deliver(ticket) {
Ok(message_id) => {
Config::XcmEventEmitter::emit_sent_event(
self.original_origin.clone(),
dest,
message_id.clone(),
);
Ok(message_id)
},
Err(e) => {
tracing::error!(target: "xcm::send", ?e, "XCM failed to deliver with error");
Config::XcmEventEmitter::emit_sent_failure_event(
self.original_origin.clone(),
dest,
e.clone(),
);
Err(e.into())
},
}
}

/// Remove the registered error handler and return it. Do not refund its weight.
Expand Down Expand Up @@ -813,6 +831,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
self.process_instruction(instr)
});
if let Err(e) = inst_res {
Config::XcmEventEmitter::emit_process_failure_event(self.original_origin.clone(), e.clone());

Choose a reason for hiding this comment

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

Move this line, emit_process_failure_event, below, the tracing. Then we can see the trace, even it fails to emit.

tracing::trace!(target: "xcm::execute", "!!! ERROR: {:?}", e);
*r = Err(ExecutorError {
index: i as u32,
Expand Down
Loading
Loading