Unverified Commit 34ad3ab3 authored by Alexander Popiak's avatar Alexander Popiak Committed by GitHub
Browse files

Fix TransactAsset Implementation (#3345)



* convert local AssetNotFound errors into XcmError::AssetNotFound

aims allow the tuple implementation of TransactAsset to iterate properly

* add more XcmError descriptions

* adjust the rest of the TransactAsset tuple implementation to the behavior of can_check_in

* fix copy paste errors

* keep iterating tuple on Unimplemented error in TransactAsset

* add tests for tuple implementation of TransactAsset

* Update xcm/src/v0/traits.rs
Co-authored-by: Andronik Ordian's avatarAndronik Ordian <write@reusable.software>
Co-authored-by: Shawn Tabrizi's avatarShawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Andronik Ordian's avatarAndronik Ordian <write@reusable.software>
parent 5dac5323
Pipeline #148974 passed with stages
in 41 minutes and 16 seconds
...@@ -24,11 +24,14 @@ use super::{MultiLocation, Xcm}; ...@@ -24,11 +24,14 @@ use super::{MultiLocation, Xcm};
#[derive(Clone, Encode, Decode, Eq, PartialEq, Debug)] #[derive(Clone, Encode, Decode, Eq, PartialEq, Debug)]
pub enum Error { pub enum Error {
Undefined, Undefined,
/// An arithmetic overflow happened.
Overflow, Overflow,
/// The operation is intentionally unsupported. /// The operation is intentionally unsupported.
Unimplemented, Unimplemented,
UnhandledXcmVersion, UnhandledXcmVersion,
/// The implementation does not handle a given XCM.
UnhandledXcmMessage, UnhandledXcmMessage,
/// The implementation does not handle an effect present in an XCM.
UnhandledEffect, UnhandledEffect,
EscalationOfPrivilege, EscalationOfPrivilege,
UntrustedReserveLocation, UntrustedReserveLocation,
...@@ -43,10 +46,15 @@ pub enum Error { ...@@ -43,10 +46,15 @@ pub enum Error {
FailedToDecode, FailedToDecode,
BadOrigin, BadOrigin,
ExceedsMaxMessageSize, 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.
FailedToTransactAsset(#[codec(skip)] &'static str), FailedToTransactAsset(#[codec(skip)] &'static str),
/// Execution of the XCM would potentially result in a greater weight used than the pre-specified /// 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. /// weight limit. The amount that is potentially required is the parameter.
WeightLimitReached(Weight), WeightLimitReached(Weight),
/// An asset wildcard was passed where it was not expected (e.g. as the asset to withdraw in a
/// `WithdrawAsset` XCM).
Wildcard, Wildcard,
/// The case where an XCM message has specified a optional weight limit and the weight required for /// The case where an XCM message has specified a optional weight limit and the weight required for
/// processing is too great. /// processing is too great.
......
...@@ -37,7 +37,7 @@ impl From<Error> for XcmError { ...@@ -37,7 +37,7 @@ impl From<Error> for XcmError {
fn from(e: Error) -> Self { fn from(e: Error) -> Self {
use XcmError::FailedToTransactAsset; use XcmError::FailedToTransactAsset;
match e { match e {
Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), Error::AssetNotFound => XcmError::AssetNotFound,
Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"),
Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"),
} }
......
...@@ -33,7 +33,7 @@ impl From<Error> for XcmError { ...@@ -33,7 +33,7 @@ impl From<Error> for XcmError {
fn from(e: Error) -> Self { fn from(e: Error) -> Self {
use XcmError::FailedToTransactAsset; use XcmError::FailedToTransactAsset;
match e { match e {
Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), Error::AssetNotFound => XcmError::AssetNotFound,
Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"),
Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"),
Error::AssetIdConversionFailed => FailedToTransactAsset("AssetIdConversionFailed"), Error::AssetIdConversionFailed => FailedToTransactAsset("AssetIdConversionFailed"),
......
...@@ -103,7 +103,7 @@ impl TransactAsset for Tuple { ...@@ -103,7 +103,7 @@ impl TransactAsset for Tuple {
fn can_check_in(origin: &MultiLocation, what: &MultiAsset) -> XcmResult { fn can_check_in(origin: &MultiLocation, what: &MultiAsset) -> XcmResult {
for_tuples!( #( for_tuples!( #(
match Tuple::can_check_in(origin, what) { match Tuple::can_check_in(origin, what) {
Err(XcmError::AssetNotFound) => (), Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (),
r => return r, r => return r,
} }
)* ); )* );
...@@ -130,7 +130,10 @@ impl TransactAsset for Tuple { ...@@ -130,7 +130,10 @@ impl TransactAsset for Tuple {
fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> XcmResult { fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> XcmResult {
for_tuples!( #( for_tuples!( #(
match Tuple::deposit_asset(what, who) { o @ Ok(_) => return o, _ => () } match Tuple::deposit_asset(what, who) {
Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (),
r => return r,
}
)* ); )* );
log::trace!( log::trace!(
target: "xcm::TransactAsset::deposit_asset", target: "xcm::TransactAsset::deposit_asset",
...@@ -138,25 +141,31 @@ impl TransactAsset for Tuple { ...@@ -138,25 +141,31 @@ impl TransactAsset for Tuple {
what, what,
who, who,
); );
Err(XcmError::Unimplemented) Err(XcmError::AssetNotFound)
} }
fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> Result<Assets, XcmError> { fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> Result<Assets, XcmError> {
for_tuples!( #( for_tuples!( #(
match Tuple::withdraw_asset(what, who) { o @ Ok(_) => return o, _ => () } match Tuple::withdraw_asset(what, who) {
Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (),
r => return r,
}
)* ); )* );
log::trace!( log::trace!(
target: "xcm::TransactAsset::withdraw_asset", target: "xcm::TransactAsset::withdraw_asset",
"did not withdraw asset: what: {:?}, who: {:?}", "did not withdraw asset: what: {:?}, who: {:?}",
what, what,
who, who,
); );
Err(XcmError::Unimplemented) Err(XcmError::AssetNotFound)
} }
fn transfer_asset(what: &MultiAsset, from: &MultiLocation, to: &MultiLocation) -> Result<Assets, XcmError> { fn transfer_asset(what: &MultiAsset, from: &MultiLocation, to: &MultiLocation) -> Result<Assets, XcmError> {
for_tuples!( #( for_tuples!( #(
match Tuple::transfer_asset(what, from, to) { o @ Ok(_) => return o, _ => () } match Tuple::transfer_asset(what, from, to) {
Err(XcmError::AssetNotFound | XcmError::Unimplemented) => (),
r => return r,
}
)* ); )* );
log::trace!( log::trace!(
target: "xcm::TransactAsset::transfer_asset", target: "xcm::TransactAsset::transfer_asset",
...@@ -165,6 +174,99 @@ impl TransactAsset for Tuple { ...@@ -165,6 +174,99 @@ impl TransactAsset for Tuple {
from, from,
to, to,
); );
Err(XcmError::Unimplemented) 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<Assets, XcmError> {
Err(XcmError::AssetNotFound)
}
fn transfer_asset(_what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation) -> Result<Assets, XcmError> {
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<Assets, XcmError> {
Err(XcmError::Overflow)
}
fn transfer_asset(_what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation) -> Result<Assets, XcmError> {
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<Assets, XcmError> {
Ok(Assets::default())
}
fn transfer_asset(_what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation) -> Result<Assets, XcmError> {
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(()));
} }
} }
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment