From 934c091421af4be362839996bfaa441ba59bf12b Mon Sep 17 00:00:00 2001 From: Ayevbeosa Iyamu <ayevbeosa.j@gmail.com> Date: Fri, 21 Feb 2025 18:48:03 +0100 Subject: [PATCH] xcm-builder: added logging for xcm filters/helpers/matchers/types (#2408) (#7003) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Added logs in pallet-xcm to help in debugging, fixes #2408, and in continuation of #4982 # Checklist - [x] https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-builder/src/ - [x] https://github.com/paritytech/polkadot-sdk/tree/master/cumulus/parachains/runtimes/assets/common/src - [x] runtime-defined XCM filters/converters (just [one example](https://github.com/paritytech/polkadot-sdk/blob/183b55aae21e97ef39192e5a358287e2b6b7043c/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs#L284)) Polkadot Address: 1Gz5aLtEu2n4jsfA6XwtZnuaRymJrDDw4kEGdNHTdxrpzrc --------- Co-authored-by: Ayevbeosa Iyamu <aiyamu@vatebra.com> Co-authored-by: Bastian Köcher <git@kchr.de> Co-authored-by: Adrian Catangiu <adrian@parity.io> Co-authored-by: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- Cargo.lock | 6 +- .../runtimes/assets/common/Cargo.toml | 4 +- .../runtimes/assets/common/src/benchmarks.rs | 32 ++++- .../assets/common/src/foreign_creators.rs | 7 +- .../assets/common/src/fungible_conversion.rs | 2 +- .../runtimes/assets/common/src/lib.rs | 1 + .../runtimes/assets/common/src/matching.rs | 34 +++-- .../bridge-hubs/bridge-hub-westend/Cargo.toml | 4 +- .../bridge-hubs/bridge-hub-westend/src/lib.rs | 30 ++-- .../bridge-hub-westend/src/xcm_config.rs | 5 +- polkadot/xcm/xcm-builder/Cargo.toml | 4 +- .../single_asset_adapter/adapter.rs | 48 +++---- polkadot/xcm/xcm-builder/src/barriers.rs | 96 +++++++------ .../xcm/xcm-builder/src/currency_adapter.rs | 22 +-- polkadot/xcm/xcm-builder/src/fee_handling.rs | 2 +- .../xcm-builder/src/filter_asset_location.rs | 6 +- .../xcm/xcm-builder/src/fungible_adapter.rs | 54 +++---- .../xcm/xcm-builder/src/fungibles_adapter.rs | 64 +++++---- polkadot/xcm/xcm-builder/src/lib.rs | 1 + .../xcm-builder/src/location_conversion.rs | 12 +- .../xcm-builder/src/nonfungible_adapter.rs | 136 ++++++++++-------- .../xcm-builder/src/nonfungibles_adapter.rs | 129 ++++++++++------- .../xcm/xcm-builder/src/origin_conversion.rs | 38 ++--- .../xcm-builder/src/process_xcm_message.rs | 17 +-- polkadot/xcm/xcm-builder/src/routing.rs | 12 +- polkadot/xcm/xcm-builder/src/weight.rs | 36 +++-- prdoc/pr_7003.prdoc | 17 +++ 27 files changed, 478 insertions(+), 341 deletions(-) create mode 100644 prdoc/pr_7003.prdoc diff --git a/Cargo.lock b/Cargo.lock index 5457e44aeaa..508b7706854 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1271,7 +1271,6 @@ dependencies = [ "cumulus-primitives-core 0.7.0", "frame-support 28.0.0", "impl-trait-for-tuples", - "log", "pallet-asset-conversion 10.0.0", "pallet-assets 29.1.0", "pallet-xcm 7.0.0", @@ -1284,6 +1283,7 @@ dependencies = [ "staging-xcm-builder 7.0.0", "staging-xcm-executor 7.0.0", "substrate-wasm-builder 17.0.0", + "tracing", ] [[package]] @@ -2955,7 +2955,6 @@ dependencies = [ "frame-system-rpc-runtime-api 26.0.0", "frame-try-runtime 0.34.0", "hex-literal", - "log", "pallet-aura 27.0.0", "pallet-authorship 28.0.0", "pallet-balances 28.0.0", @@ -3014,6 +3013,7 @@ dependencies = [ "staging-xcm-executor 7.0.0", "substrate-wasm-builder 17.0.0", "testnet-parachains-constants 1.0.0", + "tracing", "westend-runtime-constants 7.0.0", "xcm-runtime-apis 0.1.0", ] @@ -28752,7 +28752,6 @@ dependencies = [ "frame-support 28.0.0", "frame-system 28.0.0", "impl-trait-for-tuples", - "log", "pallet-asset-conversion 10.0.0", "pallet-assets 29.1.0", "pallet-balances 28.0.0", @@ -28773,6 +28772,7 @@ dependencies = [ "sp-weights 27.0.0", "staging-xcm 7.0.0", "staging-xcm-executor 7.0.0", + "tracing", ] [[package]] diff --git a/cumulus/parachains/runtimes/assets/common/Cargo.toml b/cumulus/parachains/runtimes/assets/common/Cargo.toml index de74f59f43c..11cff7dd313 100644 --- a/cumulus/parachains/runtimes/assets/common/Cargo.toml +++ b/cumulus/parachains/runtimes/assets/common/Cargo.toml @@ -14,8 +14,8 @@ workspace = true [dependencies] codec = { features = ["derive"], workspace = true } impl-trait-for-tuples = { workspace = true } -log = { workspace = true } scale-info = { features = ["derive"], workspace = true } +tracing = { workspace = true } # Substrate frame-support = { workspace = true } @@ -43,7 +43,6 @@ std = [ "codec/std", "cumulus-primitives-core/std", "frame-support/std", - "log/std", "pallet-asset-conversion/std", "pallet-assets/std", "pallet-xcm/std", @@ -51,6 +50,7 @@ std = [ "scale-info/std", "sp-api/std", "sp-runtime/std", + "tracing/std", "xcm-builder/std", "xcm-executor/std", "xcm/std", diff --git a/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs b/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs index d59fddc4e8f..28ae6be27eb 100644 --- a/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs +++ b/cumulus/parachains/runtimes/assets/common/src/benchmarks.rs @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use core::marker::PhantomData; +use core::{fmt::Debug, marker::PhantomData}; use cumulus_primitives_core::ParaId; use sp_runtime::traits::Get; use xcm::latest::prelude::*; @@ -22,8 +22,10 @@ use xcm::latest::prelude::*; pub struct AssetPairFactory<Target, SelfParaId, PalletId, L = Location>( PhantomData<(Target, SelfParaId, PalletId, L)>, ); -impl<Target: Get<L>, SelfParaId: Get<ParaId>, PalletId: Get<u32>, L: TryFrom<Location>> +impl<Target: Get<L>, SelfParaId: Get<ParaId>, PalletId: Get<u32>, L: TryFrom<Location> + Debug> pallet_asset_conversion::BenchmarkHelper<L> for AssetPairFactory<Target, SelfParaId, PalletId, L> +where + <L as TryFrom<Location>>::Error: Debug, { fn create_pair(seed1: u32, seed2: u32) -> (L, L) { let with_id = Location::new( @@ -35,9 +37,31 @@ impl<Target: Get<L>, SelfParaId: Get<ParaId>, PalletId: Get<u32>, L: TryFrom<Loc ], ); if seed1 % 2 == 0 { - (with_id.try_into().map_err(|_| "Something went wrong").unwrap(), Target::get()) + ( + with_id + .try_into() + .map_err(|error| { + tracing::error!( + target: "xcm", + ?error, + "Failed to create asset pairs when seed1 is even", + ); + "Something went wrong" + }) + .unwrap(), + Target::get(), + ) } else { - (Target::get(), with_id.try_into().map_err(|_| "Something went wrong").unwrap()) + ( + Target::get(), + with_id + .try_into() + .map_err(|error| { + tracing::error!(target: "xcm", ?error, "Failed to create asset pairs"); + "Something went wrong" + }) + .unwrap(), + ) } } } diff --git a/cumulus/parachains/runtimes/assets/common/src/foreign_creators.rs b/cumulus/parachains/runtimes/assets/common/src/foreign_creators.rs index 95edb31da06..e5b7b0d86aa 100644 --- a/cumulus/parachains/runtimes/assets/common/src/foreign_creators.rs +++ b/cumulus/parachains/runtimes/assets/common/src/foreign_creators.rs @@ -13,6 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use core::fmt::Debug; use frame_support::traits::{ ContainsPair, EnsureOrigin, EnsureOriginWithArg, Everything, OriginTrait, }; @@ -29,8 +30,8 @@ impl< IsForeign: ContainsPair<L, L>, AccountOf: ConvertLocation<AccountId>, AccountId: Clone, - RuntimeOrigin: From<XcmOrigin> + OriginTrait + Clone, - L: TryFrom<Location> + TryInto<Location> + Clone, + RuntimeOrigin: From<XcmOrigin> + OriginTrait + Clone + Debug, + L: TryFrom<Location> + TryInto<Location> + Clone + Debug, > EnsureOriginWithArg<RuntimeOrigin, L> for ForeignCreators<IsForeign, AccountOf, AccountId, L> where RuntimeOrigin::PalletsOrigin: @@ -42,8 +43,10 @@ where origin: RuntimeOrigin, asset_location: &L, ) -> core::result::Result<Self::Success, RuntimeOrigin> { + tracing::trace!(target: "xcm::try_origin", ?origin, ?asset_location, "ForeignCreators"); let origin_location = EnsureXcm::<Everything, L>::try_origin(origin.clone())?; if !IsForeign::contains(asset_location, &origin_location) { + tracing::trace!(target: "xcm::try_origin", ?asset_location, ?origin_location, "ForeignCreators: no match"); return Err(origin) } let latest_location: Location = diff --git a/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs b/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs index 27ee2d6b565..e3189de7e94 100644 --- a/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs +++ b/cumulus/parachains/runtimes/assets/common/src/fungible_conversion.rs @@ -117,7 +117,7 @@ impl< for_tuples!( #( match Tuple::contains(location) { o @ true => return o, _ => () } )* ); - log::trace!(target: "xcm::contains", "did not match location: {:?}", &location); + tracing::trace!(target: "xcm::contains", ?location, "MatchesLocation: no match"); false } } diff --git a/cumulus/parachains/runtimes/assets/common/src/lib.rs b/cumulus/parachains/runtimes/assets/common/src/lib.rs index 50b1b63146b..da024b1c945 100644 --- a/cumulus/parachains/runtimes/assets/common/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/common/src/lib.rs @@ -24,6 +24,7 @@ pub mod matching; pub mod runtime_api; extern crate alloc; +extern crate core; use crate::matching::{LocalLocationPattern, ParentLocation}; use alloc::vec::Vec; diff --git a/cumulus/parachains/runtimes/assets/common/src/matching.rs b/cumulus/parachains/runtimes/assets/common/src/matching.rs index aa9d7929cb9..23a15c9293e 100644 --- a/cumulus/parachains/runtimes/assets/common/src/matching.rs +++ b/cumulus/parachains/runtimes/assets/common/src/matching.rs @@ -13,6 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use core::fmt::Debug; use cumulus_primitives_core::ParaId; use frame_support::{ pallet_prelude::Get, @@ -33,8 +34,9 @@ impl<IsForeign: ContainsPair<Location, Location>> ContainsPair<Asset, Location> for IsForeignConcreteAsset<IsForeign> { fn contains(asset: &Asset, origin: &Location) -> bool { - log::trace!(target: "xcm::contains", "IsForeignConcreteAsset asset: {:?}, origin: {:?}", asset, origin); - matches!(asset.id, AssetId(ref id) if IsForeign::contains(id, origin)) + let result = matches!(asset.id, AssetId(ref id) if IsForeign::contains(id, origin)); + tracing::trace!(target: "xcm::contains", ?asset, ?origin, ?result, "IsForeignConcreteAsset"); + result } } @@ -43,10 +45,11 @@ impl<IsForeign: ContainsPair<Location, Location>> ContainsPair<Asset, Location> pub struct FromSiblingParachain<SelfParaId, L = Location>( core::marker::PhantomData<(SelfParaId, L)>, ); -impl<SelfParaId: Get<ParaId>, L: TryFrom<Location> + TryInto<Location> + Clone> ContainsPair<L, L> - for FromSiblingParachain<SelfParaId, L> +impl<SelfParaId: Get<ParaId>, L: TryFrom<Location> + TryInto<Location> + Clone + Debug> + ContainsPair<L, L> for FromSiblingParachain<SelfParaId, L> { fn contains(a: &L, b: &L) -> bool { + tracing::trace!(target: "xcm:contains", ?a, ?b, "FromSiblingParachain"); // We convert locations to latest let a = match ((*a).clone().try_into(), (*b).clone().try_into()) { (Ok(a), Ok(b)) if a.starts_with(&b) => a, // `a` needs to be from `b` at least @@ -70,10 +73,11 @@ pub struct FromNetwork<UniversalLocation, ExpectedNetworkId, L = Location>( impl< UniversalLocation: Get<InteriorLocation>, ExpectedNetworkId: Get<NetworkId>, - L: TryFrom<Location> + TryInto<Location> + Clone, + L: TryFrom<Location> + TryInto<Location> + Clone + Debug, > ContainsPair<L, L> for FromNetwork<UniversalLocation, ExpectedNetworkId, L> { fn contains(a: &L, b: &L) -> bool { + tracing::trace!(target: "xcm:contains", ?a, ?b, "FromNetwork"); // We convert locations to latest let a = match ((*a).clone().try_into(), (*b).clone().try_into()) { (Ok(a), Ok(b)) if a.starts_with(&b) => a, // `a` needs to be from `b` at least @@ -86,11 +90,7 @@ impl< match ensure_is_remote(universal_source.clone(), a.clone()) { Ok((network_id, _)) => network_id == ExpectedNetworkId::get(), Err(e) => { - log::trace!( - target: "xcm::contains", - "FromNetwork origin: {:?} is not remote to the universal_source: {:?} {:?}", - a, universal_source, e - ); + tracing::debug!(target: "xcm::contains", origin = ?a, ?universal_source, error = ?e, "FromNetwork origin is not remote to the universal_source"); false }, } @@ -118,15 +118,20 @@ impl< let expected_origin = OriginLocation::get(); // ensure `origin` is expected `OriginLocation` if !expected_origin.eq(&origin) { - log::trace!( + tracing::trace!( target: "xcm::contains", - "RemoteAssetFromLocation asset: {asset:?}, origin: {origin:?} is not from expected {expected_origin:?}" + ?asset, + ?origin, + ?expected_origin, + "RemoteAssetFromLocation: Asset is not from expected origin" ); return false; } else { - log::trace!( + tracing::trace!( target: "xcm::contains", - "RemoteAssetFromLocation asset: {asset:?}, origin: {origin:?}", + ?asset, + ?origin, + "RemoteAssetFromLocation", ); } @@ -138,6 +143,7 @@ impl<AssetsAllowedNetworks: Contains<Location>, OriginLocation: Get<Location>> ContainsPair<Asset, Location> for RemoteAssetFromLocation<AssetsAllowedNetworks, OriginLocation> { fn contains(asset: &Asset, origin: &Location) -> bool { + tracing::trace!(target: "xcm:contains", ?asset, ?origin, "RemoteAssetFromLocation"); <Self as ContainsPair<Location, Location>>::contains(&asset.id.0, origin) } } diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/Cargo.toml b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/Cargo.toml index 444023eac72..772968799a0 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/Cargo.toml +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/Cargo.toml @@ -17,10 +17,10 @@ substrate-wasm-builder = { optional = true, workspace = true, default-features = [dependencies] codec = { features = ["derive"], workspace = true } hex-literal = { workspace = true, default-features = true } -log = { workspace = true } scale-info = { features = ["derive"], workspace = true } serde = { optional = true, features = ["derive"], workspace = true, default-features = true } serde_json = { features = ["alloc"], workspace = true } +tracing = { workspace = true } # Substrate frame-benchmarking = { optional = true, workspace = true } @@ -162,7 +162,6 @@ std = [ "frame-system-rpc-runtime-api/std", "frame-system/std", "frame-try-runtime?/std", - "log/std", "pallet-aura/std", "pallet-authorship/std", "pallet-balances/std", @@ -215,6 +214,7 @@ std = [ "sp-version/std", "substrate-wasm-builder", "testnet-parachains-constants/std", + "tracing/std", "westend-runtime-constants/std", "xcm-builder/std", "xcm-executor/std", diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs index 69d2ed06e32..2fdd3c06197 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs @@ -813,11 +813,11 @@ impl_runtime_apis! { Ok(WeightToFee::weight_to_fee(&weight)) }, Ok(asset_id) => { - log::trace!(target: "xcm::xcm_runtime_apis", "query_weight_to_asset_fee - unhandled asset_id: {asset_id:?}!"); + tracing::trace!(target: "xcm::xcm_runtime_apis", ?asset_id, "query_weight_to_asset_fee - unhandled asset_id!"); Err(XcmPaymentApiError::AssetNotFound) }, Err(_) => { - log::trace!(target: "xcm::xcm_runtime_apis", "query_weight_to_asset_fee - failed to convert asset: {asset:?}!"); + tracing::trace!(target: "xcm::xcm_runtime_apis", ?asset, "query_weight_to_asset_fee - failed to convert asset!"); Err(XcmPaymentApiError::VersionedConversionFailed) } } @@ -1163,12 +1163,15 @@ impl_runtime_apis! { alloc::boxed::Box::new(bridge_to_rococo_config::BridgeHubRococoLocation::get()), XCM_VERSION, ).map_err(|e| { - log::error!( - "Failed to dispatch `force_xcm_version({:?}, {:?}, {:?})`, error: {:?}", - RuntimeOrigin::root(), - bridge_to_rococo_config::BridgeHubRococoLocation::get(), - XCM_VERSION, - e + let origin = RuntimeOrigin::root(); + let bridge = bridge_to_rococo_config::BridgeHubRococoLocation::get(); + tracing::error!( + target: "xcm::export_message_origin_and_destination", + ?origin, + ?bridge, + ?XCM_VERSION, + ?e, + "Failed to dispatch `force_xcm_version`", ); BenchmarkError::Stop("XcmVersion was not stored!") })?; @@ -1198,11 +1201,12 @@ impl_runtime_apis! { bp_messages::LegacyLaneId([1, 2, 3, 4]), true, ).map_err(|e| { - log::error!( - "Failed to `XcmOverBridgeHubRococo::open_bridge`({:?}, {:?})`, error: {:?}", - sibling_parachain_location, - bridge_destination_universal_location, - e + tracing::error!( + target: "xcm::export_message_origin_and_destination", + ?sibling_parachain_location, + ?bridge_destination_universal_location, + ?e, + "Failed to `XcmOverBridgeHubRococo::open_bridge`", ); BenchmarkError::Stop("Bridge was not opened!") })?; diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs index 3e844f98416..f7a1859e9e8 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs @@ -123,7 +123,9 @@ pub type XcmOriginToTransactDispatchOrigin = ( pub struct ParentOrParentsPlurality; impl Contains<Location> for ParentOrParentsPlurality { fn contains(location: &Location) -> bool { - matches!(location.unpack(), (1, []) | (1, [Plurality { .. }])) + let result = matches!(location.unpack(), (1, []) | (1, [Plurality { .. }])); + tracing::trace!(target: "xcm::contains", ?location, ?result, "ParentOrParentsPlurality matches"); + result } } @@ -300,6 +302,7 @@ impl<WaivedLocations: Contains<Location>, FeeHandler: HandleFee> FeeManager } fn handle_fee(fee: Assets, context: Option<&XcmContext>, reason: FeeReason) { + tracing::trace!(target: "xcm::handle_fee", ?fee, ?context, ?reason, "FeeManager handle_fee"); FeeHandler::handle_fee(fee, context, reason); } } diff --git a/polkadot/xcm/xcm-builder/Cargo.toml b/polkadot/xcm/xcm-builder/Cargo.toml index 764c788f2d0..31cdb3a6ec1 100644 --- a/polkadot/xcm/xcm-builder/Cargo.toml +++ b/polkadot/xcm/xcm-builder/Cargo.toml @@ -17,7 +17,6 @@ environmental = { workspace = true } frame-support = { workspace = true } frame-system = { workspace = true } impl-trait-for-tuples = { workspace = true } -log = { workspace = true } pallet-asset-conversion = { workspace = true } pallet-transaction-payment = { workspace = true } scale-info = { features = ["derive"], workspace = true } @@ -26,6 +25,7 @@ sp-core = { workspace = true } sp-io = { workspace = true } sp-runtime = { workspace = true } sp-weights = { workspace = true } +tracing = { workspace = true } xcm = { workspace = true } xcm-executor = { workspace = true } @@ -66,7 +66,6 @@ std = [ "environmental/std", "frame-support/std", "frame-system/std", - "log/std", "pallet-asset-conversion/std", "pallet-transaction-payment/std", "polkadot-parachain-primitives/std", @@ -77,6 +76,7 @@ std = [ "sp-io/std", "sp-runtime/std", "sp-weights/std", + "tracing/std", "xcm-executor/std", "xcm/std", ] diff --git a/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs b/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs index 3108068686f..07698253a79 100644 --- a/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs +++ b/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs @@ -61,9 +61,9 @@ where ) -> Result<AssetsInHolding, AssetsInHolding> { let mut give_iter = give.fungible_assets_iter(); let give_asset = give_iter.next().ok_or_else(|| { - log::trace!( + tracing::trace!( target: "xcm::SingleAssetExchangeAdapter::exchange_asset", - "No fungible asset was in `give`.", + ?give, "No fungible asset was in `give`.", ); give.clone() })?; @@ -73,21 +73,21 @@ where let want_asset = want.get(0).ok_or_else(|| give.clone())?; let (give_asset_id, give_amount) = Matcher::matches_fungibles(&give_asset).map_err(|error| { - log::trace!( + tracing::trace!( target: "xcm::SingleAssetExchangeAdapter::exchange_asset", - "Could not map XCM asset give {:?} to FRAME asset. Error: {:?}", - give_asset, - error, + ?give_asset, + ?error, + "Could not map XCM asset give to FRAME asset.", ); give.clone() })?; let (want_asset_id, want_amount) = Matcher::matches_fungibles(&want_asset).map_err(|error| { - log::trace!( + tracing::trace!( target: "xcm::SingleAssetExchangeAdapter::exchange_asset", - "Could not map XCM asset want {:?} to FRAME asset. Error: {:?}", - want_asset, - error, + ?want_asset, + ?error, + "Could not map XCM asset want to FRAME asset." ); give.clone() })?; @@ -106,10 +106,10 @@ where Some(want_amount), ) .map_err(|(credit_in, error)| { - log::error!( + tracing::debug!( target: "xcm::SingleAssetExchangeAdapter::exchange_asset", - "Could not perform the swap, error: {:?}.", - error + ?error, + "Could not perform the swap" ); drop(credit_in); give.clone() @@ -127,10 +127,10 @@ where want_amount, ) .map_err(|(credit_in, error)| { - log::error!( + tracing::debug!( target: "xcm::SingleAssetExchangeAdapter::exchange_asset", - "Could not perform the swap, error: {:?}.", - error + ?error, + "Could not perform the swap", ); drop(credit_in); give.clone() @@ -162,22 +162,22 @@ where // We first match both XCM assets to the asset ID types `AssetConversion` can handle. let (give_asset_id, give_amount) = Matcher::matches_fungibles(give_asset) .map_err(|error| { - log::trace!( + tracing::trace!( target: "xcm::SingleAssetExchangeAdapter::quote_exchange_price", - "Could not map XCM asset {:?} to FRAME asset. Error: {:?}.", - give_asset, - error, + ?give_asset, + ?error, + "Could not map XCM asset to FRAME asset." ); () }) .ok()?; let (want_asset_id, want_amount) = Matcher::matches_fungibles(want_asset) .map_err(|error| { - log::trace!( + tracing::trace!( target: "xcm::SingleAssetExchangeAdapter::quote_exchange_price", - "Could not map XCM asset {:?} to FRAME asset. Error: {:?}.", - want_asset, - error, + ?want_asset, + ?error, + "Could not map XCM asset to FRAME asset" ); () }) diff --git a/polkadot/xcm/xcm-builder/src/barriers.rs b/polkadot/xcm/xcm-builder/src/barriers.rs index 6eda6bd2bbe..6152e54d08b 100644 --- a/polkadot/xcm/xcm-builder/src/barriers.rs +++ b/polkadot/xcm/xcm-builder/src/barriers.rs @@ -34,15 +34,18 @@ use xcm_executor::traits::{CheckSuspension, DenyExecution, OnResponse, Propertie pub struct TakeWeightCredit; impl ShouldExecute for TakeWeightCredit { fn should_execute<RuntimeCall>( - _origin: &Location, - _instructions: &mut [Instruction<RuntimeCall>], + origin: &Location, + instructions: &mut [Instruction<RuntimeCall>], max_weight: Weight, properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "TakeWeightCredit origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - _origin, _instructions, max_weight, properties, + ?origin, + ?instructions, + ?max_weight, + ?properties, + "TakeWeightCredit" ); properties.weight_credit = properties .weight_credit @@ -66,12 +69,15 @@ impl<T: Contains<Location>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T> origin: &Location, instructions: &mut [Instruction<RuntimeCall>], max_weight: Weight, - _properties: &mut Properties, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "AllowTopLevelPaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, max_weight, _properties, + ?origin, + ?instructions, + ?max_weight, + ?properties, + "AllowTopLevelPaidExecutionFrom", ); ensure!(T::contains(origin), ProcessMessageError::Unsupported); @@ -173,10 +179,13 @@ impl<InnerBarrier: ShouldExecute, LocalUniversal: Get<InteriorLocation>, MaxPref max_weight: Weight, properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "WithComputedOrigin origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, max_weight, properties, + ?origin, + ?instructions, + ?max_weight, + ?properties, + "WithComputedOrigin" ); let mut actual_origin = origin.clone(); let skipped = Cell::new(0usize); @@ -230,10 +239,13 @@ impl<InnerBarrier: ShouldExecute> ShouldExecute for TrailingSetTopicAsId<InnerBa max_weight: Weight, properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "TrailingSetTopicAsId origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, max_weight, properties, + ?origin, + ?instructions, + ?max_weight, + ?properties, + "TrailingSetTopicAsId" ); let until = if let Some(SetTopic(t)) = instructions.last() { properties.message_id = Some(*t); @@ -276,13 +288,13 @@ impl<T: Contains<Location>> ShouldExecute for AllowUnpaidExecutionFrom<T> { fn should_execute<RuntimeCall>( origin: &Location, instructions: &mut [Instruction<RuntimeCall>], - _max_weight: Weight, - _properties: &mut Properties, + max_weight: Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "AllowUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, _max_weight, _properties, + ?origin, ?instructions, ?max_weight, ?properties, + "AllowUnpaidExecutionFrom" ); ensure!(T::contains(origin), ProcessMessageError::Unsupported); Ok(()) @@ -313,12 +325,12 @@ impl<T: Contains<Location>, Aliasers: ContainsPair<Location, Location>> ShouldEx origin: &Location, instructions: &mut [Instruction<Call>], max_weight: Weight, - _properties: &mut Properties, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "AllowExplicitUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, max_weight, _properties, + ?origin, ?instructions, ?max_weight, ?properties, + "AllowExplicitUnpaidExecutionFrom", ); // We will read up to 5 instructions before `UnpaidExecution`. // This allows up to 3 asset transfer instructions, thus covering all possible transfer @@ -428,13 +440,13 @@ impl<ResponseHandler: OnResponse> ShouldExecute for AllowKnownQueryResponses<Res fn should_execute<RuntimeCall>( origin: &Location, instructions: &mut [Instruction<RuntimeCall>], - _max_weight: Weight, - _properties: &mut Properties, + max_weight: Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "AllowKnownQueryResponses origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, _max_weight, _properties, + ?origin, ?instructions, ?max_weight, ?properties, + "AllowKnownQueryResponses" ); instructions .matcher() @@ -456,13 +468,13 @@ impl<T: Contains<Location>> ShouldExecute for AllowSubscriptionsFrom<T> { fn should_execute<RuntimeCall>( origin: &Location, instructions: &mut [Instruction<RuntimeCall>], - _max_weight: Weight, - _properties: &mut Properties, + max_weight: Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "AllowSubscriptionsFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, _max_weight, _properties, + ?origin, ?instructions, ?max_weight, ?properties, + "AllowSubscriptionsFrom", ); ensure!(T::contains(origin), ProcessMessageError::Unsupported); instructions @@ -487,13 +499,13 @@ impl ShouldExecute for AllowHrmpNotificationsFromRelayChain { fn should_execute<RuntimeCall>( origin: &Location, instructions: &mut [Instruction<RuntimeCall>], - _max_weight: Weight, - _properties: &mut Properties, + max_weight: Weight, + properties: &mut Properties, ) -> Result<(), ProcessMessageError> { - log::trace!( + tracing::trace!( target: "xcm::barriers", - "AllowHrmpNotificationsFromRelayChain origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, _max_weight, _properties, + ?origin, ?instructions, ?max_weight, ?properties, + "AllowHrmpNotificationsFromRelayChain" ); // accept only the Relay Chain ensure!(matches!(origin.unpack(), (1, [])), ProcessMessageError::Unsupported); @@ -560,7 +572,7 @@ impl DenyExecution for DenyReserveTransferToRelayChain { ReserveAssetDeposited { .. } if matches!(origin, Location { parents: 1, interior: Here }) => { - log::warn!( + tracing::debug!( target: "xcm::barriers", "Unexpected ReserveAssetDeposited from the Relay Chain", ); @@ -603,7 +615,7 @@ impl<Inner: DenyExecution> DenyRecursively<Inner> { // Prevent stack overflow by enforcing a recursion depth limit. recursion_count::with(|count| { if *count > xcm_executor::RECURSION_LIMIT { - log::debug!( + tracing::debug!( target: "xcm::barriers", "Recursion limit exceeded (count: {count}), origin: {:?}, xcm: {:?}, max_weight: {:?}, properties: {:?}", origin, xcm, max_weight, properties @@ -642,7 +654,7 @@ impl<Inner: DenyExecution> DenyExecution for DenyRecursively<Inner> { ) -> Result<(), ProcessMessageError> { // First, check if the top-level message should be denied. Inner::deny_execution(origin, instructions, max_weight, properties).inspect_err(|e| { - log::warn!( + tracing::debug!( target: "xcm::barriers", "DenyRecursively::Inner denied execution, origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}, error: {:?}", origin, instructions, max_weight, properties, e diff --git a/polkadot/xcm/xcm-builder/src/currency_adapter.rs b/polkadot/xcm/xcm-builder/src/currency_adapter.rs index 355d6ad8538..1681e4dc650 100644 --- a/polkadot/xcm/xcm-builder/src/currency_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/currency_adapter.rs @@ -142,8 +142,8 @@ impl< > TransactAsset for CurrencyAdapter<Currency, Matcher, AccountIdConverter, AccountId, CheckedAccount> { - fn can_check_in(_origin: &Location, what: &Asset, _context: &XcmContext) -> Result { - log::trace!(target: "xcm::currency_adapter", "can_check_in origin: {:?}, what: {:?}", _origin, what); + fn can_check_in(origin: &Location, what: &Asset, _context: &XcmContext) -> Result { + tracing::trace!(target: "xcm::currency_adapter", ?origin, ?what, "can_check_in origin"); // Check we handle this asset. let amount: Currency::Balance = Matcher::matches_fungible(what).ok_or(Error::AssetNotHandled)?; @@ -156,8 +156,8 @@ impl< } } - fn check_in(_origin: &Location, what: &Asset, _context: &XcmContext) { - log::trace!(target: "xcm::currency_adapter", "check_in origin: {:?}, what: {:?}", _origin, what); + fn check_in(origin: &Location, what: &Asset, _context: &XcmContext) { + tracing::trace!(target: "xcm::currency_adapter", ?origin, ?what, "check_in origin"); if let Some(amount) = Matcher::matches_fungible(what) { match CheckedAccount::get() { Some((checked_account, MintLocation::Local)) => @@ -169,8 +169,8 @@ impl< } } - fn can_check_out(_dest: &Location, what: &Asset, _context: &XcmContext) -> Result { - log::trace!(target: "xcm::currency_adapter", "can_check_out dest: {:?}, what: {:?}", _dest, what); + fn can_check_out(dest: &Location, what: &Asset, _context: &XcmContext) -> Result { + tracing::trace!(target: "xcm::currency_adapter", ?dest, ?what, "can_check_out"); let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotHandled)?; match CheckedAccount::get() { Some((checked_account, MintLocation::Local)) => @@ -181,8 +181,8 @@ impl< } } - fn check_out(_dest: &Location, what: &Asset, _context: &XcmContext) { - log::trace!(target: "xcm::currency_adapter", "check_out dest: {:?}, what: {:?}", _dest, what); + fn check_out(dest: &Location, what: &Asset, _context: &XcmContext) { + tracing::trace!(target: "xcm::currency_adapter", ?dest, ?what, "check_out"); if let Some(amount) = Matcher::matches_fungible(what) { match CheckedAccount::get() { Some((checked_account, MintLocation::Local)) => @@ -195,7 +195,7 @@ impl< } fn deposit_asset(what: &Asset, who: &Location, _context: Option<&XcmContext>) -> Result { - log::trace!(target: "xcm::currency_adapter", "deposit_asset what: {:?}, who: {:?}", what, who); + tracing::trace!(target: "xcm::currency_adapter", ?what, ?who, "deposit_asset"); // Check we handle this asset. let amount = Matcher::matches_fungible(&what).ok_or(Error::AssetNotHandled)?; let who = @@ -209,7 +209,7 @@ impl< who: &Location, _maybe_context: Option<&XcmContext>, ) -> result::Result<AssetsInHolding, XcmError> { - log::trace!(target: "xcm::currency_adapter", "withdraw_asset what: {:?}, who: {:?}", what, who); + tracing::trace!(target: "xcm::currency_adapter", ?what, ?who, "withdraw_asset"); // Check we handle this asset. let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotHandled)?; let who = @@ -225,7 +225,7 @@ impl< to: &Location, _context: &XcmContext, ) -> result::Result<AssetsInHolding, XcmError> { - log::trace!(target: "xcm::currency_adapter", "internal_transfer_asset asset: {:?}, from: {:?}, to: {:?}", asset, from, to); + tracing::trace!(target: "xcm::currency_adapter", ?asset, ?from, ?to, "internal_transfer_asset"); let amount = Matcher::matches_fungible(asset).ok_or(Error::AssetNotHandled)?; let from = AccountIdConverter::convert_location(from).ok_or(Error::AccountIdConversionFailed)?; diff --git a/polkadot/xcm/xcm-builder/src/fee_handling.rs b/polkadot/xcm/xcm-builder/src/fee_handling.rs index f02e6b393c8..21f6eace4fb 100644 --- a/polkadot/xcm/xcm-builder/src/fee_handling.rs +++ b/polkadot/xcm/xcm-builder/src/fee_handling.rs @@ -127,7 +127,7 @@ pub fn deposit_or_burn_fee<AssetTransactor: TransactAsset>( ) { for asset in fee.into_inner() { if let Err(e) = AssetTransactor::deposit_asset(&asset, &dest, context) { - log::trace!( + tracing::trace!( target: "xcm::fees", "`AssetTransactor::deposit_asset` returned error: {e:?}. Burning fee: {asset:?}. \ They might be burned.", diff --git a/polkadot/xcm/xcm-builder/src/filter_asset_location.rs b/polkadot/xcm/xcm-builder/src/filter_asset_location.rs index 16b7be7f3ba..55d548de453 100644 --- a/polkadot/xcm/xcm-builder/src/filter_asset_location.rs +++ b/polkadot/xcm/xcm-builder/src/filter_asset_location.rs @@ -26,7 +26,7 @@ use xcm::latest::{Asset, AssetFilter, AssetId, Location, WildAsset}; pub struct NativeAsset; impl ContainsPair<Asset, Location> for NativeAsset { fn contains(asset: &Asset, origin: &Location) -> bool { - log::trace!(target: "xcm::contains", "NativeAsset asset: {:?}, origin: {:?}", asset, origin); + tracing::trace!(target: "xcm::contains", ?asset, ?origin, "NativeAsset"); matches!(asset.id, AssetId(ref id) if id == origin) } } @@ -35,7 +35,7 @@ impl ContainsPair<Asset, Location> for NativeAsset { pub struct Case<T>(PhantomData<T>); impl<T: Get<(AssetFilter, Location)>> ContainsPair<Asset, Location> for Case<T> { fn contains(asset: &Asset, origin: &Location) -> bool { - log::trace!(target: "xcm::contains", "Case asset: {:?}, origin: {:?}", asset, origin); + tracing::trace!(target: "xcm::contains", ?asset, ?origin, "Case asset"); let (a, o) = T::get(); a.matches(asset) && &o == origin } @@ -51,7 +51,7 @@ impl<LocationFilter: Contains<Location>, AssetFilters: Get<Vec<AssetFilter>>> Contains<(Location, Vec<Asset>)> for LocationWithAssetFilters<LocationFilter, AssetFilters> { fn contains((location, assets): &(Location, Vec<Asset>)) -> bool { - log::trace!(target: "xcm::contains", "LocationWithAssetFilters location: {:?}, assets: {:?}", location, assets); + tracing::trace!(target: "xcm::contains", ?location, ?assets, "LocationWithAssetFilters"); // `location` must match the `Location` filter. if !LocationFilter::contains(location) { diff --git a/polkadot/xcm/xcm-builder/src/fungible_adapter.rs b/polkadot/xcm/xcm-builder/src/fungible_adapter.rs index 2da772deb0e..10dd929fa47 100644 --- a/polkadot/xcm/xcm-builder/src/fungible_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/fungible_adapter.rs @@ -49,10 +49,10 @@ impl< to: &Location, _context: &XcmContext, ) -> result::Result<AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: "xcm::fungible_adapter", - "internal_transfer_asset what: {:?}, from: {:?}, to: {:?}", - what, from, to + ?what, ?from, ?to, + "internal_transfer_asset", ); // Check we handle the asset let amount = Matcher::matches_fungible(what).ok_or(MatchError::AssetNotHandled)?; @@ -114,11 +114,11 @@ impl< > TransactAsset for FungibleMutateAdapter<Fungible, Matcher, AccountIdConverter, AccountId, CheckingAccount> { - fn can_check_in(_origin: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_in(origin: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { + tracing::trace!( target: "xcm::fungible_adapter", - "can_check_in origin: {:?}, what: {:?}", - _origin, what + ?origin, ?what, + "can_check_in origin", ); // Check we handle this asset let amount = Matcher::matches_fungible(what).ok_or(MatchError::AssetNotHandled)?; @@ -131,11 +131,11 @@ impl< } } - fn check_in(_origin: &Location, what: &Asset, _context: &XcmContext) { - log::trace!( + fn check_in(origin: &Location, what: &Asset, _context: &XcmContext) { + tracing::trace!( target: "xcm::fungible_adapter", - "check_in origin: {:?}, what: {:?}", - _origin, what + ?origin, ?what, + "check_in origin", ); if let Some(amount) = Matcher::matches_fungible(what) { match CheckingAccount::get() { @@ -148,12 +148,12 @@ impl< } } - fn can_check_out(_dest: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_out(dest: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { + tracing::trace!( target: "xcm::fungible_adapter", - "can_check_out dest: {:?}, what: {:?}", - _dest, - what + ?dest, + ?what, + "can_check_out", ); let amount = Matcher::matches_fungible(what).ok_or(MatchError::AssetNotHandled)?; match CheckingAccount::get() { @@ -165,12 +165,12 @@ impl< } } - fn check_out(_dest: &Location, what: &Asset, _context: &XcmContext) { - log::trace!( + fn check_out(dest: &Location, what: &Asset, _context: &XcmContext) { + tracing::trace!( target: "xcm::fungible_adapter", - "check_out dest: {:?}, what: {:?}", - _dest, - what + ?dest, + ?what, + "check_out", ); if let Some(amount) = Matcher::matches_fungible(what) { match CheckingAccount::get() { @@ -184,10 +184,10 @@ impl< } fn deposit_asset(what: &Asset, who: &Location, _context: Option<&XcmContext>) -> XcmResult { - log::trace!( + tracing::trace!( target: "xcm::fungible_adapter", - "deposit_asset what: {:?}, who: {:?}", - what, who, + ?what, ?who, + "deposit_asset", ); let amount = Matcher::matches_fungible(what).ok_or(MatchError::AssetNotHandled)?; let who = AccountIdConverter::convert_location(who) @@ -202,10 +202,10 @@ impl< who: &Location, _context: Option<&XcmContext>, ) -> result::Result<AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: "xcm::fungible_adapter", - "withdraw_asset what: {:?}, who: {:?}", - what, who, + ?what, ?who, + "withdraw_asset", ); let amount = Matcher::matches_fungible(what).ok_or(MatchError::AssetNotHandled)?; let who = AccountIdConverter::convert_location(who) diff --git a/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs b/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs index 59b4ccb13d0..f412dc42db5 100644 --- a/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs @@ -16,7 +16,7 @@ //! Adapters to work with [`frame_support::traits::fungibles`] through XCM. -use core::{marker::PhantomData, result}; +use core::{fmt::Debug, marker::PhantomData, result}; use frame_support::traits::{ tokens::{ fungibles, Fortitude::Polite, Precision::Exact, Preservation::Expendable, @@ -35,7 +35,8 @@ impl< Assets: fungibles::Mutate<AccountId>, Matcher: MatchesFungibles<Assets::AssetId, Assets::Balance>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Eq + Clone, /* can't get away without it since Currency is generic over it. */ + AccountId: Eq + Clone + Debug, /* can't get away without it since Currency is generic + * over it. */ > TransactAsset for FungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, AccountId> { fn internal_transfer_asset( @@ -44,10 +45,10 @@ impl< to: &Location, _context: &XcmContext, ) -> result::Result<xcm_executor::AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: "xcm::fungibles_adapter", - "internal_transfer_asset what: {:?}, from: {:?}, to: {:?}", - what, from, to + ?what, ?from, ?to, + "internal_transfer_asset" ); // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; @@ -55,8 +56,10 @@ impl< .ok_or(MatchError::AccountIdConversionFailed)?; let dest = AccountIdConverter::convert_location(to) .ok_or(MatchError::AccountIdConversionFailed)?; - Assets::transfer(asset_id, &source, &dest, amount, Expendable) - .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; + Assets::transfer(asset_id.clone(), &source, &dest, amount, Expendable).map_err(|e| { + tracing::debug!(target: "xcm::fungibles_adapter", error = ?e, ?asset_id, ?source, ?dest, ?amount, "Failed internal transfer asset"); + XcmError::FailedToTransactAsset(e.into()) + })?; Ok(what.clone().into()) } } @@ -200,11 +203,11 @@ impl< CheckingAccount, > { - fn can_check_in(_origin: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_in(origin: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { + tracing::trace!( target: "xcm::fungibles_adapter", - "can_check_in origin: {:?}, what: {:?}", - _origin, what + ?origin, ?what, + "can_check_in" ); // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; @@ -217,11 +220,11 @@ impl< } } - fn check_in(_origin: &Location, what: &Asset, _context: &XcmContext) { - log::trace!( + fn check_in(origin: &Location, what: &Asset, _context: &XcmContext) { + tracing::trace!( target: "xcm::fungibles_adapter", - "check_in origin: {:?}, what: {:?}", - _origin, what + ?origin, ?what, + "check_in" ); if let Ok((asset_id, amount)) = Matcher::matches_fungibles(what) { match CheckAsset::asset_checking(&asset_id) { @@ -234,11 +237,11 @@ impl< } } - fn can_check_out(_origin: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_out(origin: &Location, what: &Asset, _context: &XcmContext) -> XcmResult { + tracing::trace!( target: "xcm::fungibles_adapter", - "can_check_out origin: {:?}, what: {:?}", - _origin, what + ?origin, ?what, + "can_check_out" ); // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; @@ -251,11 +254,11 @@ impl< } } - fn check_out(_dest: &Location, what: &Asset, _context: &XcmContext) { - log::trace!( + fn check_out(dest: &Location, what: &Asset, _context: &XcmContext) { + tracing::trace!( target: "xcm::fungibles_adapter", - "check_out dest: {:?}, what: {:?}", - _dest, what + ?dest, ?what, + "check_out" ); if let Ok((asset_id, amount)) = Matcher::matches_fungibles(what) { match CheckAsset::asset_checking(&asset_id) { @@ -269,10 +272,10 @@ impl< } fn deposit_asset(what: &Asset, who: &Location, _context: Option<&XcmContext>) -> XcmResult { - log::trace!( + tracing::trace!( target: "xcm::fungibles_adapter", - "deposit_asset what: {:?}, who: {:?}", - what, who, + ?what, ?who, + "deposit_asset" ); // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; @@ -288,10 +291,10 @@ impl< who: &Location, _maybe_context: Option<&XcmContext>, ) -> result::Result<xcm_executor::AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: "xcm::fungibles_adapter", - "withdraw_asset what: {:?}, who: {:?}", - what, who, + ?what, ?who, + "withdraw_asset" ); // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; @@ -315,7 +318,8 @@ impl< Assets: fungibles::Mutate<AccountId>, Matcher: MatchesFungibles<Assets::AssetId, Assets::Balance>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Eq + Clone, /* can't get away without it since Currency is generic over it. */ + AccountId: Eq + Clone + Debug, /* can't get away without it since Currency is generic + * over it. */ CheckAsset: AssetChecking<Assets::AssetId>, CheckingAccount: Get<AccountId>, > TransactAsset diff --git a/polkadot/xcm/xcm-builder/src/lib.rs b/polkadot/xcm/xcm-builder/src/lib.rs index 4c48589e671..1c08c875eb2 100644 --- a/polkadot/xcm/xcm-builder/src/lib.rs +++ b/polkadot/xcm/xcm-builder/src/lib.rs @@ -21,6 +21,7 @@ #![cfg_attr(not(feature = "std"), no_std)] extern crate alloc; +extern crate core; #[cfg(test)] mod tests; diff --git a/polkadot/xcm/xcm-builder/src/location_conversion.rs b/polkadot/xcm/xcm-builder/src/location_conversion.rs index 1d840e9c0dd..c7aa0c8b504 100644 --- a/polkadot/xcm/xcm-builder/src/location_conversion.rs +++ b/polkadot/xcm/xcm-builder/src/location_conversion.rs @@ -392,10 +392,10 @@ impl<UniversalLocation: Get<InteriorLocation>, AccountId: From<[u8; 32]> + Clone { fn convert_location(location: &Location) -> Option<AccountId> { let universal_source = UniversalLocation::get(); - log::trace!( + tracing::trace!( target: "xcm::location_conversion", - "GlobalConsensusConvertsFor universal_source: {:?}, location: {:?}", - universal_source, location, + ?universal_source, ?location, + "GlobalConsensusConvertsFor", ); let (remote_network, remote_location) = ensure_is_remote(universal_source, location.clone()).ok()?; @@ -435,10 +435,10 @@ impl<UniversalLocation: Get<InteriorLocation>, AccountId: From<[u8; 32]> + Clone { fn convert_location(location: &Location) -> Option<AccountId> { let universal_source = UniversalLocation::get(); - log::trace!( + tracing::trace!( target: "xcm::location_conversion", - "GlobalConsensusParachainConvertsFor universal_source: {:?}, location: {:?}", - universal_source, location, + ?universal_source, ?location, + "GlobalConsensusParachainConvertsFor", ); let devolved = ensure_is_remote(universal_source, location.clone()).ok()?; let (remote_network, remote_location) = devolved; diff --git a/polkadot/xcm/xcm-builder/src/nonfungible_adapter.rs b/polkadot/xcm/xcm-builder/src/nonfungible_adapter.rs index 8e6232ea64d..08e2a9249f2 100644 --- a/polkadot/xcm/xcm-builder/src/nonfungible_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/nonfungible_adapter.rs @@ -17,7 +17,7 @@ //! Adapters to work with [`frame_support::traits::tokens::nonfungible`] through XCM. use crate::MintLocation; -use core::{marker::PhantomData, result}; +use core::{fmt::Debug, marker::PhantomData, result}; use frame_support::{ ensure, traits::{tokens::nonfungible, Get}, @@ -34,14 +34,17 @@ const LOG_TARGET: &str = "xcm::nonfungible_adapter"; /// Only works for transfers. pub struct NonFungibleTransferAdapter<NonFungible, Matcher, AccountIdConverter, AccountId>( PhantomData<(NonFungible, Matcher, AccountIdConverter, AccountId)>, -); +) +where + NonFungible: nonfungible::Transfer<AccountId>; impl< NonFungible: nonfungible::Transfer<AccountId>, Matcher: MatchesNonFungible<NonFungible::ItemId>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Clone, // can't get away without it since Currency is generic over it. - > TransactAsset - for NonFungibleTransferAdapter<NonFungible, Matcher, AccountIdConverter, AccountId> + AccountId: Clone + Debug, // can't get away without it since Currency is generic over it. + > TransactAsset for NonFungibleTransferAdapter<NonFungible, Matcher, AccountIdConverter, AccountId> +where + NonFungible::ItemId: Debug, { fn transfer_asset( what: &Asset, @@ -49,20 +52,22 @@ impl< to: &Location, context: &XcmContext, ) -> result::Result<xcm_executor::AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: LOG_TARGET, - "transfer_asset what: {:?}, from: {:?}, to: {:?}, context: {:?}", - what, - from, - to, - context, + ?what, + ?from, + ?to, + ?context, + "transfer_asset", ); // Check we handle this asset. let instance = Matcher::matches_nonfungible(what).ok_or(MatchError::AssetNotHandled)?; let destination = AccountIdConverter::convert_location(to) .ok_or(MatchError::AccountIdConversionFailed)?; - NonFungible::transfer(&instance, &destination) - .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; + NonFungible::transfer(&instance, &destination).map_err(|e| { + tracing::debug!(target: LOG_TARGET, ?e, ?instance, ?destination, "Failed to transfer non-fungible asset"); + XcmError::FailedToTransactAsset(e.into()) + })?; Ok(what.clone().into()) } } @@ -76,15 +81,21 @@ pub struct NonFungibleMutateAdapter< AccountIdConverter, AccountId, CheckingAccount, ->(PhantomData<(NonFungible, Matcher, AccountIdConverter, AccountId, CheckingAccount)>); +>(PhantomData<(NonFungible, Matcher, AccountIdConverter, AccountId, CheckingAccount)>) +where + NonFungible: nonfungible::Mutate<AccountId>, + NonFungible::ItemId: Debug; impl< NonFungible: nonfungible::Mutate<AccountId>, Matcher: MatchesNonFungible<NonFungible::ItemId>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Clone + Eq, // can't get away without it since Currency is generic over it. + AccountId: Clone + Eq + Debug, /* can't get away without it since Currency is generic + * over it. */ CheckingAccount: Get<Option<(AccountId, MintLocation)>>, > NonFungibleMutateAdapter<NonFungible, Matcher, AccountIdConverter, AccountId, CheckingAccount> +where + NonFungible::ItemId: Debug, { fn can_accrue_checked(instance: NonFungible::ItemId) -> XcmResult { ensure!(NonFungible::owner(&instance).is_none(), XcmError::NotDepositable); @@ -111,18 +122,21 @@ impl< NonFungible: nonfungible::Mutate<AccountId>, Matcher: MatchesNonFungible<NonFungible::ItemId>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Clone + Eq, // can't get away without it since Currency is generic over it. + AccountId: Clone + Eq + Debug, /* can't get away without it since Currency is generic + * over it. */ CheckingAccount: Get<Option<(AccountId, MintLocation)>>, > TransactAsset for NonFungibleMutateAdapter<NonFungible, Matcher, AccountIdConverter, AccountId, CheckingAccount> +where + NonFungible::ItemId: Debug, { - fn can_check_in(_origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_in(origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { + tracing::trace!( target: LOG_TARGET, - "can_check_in origin: {:?}, what: {:?}, context: {:?}", - _origin, - what, - context, + ?origin, + ?what, + ?context, + "can_check_in", ); // Check we handle this asset. let instance = Matcher::matches_nonfungible(what).ok_or(MatchError::AssetNotHandled)?; @@ -136,13 +150,13 @@ impl< } } - fn check_in(_origin: &Location, what: &Asset, context: &XcmContext) { - log::trace!( + fn check_in(origin: &Location, what: &Asset, context: &XcmContext) { + tracing::trace!( target: LOG_TARGET, - "check_in origin: {:?}, what: {:?}, context: {:?}", - _origin, - what, - context, + ?origin, + ?what, + ?context, + "check_in", ); if let Some(instance) = Matcher::matches_nonfungible(what) { match CheckingAccount::get() { @@ -156,13 +170,13 @@ impl< } } - fn can_check_out(_dest: &Location, what: &Asset, context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_out(dest: &Location, what: &Asset, context: &XcmContext) -> XcmResult { + tracing::trace!( target: LOG_TARGET, - "can_check_out dest: {:?}, what: {:?}, context: {:?}", - _dest, - what, - context, + ?dest, + ?what, + ?context, + "can_check_out", ); // Check we handle this asset. let instance = Matcher::matches_nonfungible(what).ok_or(MatchError::AssetNotHandled)?; @@ -176,13 +190,13 @@ impl< } } - fn check_out(_dest: &Location, what: &Asset, context: &XcmContext) { - log::trace!( + fn check_out(dest: &Location, what: &Asset, context: &XcmContext) { + tracing::trace!( target: LOG_TARGET, - "check_out dest: {:?}, what: {:?}, context: {:?}", - _dest, - what, - context, + ?dest, + ?what, + ?context, + "check_out", ); if let Some(instance) = Matcher::matches_nonfungible(what) { match CheckingAccount::get() { @@ -197,19 +211,21 @@ impl< } fn deposit_asset(what: &Asset, who: &Location, context: Option<&XcmContext>) -> XcmResult { - log::trace!( + tracing::trace!( target: LOG_TARGET, - "deposit_asset what: {:?}, who: {:?}, context: {:?}", - what, - who, - context, + ?what, + ?who, + ?context, + "deposit_asset", ); // Check we handle this asset. let instance = Matcher::matches_nonfungible(what).ok_or(MatchError::AssetNotHandled)?; let who = AccountIdConverter::convert_location(who) .ok_or(MatchError::AccountIdConversionFailed)?; - NonFungible::mint_into(&instance, &who) - .map_err(|e| XcmError::FailedToTransactAsset(e.into())) + NonFungible::mint_into(&instance, &who).map_err(|e| { + tracing::debug!(target: LOG_TARGET, ?e, ?instance, ?who, "Failed to mint asset"); + XcmError::FailedToTransactAsset(e.into()) + }) } fn withdraw_asset( @@ -217,19 +233,21 @@ impl< who: &Location, maybe_context: Option<&XcmContext>, ) -> result::Result<xcm_executor::AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: LOG_TARGET, - "withdraw_asset what: {:?}, who: {:?}, maybe_context: {:?}", - what, - who, - maybe_context, + ?what, + ?who, + ?maybe_context, + "withdraw_asset" ); // Check we handle this asset. let who = AccountIdConverter::convert_location(who) .ok_or(MatchError::AccountIdConversionFailed)?; let instance = Matcher::matches_nonfungible(what).ok_or(MatchError::AssetNotHandled)?; - NonFungible::burn(&instance, Some(&who)) - .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; + NonFungible::burn(&instance, Some(&who)).map_err(|e| { + tracing::debug!(target: LOG_TARGET, ?e, ?instance, ?who, "Failed to burn asset"); + XcmError::FailedToTransactAsset(e.into()) + })?; Ok(what.clone().into()) } } @@ -239,15 +257,21 @@ impl< /// Works for everything. pub struct NonFungibleAdapter<NonFungible, Matcher, AccountIdConverter, AccountId, CheckingAccount>( PhantomData<(NonFungible, Matcher, AccountIdConverter, AccountId, CheckingAccount)>, -); +) +where + NonFungible: nonfungible::Mutate<AccountId> + nonfungible::Transfer<AccountId>, + NonFungible::ItemId: Debug; impl< NonFungible: nonfungible::Mutate<AccountId> + nonfungible::Transfer<AccountId>, Matcher: MatchesNonFungible<NonFungible::ItemId>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Clone + Eq, // can't get away without it since Currency is generic over it. + AccountId: Clone + Eq + Debug, /* can't get away without it since Currency is generic + * over it. */ CheckingAccount: Get<Option<(AccountId, MintLocation)>>, > TransactAsset for NonFungibleAdapter<NonFungible, Matcher, AccountIdConverter, AccountId, CheckingAccount> +where + NonFungible::ItemId: Debug, { fn can_check_in(origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { NonFungibleMutateAdapter::< diff --git a/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs b/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs index 006c28954bc..cc0bed90da6 100644 --- a/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs @@ -17,7 +17,7 @@ //! Adapters to work with [`frame_support::traits::tokens::nonfungibles`] through XCM. use crate::{AssetChecking, MintLocation}; -use core::{marker::PhantomData, result}; +use core::{fmt::Debug, marker::PhantomData, result}; use frame_support::{ ensure, traits::{tokens::nonfungibles, Get}, @@ -34,13 +34,20 @@ const LOG_TARGET: &str = "xcm::nonfungibles_adapter"; /// Only works for transfers. pub struct NonFungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, AccountId>( PhantomData<(Assets, Matcher, AccountIdConverter, AccountId)>, -); +) +where + Assets: nonfungibles::Transfer<AccountId>, + Assets::CollectionId: Debug, + Assets::ItemId: Debug; impl< Assets: nonfungibles::Transfer<AccountId>, Matcher: MatchesNonFungibles<Assets::CollectionId, Assets::ItemId>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Clone, // can't get away without it since Currency is generic over it. + AccountId: Clone + Debug, // can't get away without it since Currency is generic over it. > TransactAsset for NonFungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, AccountId> +where + Assets::CollectionId: Debug, + Assets::ItemId: Debug, { fn transfer_asset( what: &Asset, @@ -48,20 +55,22 @@ impl< to: &Location, context: &XcmContext, ) -> result::Result<xcm_executor::AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: LOG_TARGET, - "transfer_asset what: {:?}, from: {:?}, to: {:?}, context: {:?}", - what, - from, - to, - context, + ?what, + ?from, + ?to, + ?context, + "transfer_asset", ); // Check we handle this asset. let (class, instance) = Matcher::matches_nonfungibles(what)?; let destination = AccountIdConverter::convert_location(to) .ok_or(MatchError::AccountIdConversionFailed)?; - Assets::transfer(&class, &instance, &destination) - .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; + Assets::transfer(&class, &instance, &destination).map_err(|e| { + tracing::debug!(target: LOG_TARGET, ?e, ?class, ?instance, ?destination, "Failed to transfer asset"); + XcmError::FailedToTransactAsset(e.into()) + })?; Ok(what.clone().into()) } } @@ -124,7 +133,8 @@ impl< Assets: nonfungibles::Mutate<AccountId>, Matcher: MatchesNonFungibles<Assets::CollectionId, Assets::ItemId>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Clone + Eq, // can't get away without it since Currency is generic over it. + AccountId: Clone + Eq + Debug, /* can't get away without it since Currency is generic + * over it. */ CheckAsset: AssetChecking<Assets::CollectionId>, CheckingAccount: Get<Option<AccountId>>, > TransactAsset @@ -136,14 +146,17 @@ impl< CheckAsset, CheckingAccount, > +where + Assets::CollectionId: Debug, + Assets::ItemId: Debug, { - fn can_check_in(_origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_in(origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { + tracing::trace!( target: LOG_TARGET, - "can_check_in origin: {:?}, what: {:?}, context: {:?}", - _origin, - what, - context, + ?origin, + ?what, + ?context, + "can_check_in", ); // Check we handle this asset. let (class, instance) = Matcher::matches_nonfungibles(what)?; @@ -156,13 +169,13 @@ impl< } } - fn check_in(_origin: &Location, what: &Asset, context: &XcmContext) { - log::trace!( + fn check_in(origin: &Location, what: &Asset, context: &XcmContext) { + tracing::trace!( target: LOG_TARGET, - "check_in origin: {:?}, what: {:?}, context: {:?}", - _origin, - what, - context, + ?origin, + ?what, + ?context, + "check_in", ); if let Ok((class, instance)) = Matcher::matches_nonfungibles(what) { match CheckAsset::asset_checking(&class) { @@ -175,13 +188,13 @@ impl< } } - fn can_check_out(_dest: &Location, what: &Asset, context: &XcmContext) -> XcmResult { - log::trace!( + fn can_check_out(dest: &Location, what: &Asset, context: &XcmContext) -> XcmResult { + tracing::trace!( target: LOG_TARGET, - "can_check_out dest: {:?}, what: {:?}, context: {:?}", - _dest, - what, - context, + ?dest, + ?what, + ?context, + "can_check_out", ); // Check we handle this asset. let (class, instance) = Matcher::matches_nonfungibles(what)?; @@ -194,13 +207,13 @@ impl< } } - fn check_out(_dest: &Location, what: &Asset, context: &XcmContext) { - log::trace!( + fn check_out(dest: &Location, what: &Asset, context: &XcmContext) { + tracing::trace!( target: LOG_TARGET, - "check_out dest: {:?}, what: {:?}, context: {:?}", - _dest, - what, - context, + ?dest, + ?what, + ?context, + "check_out", ); if let Ok((class, instance)) = Matcher::matches_nonfungibles(what) { match CheckAsset::asset_checking(&class) { @@ -214,19 +227,21 @@ impl< } fn deposit_asset(what: &Asset, who: &Location, context: Option<&XcmContext>) -> XcmResult { - log::trace!( + tracing::trace!( target: LOG_TARGET, - "deposit_asset what: {:?}, who: {:?}, context: {:?}", - what, - who, - context, + ?what, + ?who, + ?context, + "deposit_asset", ); // Check we handle this asset. let (class, instance) = Matcher::matches_nonfungibles(what)?; let who = AccountIdConverter::convert_location(who) .ok_or(MatchError::AccountIdConversionFailed)?; - Assets::mint_into(&class, &instance, &who) - .map_err(|e| XcmError::FailedToTransactAsset(e.into())) + Assets::mint_into(&class, &instance, &who).map_err(|e| { + tracing::debug!(target: LOG_TARGET, ?e, ?class, ?instance, ?who, "Failed to mint asset"); + XcmError::FailedToTransactAsset(e.into()) + }) } fn withdraw_asset( @@ -234,19 +249,21 @@ impl< who: &Location, maybe_context: Option<&XcmContext>, ) -> result::Result<xcm_executor::AssetsInHolding, XcmError> { - log::trace!( + tracing::trace!( target: LOG_TARGET, - "withdraw_asset what: {:?}, who: {:?}, maybe_context: {:?}", - what, - who, - maybe_context, + ?what, + ?who, + ?maybe_context, + "withdraw_asset", ); // Check we handle this asset. let who = AccountIdConverter::convert_location(who) .ok_or(MatchError::AccountIdConversionFailed)?; let (class, instance) = Matcher::matches_nonfungibles(what)?; - Assets::burn(&class, &instance, Some(&who)) - .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; + Assets::burn(&class, &instance, Some(&who)).map_err(|e| { + tracing::debug!(target: LOG_TARGET, ?e, ?class, ?instance, ?who, "Failed to burn asset"); + XcmError::FailedToTransactAsset(e.into()) + })?; Ok(what.clone().into()) } } @@ -261,12 +278,17 @@ pub struct NonFungiblesAdapter< AccountId, CheckAsset, CheckingAccount, ->(PhantomData<(Assets, Matcher, AccountIdConverter, AccountId, CheckAsset, CheckingAccount)>); +>(PhantomData<(Assets, Matcher, AccountIdConverter, AccountId, CheckAsset, CheckingAccount)>) +where + Assets: nonfungibles::Transfer<AccountId>, + Assets::CollectionId: Debug, + Assets::ItemId: Debug; impl< Assets: nonfungibles::Mutate<AccountId> + nonfungibles::Transfer<AccountId>, Matcher: MatchesNonFungibles<Assets::CollectionId, Assets::ItemId>, AccountIdConverter: ConvertLocation<AccountId>, - AccountId: Clone + Eq, // can't get away without it since Currency is generic over it. + AccountId: Clone + Eq + Debug, /* can't get away without it since Currency is generic + * over it. */ CheckAsset: AssetChecking<Assets::CollectionId>, CheckingAccount: Get<Option<AccountId>>, > TransactAsset @@ -278,6 +300,9 @@ impl< CheckAsset, CheckingAccount, > +where + Assets::CollectionId: Debug, + Assets::ItemId: Debug, { fn can_check_in(origin: &Location, what: &Asset, context: &XcmContext) -> XcmResult { NonFungiblesMutateAdapter::< diff --git a/polkadot/xcm/xcm-builder/src/origin_conversion.rs b/polkadot/xcm/xcm-builder/src/origin_conversion.rs index 6e73c0dae7b..20dbc9bba0a 100644 --- a/polkadot/xcm/xcm-builder/src/origin_conversion.rs +++ b/polkadot/xcm/xcm-builder/src/origin_conversion.rs @@ -39,10 +39,10 @@ where kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!( + tracing::trace!( target: "xcm::origin_conversion", - "SovereignSignedViaLocation origin: {:?}, kind: {:?}", - origin, kind, + ?origin, ?kind, + "SovereignSignedViaLocation", ); if let OriginKind::SovereignAccount = kind { let location = LocationConverter::convert_location(&origin).ok_or(origin)?; @@ -60,7 +60,7 @@ impl<RuntimeOrigin: OriginTrait> ConvertOrigin<RuntimeOrigin> for ParentAsSuperu kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!(target: "xcm::origin_conversion", "ParentAsSuperuser origin: {:?}, kind: {:?}", origin, kind); + tracing::trace!(target: "xcm::origin_conversion", ?origin, ?kind, "ParentAsSuperuser",); if kind == OriginKind::Superuser && origin.contains_parents_only(1) { Ok(RuntimeOrigin::root()) } else { @@ -80,7 +80,7 @@ impl<ParaId: IsSystem + From<u32>, RuntimeOrigin: OriginTrait> ConvertOrigin<Run kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!(target: "xcm::origin_conversion", "ChildSystemParachainAsSuperuser origin: {:?}, kind: {:?}", origin, kind); + tracing::trace!(target: "xcm::origin_conversion", ?origin, ?kind, "ChildSystemParachainAsSuperuser",); match (kind, origin.unpack()) { (OriginKind::Superuser, (0, [Junction::Parachain(id)])) if ParaId::from(*id).is_system() => @@ -101,10 +101,10 @@ impl<ParaId: IsSystem + From<u32>, RuntimeOrigin: OriginTrait> ConvertOrigin<Run kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!( + tracing::trace!( target: "xcm::origin_conversion", - "SiblingSystemParachainAsSuperuser origin: {:?}, kind: {:?}", - origin, kind, + ?origin, ?kind, + "SiblingSystemParachainAsSuperuser", ); match (kind, origin.unpack()) { (OriginKind::Superuser, (1, [Junction::Parachain(id)])) @@ -126,7 +126,7 @@ impl<ParachainOrigin: From<u32>, RuntimeOrigin: From<ParachainOrigin>> ConvertOr kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!(target: "xcm::origin_conversion", "ChildParachainAsNative origin: {:?}, kind: {:?}", origin, kind); + tracing::trace!(target: "xcm::origin_conversion", ?origin, ?kind, "ChildParachainAsNative"); match (kind, origin.unpack()) { (OriginKind::Native, (0, [Junction::Parachain(id)])) => Ok(RuntimeOrigin::from(ParachainOrigin::from(*id))), @@ -146,10 +146,10 @@ impl<ParachainOrigin: From<u32>, RuntimeOrigin: From<ParachainOrigin>> ConvertOr kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!( + tracing::trace!( target: "xcm::origin_conversion", - "SiblingParachainAsNative origin: {:?}, kind: {:?}", - origin, kind, + ?origin, ?kind, + "SiblingParachainAsNative", ); match (kind, origin.unpack()) { (OriginKind::Native, (1, [Junction::Parachain(id)])) => @@ -171,7 +171,7 @@ impl<RelayOrigin: Get<RuntimeOrigin>, RuntimeOrigin> ConvertOrigin<RuntimeOrigin kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!(target: "xcm::origin_conversion", "RelayChainAsNative origin: {:?}, kind: {:?}", origin, kind); + tracing::trace!(target: "xcm::origin_conversion", ?origin, ?kind, "RelayChainAsNative"); if kind == OriginKind::Native && origin.contains_parents_only(1) { Ok(RelayOrigin::get()) } else { @@ -191,10 +191,10 @@ where kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!( + tracing::trace!( target: "xcm::origin_conversion", - "SignedAccountId32AsNative origin: {:?}, kind: {:?}", - origin, kind, + ?origin, ?kind, + "SignedAccountId32AsNative", ); match (kind, origin.unpack()) { (OriginKind::Native, (0, [Junction::AccountId32 { id, network }])) @@ -218,10 +218,10 @@ where kind: OriginKind, ) -> Result<RuntimeOrigin, Location> { let origin = origin.into(); - log::trace!( + tracing::trace!( target: "xcm::origin_conversion", - "SignedAccountKey20AsNative origin: {:?}, kind: {:?}", - origin, kind, + ?origin, ?kind, + "SignedAccountKey20AsNative", ); match (kind, origin.unpack()) { (OriginKind::Native, (0, [Junction::AccountKey20 { key, network }])) diff --git a/polkadot/xcm/xcm-builder/src/process_xcm_message.rs b/polkadot/xcm/xcm-builder/src/process_xcm_message.rs index 67c05c116e9..ff8655a25fd 100644 --- a/polkadot/xcm/xcm-builder/src/process_xcm_message.rs +++ b/polkadot/xcm/xcm-builder/src/process_xcm_message.rs @@ -48,15 +48,16 @@ impl< id: &mut XcmHash, ) -> Result<bool, ProcessMessageError> { let versioned_message = VersionedXcm::<Call>::decode(&mut &message[..]).map_err(|e| { - log::trace!( + tracing::trace!( target: LOG_TARGET, - "`VersionedXcm` failed to decode: {e:?}", + ?e, + "`VersionedXcm` failed to decode", ); ProcessMessageError::Corrupt })?; let message = Xcm::<Call>::try_from(versioned_message).map_err(|_| { - log::trace!( + tracing::trace!( target: LOG_TARGET, "Failed to convert `VersionedXcm` into `xcm::prelude::Xcm`!", ); @@ -64,7 +65,7 @@ impl< ProcessMessageError::Unsupported })?; let pre = XcmExecutor::prepare(message).map_err(|_| { - log::trace!( + tracing::trace!( target: LOG_TARGET, "Failed to prepare message.", ); @@ -74,7 +75,7 @@ impl< // The worst-case weight: let required = pre.weight_of(); if !meter.can_consume(required) { - log::trace!( + tracing::trace!( target: LOG_TARGET, "Xcm required {required} more than remaining {}", meter.remaining(), @@ -86,14 +87,14 @@ impl< let (consumed, result) = match XcmExecutor::execute(origin.into(), pre, id, Weight::zero()) { Outcome::Complete { used } => { - log::trace!( + tracing::trace!( target: LOG_TARGET, "XCM message execution complete, used weight: {used}", ); (used, Ok(true)) }, Outcome::Incomplete { used, error } => { - log::trace!( + tracing::trace!( target: LOG_TARGET, "XCM message execution incomplete, used weight: {used}, error: {error:?}", ); @@ -101,7 +102,7 @@ impl< }, // In the error-case we assume the worst case and consume all possible weight. Outcome::Error { error } => { - log::trace!( + tracing::trace!( target: LOG_TARGET, "XCM message execution error: {error:?}", ); diff --git a/polkadot/xcm/xcm-builder/src/routing.rs b/polkadot/xcm/xcm-builder/src/routing.rs index 5b0d0a5f983..d9bb9ac5919 100644 --- a/polkadot/xcm/xcm-builder/src/routing.rs +++ b/polkadot/xcm/xcm-builder/src/routing.rs @@ -109,8 +109,10 @@ impl<Inner: SendXcm, TopicSource: SourceTopic> SendXcm for WithTopicSource<Inner message.0.push(SetTopic(unique_id)); unique_id }; - let (ticket, assets) = Inner::validate(destination, &mut Some(message)) - .map_err(|_| SendError::NotApplicable)?; + let (ticket, assets) = Inner::validate(destination, &mut Some(message.clone())).map_err(|e| { + tracing::debug!(target: "xcm::validate::WithTopicSource", ?destination, ?message, error = ?e, "Failed to validate"); + SendError::NotApplicable + })?; Ok(((ticket, unique_id), assets)) } @@ -208,9 +210,11 @@ impl<Inner: SendXcm> SendXcm for EnsureDecodableXcm<Inner> { if let Some(msg) = message { let versioned_xcm = VersionedXcm::<()>::from(msg.clone()); if versioned_xcm.validate_xcm_nesting().is_err() { - log::error!( + tracing::debug!( target: "xcm::validate_xcm_nesting", - "EnsureDecodableXcm validate_xcm_nesting error for \nversioned_xcm: {versioned_xcm:?}\nbased on xcm: {msg:?}" + ?versioned_xcm, + ?msg, + "EnsureDecodableXcm `validate_xcm_nesting` failed" ); return Err(SendError::Transport("EnsureDecodableXcm validate_xcm_nesting error")) } diff --git a/polkadot/xcm/xcm-builder/src/weight.rs b/polkadot/xcm/xcm-builder/src/weight.rs index 6521121f2c9..9e90053bf27 100644 --- a/polkadot/xcm/xcm-builder/src/weight.rs +++ b/polkadot/xcm/xcm-builder/src/weight.rs @@ -39,7 +39,7 @@ impl<T: Get<Weight>, C: Decode + GetDispatchInfo, M: Get<u32>> WeightBounds<C> for FixedWeightBounds<T, C, M> { fn weight(message: &mut Xcm<C>) -> Result<Weight, ()> { - log::trace!(target: "xcm::weight", "FixedWeightBounds message: {:?}", message); + tracing::trace!(target: "xcm::weight", ?message, "FixedWeightBounds"); let mut instructions_left = M::get(); Self::weight_with_limit(message, &mut instructions_left) } @@ -82,7 +82,7 @@ where Instruction<C>: xcm::latest::GetWeight<W>, { fn weight(message: &mut Xcm<C>) -> Result<Weight, ()> { - log::trace!(target: "xcm::weight", "WeightInfoBounds message: {:?}", message); + tracing::trace!(target: "xcm::weight", ?message, "WeightInfoBounds"); let mut instructions_left = M::get(); Self::weight_with_limit(message, &mut instructions_left) } @@ -154,28 +154,30 @@ impl<T: Get<(AssetId, u128, u128)>, R: TakeRevenue> WeightTrader for FixedRateOf payment: AssetsInHolding, context: &XcmContext, ) -> Result<AssetsInHolding, XcmError> { - log::trace!( + let (id, units_per_second, units_per_mb) = T::get(); + tracing::trace!( target: "xcm::weight", - "FixedRateOfFungible::buy_weight weight: {:?}, payment: {:?}, context: {:?}", - weight, payment, context, + ?id, ?weight, ?payment, ?context, + "FixedRateOfFungible::buy_weight", ); - let (id, units_per_second, units_per_mb) = T::get(); let amount = (units_per_second * (weight.ref_time() as u128) / (WEIGHT_REF_TIME_PER_SECOND as u128)) + (units_per_mb * (weight.proof_size() as u128) / (WEIGHT_PROOF_SIZE_PER_MB as u128)); if amount == 0 { return Ok(payment) } - let unused = - payment.checked_sub((id, amount).into()).map_err(|_| XcmError::TooExpensive)?; + let unused = payment.checked_sub((id, amount).into()).map_err(|error| { + tracing::error!(target: "xcm::weight", ?amount, ?error, "FixedRateOfFungible::buy_weight Failed to substract from payment"); + XcmError::TooExpensive + })?; self.0 = self.0.saturating_add(weight); self.1 = self.1.saturating_add(amount); Ok(unused) } fn refund_weight(&mut self, weight: Weight, context: &XcmContext) -> Option<Asset> { - log::trace!(target: "xcm::weight", "FixedRateOfFungible::refund_weight weight: {:?}, context: {:?}", weight, context); let (id, units_per_second, units_per_mb) = T::get(); + tracing::trace!(target: "xcm::weight", ?id, ?weight, ?context, "FixedRateOfFungible::refund_weight"); let weight = weight.min(self.0); let amount = (units_per_second * (weight.ref_time() as u128) / (WEIGHT_REF_TIME_PER_SECOND as u128)) + @@ -229,24 +231,30 @@ impl< payment: AssetsInHolding, context: &XcmContext, ) -> Result<AssetsInHolding, XcmError> { - log::trace!(target: "xcm::weight", "UsingComponents::buy_weight weight: {:?}, payment: {:?}, context: {:?}", weight, payment, context); + tracing::trace!(target: "xcm::weight", ?weight, ?payment, ?context, "UsingComponents::buy_weight"); let amount = WeightToFee::weight_to_fee(&weight); - let u128_amount: u128 = amount.try_into().map_err(|_| XcmError::Overflow)?; + let u128_amount: u128 = amount.try_into().map_err(|_| { + tracing::debug!(target: "xcm::weight", ?amount, "Weight fee could not be converted"); + XcmError::Overflow + })?; let required = Asset { id: AssetId(AssetIdValue::get()), fun: Fungible(u128_amount) }; - let unused = payment.checked_sub(required).map_err(|_| XcmError::TooExpensive)?; + let unused = payment.checked_sub(required).map_err(|error| { + tracing::debug!(target: "xcm::weight", ?error, "Failed to substract from payment"); + XcmError::TooExpensive + })?; self.0 = self.0.saturating_add(weight); self.1 = self.1.saturating_add(amount); Ok(unused) } fn refund_weight(&mut self, weight: Weight, context: &XcmContext) -> Option<Asset> { - log::trace!(target: "xcm::weight", "UsingComponents::refund_weight weight: {:?}, context: {:?}, available weight: {:?}, available amount: {:?}", weight, context, self.0, self.1); + tracing::trace!(target: "xcm::weight", ?weight, ?context, available_weight = ?self.0, available_amount = ?self.1, "UsingComponents::refund_weight"); let weight = weight.min(self.0); let amount = WeightToFee::weight_to_fee(&weight); self.0 -= weight; self.1 = self.1.saturating_sub(amount); let amount: u128 = amount.saturated_into(); - log::trace!(target: "xcm::weight", "UsingComponents::refund_weight amount to refund: {:?}", amount); + tracing::trace!(target: "xcm::weight", ?amount, "UsingComponents::refund_weight"); if amount > 0 { Some((AssetIdValue::get(), amount).into()) } else { diff --git a/prdoc/pr_7003.prdoc b/prdoc/pr_7003.prdoc new file mode 100644 index 00000000000..4c5ecf733b0 --- /dev/null +++ b/prdoc/pr_7003.prdoc @@ -0,0 +1,17 @@ +title: Added logging for xcm filters/helpers/matchers/types + +doc: + - audience: Runtime Dev + description: | + This PR adds error logs to assist in debugging xcm. + Specifically, for filters, helpers, matchers. + Additionally, it replaces the usages of `log` with `tracing`. + +crates: + - name: staging-xcm-builder + bump: minor + - name: assets-common + bump: minor + - name: bridge-hub-westend-runtime + bump: minor + -- GitLab