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

polkadot/xcm/pallet-xcm/src/lib.rs

@@ -330,7 +330,7 @@ pub mod pallet {
 			Self::deposit_event(Event::Attempted { outcome: outcome.clone() });
 			let weight_used = outcome.weight_used();
 			outcome.ensure_complete().map_err(|error| {
-				log::error!(target: "xcm::pallet_xcm::execute", "XCM execution failed with error {:?}", error);
+				log::error!(target: "xcm::pallet_xcm::execute", "XCM execution failed with error: {error:?}");
 				Error::<T>::LocalExecutionIncomplete.with_weight(
 					weight_used.saturating_add(
 						<Self::WeightInfo as ExecuteControllerWeightInfo>::execute(),
@@ -1330,7 +1330,7 @@ pub mod pallet {
 				weight,
 			);
 			outcome.ensure_complete().map_err(|error| {
-				log::error!(target: "xcm::pallet_xcm::claim_assets", "XCM execution failed with error: {:?}", error);
+				log::error!(target: "xcm::pallet_xcm::claim_assets", "XCM execution failed with error: {error:?}");
 				Error::<T>::LocalExecutionIncomplete
 			})?;
 			Ok(())
@@ -1798,7 +1798,7 @@ impl<T: Config> Pallet<T> {
 		outcome.ensure_complete().map_err(|error| {
 			log::error!(
 				target: "xcm::pallet_xcm::execute_xcm_transfer",
-				"XCM execution failed with error {:?}", error
+				"XCM execution failed with error: {error:?} for outcome: {outcome:?}",
 			);
 			Error::<T>::LocalExecutionIncomplete
 		})?;
@@ -1810,7 +1810,7 @@ impl<T: Config> Pallet<T> {
 				Self::charge_fees(origin.clone(), price).map_err(|error| {
 					log::error!(
 						target: "xcm::pallet_xcm::execute_xcm_transfer",
-						"Unable to charge fee with error {:?}", error
+						"Unable to charge fee with error {error:?} for price: {price:?} on origin {origin:?}",
 					);
 					Error::<T>::FeesNotMet
 				})?;
@@ -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: {e:?} for dest: {dest:?} and context {context:?}");
+						Error::<T>::CannotReanchor
+					})?;
 				// buy execution using `fees` batched together with above `reanchored_assets`
 				remote.inner_mut().push(BuyExecution { fees: reanchored_fees, weight_limit });
 			},
@@ -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:?} for dest: {dest:?} and context {context:?}");
+				Error::<T>::CannotReanchor
+			})?;
 
 		// XCM instructions to be executed on local chain
 		let mut local_execute_xcm = Xcm(vec![
@@ -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:?} for dest: {dest:?} and context {context:?}");
+				Error::<T>::CannotReanchor
+			})?;
 		let fees: Assets = fees.into();
 
 		let local_execute_xcm = Xcm(vec![
@@ -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:?} for dest: {dest:?} and context {context:?}");
+				Error::<T>::CannotReanchor
+			})?;
 
 		// XCM instructions to be executed on local chain
 		let mut local_execute_xcm = Xcm(vec![
@@ -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:?} for reserve: {reserve:?} and context {context:?}");
+				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:?} for dest: {dest:?} and context {context:?}");
+				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:?} for reserve: {reserve:?} and context {context:?}");
+			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() }]);
@@ -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:?} for dest: {dest:?} and context {context:?}");
+				Error::<T>::CannotReanchor
+			})?;
 
 		// XcmContext irrelevant in teleports checks
 		let dummy_context =
@@ -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 can_check_out fees: {fees:?} for dest: {dest:?} with error: {e:?}");
+			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(
@@ -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:?} for dest: {dest:?} and context {context:?}");
+				Error::<T>::CannotReanchor
+			})?;
 
 		// XcmContext irrelevant in teleports checks
 		let dummy_context =
@@ -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 can_check_out asset: {asset:?} for dest: {dest:?} with error: {e:?}");
+				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
@@ -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 with error: {e:?} for price: {price:?} and fee_payer {fee_payer:?}");
+				SendError::Fees
+			})?;
 		}
 		T::XcmRouter::deliver(ticket)
 	}
@@ -2514,16 +2550,14 @@ impl<T: Config> Pallet<T> {
 		let origin_location: Location = origin_location.try_into().map_err(|error| {
 			log::error!(
 				target: "xcm::DryRunApi::dry_run_xcm",
-				"Location version conversion failed with error: {:?}",
-				error,
+				"Location version conversion failed with error: {error:?}"
 			);
 			XcmDryRunApiError::VersionedConversionFailed
 		})?;
 		let xcm: Xcm<RuntimeCall> = xcm.try_into().map_err(|error| {
 			log::error!(
 				target: "xcm::DryRunApi::dry_run_xcm",
-				"Xcm version conversion failed with error {:?}",
-				error,
+				"Xcm version conversion failed with error {error:?}"
 			);
 			XcmDryRunApiError::VersionedConversionFailed
 		})?;
@@ -2561,7 +2595,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: {message:?} with error: {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");
@@ -2577,19 +2614,28 @@ 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: {destination:?} with error: {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: {message:?} with error {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);
+			log::error!(target: "xcm::pallet_xcm::query_delivery_fees", "Failed to validate send to destination: {destination:?} with message: {message:?} with error: {error:?}");
 			XcmPaymentApiError::Unroutable
 		})?;
 
 		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: {result_version:?} with error: {e:?}");
+				XcmPaymentApiError::VersionedConversionFailed
+			})
 	}
 
 	/// Create a new expectation of a query response with the querier being here.

polkadot/xcm/xcm-executor/src/lib.rs

@@ -508,7 +508,10 @@
 			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| {
+					tracing::error!(target: "xcm::fees", "Failed to take asset_to_pay_for_fees: {asset_to_pay_for_fees:?} (based on fee: {fee:?}) from holding with error: {e:?}");
+					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)?;
@@ -525,8 +528,7 @@
 			.map_err(|given_assets| {
 				tracing::error!(
 					target: "xcm::fees",
-					?given_assets,
-					"Swap was deemed necessary but couldn't be done",
+					"Swap was deemed necessary but couldn't be done: {given_assets:?} for withdrawn_fee_asset: {withdrawn_fee_asset:?} and asset_needed_for_fees: {asset_needed_for_fees:?}",
 				);
 				XcmError::FeesNotMet
 			})?
@@ -587,8 +589,10 @@
 		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| {
+					tracing::error!(target: "xcm::xcm_executor::to_querier", "Failed to re-anchor local_querier: {e:?} for destination: {destination:?} and context: {:?}", Config::UniversalLocation::get());
+					XcmError::ReanchorFailed
+				})?,
 			),
 		})
 	}
@@ -617,7 +621,7 @@
 		let reanchor_context = Config::UniversalLocation::get();
 		let reanchored =
 			reanchorable.reanchored(&destination, &reanchor_context).map_err(|error| {
-				tracing::error!(target: "xcm::reanchor", ?error, "Failed reanchoring with error");
+				tracing::error!(target: "xcm::reanchor", "Failed reanchoring with error: {error:?} for destination: {destination:?} and context: {context:?}");
 				XcmError::ReanchorFailed
 			})?;
 		Ok((reanchored, reanchor_context))
@@ -923,7 +927,10 @@
 				.as_mut()
 				.ok_or(XcmError::BadOrigin)?
 				.append_with(who)
-				.map_err(|_| XcmError::LocationFull),
+				.map_err(|e| {
+					tracing::error!(target: "xcm::process_instruction::descend_origin", "Failed to append junctions: {e:?} with who: {who:?}");
+					XcmError::LocationFull
+				}),
 			ClearOrigin => {
 				self.context.origin = None;
 				Ok(())
@@ -1087,7 +1094,10 @@
 				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| {
+						tracing::error!(target: "xcm::process_instruction::buy_execution", "Failed to take fees {fees:?} from holding with error: {e:?}");
+						XcmError::NotHoldingFees
+					})?;
 				let result = || -> Result<(), XcmError> {
 					let unspent = self.trader.buy_weight(weight, max_fee, &self.context)?;
 					self.holding.subsume_assets(unspent);
@@ -1150,7 +1160,10 @@
 				Ok(())
 			},
 			ExpectAsset(assets) =>
-				self.holding.ensure_contains(&assets).map_err(|_| XcmError::ExpectationFalse),
+				self.holding.ensure_contains(&assets).map_err(|e| {
+					tracing::error!(target: "xcm::process_instruction::expect_asset", "Failed ensure_contains for assets: {assets:?} with error: {e:?}");
+					XcmError::ExpectationFalse
+				}),
 			ExpectOrigin(origin) => {
 				ensure!(self.context.origin == origin, XcmError::ExpectationFalse);
 				Ok(())
@@ -1272,9 +1285,10 @@
 					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| {
+						tracing::error!(target: "xcm::xcm_executor::process_instruction", "Failed to re-anchor origin: {origin:?} for unlocker: {unlocker:?} and context: {context:?}");
+						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)?;