Tidy up XCM errors in preparation for v2. (#3988)

* Tidy up XCM errors

* Tidy up errors

* Re-order

* Fixes

* Formatting

* map undefined errors

* add functor to dictionary

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

scripts/gitlab/lingua.dic

@@ -304,3 +304,4 @@ yml
 decrement
 DM
 ParaId
+functor

xcm/pallet-xcm/src/lib.rs

@@ -1098,6 +1098,7 @@ pub mod pallet {
 			Ok(())
 		}
 	}
+
 	impl<T: Config> DropAssets for Pallet<T> {
 		fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight {
 			if assets.is_empty() {

xcm/pallet-xcm/src/tests.rs

@@ -74,7 +74,7 @@ fn report_outcome_notify_works() {
 			Parachain(PARA_ID).into(),
 			Xcm(vec![QueryResponse {
 				query_id: 0,
-				response: Response::ExecutionResult(Ok(())),
+				response: Response::ExecutionResult(None),
 				max_weight: 1_000_000,
 			}]),
 			1_000_000_000,
@@ -86,7 +86,7 @@ fn report_outcome_notify_works() {
 				Event::TestNotifier(pallet_test_notifier::Event::ResponseReceived(
 					Parachain(PARA_ID).into(),
 					0,
-					Response::ExecutionResult(Ok(())),
+					Response::ExecutionResult(None),
 				)),
 				Event::XcmPallet(crate::Event::Notified(0, 4, 2)),
 			]
@@ -128,18 +128,18 @@ fn report_outcome_works() {
 			Parachain(PARA_ID).into(),
 			Xcm(vec![QueryResponse {
 				query_id: 0,
-				response: Response::ExecutionResult(Ok(())),
+				response: Response::ExecutionResult(None),
 				max_weight: 0,
 			}]),
 			1_000_000_000,
 		);
 		assert_eq!(r, Outcome::Complete(1_000));
 		assert_eq!(
 			last_event(),
-			Event::XcmPallet(crate::Event::ResponseReady(0, Response::ExecutionResult(Ok(())),))
+			Event::XcmPallet(crate::Event::ResponseReady(0, Response::ExecutionResult(None),))
 		);
 
-		let response = Some((Response::ExecutionResult(Ok(())), 1));
+		let response = Some((Response::ExecutionResult(None), 1));
 		assert_eq!(XcmPallet::take_response(0), response);
 	});
 }

xcm/src/v2/mod.rs

@@ -152,7 +152,7 @@ pub enum Response {
 	/// Some assets.
 	Assets(MultiAssets),
 	/// The outcome of an XCM instruction.
-	ExecutionResult(result::Result<(), (u32, Error)>),
+	ExecutionResult(Option<(u32, Error)>),
 	/// An XCM version.
 	Version(super::Version),
 }

xcm/src/v2/traits.rs

@@ -24,90 +24,90 @@ use super::*;
 
 #[derive(Copy, Clone, Encode, Decode, Eq, PartialEq, Debug, TypeInfo)]
 pub enum Error {
-	Undefined,
+	// Errors that happen due to instructions being executed. These alone are defined in the
+	// XCM specification.
 	/// An arithmetic overflow happened.
+	#[codec(index = 0)]
 	Overflow,
-	/// The operation is intentionally unsupported.
+	/// The instruction is intentionally unsupported.
+	#[codec(index = 1)]
 	Unimplemented,
-	UnhandledXcmVersion,
-	/// The implementation does not handle a given XCM.
-	UnhandledXcmMessage,
-	/// The implementation does not handle an effect present in an XCM.
-	UnhandledEffect,
-	EscalationOfPrivilege,
+	/// Origin Register does not contain a value value for a reserve transfer notification.
+	#[codec(index = 2)]
 	UntrustedReserveLocation,
+	/// Origin Register does not contain a value value for a teleport notification.
+	#[codec(index = 3)]
 	UntrustedTeleportLocation,
-	DestinationBufferOverflow,
+	/// `MultiLocation` value too large to descend further.
+	#[codec(index = 4)]
 	MultiLocationFull,
+	/// `MultiLocation` value ascend more parents than known ancestors of local location.
+	#[codec(index = 5)]
 	MultiLocationNotInvertible,
-	FailedToDecode,
+	/// The Origin Register does not contain a valid value for instruction.
+	#[codec(index = 6)]
 	BadOrigin,
-	ExceedsMaxMessageSize,
-	/// An asset transaction (like withdraw or deposit) failed.
-	/// See implementers of the `TransactAsset` trait for sources.
-	/// Causes can include type conversion failures between id or balance types.
+	/// The location parameter is not a valid value for the instruction.
+	#[codec(index = 7)]
+	InvalidLocation,
+	/// The given asset is not handled.
+	#[codec(index = 8)]
+	AssetNotFound,
+	/// An asset transaction (like withdraw or deposit) failed (typically due to type conversions).
+	#[codec(index = 9)]
 	FailedToTransactAsset(#[codec(skip)] &'static str),
-	/// Execution of the XCM would potentially result in a greater weight used than the pre-specified
-	/// weight limit. The amount that is potentially required is the parameter.
-	WeightLimitReached(Weight),
-	/// An asset wildcard was passed where it was not expected (e.g. as the asset to withdraw in a
-	/// `WithdrawAsset` XCM).
-	Wildcard,
-	/// The case where an XCM message has specified a optional weight limit and the weight required for
-	/// processing is too great.
-	///
-	/// Used by:
-	/// - `Transact`
-	TooMuchWeightRequired,
-	/// The fees specified by the XCM message were not found in the holding register.
-	///
-	/// Used by:
-	/// - `BuyExecution`
-	NotHoldingFees,
-	/// The weight of an XCM message is not computable ahead of execution. This generally means at least part
-	/// of the message is invalid, which could be due to it containing overly nested structures or an invalid
-	/// nested data segment (e.g. for the call in `Transact`).
-	WeightNotComputable,
-	/// The XCM did not pass the barrier condition for execution. The barrier condition differs on different
-	/// chains and in different circumstances, but generally it means that the conditions surrounding the message
-	/// were not such that the chain considers the message worth spending time executing. Since most chains
-	/// lift the barrier to execution on appropriate payment, presentation of an NFT voucher, or based on the
-	/// message origin, it means that none of those were the case.
-	Barrier,
-	/// Indicates that it is not possible for a location to have an asset be withdrawn or transferred from its
-	/// ownership. This probably means it doesn't own (enough of) it, but may also indicate that it is under a
-	/// lock, hold, freeze or is otherwise unavailable.
+	/// An asset cannot be withdrawn, potentially due to lack of ownership, availability or rights.
+	#[codec(index = 10)]
 	NotWithdrawable,
-	/// Indicates that the consensus system cannot deposit an asset under the ownership of a particular location.
+	/// An asset cannot be deposited under the ownership of a particular location.
+	#[codec(index = 11)]
 	LocationCannotHold,
-	/// The assets given to purchase weight is are insufficient for the weight desired.
-	TooExpensive,
-	/// The given asset is not handled.
-	AssetNotFound,
-	/// The given message cannot be translated into a format that the destination can be expected to interpret.
+	/// Attempt to send a message greater than the maximum supported by the transport protocol.
+	#[codec(index = 12)]
+	ExceedsMaxMessageSize,
+	/// The given message cannot be translated into a format supported by the destination.
+	#[codec(index = 13)]
 	DestinationUnsupported,
-	/// `execute_xcm` has been called too many times recursively.
-	RecursionLimitReached,
 	/// Destination is routable, but there is some issue with the transport mechanism.
-	///
-	/// A human-readable explanation of the specific issue is provided.
+	#[codec(index = 14)]
 	Transport(#[codec(skip)] &'static str),
 	/// Destination is known to be unroutable.
+	#[codec(index = 15)]
 	Unroutable,
-	/// The weight required was not specified when it should have been.
-	UnknownWeightRequired,
-	/// An error was intentionally forced. A code is included.
-	Trap(u64),
-	/// The given claim could not be recognized/found.
+	/// Used by `ClaimAsset` when the given claim could not be recognized/found.
+	#[codec(index = 16)]
 	UnknownClaim,
-	/// The location given was invalid for some reason specific to the operation at hand.
-	InvalidLocation,
-}
+	/// Used by `Transact` when the functor cannot be decoded.
+	#[codec(index = 17)]
+	FailedToDecode,
+	/// Used by `Transact` to indicate that the given weight limit could be breached by the functor.
+	#[codec(index = 18)]
+	TooMuchWeightRequired,
+	/// Used by `BuyExecution` when the Holding Register does not contain payable fees.
+	#[codec(index = 19)]
+	NotHoldingFees,
+	/// Used by `BuyExecution` when the fees declared to purchase weight are insufficient.
+	#[codec(index = 20)]
+	TooExpensive,
+	/// Used by the `Trap` instruction to force an error intentionally. Its code is included.
+	#[codec(index = 21)]
+	Trap(u64),
 
-impl From<()> for Error {
-	fn from(_: ()) -> Self {
-		Self::Undefined
-	}
+	// Errors that happen prior to instructions being executed. These fall outside of the XCM spec.
+	/// XCM version not able to be handled.
+	UnhandledXcmVersion,
+	/// Execution of the XCM would potentially result in a greater weight used than weight limit.
+	WeightLimitReached(Weight),
+	/// The XCM did not pass the barrier condition for execution.
+	///
+	/// The barrier condition differs on different chains and in different circumstances, but
+	/// generally it means that the conditions surrounding the message were not such that the chain
+	/// considers the message worth spending time executing. Since most chains lift the barrier to
+	/// execution on appropriate payment, presentation of an NFT voucher, or based on the message
+	/// origin, it means that none of those were the case.
+	Barrier,
+	/// The weight of an XCM message is not computable ahead of execution.
+	WeightNotComputable,
 }
 
 impl From<SendError> for Error {

xcm/xcm-executor/integration-tests/src/lib.rs

@@ -117,7 +117,7 @@ fn query_response_fires() {
 
 	let mut block_builder = client.init_polkadot_block_builder();
 
-	let response = Response::ExecutionResult(Ok(()));
+	let response = Response::ExecutionResult(None);
 	let max_weight = 1_000_000;
 	let msg = Xcm(vec![QueryResponse { query_id, response, max_weight }]);
 	let msg = Box::new(VersionedXcm::from(msg));
@@ -148,13 +148,13 @@ fn query_response_fires() {
 				r.event,
 				polkadot_test_runtime::Event::Xcm(pallet_xcm::Event::ResponseReady(
 					q,
-					Response::ExecutionResult(Ok(())),
+					Response::ExecutionResult(None),
 				)) if q == query_id,
 			)));
 			assert_eq!(
 				polkadot_test_runtime::Xcm::query(query_id),
 				Some(QueryStatus::Ready {
-					response: VersionedResponse::V2(Response::ExecutionResult(Ok(()))),
+					response: VersionedResponse::V2(Response::ExecutionResult(None)),
 					at: 2u32.into()
 				}),
 			)
@@ -206,7 +206,7 @@ fn query_response_elicits_handler() {
 
 	let mut block_builder = client.init_polkadot_block_builder();
 
-	let response = Response::ExecutionResult(Ok(()));
+	let response = Response::ExecutionResult(None);
 	let max_weight = 1_000_000;
 	let msg = Xcm(vec![QueryResponse { query_id, response, max_weight }]);
 
@@ -237,7 +237,7 @@ fn query_response_elicits_handler() {
 				TestNotifier(ResponseReceived(
 					MultiLocation { parents: 0, interior: X1(Junction::AccountId32 { .. }) },
 					q,
-					Response::ExecutionResult(Ok(())),
+					Response::ExecutionResult(None),
 				)) if q == query_id,
 			)));
 		});

xcm/xcm-executor/src/lib.rs

@@ -270,7 +270,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				for asset in assets.inner() {
 					Config::AssetTransactor::beam_asset(asset, origin, &dest)?;
 				}
-				assets.reanchor(&inv_dest)?;
+				assets.reanchor(&inv_dest).map_err(|()| XcmError::MultiLocationFull)?;
 				let mut message = vec![ReserveAssetDeposited(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(dest, Xcm(message)).map_err(Into::into)
@@ -345,10 +345,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			ReportError { query_id, dest, max_response_weight: max_weight } => {
 				// Report the given result by sending a QueryResponse XCM to a previously given outcome
 				// destination if one was registered.
-				let response = Response::ExecutionResult(match self.error {
-					None => Ok(()),
-					Some(e) => Err(e),
-				});
+				let response = Response::ExecutionResult(self.error);
 				let message = QueryResponse { query_id, response, max_weight };
 				Config::XcmSender::send_xcm(dest, Xcm(vec![message]))?;
 				Ok(())
@@ -413,14 +410,16 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				Ok(())
 			},
 			SetErrorHandler(mut handler) => {
-				let handler_weight = Config::Weigher::weight(&mut handler)?;
+				let handler_weight = Config::Weigher::weight(&mut handler)
+					.map_err(|()| XcmError::WeightNotComputable)?;
 				self.total_surplus.saturating_accrue(self.error_handler_weight);
 				self.error_handler = handler;
 				self.error_handler_weight = handler_weight;
 				Ok(())
 			},
 			SetAppendix(mut appendix) => {
-				let appendix_weight = Config::Weigher::weight(&mut appendix)?;
+				let appendix_weight = Config::Weigher::weight(&mut appendix)
+					.map_err(|()| XcmError::WeightNotComputable)?;
 				self.total_surplus.saturating_accrue(self.appendix_weight);
 				self.appendix = appendix;
 				self.appendix_weight = appendix_weight;