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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
+88 −25
Open

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 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
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1833,7 +1833,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: {:?}", e);
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 @@ -1898,7 +1901,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 @@ -1941,7 +1947,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 @@ -1979,7 +1988,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 @@ -2033,13 +2045,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 @@ -2081,7 +2102,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 @@ -2095,7 +2119,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 @@ -2140,7 +2167,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 @@ -2155,7 +2185,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 @@ -2428,7 +2461,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 @@ -2534,7 +2570,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 @@ -2550,10 +2589,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 @@ -2562,7 +2607,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
36 changes: 27 additions & 9 deletions polkadot/xcm/xcm-executor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,13 @@ impl<Config: config::Config> XcmExecutor<Config> {
}
fee
} else {
self.holding.try_take(fee.into()).map_err(|_| XcmError::NotHoldingFees)?.into()
self.holding
.try_take(fee.clone().into())
.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
})?
.into()
};
Config::FeeManager::handle_fee(paid, Some(&self.context), reason);
Ok(())
Expand All @@ -477,8 +483,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 @@ -805,7 +813,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 @@ -947,7 +958,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
let old_holding = self.holding.clone();
// 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 @@ -1010,7 +1024,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 @@ -1132,9 +1149,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