diff --git a/polkadot/xcm/xcm-builder/src/currency_adapter.rs b/polkadot/xcm/xcm-builder/src/currency_adapter.rs index 86a2f54901558cdc4fc356696df8a00c5533c2f7..dfdfd4c6964a482ecb8a87fcd536cd4231d32b8f 100644 --- a/polkadot/xcm/xcm-builder/src/currency_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/currency_adapter.rs @@ -17,8 +17,8 @@ //! Adapters to work with `frame_support::traits::Currency` through XCM. use frame_support::traits::{ExistenceRequirement::AllowDeath, Get, WithdrawReasons}; -use sp_runtime::traits::{CheckedSub, SaturatedConversion}; -use sp_std::{convert::TryInto, marker::PhantomData, result}; +use sp_runtime::traits::CheckedSub; +use sp_std::{marker::PhantomData, result}; use xcm::latest::{Error as XcmError, MultiAsset, MultiLocation, Result}; use xcm_executor::{ traits::{Convert, MatchesFungible, TransactAsset}, @@ -31,8 +31,6 @@ enum Error { AssetNotFound, /// `MultiLocation` to `AccountId` conversion failed. AccountIdConversionFailed, - /// `u128` amount to currency `Balance` conversion failed. - AmountToBalanceConversionFailed, } impl From<Error> for XcmError { @@ -41,8 +39,6 @@ impl From<Error> for XcmError { match e { Error::AssetNotFound => XcmError::AssetNotFound, Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), - Error::AmountToBalanceConversionFailed => - FailedToTransactAsset("AmountToBalanceConversionFailed"), } } } @@ -149,27 +145,37 @@ impl< fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> Result { log::trace!(target: "xcm::currency_adapter", "deposit_asset what: {:?}, who: {:?}", what, who); // Check we handle this asset. - let amount: u128 = - Matcher::matches_fungible(&what).ok_or(Error::AssetNotFound)?.saturated_into(); + let amount = Matcher::matches_fungible(&what).ok_or(Error::AssetNotFound)?; let who = AccountIdConverter::convert_ref(who).map_err(|()| Error::AccountIdConversionFailed)?; - let balance_amount = - amount.try_into().map_err(|_| Error::AmountToBalanceConversionFailed)?; - let _imbalance = Currency::deposit_creating(&who, balance_amount); + let _imbalance = Currency::deposit_creating(&who, amount); Ok(()) } fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> result::Result<Assets, XcmError> { log::trace!(target: "xcm::currency_adapter", "withdraw_asset what: {:?}, who: {:?}", what, who); // Check we handle this asset. - let amount: u128 = - Matcher::matches_fungible(what).ok_or(Error::AssetNotFound)?.saturated_into(); + let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotFound)?; let who = AccountIdConverter::convert_ref(who).map_err(|()| Error::AccountIdConversionFailed)?; - let balance_amount = - amount.try_into().map_err(|_| Error::AmountToBalanceConversionFailed)?; - Currency::withdraw(&who, balance_amount, WithdrawReasons::TRANSFER, AllowDeath) + Currency::withdraw(&who, amount, WithdrawReasons::TRANSFER, AllowDeath) .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; Ok(what.clone().into()) } + + fn internal_transfer_asset( + asset: &MultiAsset, + from: &MultiLocation, + to: &MultiLocation, + ) -> result::Result<Assets, XcmError> { + log::trace!(target: "xcm::currency_adapter", "internal_transfer_asset asset: {:?}, from: {:?}, to: {:?}", asset, from, to); + let amount = Matcher::matches_fungible(asset).ok_or(Error::AssetNotFound)?; + let from = + AccountIdConverter::convert_ref(from).map_err(|()| Error::AccountIdConversionFailed)?; + let to = + AccountIdConverter::convert_ref(to).map_err(|()| Error::AccountIdConversionFailed)?; + Currency::transfer(&from, &to, amount, AllowDeath) + .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; + Ok(asset.clone().into()) + } } diff --git a/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs b/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs index 082e3b6955dfe1ab18b3fe04af48940d6db4ea1a..d06f92d97db9aa968b75bb3c395efa516352a7a8 100644 --- a/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs +++ b/polkadot/xcm/xcm-builder/src/fungibles_adapter.rs @@ -118,14 +118,14 @@ impl< AccountId: Clone, // can't get away without it since Currency is generic over it. > TransactAsset for FungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, AccountId> { - fn transfer_asset( + fn internal_transfer_asset( what: &MultiAsset, from: &MultiLocation, to: &MultiLocation, ) -> result::Result<xcm_executor::Assets, XcmError> { log::trace!( target: "xcm::fungibles_adapter", - "transfer_asset what: {:?}, from: {:?}, to: {:?}", + "internal_transfer_asset what: {:?}, from: {:?}, to: {:?}", what, from, to ); // Check we handle this asset. @@ -325,12 +325,12 @@ impl< >::withdraw_asset(what, who) } - fn transfer_asset( + fn internal_transfer_asset( what: &MultiAsset, from: &MultiLocation, to: &MultiLocation, ) -> result::Result<xcm_executor::Assets, XcmError> { - FungiblesTransferAdapter::<Assets, Matcher, AccountIdConverter, AccountId>::transfer_asset( + FungiblesTransferAdapter::<Assets, Matcher, AccountIdConverter, AccountId>::internal_transfer_asset( what, from, to, ) } diff --git a/polkadot/xcm/xcm-builder/tests/mock/mod.rs b/polkadot/xcm/xcm-builder/tests/mock/mod.rs index f11b628f65bc2f8203bf7026eb290eb282e92c7b..9599efcd7f29f3b420297400b2bc95f995b678f0 100644 --- a/polkadot/xcm/xcm-builder/tests/mock/mod.rs +++ b/polkadot/xcm/xcm-builder/tests/mock/mod.rs @@ -123,7 +123,7 @@ parameter_types! { pub type SovereignAccountOf = (ChildParachainConvertsVia<ParaId, AccountId>, AccountId32Aliases<KusamaNetwork, AccountId>); -pub type LocalAssetTransactor = XcmCurrencyAdapter< +pub type LocalCurrencyAdapter = XcmCurrencyAdapter< Balances, IsConcrete<KsmLocation>, SovereignAccountOf, @@ -131,6 +131,8 @@ pub type LocalAssetTransactor = XcmCurrencyAdapter< CheckAccount, >; +pub type LocalAssetTransactor = (LocalCurrencyAdapter,); + type LocalOriginConverter = ( SovereignSignedViaLocation<SovereignAccountOf, Origin>, ChildParachainAsNative<origin::Origin, Origin>, diff --git a/polkadot/xcm/xcm-builder/tests/scenarios.rs b/polkadot/xcm/xcm-builder/tests/scenarios.rs index 077a6590d57643e109cc1a7dc82af0b03adb7176..0bc6cfc8ce7da523889237b52248955315243410 100644 --- a/polkadot/xcm/xcm-builder/tests/scenarios.rs +++ b/polkadot/xcm/xcm-builder/tests/scenarios.rs @@ -17,7 +17,8 @@ mod mock; use mock::{ - kusama_like_with_balances, AccountId, Balance, Balances, BaseXcmWeight, XcmConfig, CENTS, + kusama_like_with_balances, AccountId, Balance, Balances, BaseXcmWeight, System, XcmConfig, + CENTS, }; use polkadot_parachain::primitives::Id as ParaId; use sp_runtime::traits::AccountIdConversion; @@ -66,6 +67,36 @@ fn withdraw_and_deposit_works() { }); } +/// Scenario: +/// Alice simply wants to transfer funds to Bob's account via XCM. +/// +/// Asserts that the balances are updated correctly and the correct events are fired. +#[test] +fn transfer_asset_works() { + let bob = AccountId::new([1u8; 32]); + let balances = vec![(ALICE, INITIAL_BALANCE), (bob.clone(), INITIAL_BALANCE)]; + kusama_like_with_balances(balances).execute_with(|| { + let amount = REGISTER_AMOUNT; + let weight = BaseXcmWeight::get(); + // Use `execute_xcm_in_credit` here to pass through the barrier + let r = XcmExecutor::<XcmConfig>::execute_xcm_in_credit( + AccountId32 { network: NetworkId::Any, id: ALICE.into() }, + Xcm(vec![TransferAsset { + assets: (Here, amount).into(), + beneficiary: AccountId32 { network: NetworkId::Any, id: bob.clone().into() }.into(), + }]), + weight, + weight, + ); + System::assert_last_event( + pallet_balances::Event::Transfer { from: ALICE, to: bob.clone(), amount }.into(), + ); + assert_eq!(r, Outcome::Complete(weight)); + assert_eq!(Balances::free_balance(ALICE), INITIAL_BALANCE - amount); + assert_eq!(Balances::free_balance(bob), INITIAL_BALANCE + amount); + }); +} + /// Scenario: /// A parachain wants to be notified that a transfer worked correctly. /// It includes a `QueryHolding` order after the deposit to get notified on success. diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs index f57e72f2ac7d4bdb709da805b02cb567803b8289..a68f2db71b6c7e970715bad0906a0455087bc9a7 100644 --- a/polkadot/xcm/xcm-executor/src/lib.rs +++ b/polkadot/xcm/xcm-executor/src/lib.rs @@ -297,7 +297,7 @@ impl<Config: config::Config> XcmExecutor<Config> { // Take `assets` from the origin account (on-chain) and place into dest account. let origin = self.origin.as_ref().ok_or(XcmError::BadOrigin)?; for asset in assets.inner() { - Config::AssetTransactor::beam_asset(&asset, origin, &beneficiary)?; + Config::AssetTransactor::transfer_asset(&asset, origin, &beneficiary)?; } Ok(()) }, @@ -305,7 +305,7 @@ impl<Config: config::Config> XcmExecutor<Config> { let origin = self.origin.as_ref().ok_or(XcmError::BadOrigin)?; // Take `assets` from the origin account (on-chain) and place into dest account. for asset in assets.inner() { - Config::AssetTransactor::beam_asset(asset, origin, &dest)?; + Config::AssetTransactor::transfer_asset(asset, origin, &dest)?; } let ancestry = Config::LocationInverter::ancestry(); assets.reanchor(&dest, &ancestry).map_err(|()| XcmError::MultiLocationFull)?; diff --git a/polkadot/xcm/xcm-executor/src/traits/transact_asset.rs b/polkadot/xcm/xcm-executor/src/traits/transact_asset.rs index 78e2180ed7a4b2cac1a79a46356db8f5dde4fdae..5b8c37bb59a0a9961f5cacda760f09059f202769 100644 --- a/polkadot/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/polkadot/xcm/xcm-executor/src/traits/transact_asset.rs @@ -78,7 +78,13 @@ pub trait TransactAsset { /// Move an `asset` `from` one location in `to` another location. /// /// Returns `XcmError::FailedToTransactAsset` if transfer failed. - fn transfer_asset( + /// + /// ## Notes + /// This function is meant to only be implemented by the type implementing `TransactAsset`, and + /// not be called directly. Most common API usages will instead call `transfer_asset`, which in + /// turn has a default implementation that calls `internal_transfer_asset`. As such, **please + /// do not call this method directly unless you know what you're doing**. + fn internal_transfer_asset( _asset: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation, @@ -88,13 +94,14 @@ pub trait TransactAsset { /// Move an `asset` `from` one location in `to` another location. /// - /// Attempts to use `transfer_asset` and if not available then falls back to using a two-part withdraw/deposit. - fn beam_asset( + /// Attempts to use `internal_transfer_asset` and if not available then falls back to using a + /// two-part withdraw/deposit. + fn transfer_asset( asset: &MultiAsset, from: &MultiLocation, to: &MultiLocation, ) -> Result<Assets, XcmError> { - match Self::transfer_asset(asset, from, to) { + match Self::internal_transfer_asset(asset, from, to) { Err(XcmError::Unimplemented) => { let assets = Self::withdraw_asset(asset, from)?; // Not a very forgiving attitude; once we implement roll-backs then it'll be nicer. @@ -168,19 +175,19 @@ impl TransactAsset for Tuple { Err(XcmError::AssetNotFound) } - fn transfer_asset( + fn internal_transfer_asset( what: &MultiAsset, from: &MultiLocation, to: &MultiLocation, ) -> Result<Assets, XcmError> { for_tuples!( #( - match Tuple::transfer_asset(what, from, to) { + match Tuple::internal_transfer_asset(what, from, to) { Err(XcmError::AssetNotFound) | Err(XcmError::Unimplemented) => (), r => return r, } )* ); log::trace!( - target: "xcm::TransactAsset::transfer_asset", + target: "xcm::TransactAsset::internal_transfer_asset", "did not transfer asset: what: {:?}, from: {:?}, to: {:?}", what, from, @@ -212,7 +219,7 @@ mod tests { Err(XcmError::AssetNotFound) } - fn transfer_asset( + fn internal_transfer_asset( _what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation, @@ -235,7 +242,7 @@ mod tests { Err(XcmError::Overflow) } - fn transfer_asset( + fn internal_transfer_asset( _what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation, @@ -258,7 +265,7 @@ mod tests { Ok(Assets::default()) } - fn transfer_asset( + fn internal_transfer_asset( _what: &MultiAsset, _from: &MultiLocation, _to: &MultiLocation,