From a3f0c4a7fa8009fb00276a12eb9b73828c475887 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 22 Jun 2021 16:32:52 +0200 Subject: [PATCH 1/7] convert local AssetNotFound errors into XcmError::AssetNotFound aims allow the tuple implementation of TransactAsset to iterate properly --- xcm/xcm-builder/src/currency_adapter.rs | 2 +- xcm/xcm-executor/src/traits/matches_fungibles.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xcm/xcm-builder/src/currency_adapter.rs b/xcm/xcm-builder/src/currency_adapter.rs index 9f4b1546afc8..744835f08959 100644 --- a/xcm/xcm-builder/src/currency_adapter.rs +++ b/xcm/xcm-builder/src/currency_adapter.rs @@ -37,7 +37,7 @@ impl From for XcmError { fn from(e: Error) -> Self { use XcmError::FailedToTransactAsset; match e { - Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), + Error::AssetNotFound => XcmError::AssetNotFound, Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), } diff --git a/xcm/xcm-executor/src/traits/matches_fungibles.rs b/xcm/xcm-executor/src/traits/matches_fungibles.rs index 75de0ae8be44..e20bb0f5c128 100644 --- a/xcm/xcm-executor/src/traits/matches_fungibles.rs +++ b/xcm/xcm-executor/src/traits/matches_fungibles.rs @@ -33,7 +33,7 @@ impl From for XcmError { fn from(e: Error) -> Self { use XcmError::FailedToTransactAsset; match e { - Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), + Error::AssetNotFound => XcmError::AssetNotFound, Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), Error::AssetIdConversionFailed => FailedToTransactAsset("AssetIdConversionFailed"), From 189519abcc8035e832582de6cb75385bbf47b713 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 23 Jun 2021 16:49:02 +0200 Subject: [PATCH 2/7] add more XcmError descriptions --- xcm/src/v0/traits.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/xcm/src/v0/traits.rs b/xcm/src/v0/traits.rs index 8664484c87ce..64d5eb152d59 100644 --- a/xcm/src/v0/traits.rs +++ b/xcm/src/v0/traits.rs @@ -24,11 +24,14 @@ use super::{MultiLocation, Xcm}; #[derive(Clone, Encode, Decode, Eq, PartialEq, Debug)] pub enum Error { Undefined, + /// An arithmetic overflow happened. Overflow, /// The operation is intentionally unsupported. Unimplemented, UnhandledXcmVersion, + /// The implementation does not handle a given XCM. UnhandledXcmMessage, + /// The implementation does not handle an effect present in an XCM. UnhandledEffect, EscalationOfPrivilege, UntrustedReserveLocation, @@ -43,10 +46,15 @@ pub enum Error { FailedToDecode, BadOrigin, ExceedsMaxMessageSize, + /// An asset transaction (like withdraw or deposit) failed. + /// See implementors of the `TransactAsset` trait for sources. + /// Causes can include type conversion failures between id or balance types. 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. From f5d5e92f8193c934fa7b5909f2b57466f0086dcf Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 23 Jun 2021 16:50:01 +0200 Subject: [PATCH 3/7] adjust the rest of the TransactAsset tuple implementation to the behavior of can_check_in --- xcm/xcm-executor/src/traits/transact_asset.rs | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index 795bb0f49bd4..71774b443ed7 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -121,20 +121,29 @@ impl TransactAsset for Tuple { } fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> XcmResult { for_tuples!( #( - match Tuple::deposit_asset(what, who) { o @ Ok(_) => return o, _ => () } + match Tuple::deposit_asset(origin, what) { + Err(XcmError::AssetNotFound) => (), + r => return r, + } )* ); - Err(XcmError::Unimplemented) + Err(XcmError::AssetNotFound) } fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> Result { for_tuples!( #( - match Tuple::withdraw_asset(what, who) { o @ Ok(_) => return o, _ => () } + match Tuple::transfer_asset(origin, what) { + Err(XcmError::AssetNotFound) => (), + r => return r, + } )* ); - Err(XcmError::Unimplemented) + Err(XcmError::AssetNotFound) } fn transfer_asset(what: &MultiAsset, from: &MultiLocation, to: &MultiLocation) -> Result { for_tuples!( #( - match Tuple::transfer_asset(what, from, to) { o @ Ok(_) => return o, _ => () } + match Tuple::transfer_asset(origin, what) { + Err(XcmError::AssetNotFound) => (), + r => return r, + } )* ); - Err(XcmError::Unimplemented) + Err(XcmError::AssetNotFound) } } From f8855d1f69c62fd0b2e6f703e801075cae8f8b31 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 23 Jun 2021 17:03:15 +0200 Subject: [PATCH 4/7] fix copy paste errors --- xcm/xcm-executor/src/traits/transact_asset.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index 71774b443ed7..2ed81f18062d 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -121,7 +121,7 @@ impl TransactAsset for Tuple { } fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> XcmResult { for_tuples!( #( - match Tuple::deposit_asset(origin, what) { + match Tuple::deposit_asset(what, who) { Err(XcmError::AssetNotFound) => (), r => return r, } @@ -130,7 +130,7 @@ impl TransactAsset for Tuple { } fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> Result { for_tuples!( #( - match Tuple::transfer_asset(origin, what) { + match Tuple::withdraw_asset(what, who) { Err(XcmError::AssetNotFound) => (), r => return r, } @@ -139,7 +139,7 @@ impl TransactAsset for Tuple { } fn transfer_asset(what: &MultiAsset, from: &MultiLocation, to: &MultiLocation) -> Result { for_tuples!( #( - match Tuple::transfer_asset(origin, what) { + match Tuple::transfer_asset(what, from, to) { Err(XcmError::AssetNotFound) => (), r => return r, } From 69e530d9de242dabebca354eaf9ca7d2f95d7313 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 21 Jul 2021 17:40:52 +0200 Subject: [PATCH 5/7] keep iterating tuple on Unimplemented error in TransactAsset --- xcm/xcm-executor/src/traits/transact_asset.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index 2b63a6a8e043..06626d44d21e 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -103,7 +103,7 @@ impl TransactAsset for Tuple { fn can_check_in(origin: &MultiLocation, what: &MultiAsset) -> XcmResult { for_tuples!( #( match Tuple::can_check_in(origin, what) { - Err(XcmError::AssetNotFound) => (), + Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (), r => return r, } )* ); @@ -131,7 +131,7 @@ impl TransactAsset for Tuple { fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> XcmResult { for_tuples!( #( match Tuple::deposit_asset(what, who) { - Err(XcmError::AssetNotFound) => (), + Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (), r => return r, } )* ); @@ -147,7 +147,7 @@ impl TransactAsset for Tuple { fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> Result { for_tuples!( #( match Tuple::withdraw_asset(what, who) { - Err(XcmError::AssetNotFound) => (), + Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (), r => return r, } )* ); @@ -163,7 +163,7 @@ impl TransactAsset for Tuple { fn transfer_asset(what: &MultiAsset, from: &MultiLocation, to: &MultiLocation) -> Result { for_tuples!( #( match Tuple::transfer_asset(what, from, to) { - Err(XcmError::AssetNotFound) => (), + Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (), r => return r, } )* ); From 1bb97cdff108f71f9fb4eee252aa7108c0d909af Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 21 Jul 2021 18:22:30 +0200 Subject: [PATCH 6/7] add tests for tuple implementation of TransactAsset --- xcm/xcm-executor/src/traits/transact_asset.rs | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index 06626d44d21e..a967f1f6909a 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -177,3 +177,96 @@ impl TransactAsset for Tuple { Err(XcmError::AssetNotFound) } } + +#[cfg(test)] +mod tests { + use super::*; + + pub struct UnimplementedTransactor; + impl TransactAsset for UnimplementedTransactor {} + + pub struct NotFoundTransactor; + impl TransactAsset for NotFoundTransactor { + fn can_check_in(_origin: &MultiLocation, _what: &MultiAsset) -> XcmResult { + Err(XcmError::AssetNotFound) + } + + fn deposit_asset(_what: &MultiAsset, _who: &MultiLocation) -> XcmResult { + Err(XcmError::AssetNotFound) + } + + fn withdraw_asset(_what: &MultiAsset, _who: &MultiLocation) -> Result { + Err(XcmError::AssetNotFound) + } + + fn transfer_asset(_what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation) -> Result { + Err(XcmError::AssetNotFound) + } + } + + pub struct OverflowTransactor; + impl TransactAsset for OverflowTransactor { + fn can_check_in(_origin: &MultiLocation, _what: &MultiAsset) -> XcmResult { + Err(XcmError::Overflow) + } + + fn deposit_asset(_what: &MultiAsset, _who: &MultiLocation) -> XcmResult { + Err(XcmError::Overflow) + } + + fn withdraw_asset(_what: &MultiAsset, _who: &MultiLocation) -> Result { + Err(XcmError::Overflow) + } + + fn transfer_asset(_what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation) -> Result { + Err(XcmError::Overflow) + } + } + + pub struct SuccessfulTransactor; + impl TransactAsset for SuccessfulTransactor { + fn can_check_in(_origin: &MultiLocation, _what: &MultiAsset) -> XcmResult { + Ok(()) + } + + fn deposit_asset(_what: &MultiAsset, _who: &MultiLocation) -> XcmResult { + Ok(()) + } + + fn withdraw_asset(_what: &MultiAsset, _who: &MultiLocation) -> Result { + Ok(Assets::default()) + } + + fn transfer_asset(_what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation) -> Result { + Ok(Assets::default()) + } + } + + #[test] + fn defaults_to_asset_not_found() { + type MultiTransactor = (UnimplementedTransactor, NotFoundTransactor, UnimplementedTransactor); + + assert_eq!(MultiTransactor::deposit_asset(&MultiAsset::All, &MultiLocation::Null), Err(XcmError::AssetNotFound)); + } + + #[test] + fn unimplemented_and_not_found_continue_iteration() { + type MultiTransactor = (UnimplementedTransactor, NotFoundTransactor, SuccessfulTransactor); + + assert_eq!(MultiTransactor::deposit_asset(&MultiAsset::All, &MultiLocation::Null), Ok(())); + } + + #[test] + fn unexpected_error_stops_iteration() { + type MultiTransactor = (OverflowTransactor, SuccessfulTransactor); + + assert_eq!(MultiTransactor::deposit_asset(&MultiAsset::All, &MultiLocation::Null), Err(XcmError::Overflow)); + } + + #[test] + fn success_stops_iteration() { + type MultiTransactor = (SuccessfulTransactor, OverflowTransactor); + + assert_eq!(MultiTransactor::deposit_asset(&MultiAsset::All, &MultiLocation::Null), Ok(())); + } +} From 7520381668cf142e48b2342122b39295f0a125ff Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 21 Jul 2021 13:44:23 -0400 Subject: [PATCH 7/7] Update xcm/src/v0/traits.rs Co-authored-by: Andronik Ordian --- xcm/src/v0/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/src/v0/traits.rs b/xcm/src/v0/traits.rs index fa17979b410c..9a01f227e766 100644 --- a/xcm/src/v0/traits.rs +++ b/xcm/src/v0/traits.rs @@ -47,7 +47,7 @@ pub enum Error { BadOrigin, ExceedsMaxMessageSize, /// An asset transaction (like withdraw or deposit) failed. - /// See implementors of the `TransactAsset` trait for sources. + /// See implementers of the `TransactAsset` trait for sources. /// Causes can include type conversion failures between id or balance types. FailedToTransactAsset(#[codec(skip)] &'static str), /// Execution of the XCM would potentially result in a greater weight used than the pre-specified