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

pallet-xcm: added useful error logs (#2408) #4982

Merged
merged 46 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
991c9da
pallet-xcm: added useful error logs (#2408)
Jul 9, 2024
84e540a
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Jul 9, 2024
3cb6ad3
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Jul 9, 2024
eae27b6
xcm-executor: added logs
Jul 11, 2024
e9ecfba
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Jul 11, 2024
7a4eef7
Apply suggestions from code review
acatangiu Oct 2, 2024
6bbfffa
Apply suggestions from code review
acatangiu Oct 2, 2024
6aa4816
Merge branch 'master' into ayevbeosa-log-xcm-errors
Oct 3, 2024
6a3e140
added logs
Oct 3, 2024
03e0b88
Update polkadot/xcm/pallet-xcm/src/lib.rs
bkontur Oct 3, 2024
92e5cbd
Update polkadot/xcm/pallet-xcm/src/lib.rs
bkontur Oct 4, 2024
9104421
Update polkadot/xcm/pallet-xcm/src/lib.rs
bkontur Oct 4, 2024
5011a47
Update polkadot/xcm/pallet-xcm/src/lib.rs
ayevbeosa Oct 4, 2024
208f62d
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Oct 4, 2024
601e110
Update polkadot/xcm/pallet-xcm/src/lib.rs
bkontur Oct 4, 2024
081c929
Update polkadot/xcm/pallet-xcm/src/lib.rs
bkontur Oct 4, 2024
cbc97b5
Update polkadot/xcm/xcm-executor/src/lib.rs
bkontur Oct 4, 2024
9d3ec46
Update polkadot/xcm/pallet-xcm/src/lib.rs
bkontur Oct 4, 2024
40ec293
Update polkadot/xcm/xcm-executor/src/lib.rs
bkontur Oct 4, 2024
76ecb82
Apply suggestions from code review
bkontur Oct 4, 2024
bf76568
Update polkadot/xcm/xcm-executor/src/lib.rs
bkontur Oct 4, 2024
f520ea3
convert `log::error` to `tracing::error`, extend logs with more info
Oct 5, 2024
687adc9
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Oct 5, 2024
0e2ac5a
extend logs with more info
Oct 5, 2024
9b9f309
Update polkadot/xcm/xcm-executor/src/lib.rs
ayevbeosa Oct 7, 2024
58f374b
change tracing:error format
Oct 7, 2024
e8abe1f
Merge branch 'master' into ayevbeosa-log-xcm-errors
bkontur Oct 8, 2024
cfd1ae4
used `tracing` instead of `log`
Oct 8, 2024
100c576
Merge branch 'master' into ayevbeosa-log-xcm-errors
bkontur Oct 9, 2024
4f1cf76
fix compile errors
Oct 10, 2024
f0e12ca
replaced log with tracing
Oct 10, 2024
9af6495
prefer to use tracing instead of log
Oct 10, 2024
841dcb4
fix compile issues
Oct 11, 2024
25faf07
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Oct 11, 2024
44f184b
fix compile errors
Oct 11, 2024
4f9d0dc
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Oct 11, 2024
fc0bb42
use tracing instead log
Oct 12, 2024
003cde7
Merge branch 'master' into ayevbeosa-log-xcm-errors
acatangiu Oct 14, 2024
753c51f
fix toml formattiing issues
Oct 14, 2024
e4d5945
Merge branch 'ayevbeosa-log-xcm-errors' of https://github.com/ayevbeo…
Oct 14, 2024
47edc33
Merge branch 'master' into ayevbeosa-log-xcm-errors
ayevbeosa Oct 14, 2024
ef5bc73
Add prdoc
Oct 14, 2024
c467fee
Update prdoc/pr_4982.prdoc
bkontur Oct 14, 2024
8b96b7f
Update prdoc/pr_4982.prdoc
bkontur Oct 14, 2024
25b4a75
Merge branch 'master' into ayevbeosa-log-xcm-errors
bkontur Oct 15, 2024
89b01b9
Merge branch 'master' into ayevbeosa-log-xcm-errors
bkontur Oct 15, 2024
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
80 changes: 64 additions & 16 deletions polkadot/xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1836,7 +1836,10 @@ impl<T: Config> Pallet<T> {
// no custom fees instructions, they are batched together with `assets` transfer;
// BuyExecution happens after receiving all `assets`
let reanchored_fees =
fees.reanchored(&dest, &context).map_err(|_| Error::<T>::CannotReanchor)?;
fees.reanchored(&dest, &context).map_err(|e| {
log::error!(target: "xcm::pallet_xcm::add_fees_to_xcm", "Failed to re-anchor fees: {:?} for dest: {:?} and context {:?}", e, dest, context);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
Error::<T>::CannotReanchor
})?;
// buy execution using `fees` batched together with above `reanchored_assets`
remote.inner_mut().push(BuyExecution { fees: reanchored_fees, weight_limit });
},
Expand Down Expand Up @@ -1901,7 +1904,10 @@ impl<T: Config> Pallet<T> {
let mut reanchored_assets = assets.clone();
reanchored_assets
.reanchor(&dest, &context)
.map_err(|_| Error::<T>::CannotReanchor)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::local_reserve_transfer_programs", "Failed to re-anchor assets: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
Error::<T>::CannotReanchor
})?;

// XCM instructions to be executed on local chain
let mut local_execute_xcm = Xcm(vec![
Expand Down Expand Up @@ -1948,7 +1954,10 @@ impl<T: Config> Pallet<T> {
let reanchored_fees = fees
.clone()
.reanchored(&dest, &context)
.map_err(|_| Error::<T>::CannotReanchor)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::destination_reserve_fees_instructions", "Failed to re-anchor fees: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
Error::<T>::CannotReanchor
})?;
let fees: Assets = fees.into();

let local_execute_xcm = Xcm(vec![
Expand Down Expand Up @@ -1992,7 +2001,10 @@ impl<T: Config> Pallet<T> {
let mut reanchored_assets = assets.clone();
reanchored_assets
.reanchor(&dest, &context)
.map_err(|_| Error::<T>::CannotReanchor)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::destination_reserve_transfer_programs", "Failed to re-anchor assets: {:?}", e);
Error::<T>::CannotReanchor
})?;

// XCM instructions to be executed on local chain
let mut local_execute_xcm = Xcm(vec![
Expand Down Expand Up @@ -2046,13 +2058,22 @@ impl<T: Config> Pallet<T> {
// identifies fee item as seen by `reserve` - to be used at reserve chain
let reserve_fees = fees_half_1
.reanchored(&reserve, &context)
.map_err(|_| Error::<T>::CannotReanchor)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::remote_reserve_transfer_program", "Failed to re-anchor reserve_fees: {:?}", e);
Error::<T>::CannotReanchor
})?;
// identifies fee item as seen by `dest` - to be used at destination chain
let dest_fees = fees_half_2
.reanchored(&dest, &context)
.map_err(|_| Error::<T>::CannotReanchor)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::remote_reserve_transfer_program", "Failed to re-anchor dest_fees: {:?}", e);
Error::<T>::CannotReanchor
})?;
// identifies `dest` as seen by `reserve`
let dest = dest.reanchored(&reserve, &context).map_err(|_| Error::<T>::CannotReanchor)?;
let dest = dest.reanchored(&reserve, &context).map_err(|e| {
log::error!(target: "xcm::pallet_xcm::remote_reserve_transfer_program", "Failed to re-anchor dest: {:?}", e);
Error::<T>::CannotReanchor
})?;
// xcm to be executed at dest
let mut xcm_on_dest =
Xcm(vec![BuyExecution { fees: dest_fees, weight_limit: weight_limit.clone() }]);
Expand Down Expand Up @@ -2098,7 +2119,10 @@ impl<T: Config> Pallet<T> {
let reanchored_fees = fees
.clone()
.reanchored(&dest, &context)
.map_err(|_| Error::<T>::CannotReanchor)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::teleport_fees_instructions", "Failed to re-anchor fees: {:?}", e);
Error::<T>::CannotReanchor
})?;

// XcmContext irrelevant in teleports checks
let dummy_context =
Expand All @@ -2112,7 +2136,10 @@ impl<T: Config> Pallet<T> {
&fees,
&dummy_context,
)
.map_err(|_| Error::<T>::CannotCheckOutTeleport)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::teleport_fees_instructions", "Failed to teleport fees: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
Error::<T>::CannotCheckOutTeleport
})?;
// safe to do this here, we're in a transactional call that will be reverted on any
// errors down the line
<T::XcmExecutor as XcmAssetTransfers>::AssetTransactor::check_out(
Expand Down Expand Up @@ -2163,7 +2190,10 @@ impl<T: Config> Pallet<T> {
let mut reanchored_assets = assets.clone();
reanchored_assets
.reanchor(&dest, &context)
.map_err(|_| Error::<T>::CannotReanchor)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::teleport_assets_program", "Failed to re-anchor asset: {:?}", e);
Error::<T>::CannotReanchor
})?;

// XcmContext irrelevant in teleports checks
let dummy_context =
Expand All @@ -2178,7 +2208,10 @@ impl<T: Config> Pallet<T> {
asset,
&dummy_context,
)
.map_err(|_| Error::<T>::CannotCheckOutTeleport)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::teleport_assets_program", "Failed to teleport asset: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
Error::<T>::CannotCheckOutTeleport
})?;
}
for asset in assets.inner() {
// safe to do this here, we're in a transactional call that will be reverted on any
Expand Down Expand Up @@ -2451,7 +2484,10 @@ impl<T: Config> Pallet<T> {
log::debug!(target: "xcm::send_xcm", "dest: {:?}, message: {:?}", &dest, &message);
let (ticket, price) = validate_send::<T::XcmRouter>(dest, message)?;
if let Some(fee_payer) = maybe_fee_payer {
Self::charge_fees(fee_payer, price).map_err(|_| SendError::Fees)?;
Self::charge_fees(fee_payer, price).map_err(|e| {
log::error!(target: "xcm::pallet_xcm::send_xcm", "Charging fees failed: {:?}", e);
SendError::Fees
})?;
}
T::XcmRouter::deliver(ticket)
}
Expand Down Expand Up @@ -2561,7 +2597,10 @@ impl<T: Config> Pallet<T> {

pub fn query_xcm_weight(message: VersionedXcm<()>) -> Result<Weight, XcmPaymentApiError> {
let message = Xcm::<()>::try_from(message)
.map_err(|_| XcmPaymentApiError::VersionedConversionFailed)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::query_xcm_weight", "Failed to convert versioned `message`: {:?}", e);
XcmPaymentApiError::VersionedConversionFailed
})?;

T::Weigher::weight(&mut message.into()).map_err(|()| {
log::error!(target: "xcm::pallet_xcm::query_xcm_weight", "Error when querying XCM weight");
Expand All @@ -2577,10 +2616,16 @@ impl<T: Config> Pallet<T> {

let destination = destination
.try_into()
.map_err(|_| XcmPaymentApiError::VersionedConversionFailed)?;
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::query_delivery_fees", "Failed to convert versioned `destination`: {:?}", e);
XcmPaymentApiError::VersionedConversionFailed
})?;

let message =
message.try_into().map_err(|_| XcmPaymentApiError::VersionedConversionFailed)?;
message.try_into().map_err(|e| {
log::error!(target: "xcm::pallet_xcm::query_delivery_fees", "Failed to convert versioned `message`: {:?}", e);
XcmPaymentApiError::VersionedConversionFailed
})?;

let (_, fees) = validate_send::<T::XcmRouter>(destination, message).map_err(|error| {
log::error!(target: "xcm::pallet_xcm::query_delivery_fees", "Error when querying delivery fees: {:?}", error);
Expand All @@ -2589,7 +2634,10 @@ impl<T: Config> Pallet<T> {

VersionedAssets::from(fees)
.into_version(result_version)
.map_err(|_| XcmPaymentApiError::VersionedConversionFailed)
.map_err(|e| {
log::error!(target: "xcm::pallet_xcm::query_delivery_fees", "Failed to convert fees into version: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
XcmPaymentApiError::VersionedConversionFailed
})
}

/// Create a new expectation of a query response with the querier being here.
Expand Down
33 changes: 24 additions & 9 deletions polkadot/xcm/xcm-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
let assets_taken_from_holding_to_pay_delivery_fees = self
.holding
.try_take(asset_to_pay_for_fees.clone().into())
.map_err(|_| XcmError::NotHoldingFees)?;
.map_err(|e| {
log::error!(target: "xcm::xcm_executor::take_fee", "Failed to take fees {:?} from holding. {:?}", fee, e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
XcmError::NotHoldingFees
})?;
tracing::trace!(target: "xcm::fees", ?assets_taken_from_holding_to_pay_delivery_fees);
let mut iter = assets_taken_from_holding_to_pay_delivery_fees.fungible_assets_iter();
let asset = iter.next().ok_or(XcmError::NotHoldingFees)?;
Expand Down Expand Up @@ -587,8 +590,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
Ok(match local_querier {
None => None,
Some(q) => Some(
q.reanchored(&destination, &Config::UniversalLocation::get())
.map_err(|_| XcmError::ReanchorFailed)?,
q.reanchored(&destination, &Config::UniversalLocation::get()).map_err(|e| {
log::error!(target: "xcm::xcm_executor::to_querier", "Failed to re-anchor local_querier: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
XcmError::ReanchorFailed
})?,
),
})
}
Expand Down Expand Up @@ -923,7 +928,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
.as_mut()
.ok_or(XcmError::BadOrigin)?
.append_with(who)
.map_err(|_| XcmError::LocationFull),
.map_err(|e| {
log::error!(target: "xcm::process_instruction::descend_origin", "Failed to append to Location: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
XcmError::LocationFull
}),
ClearOrigin => {
self.context.origin = None;
Ok(())
Expand Down Expand Up @@ -1087,7 +1095,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
tracing::trace!(target: "xcm::executor::BuyExecution", asset_used_for_fees = ?self.asset_used_for_fees);
// pay for `weight` using up to `fees` of the holding register.
let max_fee =
self.holding.try_take(fees.into()).map_err(|_| XcmError::NotHoldingFees)?;
self.holding.try_take(fees.clone().into()).map_err(|e| {
log::error!(target: "xcm::process_instruction::buy_execution", "Failed to take fees {:?} from holding. {:?}", fees, e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
XcmError::NotHoldingFees
})?;
let result = || -> Result<(), XcmError> {
let unspent = self.trader.buy_weight(weight, max_fee, &self.context)?;
self.holding.subsume_assets(unspent);
Expand Down Expand Up @@ -1150,7 +1161,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
Ok(())
},
ExpectAsset(assets) =>
self.holding.ensure_contains(&assets).map_err(|_| XcmError::ExpectationFalse),
self.holding.ensure_contains(&assets).map_err(|e| {
log::error!(target: "xcm::process_instruction::expect_asset", "Holding contains no asset: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
XcmError::ExpectationFalse
}),
ExpectOrigin(origin) => {
ensure!(self.context.origin == origin, XcmError::ExpectationFalse);
Ok(())
Expand Down Expand Up @@ -1272,9 +1286,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
let (remote_asset, context) = Self::try_reanchor(asset.clone(), &unlocker)?;
let lock_ticket =
Config::AssetLocker::prepare_lock(unlocker.clone(), asset, origin.clone())?;
let owner = origin
.reanchored(&unlocker, &context)
.map_err(|_| XcmError::ReanchorFailed)?;
let owner = origin.reanchored(&unlocker, &context).map_err(|e| {
log::error!(target: "xcm::xcm_executor::process_instruction", "Failed to re-anchor: {:?}", e);
bkontur marked this conversation as resolved.
Show resolved Hide resolved
XcmError::ReanchorFailed
})?;
let msg = Xcm::<()>(vec![NoteUnlockable { asset: remote_asset, owner }]);
let (ticket, price) = validate_send::<Config::XcmSender>(unlocker, msg)?;
self.take_fee(price, FeeReason::LockAsset)?;
Expand Down
Loading