From 8a9bc2154adb70e31e84576d7c0290355b5d0f08 Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Fri, 8 Jul 2022 15:06:46 +0300 Subject: [PATCH 01/15] Works with boudned vector Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 118 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 111 insertions(+), 7 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 99a537cf2..b8b66aa99 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -117,6 +117,9 @@ pub mod module { /// The way to retreave the reserve of a MultiAsset. This can be /// configured to accept absolute or relative paths for self tokens type ReserveProvider: Reserve; + + type XcmSender: SendXcm; + type MaxTransactSize: Get; } #[pallet::event] @@ -342,6 +345,31 @@ pub mod module { Self::do_transfer_multicurrencies(who, currencies, fee_item, dest, dest_weight) } + #[pallet::weight(0)] + #[transactional] + pub fn transfer_with_transact( + origin: OriginFor, + currency_id: T::CurrencyId, + amount: T::Balance, + dest_id: u32, + dest_weight: Weight, + // encoded_call_data: Vec, + encoded_call_data: BoundedVec, + transact_fee: T::Balance, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + + Self::do_transfer_with_transact( + who, + currency_id, + amount, + dest_id, + dest_weight, + encoded_call_data, + transact_fee, + ) + } + /// Transfer several `MultiAsset` specifying the item to be used as fee /// /// `dest_weight` is the weight for XCM execution on the dest chain, and @@ -373,11 +401,86 @@ pub mod module { // We first grab the fee let fee: &MultiAsset = assets.get(fee_item as usize).ok_or(Error::::AssetIndexNonExistent)?; - Self::do_transfer_multiassets(who, assets.clone(), fee.clone(), dest, dest_weight) + Self::do_transfer_multiassets(who, assets.clone(), fee.clone(), dest, dest_weight, None) } } impl Pallet { + fn do_transfer_with_transact( + who: T::AccountId, + currency_id: T::CurrencyId, + amount: T::Balance, + dest_id: u32, + dest_weight: Weight, + // encoded_call_data: Vec, + encoded_call_data: BoundedVec, + transact_fee: T::Balance, + ) -> DispatchResult { + let location: MultiLocation = + T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; + + let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); + + let mut dest_chain_location: MultiLocation = (1, Parachain(dest_id)).into(); + let _ = dest_chain_location.append_with(origin_location.clone().interior); + + ensure!(!amount.is_zero(), Error::::ZeroAmount); + ensure!( + T::MultiLocationsFilter::contains(&dest_chain_location), + Error::::NotSupportedMultiLocation + ); + + let asset: MultiAsset = (location.clone(), amount.into()).into(); + let transact_fee_asset: MultiAsset = (location, transact_fee.into()).into(); + + let mut override_dest = T::SelfLocation::get(); + let _ = override_dest.append_with(origin_location.clone().interior); + + let _ = Self::do_transfer_multiassets( + who.clone(), + vec![asset.clone()].into(), + asset, + dest_chain_location.clone(), + dest_weight, + // TODO: not a todo, but this part is important + Some(override_dest), + ); + + let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); + // TODO: is this the best way to get dest location + let target_chain_location = dest_chain_location.chain_part().ok_or(Error::::AssetHasNoReserve)?; + let ancestry = T::LocationInverter::ancestry(); + + let transact_fee_asset = transact_fee_asset + .clone() + .reanchored(&target_chain_location, &ancestry) + .map_err(|_| Error::::CannotReanchor)?; + + //let double_encoded: DoubleEncoded = call.into(); + let mut transact_fee_assets = MultiAssets::new(); + transact_fee_assets.push(transact_fee_asset.clone()); + let instructions = Xcm(vec![ + DescendOrigin(origin_location.clone().interior), + WithdrawAsset(transact_fee_assets), + BuyExecution { + fees: transact_fee_asset, + weight_limit: WeightLimit::Limited(dest_weight), + }, + Transact { + origin_type: OriginKind::SovereignAccount, + require_weight_at_most: dest_weight, + call: encoded_call_data.into_inner().into(), + //call, + }, + // TODO: + // RefundSurplus + // DepositAsset(user_account or back to sovereign_account) + ]); + + let _res = T::XcmSender::send_xcm(target_chain_location.clone(), instructions); + Ok(()) + } + fn do_transfer( who: T::AccountId, currency_id: T::CurrencyId, @@ -395,7 +498,7 @@ pub mod module { ); let asset: MultiAsset = (location, amount.into()).into(); - Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight) + Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight, None) } fn do_transfer_with_fee( @@ -424,7 +527,7 @@ pub mod module { assets.push(asset); assets.push(fee_asset.clone()); - Self::do_transfer_multiassets(who, assets, fee_asset, dest, dest_weight) + Self::do_transfer_multiassets(who, assets, fee_asset, dest, dest_weight, None) } fn do_transfer_multiasset( @@ -433,7 +536,7 @@ pub mod module { dest: MultiLocation, dest_weight: Weight, ) -> DispatchResult { - Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight) + Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight, None) } fn do_transfer_multiasset_with_fee( @@ -448,7 +551,7 @@ pub mod module { assets.push(asset); assets.push(fee.clone()); - Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight)?; + Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight, None)?; Ok(()) } @@ -492,7 +595,7 @@ pub mod module { let fee: MultiAsset = (fee_location, (*fee_amount).into()).into(); - Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight) + Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight, None) } fn do_transfer_multiassets( @@ -501,6 +604,7 @@ pub mod module { fee: MultiAsset, dest: MultiLocation, dest_weight: Weight, + override_recipient: Option, ) -> DispatchResult { ensure!( assets.len() <= T::MaxAssetsForTransfer::get(), @@ -588,7 +692,7 @@ pub mod module { fee.clone(), non_fee_reserve, &dest, - None, + override_recipient, dest_weight, )?; } From 2bad165a4ad720339bbd159cb56fc377bfa8c45b Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Mon, 11 Jul 2022 14:08:28 +0300 Subject: [PATCH 02/15] Add transact_only and clean up Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 182 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 157 insertions(+), 25 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index b8b66aa99..7ebb26e15 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -345,6 +345,21 @@ pub mod module { Self::do_transfer_multicurrencies(who, currencies, fee_item, dest, dest_weight) } + #[pallet::weight(0)] + #[transactional] + pub fn transact( + origin: OriginFor, + currency_id: T::CurrencyId, + dest_id: u32, + dest_weight: Weight, + encoded_call_data: BoundedVec, + transact_fee: T::Balance, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + + Self::do_transact(who, currency_id, dest_id, dest_weight, encoded_call_data, transact_fee) + } + #[pallet::weight(0)] #[transactional] pub fn transfer_with_transact( @@ -353,7 +368,6 @@ pub mod module { amount: T::Balance, dest_id: u32, dest_weight: Weight, - // encoded_call_data: Vec, encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { @@ -406,13 +420,86 @@ pub mod module { } impl Pallet { + fn do_transact( + who: T::AccountId, + currency_id: T::CurrencyId, + dest_chain_id: u32, + dest_weight: Weight, + encoded_call_data: BoundedVec, + transact_fee: T::Balance, + ) -> DispatchResult { + let location: MultiLocation = + T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; + + let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); + let mut dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); + // // Need to append some interior to pass is_valid check later on. + // // Only the chain part is needed for everything else. + // let _ = dest_chain_location.append_with(origin_location.clone().interior); + + // ensure!(!amount.is_zero(), Error::::ZeroAmount); + // ensure!( + // T::MultiLocationsFilter::contains(&dest_chain_location), + // Error::::NotSupportedMultiLocation + // ); + + // let transfer_asset: MultiAsset = (location.clone(), amount.into()).into(); + + // let mut override_recipient = T::SelfLocation::get(); + // let _ = override_recipient.append_with(origin_location.clone().interior); + + // let _ = Self::do_transfer_multiassets( + // who.clone(), + // vec![transfer_asset.clone()].into(), + // transfer_asset, + // dest_chain_location.clone(), + // dest_weight, + // Some(override_recipient.clone()), + // ); + + let transact_fee_asset: MultiAsset = (location, transact_fee.into()).into(); + + // We can get rid of the interior now. + dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; + let ancestry = T::LocationInverter::ancestry(); + let transact_fee_asset = transact_fee_asset + .clone() + .reanchored(&dest_chain_location, &ancestry) + .map_err(|_| Error::::CannotReanchor)?; + + let mut transact_fee_assets = MultiAssets::new(); + transact_fee_assets.push(transact_fee_asset.clone()); + let instructions = Xcm(vec![ + DescendOrigin(origin_location.clone().interior), + WithdrawAsset(transact_fee_assets.clone()), + BuyExecution { + fees: transact_fee_asset, + weight_limit: WeightLimit::Limited(dest_weight), + }, + Transact { + origin_type: OriginKind::SovereignAccount, + require_weight_at_most: dest_weight, + call: encoded_call_data.into_inner().into(), + }, + // RefundSurplus, + // DepositAsset { + // assets: All.into(), + // max_assets: transact_fee_assets.len() as u32, + // beneficiary: override_recipient, + // }, + ]); + + let _res = T::XcmSender::send_xcm(dest_chain_location, instructions); + + Ok(()) + } + fn do_transfer_with_transact( who: T::AccountId, currency_id: T::CurrencyId, amount: T::Balance, - dest_id: u32, + dest_chain_id: u32, dest_weight: Weight, - // encoded_call_data: Vec, encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { @@ -420,8 +507,9 @@ pub mod module { T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); - - let mut dest_chain_location: MultiLocation = (1, Parachain(dest_id)).into(); + let mut dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); + // Need to append some interior to pass is_valid check later on. + // Only the chain part is needed for everything else. let _ = dest_chain_location.append_with(origin_location.clone().interior); ensure!(!amount.is_zero(), Error::::ZeroAmount); @@ -430,57 +518,101 @@ pub mod module { Error::::NotSupportedMultiLocation ); - let asset: MultiAsset = (location.clone(), amount.into()).into(); - let transact_fee_asset: MultiAsset = (location, transact_fee.into()).into(); + let transfer_asset: MultiAsset = (location.clone(), amount.into()).into(); - let mut override_dest = T::SelfLocation::get(); - let _ = override_dest.append_with(origin_location.clone().interior); + let mut override_recipient = T::SelfLocation::get(); + let _ = override_recipient.append_with(origin_location.clone().interior); let _ = Self::do_transfer_multiassets( who.clone(), - vec![asset.clone()].into(), - asset, + vec![transfer_asset.clone()].into(), + transfer_asset, dest_chain_location.clone(), dest_weight, - // TODO: not a todo, but this part is important - Some(override_dest), + Some(override_recipient.clone()), ); - let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); - // TODO: is this the best way to get dest location - let target_chain_location = dest_chain_location.chain_part().ok_or(Error::::AssetHasNoReserve)?; - let ancestry = T::LocationInverter::ancestry(); + let transact_fee_asset: MultiAsset = (location, transact_fee.into()).into(); + // We can get rid of the interior now. + dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; + let ancestry = T::LocationInverter::ancestry(); let transact_fee_asset = transact_fee_asset .clone() - .reanchored(&target_chain_location, &ancestry) + .reanchored(&dest_chain_location, &ancestry) .map_err(|_| Error::::CannotReanchor)?; - //let double_encoded: DoubleEncoded = call.into(); let mut transact_fee_assets = MultiAssets::new(); transact_fee_assets.push(transact_fee_asset.clone()); let instructions = Xcm(vec![ DescendOrigin(origin_location.clone().interior), - WithdrawAsset(transact_fee_assets), + WithdrawAsset(transact_fee_assets.clone()), BuyExecution { fees: transact_fee_asset, weight_limit: WeightLimit::Limited(dest_weight), }, Transact { + // SovereignAccount of the user, not the chain origin_type: OriginKind::SovereignAccount, require_weight_at_most: dest_weight, call: encoded_call_data.into_inner().into(), - //call, }, - // TODO: - // RefundSurplus - // DepositAsset(user_account or back to sovereign_account) + RefundSurplus, + DepositAsset { + assets: All.into(), + max_assets: transact_fee_assets.len() as u32, + beneficiary: override_recipient, + }, ]); - let _res = T::XcmSender::send_xcm(target_chain_location.clone(), instructions); + let _res = T::XcmSender::send_xcm(dest_chain_location, instructions); + Ok(()) } + // fn create_transact( + // asset_location: MultiLocation, + // transact_fee: T::Balance, + // dest_chain_location: MultiLocation, + // origin_location: MultiLocation, + // override_recipient: MultiLocation, + // dest_weight: Weight, + // encoded_call_data: BoundedVec, + // ) { + // let transact_fee_asset: MultiAsset = (asset_location, transact_fee.into()).into(); + + // // We can get rid of the interior now. + // let dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; + // let ancestry = T::LocationInverter::ancestry(); + // let transact_fee_asset = transact_fee_asset + // .clone() + // .reanchored(&dest_chain_location, &ancestry) + // .map_err(|_| Error::::CannotReanchor)?; + + // let mut transact_fee_assets = MultiAssets::new(); + // transact_fee_assets.push(transact_fee_asset.clone()); + // let instructions = Xcm(vec![ + // DescendOrigin(origin_location.clone().interior), + // WithdrawAsset(transact_fee_assets.clone()), + // BuyExecution { + // fees: transact_fee_asset, + // weight_limit: WeightLimit::Limited(dest_weight), + // }, + // Transact { + // // SovereignAccount of the user, not the chain + // origin_type: OriginKind::SovereignAccount, + // require_weight_at_most: dest_weight, + // call: encoded_call_data.into_inner().into(), + // }, + // RefundSurplus, + // DepositAsset { + // assets: All.into(), + // max_assets: transact_fee_assets.len() as u32, + // beneficiary: override_recipient, + // }, + // ]); + // } + fn do_transfer( who: T::AccountId, currency_id: T::CurrencyId, From f8150b39c0c5bf305e5b98c8d1801e19fefa2f56 Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Mon, 11 Jul 2022 20:21:55 +0300 Subject: [PATCH 03/15] Clean up Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 187 ++++++++++++++++----------------------------- 1 file changed, 65 insertions(+), 122 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 7ebb26e15..dcc92ade8 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -119,6 +119,8 @@ pub mod module { type ReserveProvider: Reserve; type XcmSender: SendXcm; + + #[pallet::constant] type MaxTransactSize: Get; } @@ -178,6 +180,8 @@ pub mod module { NotSupportedMultiLocation, /// MinXcmFee not registered for certain reserve location MinXcmFeeNotDefined, + SendFailure, + TransactTooLarge, } #[pallet::hooks] @@ -355,9 +359,11 @@ pub mod module { encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { + ensure!(T::MaxTransactSize::get() <= 256u32, Error::::TransactTooLarge); + let who = ensure_signed(origin)?; - Self::do_transact(who, currency_id, dest_id, dest_weight, encoded_call_data, transact_fee) + Self::do_transact(who, currency_id, transact_fee, dest_id, dest_weight, encoded_call_data) } #[pallet::weight(0)] @@ -371,6 +377,8 @@ pub mod module { encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { + ensure!(T::MaxTransactSize::get() <= 256u32, Error::::TransactTooLarge); + let who = ensure_signed(origin)?; Self::do_transfer_with_transact( @@ -422,74 +430,27 @@ pub mod module { impl Pallet { fn do_transact( who: T::AccountId, - currency_id: T::CurrencyId, + transact_currency_id: T::CurrencyId, + transact_fee_amount: T::Balance, dest_chain_id: u32, dest_weight: Weight, encoded_call_data: BoundedVec, - transact_fee: T::Balance, ) -> DispatchResult { - let location: MultiLocation = - T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; + let transact_fee_location: MultiLocation = T::CurrencyIdConvert::convert(transact_currency_id) + .ok_or(Error::::NotCrossChainTransferableCurrency)?; let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); - let mut dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); - // // Need to append some interior to pass is_valid check later on. - // // Only the chain part is needed for everything else. - // let _ = dest_chain_location.append_with(origin_location.clone().interior); - - // ensure!(!amount.is_zero(), Error::::ZeroAmount); - // ensure!( - // T::MultiLocationsFilter::contains(&dest_chain_location), - // Error::::NotSupportedMultiLocation - // ); - - // let transfer_asset: MultiAsset = (location.clone(), amount.into()).into(); - - // let mut override_recipient = T::SelfLocation::get(); - // let _ = override_recipient.append_with(origin_location.clone().interior); - - // let _ = Self::do_transfer_multiassets( - // who.clone(), - // vec![transfer_asset.clone()].into(), - // transfer_asset, - // dest_chain_location.clone(), - // dest_weight, - // Some(override_recipient.clone()), - // ); - - let transact_fee_asset: MultiAsset = (location, transact_fee.into()).into(); - - // We can get rid of the interior now. - dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; - let ancestry = T::LocationInverter::ancestry(); - let transact_fee_asset = transact_fee_asset - .clone() - .reanchored(&dest_chain_location, &ancestry) - .map_err(|_| Error::::CannotReanchor)?; - - let mut transact_fee_assets = MultiAssets::new(); - transact_fee_assets.push(transact_fee_asset.clone()); - let instructions = Xcm(vec![ - DescendOrigin(origin_location.clone().interior), - WithdrawAsset(transact_fee_assets.clone()), - BuyExecution { - fees: transact_fee_asset, - weight_limit: WeightLimit::Limited(dest_weight), - }, - Transact { - origin_type: OriginKind::SovereignAccount, - require_weight_at_most: dest_weight, - call: encoded_call_data.into_inner().into(), - }, - // RefundSurplus, - // DepositAsset { - // assets: All.into(), - // max_assets: transact_fee_assets.len() as u32, - // beneficiary: override_recipient, - // }, - ]); + let dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); - let _res = T::XcmSender::send_xcm(dest_chain_location, instructions); + Self::send_transact( + transact_fee_location, + transact_fee_amount, + dest_chain_location.clone(), + origin_location, + dest_weight, + encoded_call_data, + None, + )?; Ok(()) } @@ -501,9 +462,9 @@ pub mod module { dest_chain_id: u32, dest_weight: Weight, encoded_call_data: BoundedVec, - transact_fee: T::Balance, + transact_fee_amount: T::Balance, ) -> DispatchResult { - let location: MultiLocation = + let transact_fee_location: MultiLocation = T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); @@ -518,24 +479,44 @@ pub mod module { Error::::NotSupportedMultiLocation ); - let transfer_asset: MultiAsset = (location.clone(), amount.into()).into(); - + let transfer_asset: MultiAsset = (transact_fee_location.clone(), amount.into()).into(); let mut override_recipient = T::SelfLocation::get(); let _ = override_recipient.append_with(origin_location.clone().interior); - let _ = Self::do_transfer_multiassets( + Self::do_transfer_multiassets( who.clone(), vec![transfer_asset.clone()].into(), transfer_asset, dest_chain_location.clone(), dest_weight, Some(override_recipient.clone()), - ); + )?; - let transact_fee_asset: MultiAsset = (location, transact_fee.into()).into(); + Self::send_transact( + transact_fee_location, + transact_fee_amount, + dest_chain_location.clone(), + origin_location, + dest_weight, + encoded_call_data, + Some(override_recipient), + )?; + Ok(()) + } + + fn send_transact( + transact_fee_location: MultiLocation, + transact_fee_amount: T::Balance, + dest_chain_location: MultiLocation, + origin_location: MultiLocation, + dest_weight: Weight, + encoded_call_data: BoundedVec, + override_recipient: Option, + ) -> DispatchResult { + let transact_fee_asset: MultiAsset = (transact_fee_location, transact_fee_amount.into()).into(); // We can get rid of the interior now. - dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; + let dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; let ancestry = T::LocationInverter::ancestry(); let transact_fee_asset = transact_fee_asset .clone() @@ -544,7 +525,7 @@ pub mod module { let mut transact_fee_assets = MultiAssets::new(); transact_fee_assets.push(transact_fee_asset.clone()); - let instructions = Xcm(vec![ + let mut instructions = vec![ DescendOrigin(origin_location.clone().interior), WithdrawAsset(transact_fee_assets.clone()), BuyExecution { @@ -557,62 +538,24 @@ pub mod module { require_weight_at_most: dest_weight, call: encoded_call_data.into_inner().into(), }, - RefundSurplus, - DepositAsset { - assets: All.into(), - max_assets: transact_fee_assets.len() as u32, - beneficiary: override_recipient, - }, - ]); + ]; + + if let Some(recipient) = override_recipient { + instructions.extend(vec![ + RefundSurplus, + DepositAsset { + assets: All.into(), + max_assets: transact_fee_assets.len() as u32, + beneficiary: recipient, + }, + ]); + } - let _res = T::XcmSender::send_xcm(dest_chain_location, instructions); + T::XcmSender::send_xcm(dest_chain_location, Xcm(instructions)).map_err(|_| Error::::SendFailure)?; Ok(()) } - // fn create_transact( - // asset_location: MultiLocation, - // transact_fee: T::Balance, - // dest_chain_location: MultiLocation, - // origin_location: MultiLocation, - // override_recipient: MultiLocation, - // dest_weight: Weight, - // encoded_call_data: BoundedVec, - // ) { - // let transact_fee_asset: MultiAsset = (asset_location, transact_fee.into()).into(); - - // // We can get rid of the interior now. - // let dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; - // let ancestry = T::LocationInverter::ancestry(); - // let transact_fee_asset = transact_fee_asset - // .clone() - // .reanchored(&dest_chain_location, &ancestry) - // .map_err(|_| Error::::CannotReanchor)?; - - // let mut transact_fee_assets = MultiAssets::new(); - // transact_fee_assets.push(transact_fee_asset.clone()); - // let instructions = Xcm(vec![ - // DescendOrigin(origin_location.clone().interior), - // WithdrawAsset(transact_fee_assets.clone()), - // BuyExecution { - // fees: transact_fee_asset, - // weight_limit: WeightLimit::Limited(dest_weight), - // }, - // Transact { - // // SovereignAccount of the user, not the chain - // origin_type: OriginKind::SovereignAccount, - // require_weight_at_most: dest_weight, - // call: encoded_call_data.into_inner().into(), - // }, - // RefundSurplus, - // DepositAsset { - // assets: All.into(), - // max_assets: transact_fee_assets.len() as u32, - // beneficiary: override_recipient, - // }, - // ]); - // } - fn do_transfer( who: T::AccountId, currency_id: T::CurrencyId, From 4d16c28211363ad1c399e0b47a1d27be3d7b2f7f Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Mon, 11 Jul 2022 21:28:56 +0300 Subject: [PATCH 04/15] Add weights Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index dcc92ade8..9a6c17104 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -349,7 +349,7 @@ pub mod module { Self::do_transfer_multicurrencies(who, currencies, fee_item, dest, dest_weight) } - #[pallet::weight(0)] + #[pallet::weight(Pallet::::weight_of_send_transact())] #[transactional] pub fn transact( origin: OriginFor, @@ -366,13 +366,13 @@ pub mod module { Self::do_transact(who, currency_id, transact_fee, dest_id, dest_weight, encoded_call_data) } - #[pallet::weight(0)] + #[pallet::weight(Pallet::::weight_of_transfer_with_transact(currency_id.clone(), *amount, *dest_chain_id))] #[transactional] pub fn transfer_with_transact( origin: OriginFor, currency_id: T::CurrencyId, amount: T::Balance, - dest_id: u32, + dest_chain_id: u32, dest_weight: Weight, encoded_call_data: BoundedVec, transact_fee: T::Balance, @@ -385,7 +385,7 @@ pub mod module { who, currency_id, amount, - dest_id, + dest_chain_id, dest_weight, encoded_call_data, transact_fee, @@ -998,6 +998,29 @@ pub mod module { } } + /// Returns weight of `send_transact` call. + fn weight_of_send_transact() -> Weight { + let mut msg = Xcm(vec![]); + return T::Weigher::weight(&mut msg) + .map_or(Weight::max_value(), |w| T::BaseXcmWeight::get().saturating_add(w)); + } + + /// Returns weight of `transfer_with_transact` call. + fn weight_of_transfer_with_transact( + currency_id: T::CurrencyId, + amount: T::Balance, + dest_chain_id: u32, + ) -> Weight { + let dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); + if let Some(location) = T::CurrencyIdConvert::convert(currency_id) { + let asset = (location, amount.into()).into(); + Self::weight_of_transfer_multiasset(&asset, &VersionedMultiLocation::V1(dest_chain_location)) + + Self::weight_of_send_transact() + } else { + 0 + } + } + /// Returns weight of `transfer` call. fn weight_of_transfer_multicurrencies( currencies: &[(T::CurrencyId, T::Balance)], From dfd223cd2c4c818ffc874e28784514e6924b00d3 Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Tue, 12 Jul 2022 18:37:37 +0300 Subject: [PATCH 05/15] Add tests with unpaid execution everything barier Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 2 + xtokens/src/mock/para.rs | 102 +++++++++++++++++-- xtokens/src/mock/para_relative_view.rs | 3 + xtokens/src/mock/relay.rs | 7 +- xtokens/src/tests.rs | 130 ++++++++++++++++++++++++- 5 files changed, 232 insertions(+), 12 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 9a6c17104..e301d05c0 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -738,6 +738,7 @@ pub mod module { let mut assets_to_fee_reserve = MultiAssets::new(); let asset_to_fee_reserve = subtract_fee(&fee, min_xcm_fee); assets_to_fee_reserve.push(asset_to_fee_reserve.clone()); + println!("im not here right ?//////////////////////////////// \n"); // First xcm sent to fee reserve chain and routed to dest chain. Self::execute_and_send_reserve_kind_xcm( @@ -806,6 +807,7 @@ pub mod module { }; let weight = T::Weigher::weight(&mut msg).map_err(|()| Error::::UnweighableMessage)?; + println!("origin_location: {:?} \n", origin_location); T::XcmExecutor::execute_xcm_in_credit(origin_location, msg, weight, weight) .ensure_complete() .map_err(|error| { diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 2dde03460..df007864e 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -2,8 +2,8 @@ use super::{Amount, Balance, CurrencyId, CurrencyIdConvert, ParachainXcmRouter}; use crate as orml_xtokens; use frame_support::{ - construct_runtime, match_types, parameter_types, - traits::{ConstU128, ConstU32, ConstU64, Everything, Get, Nothing}, + construct_runtime, ensure, match_types, parameter_types, + traits::{ConstU128, ConstU32, ConstU64, Contains, Everything, Get, Nothing}, weights::{constants::WEIGHT_PER_SECOND, Weight}, }; use frame_system::EnsureRoot; @@ -13,17 +13,22 @@ use sp_runtime::{ traits::{Convert, IdentityLookup, Zero}, AccountId32, }; +use sp_std::marker::PhantomData; use cumulus_primitives_core::{ChannelStatus, GetChannelInfo, ParaId}; use pallet_xcm::XcmPassthrough; use polkadot_parachain::primitives::Sibling; use xcm::latest::prelude::*; use xcm_builder::{ - AccountId32Aliases, AllowTopLevelPaidExecutionFrom, EnsureXcmOrigin, FixedWeightBounds, LocationInverter, - ParentIsPreset, RelayChainAsNative, SiblingParachainAsNative, SiblingParachainConvertsVia, - SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, + Account32Hash, AccountId32Aliases, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, EnsureXcmOrigin, + FixedWeightBounds, LocationInverter, ParentIsPreset, RelayChainAsNative, SiblingParachainAsNative, + SiblingParachainConvertsVia, SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, + TakeWeightCredit, +}; +use xcm_executor::{ + traits::{ShouldExecute, WeightTrader}, + Assets, Config, XcmExecutor, }; -use xcm_executor::{traits::WeightTrader, Assets, Config, XcmExecutor}; use orml_traits::{location::AbsoluteReserveProvider, parameter_type_with_key}; use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset}; @@ -109,6 +114,7 @@ pub type LocationToAccountId = ( ParentIsPreset, SiblingParachainConvertsVia, AccountId32Aliases, + Account32Hash, ); pub type XcmOriginToCallOrigin = ( @@ -130,8 +136,79 @@ pub type LocalAssetTransactor = MultiCurrencyAdapter< (), >; +match_types! { + pub type ParentOrFriends: impl Contains = { + MultiLocation { parents: 1, interior: Here } | + MultiLocation { parents: 1, interior: X1(Parachain(1)) } | + MultiLocation { parents: 1, interior: X1(Parachain(2)) } | + MultiLocation { parents: 1, interior: X1(Parachain(3)) } | + MultiLocation { parents: 1, interior: X1(Parachain(4)) } + }; +} + pub type XcmRouter = ParachainXcmRouter; -pub type Barrier = (TakeWeightCredit, AllowTopLevelPaidExecutionFrom); + +// /// Allows execution from `origin` if it is contained in `T` (i.e. +// /// `T::Contains(origin)`) taking payments into account. +// /// +// /// Only allows for `TeleportAsset`, `WithdrawAsset`, `ClaimAsset` and +// /// `ReserveAssetDeposit` XCMs because they are the only ones that place +// assets /// in the Holding Register to pay for execution. +// pub struct AllowTopLevelPaidExecutionFrom(PhantomData); +// impl> ShouldExecute for +// AllowTopLevelPaidExecutionFrom { fn should_execute( +// origin: &MultiLocation, +// message: &mut Xcm, +// max_weight: Weight, +// _weight_credit: &mut Weight, +// ) -> Result<(), ()> { +// // log::trace!( +// // target: "xcm::barriers", +// // "AllowTopLevelPaidExecutionFrom origin: {:?}, message: {:?}, max_weight: +// // {:?}, weight_credit: {:?}", origin, message, max_weight, _weight_credit, +// // ); +// ensure!(T::contains(origin), ()); +// let mut iter = message.0.iter_mut(); +// let i = iter.next().ok_or(())?; +// match i { +// ReceiveTeleportedAsset(..) +// | WithdrawAsset(..) +// | ReserveAssetDeposited(..) +// | ClaimAsset { .. } +// | Transact { .. } => (), +// _ => return Err(()), +// } +// let mut i = iter.next().ok_or(())?; +// while let ClearOrigin = i { +// i = iter.next().ok_or(())?; +// } +// match i { +// BuyExecution { +// weight_limit: Limited(ref mut weight), +// .. +// } if *weight >= max_weight => { +// *weight = max_weight; +// Ok(()) +// } +// BuyExecution { +// ref mut weight_limit, .. +// } if weight_limit == &Unlimited => { +// *weight_limit = Limited(max_weight); +// Ok(()) +// } +// _ => Err(()), +// } +// } +// } +pub type Barrier = ( + TakeWeightCredit, + AllowTopLevelPaidExecutionFrom, + // This has to be after AllowTopLevelPaidExecutionFrom or 2 tests will fail: + // tests::sending_sibling_asset_to_reserve_sibling_with_relay_fee_not_enough + // tests::sending_sibling_asset_to_reserve_sibling_with_relay_fee_works + AllowUnpaidExecutionFrom, +); +//pub type Barrier = AllowUnpaidExecutionFrom; /// A trader who believes all tokens are created equal to "weight" of any chain, /// which is not true, but good enough to mock the fee payment of XCM execution. @@ -154,16 +231,22 @@ impl WeightTrader for AllTokensAreCreatedEqualToWeight { id: asset_id.clone(), fun: Fungible(weight as u128), }; - + println!("check 1 \n"); + println!("required: {:?} \n", required); if let MultiAsset { fun: _, id: Concrete(ref id), } = &required { self.0 = id.clone(); + println!("id: {:?} \n", id); } + println!("check 2 \n"); let unused = payment.checked_sub(required).map_err(|_| XcmError::TooExpensive)?; + println!("check 3 \n"); + println!("unused: {:?} \n", unused); + Ok(unused) } @@ -281,6 +364,7 @@ parameter_type_with_key! { _ => None, } }; + } impl orml_xtokens::Config for Runtime { @@ -298,6 +382,8 @@ impl orml_xtokens::Config for Runtime { type LocationInverter = LocationInverter; type MaxAssetsForTransfer = MaxAssetsForTransfer; type ReserveProvider = AbsoluteReserveProvider; + type XcmSender = XcmRouter; + type MaxTransactSize = ConstU32<256>; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_relative_view.rs b/xtokens/src/mock/para_relative_view.rs index f6786d09f..2487cefe6 100644 --- a/xtokens/src/mock/para_relative_view.rs +++ b/xtokens/src/mock/para_relative_view.rs @@ -326,6 +326,7 @@ impl Convert> for RelativeCurrencyIdConvert { parameter_types! { pub SelfLocation: MultiLocation = MultiLocation::here(); pub const MaxAssetsForTransfer: usize = 2; + pub const MaxTransactSize: u32 = 256; } match_types! { @@ -365,6 +366,8 @@ impl orml_xtokens::Config for Runtime { type LocationInverter = LocationInverter; type MaxAssetsForTransfer = MaxAssetsForTransfer; type ReserveProvider = RelativeReserveProvider; + type XcmSender = XcmRouter; + type MaxTransactSize = MaxTransactSize; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/relay.rs b/xtokens/src/mock/relay.rs index ba8e5c57f..d09747992 100644 --- a/xtokens/src/mock/relay.rs +++ b/xtokens/src/mock/relay.rs @@ -11,9 +11,9 @@ use cumulus_primitives_core::ParaId; use polkadot_runtime_parachains::{configuration, origin, shared, ump}; use xcm::latest::prelude::*; use xcm_builder::{ - AccountId32Aliases, AllowTopLevelPaidExecutionFrom, ChildParachainAsNative, ChildParachainConvertsVia, - CurrencyAdapter as XcmCurrencyAdapter, FixedWeightBounds, IsConcrete, LocationInverter, SignedAccountId32AsNative, - SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, + Account32Hash, AccountId32Aliases, AllowTopLevelPaidExecutionFrom, ChildParachainAsNative, + ChildParachainConvertsVia, CurrencyAdapter as XcmCurrencyAdapter, FixedWeightBounds, IsConcrete, LocationInverter, + SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, }; use xcm_executor::{Config, XcmExecutor}; @@ -74,6 +74,7 @@ parameter_types! { pub type SovereignAccountOf = ( ChildParachainConvertsVia, AccountId32Aliases, + // Account32Hash, ); pub type LocalAssetTransactor = diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 35d78fff8..84f9eb058 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -3,7 +3,7 @@ use super::*; use codec::Encode; use cumulus_primitives_core::ParaId; -use frame_support::{assert_err, assert_noop, assert_ok, traits::Currency}; +use frame_support::{assert_err, assert_noop, assert_ok, traits::Currency, BoundedVec}; use mock::*; use orml_traits::{ConcreteFungibleAsset, MultiCurrency}; use polkadot_parachain::primitives::Sibling; @@ -300,6 +300,134 @@ fn send_sibling_asset_to_reserve_sibling() { }); } +#[test] +fn transfer_with_transact_self_reserve_sibling() { + TestNet::reset(); + + ParaA::execute_with(|| { + assert_ok!(ParaTokens::deposit(CurrencyId::A, &ALICE, 100_000)); + }); + + // ParaB::execute_with(|| { + // assert_ok!(ParaTokens::deposit(CurrencyId::B, &sibling_a_account(), + // 100_000)); assert_ok!(ParaTokens::deposit(CurrencyId::A, &BOB, 100_000)); + // }); + + let remark = para::Call::System(frame_system::Call::::remark_with_event { remark: vec![1, 2, 3] }); + let encoded_call: Vec = remark.encode().into(); + let bounded_vec = BoundedVec::try_from(encoded_call).unwrap(); + + ParaA::execute_with(|| { + assert_ok!(ParaXTokens::transfer_with_transact( + Some(ALICE).into(), + CurrencyId::A, + 50_000, + 2, // PARA_B_ID, + 4_000, + bounded_vec.clone(), + // Will cost at 3_000 + 10_000 + )); + + assert_eq!(ParaTokens::free_balance(CurrencyId::A, &ALICE), 50_000); + }); + + ParaB::execute_with(|| { + assert!(para::System::events() + .iter() + .any(|r| matches!(r.event, para::Event::System(frame_system::Event::Remarked { .. })))); + + // assert_eq!(ParaTokens::free_balance(CurrencyId::A, &ALICE), 50_000); + // assert_eq!(ParaTokens::free_balance(CurrencyId::B, &BOB), 460); + }); +} + +#[test] +fn transfer_with_transact_to_non_reserve_sibling() { + TestNet::reset(); + + ParaA::execute_with(|| { + assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 1_000); + assert_ok!(ParaTokens::deposit(CurrencyId::R, &ALICE, 100_000)); + }); + + Relay::execute_with(|| { + let _ = RelayBalances::deposit_creating(¶_a_account(), 100_000); + }); + + let remark = para::Call::System(frame_system::Call::::remark_with_event { remark: vec![1, 2, 3] }); + let encoded_call: Vec = remark.encode().into(); + let bounded_vec = BoundedVec::try_from(encoded_call).unwrap(); + + ParaA::execute_with(|| { + assert_ok!(ParaXTokens::transfer_with_transact( + Some(ALICE).into(), + CurrencyId::R, + 50_000, + 2, // PARA_B_ID, + 4_000, + bounded_vec.clone(), + // Will cost at 3_000 + 10_000 + )); + + assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 51_000); + }); + + ParaB::execute_with(|| { + assert!(para::System::events() + .iter() + .any(|r| matches!(r.event, para::Event::System(frame_system::Event::Remarked { .. })))); + + // assert_eq!(ParaTokens::free_balance(CurrencyId::R, + // &sibling_a_account()), 50_000); + // assert_eq!(ParaTokens::free_balance(CurrencyId::R, &BOB), 460); + }); +} + +#[test] +fn transfer_with_transact_to_reserve_sibling() { + TestNet::reset(); + + ParaA::execute_with(|| { + assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 100_000)); + assert_ok!(ParaTokens::deposit(CurrencyId::A, &sibling_b_account(), 100_000)); + }); + + ParaB::execute_with(|| { + assert_ok!(ParaTokens::deposit(CurrencyId::B, &sibling_a_account(), 100_000)); + assert_ok!(ParaTokens::deposit(CurrencyId::A, &BOB, 100_000)); + }); + + let remark = para::Call::System(frame_system::Call::::remark_with_event { remark: vec![1, 2, 3] }); + let encoded_call: Vec = remark.encode().into(); + let bounded_vec = BoundedVec::try_from(encoded_call).unwrap(); + + ParaA::execute_with(|| { + assert_ok!(ParaXTokens::transfer_with_transact( + Some(ALICE).into(), + CurrencyId::B, + 50_000, + 2, // PARA_B_ID, + 4_000, + bounded_vec.clone(), + // Will cost at 3_000 + 10_000 + )); + + assert_eq!(ParaTokens::free_balance(CurrencyId::B, &ALICE), 50_000); + }); + + ParaB::execute_with(|| { + assert!(para::System::events() + .iter() + .any(|r| matches!(r.event, para::Event::System(frame_system::Event::Remarked { .. })))); + + assert_eq!(ParaTokens::free_balance(CurrencyId::B, &sibling_a_account()), 50_000); + // assert_eq!(ParaTokens::free_balance(CurrencyId::B, &BOB), 460); + }); +} + #[test] fn send_sibling_asset_to_reserve_sibling_with_fee() { TestNet::reset(); From 6bcf2bbea6949469d9bbaba58991757a4a8e7b32 Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Tue, 12 Jul 2022 22:34:29 +0300 Subject: [PATCH 06/15] Add AllowTransactFrom barrier Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 4 +- xtokens/src/mock/para.rs | 117 +++++++++++++++++++++------------------ xtokens/src/tests.rs | 2 +- 3 files changed, 68 insertions(+), 55 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index e301d05c0..272e923ca 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -530,7 +530,9 @@ pub mod module { WithdrawAsset(transact_fee_assets.clone()), BuyExecution { fees: transact_fee_asset, - weight_limit: WeightLimit::Limited(dest_weight), + // TODO: make it work with Limited... + // weight_limit: WeightLimit::Limited(dest_weight), + weight_limit: WeightLimit::Unlimited, }, Transact { // SovereignAccount of the user, not the chain diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index df007864e..368893b37 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -148,65 +148,76 @@ match_types! { pub type XcmRouter = ParachainXcmRouter; -// /// Allows execution from `origin` if it is contained in `T` (i.e. -// /// `T::Contains(origin)`) taking payments into account. -// /// -// /// Only allows for `TeleportAsset`, `WithdrawAsset`, `ClaimAsset` and -// /// `ReserveAssetDeposit` XCMs because they are the only ones that place -// assets /// in the Holding Register to pay for execution. -// pub struct AllowTopLevelPaidExecutionFrom(PhantomData); -// impl> ShouldExecute for -// AllowTopLevelPaidExecutionFrom { fn should_execute( -// origin: &MultiLocation, -// message: &mut Xcm, -// max_weight: Weight, -// _weight_credit: &mut Weight, -// ) -> Result<(), ()> { -// // log::trace!( -// // target: "xcm::barriers", -// // "AllowTopLevelPaidExecutionFrom origin: {:?}, message: {:?}, max_weight: -// // {:?}, weight_credit: {:?}", origin, message, max_weight, _weight_credit, -// // ); -// ensure!(T::contains(origin), ()); -// let mut iter = message.0.iter_mut(); -// let i = iter.next().ok_or(())?; -// match i { -// ReceiveTeleportedAsset(..) -// | WithdrawAsset(..) -// | ReserveAssetDeposited(..) -// | ClaimAsset { .. } -// | Transact { .. } => (), -// _ => return Err(()), -// } -// let mut i = iter.next().ok_or(())?; -// while let ClearOrigin = i { -// i = iter.next().ok_or(())?; -// } -// match i { -// BuyExecution { -// weight_limit: Limited(ref mut weight), -// .. -// } if *weight >= max_weight => { -// *weight = max_weight; -// Ok(()) -// } -// BuyExecution { -// ref mut weight_limit, .. -// } if weight_limit == &Unlimited => { -// *weight_limit = Limited(max_weight); -// Ok(()) -// } -// _ => Err(()), -// } -// } -// } +/// Allows execution from `origin` if it is contained in `T` (i.e. +/// `T::Contains(origin)`) taking payments into account. +/// +/// Only allows for `TeleportAsset`, `WithdrawAsset`, `ClaimAsset` and +/// `ReserveAssetDeposit` XCMs because they are the only ones that place +/// assets in the Holding Register to pay for execution. +pub struct AllowTransactFrom(PhantomData); +impl> ShouldExecute for AllowTransactFrom { + fn should_execute( + origin: &MultiLocation, + message: &mut Xcm, + max_weight: Weight, + _weight_credit: &mut Weight, + ) -> Result<(), ()> { + // log::trace!( + // target: "xcm::barriers", + // "AllowTopLevelPaidExecutionFrom origin: {:?}, message: {:?}, max_weight: + // {:?}, weight_credit: {:?}", origin, message, max_weight, _weight_credit, + // ); + println!("AllowTransactFrom message: {:?} \n", message); + println!("AllowTransactFrom max_weight: {:?} \n", max_weight); + ensure!(T::contains(origin), ()); + + let mut iter = message.0.iter_mut(); + let i = iter.next().ok_or(())?; + println!("AllowTransactFrom 1"); + // TODO: maybe check that we're not descending into Here + match i { + DescendOrigin(..) => (), + _ => return Err(()), + } + let i = iter.next().ok_or(())?; + println!("AllowTransactFrom 2"); + match i { + WithdrawAsset(..) => (), + _ => return Err(()), + } + let i = iter.next().ok_or(())?; + println!("AllowTransactFrom 3"); + println!("AllowTransactFrom i: {:?} \n", i); + match i { + BuyExecution { + weight_limit: Limited(ref mut weight), + .. + } if *weight >= max_weight => { + *weight = max_weight; + Ok(()) + } + BuyExecution { + ref mut weight_limit, .. + } if weight_limit == &Unlimited => { + *weight_limit = Limited(max_weight); + Ok(()) + } + _ => { + println!("AllowTransactFrom 4"); + + Err(()) + } + } + } +} pub type Barrier = ( TakeWeightCredit, AllowTopLevelPaidExecutionFrom, + AllowTransactFrom, // This has to be after AllowTopLevelPaidExecutionFrom or 2 tests will fail: // tests::sending_sibling_asset_to_reserve_sibling_with_relay_fee_not_enough // tests::sending_sibling_asset_to_reserve_sibling_with_relay_fee_works - AllowUnpaidExecutionFrom, + // AllowUnpaidExecutionFrom, ); //pub type Barrier = AllowUnpaidExecutionFrom; diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 84f9eb058..5c2fcc444 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -324,7 +324,7 @@ fn transfer_with_transact_self_reserve_sibling() { 50_000, 2, // PARA_B_ID, 4_000, - bounded_vec.clone(), + bounded_vec, // Will cost at 3_000 10_000 )); From 1bb7400f29737e1b474838c504dcce4716ccf9dc Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 11:04:56 +0300 Subject: [PATCH 07/15] Assert with user sovereign account Signed-off-by: Georgi Zlatarev --- xtokens/src/mock/para.rs | 12 ++- xtokens/src/mock/relay.rs | 6 +- xtokens/src/tests.rs | 185 +++++++++++++++++++++++++++++++------- 3 files changed, 166 insertions(+), 37 deletions(-) diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 368893b37..f75c70b6a 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -110,11 +110,13 @@ parameter_types! { pub Ancestry: MultiLocation = Parachain(ParachainInfo::parachain_id().into()).into(); } +pub type MyAccount32Hash = Account32Hash; + pub type LocationToAccountId = ( ParentIsPreset, SiblingParachainConvertsVia, AccountId32Aliases, - Account32Hash, + MyAccount32Hash, ); pub type XcmOriginToCallOrigin = ( @@ -270,6 +272,8 @@ impl WeightTrader for AllTokensAreCreatedEqualToWeight { } } +pub type FixedWeigher = FixedWeightBounds, Call, ConstU32<100>>; + pub struct XcmConfig; impl Config for XcmConfig { type Call = Call; @@ -280,7 +284,7 @@ impl Config for XcmConfig { type IsTeleporter = (); type LocationInverter = LocationInverter; type Barrier = Barrier; - type Weigher = FixedWeightBounds, Call, ConstU32<100>>; + type Weigher = FixedWeigher; type Trader = AllTokensAreCreatedEqualToWeight; type ResponseHandler = (); type AssetTrap = PolkadotXcm; @@ -331,7 +335,7 @@ impl pallet_xcm::Config for Runtime { type XcmExecutor = XcmExecutor; type XcmTeleportFilter = Nothing; type XcmReserveTransferFilter = Everything; - type Weigher = FixedWeightBounds, Call, ConstU32<100>>; + type Weigher = FixedWeigher; type LocationInverter = LocationInverter; type Origin = Origin; type Call = Call; @@ -388,7 +392,7 @@ impl orml_xtokens::Config for Runtime { type MultiLocationsFilter = ParentOrParachains; type MinXcmFee = ParachainMinFee; type XcmExecutor = XcmExecutor; - type Weigher = FixedWeightBounds, Call, ConstU32<100>>; + type Weigher = FixedWeigher; type BaseXcmWeight = ConstU64<100_000_000>; type LocationInverter = LocationInverter; type MaxAssetsForTransfer = MaxAssetsForTransfer; diff --git a/xtokens/src/mock/relay.rs b/xtokens/src/mock/relay.rs index d09747992..3110cfdc5 100644 --- a/xtokens/src/mock/relay.rs +++ b/xtokens/src/mock/relay.rs @@ -89,6 +89,8 @@ type LocalOriginConverter = ( pub type XcmRouter = super::RelayChainXcmRouter; pub type Barrier = (TakeWeightCredit, AllowTopLevelPaidExecutionFrom); +pub type RelayWeigher = FixedWeightBounds, Call, ConstU32<100>>; + pub struct XcmConfig; impl Config for XcmConfig { type Call = Call; @@ -99,7 +101,7 @@ impl Config for XcmConfig { type IsTeleporter = (); type LocationInverter = LocationInverter; type Barrier = Barrier; - type Weigher = FixedWeightBounds, Call, ConstU32<100>>; + type Weigher = RelayWeigher; type Trader = UsingComponents, KsmLocation, AccountId, Balances, ()>; type ResponseHandler = (); type AssetTrap = (); @@ -119,7 +121,7 @@ impl pallet_xcm::Config for Runtime { type XcmExecutor = XcmExecutor; type XcmTeleportFilter = Everything; type XcmReserveTransferFilter = Everything; - type Weigher = FixedWeightBounds, Call, ConstU32<100>>; + type Weigher = RelayWeigher; type LocationInverter = LocationInverter; type Origin = Origin; type Call = Call; diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 5c2fcc444..4cc74fe68 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -4,10 +4,17 @@ use super::*; use codec::Encode; use cumulus_primitives_core::ParaId; use frame_support::{assert_err, assert_noop, assert_ok, traits::Currency, BoundedVec}; -use mock::*; +use mock::{ + para::{FixedWeigher, MyAccount32Hash}, + relay::RelayWeigher, + *, +}; use orml_traits::{ConcreteFungibleAsset, MultiCurrency}; use polkadot_parachain::primitives::Sibling; use sp_runtime::{traits::AccountIdConversion, AccountId32}; +use xcm::latest::OriginKind::SovereignAccount; +use xcm_builder::Account32Hash; +use xcm_executor::traits::Convert; use xcm_simulator::TestExt; fn para_a_account() -> AccountId32 { @@ -308,11 +315,6 @@ fn transfer_with_transact_self_reserve_sibling() { assert_ok!(ParaTokens::deposit(CurrencyId::A, &ALICE, 100_000)); }); - // ParaB::execute_with(|| { - // assert_ok!(ParaTokens::deposit(CurrencyId::B, &sibling_a_account(), - // 100_000)); assert_ok!(ParaTokens::deposit(CurrencyId::A, &BOB, 100_000)); - // }); - let remark = para::Call::System(frame_system::Call::::remark_with_event { remark: vec![1, 2, 3] }); let encoded_call: Vec = remark.encode().into(); let bounded_vec = BoundedVec::try_from(encoded_call).unwrap(); @@ -336,23 +338,61 @@ fn transfer_with_transact_self_reserve_sibling() { assert!(para::System::events() .iter() .any(|r| matches!(r.event, para::Event::System(frame_system::Event::Remarked { .. })))); - - // assert_eq!(ParaTokens::free_balance(CurrencyId::A, &ALICE), 50_000); - // assert_eq!(ParaTokens::free_balance(CurrencyId::B, &BOB), 460); + let user_multilocaiton = ( + Parent, + Parachain(1), + Junction::AccountId32 { + network: NetworkId::Any, + id: ALICE.into(), + }, + ) + .into(); + + let mut token_transfer_msg = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); + let token_transfer_weight = FixedWeigher::weight(&mut token_transfer_msg).unwrap(); + println!("token_transfer_weight: {:?} \n", token_transfer_weight); + + let mut transact_msg = Xcm(vec![ + ClearOrigin, + ClearOrigin, + ClearOrigin, + ClearOrigin, + ClearOrigin, + Transact { + origin_type: SovereignAccount, + require_weight_at_most: 4_000, + call: vec![].into(), + }, + ]); + let transact_msg_weight = FixedWeigher::weight(&mut transact_msg).unwrap(); + println!("transact_msg_weight: {:?} \n", transact_msg_weight); + + let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); + // Token transfer --> 50_000 - 40 = 49_960. + // Weight is 40 because the token transfer has 4 messages each with 10 weight. + // Transact message --> 10_000 - 4_060 = 5_940 refunded. + // Weight is 4060 because there are 6 instructions * 10 weight + the 4000 + // required_weight_at_most in the Transact instruction + // Finally --> 50_000 - 40 - 4_060 = 45_900. + assert_eq!( + ParaTokens::free_balance(CurrencyId::A, &user_sovereign_account), + 50_000u128 - token_transfer_weight as u128 - transact_msg_weight as u128 + ); }); } #[test] -fn transfer_with_transact_to_non_reserve_sibling() { +fn transfer_with_transact_to_reserve_sibling() { TestNet::reset(); ParaA::execute_with(|| { - assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 1_000); - assert_ok!(ParaTokens::deposit(CurrencyId::R, &ALICE, 100_000)); + assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 100_000)); + assert_ok!(ParaTokens::deposit(CurrencyId::A, &sibling_b_account(), 100_000)); }); - Relay::execute_with(|| { - let _ = RelayBalances::deposit_creating(¶_a_account(), 100_000); + ParaB::execute_with(|| { + assert_ok!(ParaTokens::deposit(CurrencyId::B, &sibling_a_account(), 100_000)); + assert_ok!(ParaTokens::deposit(CurrencyId::A, &BOB, 100_000)); }); let remark = para::Call::System(frame_system::Call::::remark_with_event { remark: vec![1, 2, 3] }); @@ -362,7 +402,7 @@ fn transfer_with_transact_to_non_reserve_sibling() { ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( Some(ALICE).into(), - CurrencyId::R, + CurrencyId::B, 50_000, 2, // PARA_B_ID, 4_000, @@ -371,7 +411,7 @@ fn transfer_with_transact_to_non_reserve_sibling() { 10_000 )); - assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 51_000); + assert_eq!(ParaTokens::free_balance(CurrencyId::B, &ALICE), 50_000); }); ParaB::execute_with(|| { @@ -379,24 +419,62 @@ fn transfer_with_transact_to_non_reserve_sibling() { .iter() .any(|r| matches!(r.event, para::Event::System(frame_system::Event::Remarked { .. })))); - // assert_eq!(ParaTokens::free_balance(CurrencyId::R, - // &sibling_a_account()), 50_000); - // assert_eq!(ParaTokens::free_balance(CurrencyId::R, &BOB), 460); + assert_eq!(ParaTokens::free_balance(CurrencyId::B, &sibling_a_account()), 50_000); + + let user_multilocaiton = ( + Parent, + Parachain(1), + Junction::AccountId32 { + network: NetworkId::Any, + id: ALICE.into(), + }, + ) + .into(); + + let mut token_transfer_msg = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); + let token_transfer_weight = FixedWeigher::weight(&mut token_transfer_msg).unwrap(); + println!("token_transfer_weight: {:?} \n", token_transfer_weight); + + let mut transact_msg = Xcm(vec![ + ClearOrigin, + ClearOrigin, + ClearOrigin, + ClearOrigin, + ClearOrigin, + Transact { + origin_type: SovereignAccount, + require_weight_at_most: 4_000, + call: vec![].into(), + }, + ]); + let transact_msg_weight = FixedWeigher::weight(&mut transact_msg).unwrap(); + println!("transact_msg_weight: {:?} \n", transact_msg_weight); + + let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); + // Token transfer --> 50_000 - 40 = 49_960. + // Weight is 40 because the token transfer has 4 messages each with 10 weight. + // Transact message --> 10_000 - 4_060 = 5_940 refunded. + // Weight is 4060 because there are 6 instructions * 10 weight + the 4000 + // required_weight_at_most in the Transact instruction + // Finally --> 50_000 - 40 - 4_060 = 45_900. + assert_eq!( + ParaTokens::free_balance(CurrencyId::B, &user_sovereign_account), + 50_000u128 - token_transfer_weight as u128 - transact_msg_weight as u128 + ); }); } #[test] -fn transfer_with_transact_to_reserve_sibling() { +fn transfer_with_transact_to_non_reserve_sibling() { TestNet::reset(); ParaA::execute_with(|| { - assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 100_000)); - assert_ok!(ParaTokens::deposit(CurrencyId::A, &sibling_b_account(), 100_000)); + assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 1_000); + assert_ok!(ParaTokens::deposit(CurrencyId::R, &ALICE, 100_000)); }); - ParaB::execute_with(|| { - assert_ok!(ParaTokens::deposit(CurrencyId::B, &sibling_a_account(), 100_000)); - assert_ok!(ParaTokens::deposit(CurrencyId::A, &BOB, 100_000)); + Relay::execute_with(|| { + let _ = RelayBalances::deposit_creating(¶_a_account(), 100_000); }); let remark = para::Call::System(frame_system::Call::::remark_with_event { remark: vec![1, 2, 3] }); @@ -406,7 +484,7 @@ fn transfer_with_transact_to_reserve_sibling() { ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( Some(ALICE).into(), - CurrencyId::B, + CurrencyId::R, 50_000, 2, // PARA_B_ID, 4_000, @@ -415,7 +493,7 @@ fn transfer_with_transact_to_reserve_sibling() { 10_000 )); - assert_eq!(ParaTokens::free_balance(CurrencyId::B, &ALICE), 50_000); + assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 51_000); }); ParaB::execute_with(|| { @@ -423,8 +501,55 @@ fn transfer_with_transact_to_reserve_sibling() { .iter() .any(|r| matches!(r.event, para::Event::System(frame_system::Event::Remarked { .. })))); - assert_eq!(ParaTokens::free_balance(CurrencyId::B, &sibling_a_account()), 50_000); - // assert_eq!(ParaTokens::free_balance(CurrencyId::B, &BOB), 460); + let user_multilocaiton = ( + Parent, + Parachain(1), + Junction::AccountId32 { + network: NetworkId::Any, + id: ALICE.into(), + }, + ) + .into(); + + let mut token_transfer_msg = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); + let token_transfer_weight = FixedWeigher::weight(&mut token_transfer_msg).unwrap(); + println!("token_transfer_weight: {:?} \n", token_transfer_weight); + let mut token_transfer_msg_on_relay = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); + let token_transfer_weight_on_relay = RelayWeigher::weight(&mut token_transfer_msg_on_relay).unwrap(); + + let mut transact_msg = Xcm(vec![ + ClearOrigin, + ClearOrigin, + ClearOrigin, + ClearOrigin, + ClearOrigin, + Transact { + origin_type: SovereignAccount, + require_weight_at_most: 4_000, + call: vec![].into(), + }, + ]); + let transact_msg_weight = FixedWeigher::weight(&mut transact_msg).unwrap(); + println!("transact_msg_weight: {:?} \n", transact_msg_weight); + + let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); + // Token transfer --> 50_000 - 40 = 49_960. + // Weight is 40 because the token transfer has 4 messages each with 10 weight. + // Transact message --> 10_000 - 4_060 = 5_940 refunded. + // Weight is 4060 because there are 6 instructions * 10 weight + the 4000 + // required_weight_at_most in the Transact instruction + // Finally --> 50_000 - 40 - 4_060 = 45_900. + assert_eq!( + ParaTokens::free_balance(CurrencyId::R, &user_sovereign_account), + 50_000u128 + - token_transfer_weight_on_relay as u128 + - token_transfer_weight as u128 + - transact_msg_weight as u128 + ); + + // assert_eq!(ParaTokens::free_balance(CurrencyId::R, + // &sibling_a_account()), 50_000); + // assert_eq!(ParaTokens::free_balance(CurrencyId::R, &BOB), 460); }); } @@ -1093,8 +1218,6 @@ fn send_as_sovereign() { }); ParaA::execute_with(|| { - use xcm::latest::OriginKind::SovereignAccount; - let call = relay::Call::System(frame_system::Call::::remark_with_event { remark: vec![1, 1, 1] }); let assets: MultiAsset = (Here, 1_000_000_000_000).into(); From 87242813ffe50186d3cfe93ea4948d2bd8b2d36f Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 11:09:58 +0300 Subject: [PATCH 08/15] Make the Barrier accept Limited weight Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 2 +- xtokens/src/mock/para.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 272e923ca..1fc1badb1 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -532,7 +532,7 @@ pub mod module { fees: transact_fee_asset, // TODO: make it work with Limited... // weight_limit: WeightLimit::Limited(dest_weight), - weight_limit: WeightLimit::Unlimited, + weight_limit: WeightLimit::Limited(dest_weight), }, Transact { // SovereignAccount of the user, not the chain diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index f75c70b6a..7d08ec2de 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -194,7 +194,9 @@ impl> ShouldExecute for AllowTransactFrom { BuyExecution { weight_limit: Limited(ref mut weight), .. - } if *weight >= max_weight => { + } => { + // max_weight will always be more than weight because + // max_weight = weight + weigher::weight(incoming_message) *weight = max_weight; Ok(()) } From a2a315017d22879b0e394673c62abbec1c8c7ce0 Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 11:13:50 +0300 Subject: [PATCH 09/15] Make the Barrier accept Limited weight Signed-off-by: Georgi Zlatarev --- xtokens/src/mock/para.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 7d08ec2de..cca15cd3e 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -191,18 +191,9 @@ impl> ShouldExecute for AllowTransactFrom { println!("AllowTransactFrom 3"); println!("AllowTransactFrom i: {:?} \n", i); match i { - BuyExecution { - weight_limit: Limited(ref mut weight), - .. - } => { - // max_weight will always be more than weight because - // max_weight = weight + weigher::weight(incoming_message) - *weight = max_weight; - Ok(()) - } BuyExecution { ref mut weight_limit, .. - } if weight_limit == &Unlimited => { + } => { *weight_limit = Limited(max_weight); Ok(()) } From 7241e8cfa747d6e31c16ba436b514d044118c97c Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 11:19:24 +0300 Subject: [PATCH 10/15] Expect refund instructions as well Signed-off-by: Georgi Zlatarev --- xtokens/src/mock/para.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index cca15cd3e..5768307d8 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -195,14 +195,30 @@ impl> ShouldExecute for AllowTransactFrom { ref mut weight_limit, .. } => { *weight_limit = Limited(max_weight); - Ok(()) } _ => { println!("AllowTransactFrom 4"); - Err(()) + return Err(()); } } + let i = iter.next().ok_or(())?; + match i { + Transact { .. } => (), + _ => return Err(()), + } + let i = iter.next().ok_or(())?; + match i { + RefundSurplus { .. } => (), + _ => return Err(()), + } + let i = iter.next().ok_or(())?; + match i { + DepositAsset { .. } => (), + _ => return Err(()), + } + + Ok(()) } } pub type Barrier = ( From e057b51cd3c89337aa17548d130baa32c439b48e Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 11:25:44 +0300 Subject: [PATCH 11/15] Refund surplus for transact-only as well Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 1fc1badb1..1978112fd 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -442,6 +442,8 @@ pub mod module { let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); let dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); + let refund_recipient = T::SelfLocation::get(); + Self::send_transact( transact_fee_location, transact_fee_amount, @@ -449,7 +451,7 @@ pub mod module { origin_location, dest_weight, encoded_call_data, - None, + refund_recipient, )?; Ok(()) @@ -499,7 +501,7 @@ pub mod module { origin_location, dest_weight, encoded_call_data, - Some(override_recipient), + override_recipient, )?; Ok(()) @@ -512,7 +514,7 @@ pub mod module { origin_location: MultiLocation, dest_weight: Weight, encoded_call_data: BoundedVec, - override_recipient: Option, + refund_recipient: MultiLocation, ) -> DispatchResult { let transact_fee_asset: MultiAsset = (transact_fee_location, transact_fee_amount.into()).into(); // We can get rid of the interior now. @@ -525,13 +527,11 @@ pub mod module { let mut transact_fee_assets = MultiAssets::new(); transact_fee_assets.push(transact_fee_asset.clone()); - let mut instructions = vec![ + let instructions = vec![ DescendOrigin(origin_location.clone().interior), WithdrawAsset(transact_fee_assets.clone()), BuyExecution { fees: transact_fee_asset, - // TODO: make it work with Limited... - // weight_limit: WeightLimit::Limited(dest_weight), weight_limit: WeightLimit::Limited(dest_weight), }, Transact { @@ -540,19 +540,14 @@ pub mod module { require_weight_at_most: dest_weight, call: encoded_call_data.into_inner().into(), }, + RefundSurplus, + DepositAsset { + assets: All.into(), + max_assets: transact_fee_assets.len() as u32, + beneficiary: refund_recipient, + }, ]; - if let Some(recipient) = override_recipient { - instructions.extend(vec![ - RefundSurplus, - DepositAsset { - assets: All.into(), - max_assets: transact_fee_assets.len() as u32, - beneficiary: recipient, - }, - ]); - } - T::XcmSender::send_xcm(dest_chain_location, Xcm(instructions)).map_err(|_| Error::::SendFailure)?; Ok(()) From 68edfe760bb006ef90357305f849dcd5d5289c5c Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 15:00:51 +0300 Subject: [PATCH 12/15] Clean up Signed-off-by: Georgi Zlatarev --- xtokens/Cargo.toml | 2 + xtokens/src/lib.rs | 52 +++++++++--------- xtokens/src/mock/para.rs | 83 ++++++++++++---------------- xtokens/src/mock/relay.rs | 7 ++- xtokens/src/tests.rs | 111 +++++++++++++++++--------------------- 5 files changed, 112 insertions(+), 143 deletions(-) diff --git a/xtokens/Cargo.toml b/xtokens/Cargo.toml index dd1b7e1c4..e86c24e01 100644 --- a/xtokens/Cargo.toml +++ b/xtokens/Cargo.toml @@ -11,6 +11,7 @@ edition = "2021" scale-info = { version = "2.1.2", default-features = false, features = ["derive"] } serde = { version = "1.0.136", optional = true } codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false } +log = "0.4.16" # substrate sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false , branch = "polkadot-v0.9.24" } @@ -59,6 +60,7 @@ std = [ "serde", "codec/std", "scale-info/std", + "log/std", "sp-runtime/std", "sp-std/std", "sp-io/std", diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 1978112fd..b996592f3 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -359,6 +359,7 @@ pub mod module { encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { + // TODO: make the limit u8 or hard-code the constant in the ORML code ensure!(T::MaxTransactSize::get() <= 256u32, Error::::TransactTooLarge); let who = ensure_signed(origin)?; @@ -377,6 +378,7 @@ pub mod module { encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { + // TODO: make the limit u8 or hard-code the constant in the ORML code ensure!(T::MaxTransactSize::get() <= 256u32, Error::::TransactTooLarge); let who = ensure_signed(origin)?; @@ -439,16 +441,14 @@ pub mod module { let transact_fee_location: MultiLocation = T::CurrencyIdConvert::convert(transact_currency_id) .ok_or(Error::::NotCrossChainTransferableCurrency)?; - let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); + let origin_location_interior = T::AccountIdToMultiLocation::convert(who).interior; let dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); - let refund_recipient = T::SelfLocation::get(); - Self::send_transact( transact_fee_location, transact_fee_amount, - dest_chain_location.clone(), - origin_location, + dest_chain_location, + origin_location_interior, dest_weight, encoded_call_data, refund_recipient, @@ -466,27 +466,27 @@ pub mod module { encoded_call_data: BoundedVec, transact_fee_amount: T::Balance, ) -> DispatchResult { - let transact_fee_location: MultiLocation = - T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; + ensure!(!amount.is_zero(), Error::::ZeroAmount); let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); let mut dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); - // Need to append some interior to pass is_valid check later on. - // Only the chain part is needed for everything else. - let _ = dest_chain_location.append_with(origin_location.clone().interior); - - ensure!(!amount.is_zero(), Error::::ZeroAmount); + let origin_location_interior = origin_location.clone().interior; + // Need to append some interior to pass the `contains()` check and later the + // `is_valid()` check in `do_transfer_multiassets()`. Only the chain part is + // needed afterwards. + let _ = dest_chain_location.append_with(origin_location_interior.clone()); ensure!( T::MultiLocationsFilter::contains(&dest_chain_location), Error::::NotSupportedMultiLocation ); + let transact_fee_location: MultiLocation = + T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; let transfer_asset: MultiAsset = (transact_fee_location.clone(), amount.into()).into(); let mut override_recipient = T::SelfLocation::get(); - let _ = override_recipient.append_with(origin_location.clone().interior); - + let _ = override_recipient.append_with(origin_location_interior.clone()); Self::do_transfer_multiassets( - who.clone(), + who, vec![transfer_asset.clone()].into(), transfer_asset, dest_chain_location.clone(), @@ -494,11 +494,12 @@ pub mod module { Some(override_recipient.clone()), )?; + let dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; Self::send_transact( transact_fee_location, transact_fee_amount, - dest_chain_location.clone(), - origin_location, + dest_chain_location, + origin_location_interior, dest_weight, encoded_call_data, override_recipient, @@ -511,25 +512,24 @@ pub mod module { transact_fee_location: MultiLocation, transact_fee_amount: T::Balance, dest_chain_location: MultiLocation, - origin_location: MultiLocation, + origin_location_interior: Junctions, dest_weight: Weight, encoded_call_data: BoundedVec, refund_recipient: MultiLocation, ) -> DispatchResult { - let transact_fee_asset: MultiAsset = (transact_fee_location, transact_fee_amount.into()).into(); - // We can get rid of the interior now. - let dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; let ancestry = T::LocationInverter::ancestry(); - let transact_fee_asset = transact_fee_asset + let mut transact_fee_asset: MultiAsset = (transact_fee_location, transact_fee_amount.into()).into(); + transact_fee_asset = transact_fee_asset .clone() .reanchored(&dest_chain_location, &ancestry) .map_err(|_| Error::::CannotReanchor)?; let mut transact_fee_assets = MultiAssets::new(); transact_fee_assets.push(transact_fee_asset.clone()); + let transact_fee_assets_len = transact_fee_assets.len(); let instructions = vec![ - DescendOrigin(origin_location.clone().interior), - WithdrawAsset(transact_fee_assets.clone()), + DescendOrigin(origin_location_interior), + WithdrawAsset(transact_fee_assets), BuyExecution { fees: transact_fee_asset, weight_limit: WeightLimit::Limited(dest_weight), @@ -543,7 +543,7 @@ pub mod module { RefundSurplus, DepositAsset { assets: All.into(), - max_assets: transact_fee_assets.len() as u32, + max_assets: transact_fee_assets_len as u32, beneficiary: refund_recipient, }, ]; @@ -735,7 +735,6 @@ pub mod module { let mut assets_to_fee_reserve = MultiAssets::new(); let asset_to_fee_reserve = subtract_fee(&fee, min_xcm_fee); assets_to_fee_reserve.push(asset_to_fee_reserve.clone()); - println!("im not here right ?//////////////////////////////// \n"); // First xcm sent to fee reserve chain and routed to dest chain. Self::execute_and_send_reserve_kind_xcm( @@ -804,7 +803,6 @@ pub mod module { }; let weight = T::Weigher::weight(&mut msg).map_err(|()| Error::::UnweighableMessage)?; - println!("origin_location: {:?} \n", origin_location); T::XcmExecutor::execute_xcm_in_credit(origin_location, msg, weight, weight) .ensure_complete() .map_err(|error| { diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 5768307d8..d559392f5 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -20,10 +20,9 @@ use pallet_xcm::XcmPassthrough; use polkadot_parachain::primitives::Sibling; use xcm::latest::prelude::*; use xcm_builder::{ - Account32Hash, AccountId32Aliases, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, EnsureXcmOrigin, - FixedWeightBounds, LocationInverter, ParentIsPreset, RelayChainAsNative, SiblingParachainAsNative, - SiblingParachainConvertsVia, SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, - TakeWeightCredit, + Account32Hash, AccountId32Aliases, AllowTopLevelPaidExecutionFrom, EnsureXcmOrigin, FixedWeightBounds, + LocationInverter, ParentIsPreset, RelayChainAsNative, SiblingParachainAsNative, SiblingParachainConvertsVia, + SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, }; use xcm_executor::{ traits::{ShouldExecute, WeightTrader}, @@ -150,12 +149,12 @@ match_types! { pub type XcmRouter = ParachainXcmRouter; -/// Allows execution from `origin` if it is contained in `T` (i.e. -/// `T::Contains(origin)`) taking payments into account. +/// Allows execution of a Transact instruction from `origin` if it is contained +/// in `T` (i.e. `T::Contains(origin)`) taking payments into account. /// -/// Only allows for `TeleportAsset`, `WithdrawAsset`, `ClaimAsset` and -/// `ReserveAssetDeposit` XCMs because they are the only ones that place -/// assets in the Holding Register to pay for execution. +/// Only allows for a specific set of instructions: +/// DescendOrigin + WithdrawAsset + BuyExecution + Transact + RefundSurplus + +/// DepositAsset pub struct AllowTransactFrom(PhantomData); impl> ShouldExecute for AllowTransactFrom { fn should_execute( @@ -164,56 +163,51 @@ impl> ShouldExecute for AllowTransactFrom { max_weight: Weight, _weight_credit: &mut Weight, ) -> Result<(), ()> { - // log::trace!( - // target: "xcm::barriers", - // "AllowTopLevelPaidExecutionFrom origin: {:?}, message: {:?}, max_weight: - // {:?}, weight_credit: {:?}", origin, message, max_weight, _weight_credit, - // ); - println!("AllowTransactFrom message: {:?} \n", message); - println!("AllowTransactFrom max_weight: {:?} \n", max_weight); + log::trace!( + target: "xcm::barriers", + "AllowTransactFrom origin: {:?}, message: {:?}, max_weight: + {:?}, weight_credit: {:?}", origin, message, max_weight, _weight_credit, + ); + ensure!(T::contains(origin), ()); let mut iter = message.0.iter_mut(); - let i = iter.next().ok_or(())?; - println!("AllowTransactFrom 1"); + let next_instruction = iter.next().ok_or(())?; // TODO: maybe check that we're not descending into Here - match i { + match next_instruction { DescendOrigin(..) => (), _ => return Err(()), } - let i = iter.next().ok_or(())?; - println!("AllowTransactFrom 2"); - match i { + let next_instruction = iter.next().ok_or(())?; + match next_instruction { WithdrawAsset(..) => (), _ => return Err(()), } - let i = iter.next().ok_or(())?; - println!("AllowTransactFrom 3"); - println!("AllowTransactFrom i: {:?} \n", i); - match i { + let next_instruction = iter.next().ok_or(())?; + match next_instruction { BuyExecution { ref mut weight_limit, .. } => { *weight_limit = Limited(max_weight); } _ => { - println!("AllowTransactFrom 4"); - return Err(()); } } - let i = iter.next().ok_or(())?; - match i { + let next_instruction = iter.next().ok_or(())?; + match next_instruction { + // TODO: can at least check the length of the encoded vec, need a PR in polkadot for that + // TODO: if possible decode the call and match against a configurable set of allowed calls Transact { .. } => (), _ => return Err(()), } - let i = iter.next().ok_or(())?; - match i { + let next_instruction = iter.next().ok_or(())?; + match next_instruction { RefundSurplus { .. } => (), _ => return Err(()), } - let i = iter.next().ok_or(())?; - match i { + let next_instruction = iter.next().ok_or(())?; + match next_instruction { DepositAsset { .. } => (), _ => return Err(()), } @@ -225,12 +219,7 @@ pub type Barrier = ( TakeWeightCredit, AllowTopLevelPaidExecutionFrom, AllowTransactFrom, - // This has to be after AllowTopLevelPaidExecutionFrom or 2 tests will fail: - // tests::sending_sibling_asset_to_reserve_sibling_with_relay_fee_not_enough - // tests::sending_sibling_asset_to_reserve_sibling_with_relay_fee_works - // AllowUnpaidExecutionFrom, ); -//pub type Barrier = AllowUnpaidExecutionFrom; /// A trader who believes all tokens are created equal to "weight" of any chain, /// which is not true, but good enough to mock the fee payment of XCM execution. @@ -249,25 +238,21 @@ impl WeightTrader for AllTokensAreCreatedEqualToWeight { .next() .expect("Payment must be something; qed") .0; + let required = MultiAsset { id: asset_id.clone(), fun: Fungible(weight as u128), }; - println!("check 1 \n"); - println!("required: {:?} \n", required); + if let MultiAsset { fun: _, id: Concrete(ref id), } = &required { self.0 = id.clone(); - println!("id: {:?} \n", id); } - println!("check 2 \n"); let unused = payment.checked_sub(required).map_err(|_| XcmError::TooExpensive)?; - println!("check 3 \n"); - println!("unused: {:?} \n", unused); Ok(unused) } @@ -281,7 +266,7 @@ impl WeightTrader for AllTokensAreCreatedEqualToWeight { } } -pub type FixedWeigher = FixedWeightBounds, Call, ConstU32<100>>; +pub type ParaWeigher = FixedWeightBounds, Call, ConstU32<100>>; pub struct XcmConfig; impl Config for XcmConfig { @@ -293,7 +278,7 @@ impl Config for XcmConfig { type IsTeleporter = (); type LocationInverter = LocationInverter; type Barrier = Barrier; - type Weigher = FixedWeigher; + type Weigher = ParaWeigher; type Trader = AllTokensAreCreatedEqualToWeight; type ResponseHandler = (); type AssetTrap = PolkadotXcm; @@ -344,7 +329,7 @@ impl pallet_xcm::Config for Runtime { type XcmExecutor = XcmExecutor; type XcmTeleportFilter = Nothing; type XcmReserveTransferFilter = Everything; - type Weigher = FixedWeigher; + type Weigher = ParaWeigher; type LocationInverter = LocationInverter; type Origin = Origin; type Call = Call; @@ -401,7 +386,7 @@ impl orml_xtokens::Config for Runtime { type MultiLocationsFilter = ParentOrParachains; type MinXcmFee = ParachainMinFee; type XcmExecutor = XcmExecutor; - type Weigher = FixedWeigher; + type Weigher = ParaWeigher; type BaseXcmWeight = ConstU64<100_000_000>; type LocationInverter = LocationInverter; type MaxAssetsForTransfer = MaxAssetsForTransfer; diff --git a/xtokens/src/mock/relay.rs b/xtokens/src/mock/relay.rs index 3110cfdc5..709e853e2 100644 --- a/xtokens/src/mock/relay.rs +++ b/xtokens/src/mock/relay.rs @@ -11,9 +11,9 @@ use cumulus_primitives_core::ParaId; use polkadot_runtime_parachains::{configuration, origin, shared, ump}; use xcm::latest::prelude::*; use xcm_builder::{ - Account32Hash, AccountId32Aliases, AllowTopLevelPaidExecutionFrom, ChildParachainAsNative, - ChildParachainConvertsVia, CurrencyAdapter as XcmCurrencyAdapter, FixedWeightBounds, IsConcrete, LocationInverter, - SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, + AccountId32Aliases, AllowTopLevelPaidExecutionFrom, ChildParachainAsNative, ChildParachainConvertsVia, + CurrencyAdapter as XcmCurrencyAdapter, FixedWeightBounds, IsConcrete, LocationInverter, SignedAccountId32AsNative, + SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, }; use xcm_executor::{Config, XcmExecutor}; @@ -74,7 +74,6 @@ parameter_types! { pub type SovereignAccountOf = ( ChildParachainConvertsVia, AccountId32Aliases, - // Account32Hash, ); pub type LocalAssetTransactor = diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 4cc74fe68..d37965e26 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -5,7 +5,7 @@ use codec::Encode; use cumulus_primitives_core::ParaId; use frame_support::{assert_err, assert_noop, assert_ok, traits::Currency, BoundedVec}; use mock::{ - para::{FixedWeigher, MyAccount32Hash}, + para::{MyAccount32Hash, ParaWeigher}, relay::RelayWeigher, *, }; @@ -13,7 +13,6 @@ use orml_traits::{ConcreteFungibleAsset, MultiCurrency}; use polkadot_parachain::primitives::Sibling; use sp_runtime::{traits::AccountIdConversion, AccountId32}; use xcm::latest::OriginKind::SovereignAccount; -use xcm_builder::Account32Hash; use xcm_executor::traits::Convert; use xcm_simulator::TestExt; @@ -319,19 +318,22 @@ fn transfer_with_transact_self_reserve_sibling() { let encoded_call: Vec = remark.encode().into(); let bounded_vec = BoundedVec::try_from(encoded_call).unwrap(); + let transfer_amount = 50_000u128; + let transact_fee_amount = 10_000u128; + let dest_weight = 4_000; + ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( Some(ALICE).into(), CurrencyId::A, - 50_000, + transfer_amount, 2, // PARA_B_ID, - 4_000, + dest_weight, bounded_vec, - // Will cost at 3_000 - 10_000 + transact_fee_amount )); - assert_eq!(ParaTokens::free_balance(CurrencyId::A, &ALICE), 50_000); + assert_eq!(ParaTokens::free_balance(CurrencyId::A, &ALICE), transfer_amount); }); ParaB::execute_with(|| { @@ -349,8 +351,7 @@ fn transfer_with_transact_self_reserve_sibling() { .into(); let mut token_transfer_msg = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); - let token_transfer_weight = FixedWeigher::weight(&mut token_transfer_msg).unwrap(); - println!("token_transfer_weight: {:?} \n", token_transfer_weight); + let token_transfer_weight = ParaWeigher::weight(&mut token_transfer_msg).unwrap() as u128; let mut transact_msg = Xcm(vec![ ClearOrigin, @@ -360,23 +361,18 @@ fn transfer_with_transact_self_reserve_sibling() { ClearOrigin, Transact { origin_type: SovereignAccount, - require_weight_at_most: 4_000, + require_weight_at_most: dest_weight, call: vec![].into(), }, ]); - let transact_msg_weight = FixedWeigher::weight(&mut transact_msg).unwrap(); - println!("transact_msg_weight: {:?} \n", transact_msg_weight); + let transact_msg_weight = ParaWeigher::weight(&mut transact_msg).unwrap() as u128; let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); - // Token transfer --> 50_000 - 40 = 49_960. - // Weight is 40 because the token transfer has 4 messages each with 10 weight. - // Transact message --> 10_000 - 4_060 = 5_940 refunded. - // Weight is 4060 because there are 6 instructions * 10 weight + the 4000 - // required_weight_at_most in the Transact instruction - // Finally --> 50_000 - 40 - 4_060 = 45_900. + assert_eq!( ParaTokens::free_balance(CurrencyId::A, &user_sovereign_account), - 50_000u128 - token_transfer_weight as u128 - transact_msg_weight as u128 + // AllTokensAreCreatedEqualToWeight means 1 to 1 mapping of the weight to fee + transfer_amount - token_transfer_weight - transact_msg_weight ); }); } @@ -399,19 +395,22 @@ fn transfer_with_transact_to_reserve_sibling() { let encoded_call: Vec = remark.encode().into(); let bounded_vec = BoundedVec::try_from(encoded_call).unwrap(); + let transfer_amount = 50_000u128; + let transact_fee_amount = 10_000u128; + let dest_weight = 4_000; + ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( Some(ALICE).into(), CurrencyId::B, - 50_000, + transfer_amount, 2, // PARA_B_ID, - 4_000, + dest_weight, bounded_vec.clone(), - // Will cost at 3_000 - 10_000 + transact_fee_amount )); - assert_eq!(ParaTokens::free_balance(CurrencyId::B, &ALICE), 50_000); + assert_eq!(ParaTokens::free_balance(CurrencyId::B, &ALICE), transfer_amount); }); ParaB::execute_with(|| { @@ -419,7 +418,10 @@ fn transfer_with_transact_to_reserve_sibling() { .iter() .any(|r| matches!(r.event, para::Event::System(frame_system::Event::Remarked { .. })))); - assert_eq!(ParaTokens::free_balance(CurrencyId::B, &sibling_a_account()), 50_000); + assert_eq!( + ParaTokens::free_balance(CurrencyId::B, &sibling_a_account()), + transfer_amount + ); let user_multilocaiton = ( Parent, @@ -432,8 +434,7 @@ fn transfer_with_transact_to_reserve_sibling() { .into(); let mut token_transfer_msg = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); - let token_transfer_weight = FixedWeigher::weight(&mut token_transfer_msg).unwrap(); - println!("token_transfer_weight: {:?} \n", token_transfer_weight); + let token_transfer_weight = ParaWeigher::weight(&mut token_transfer_msg).unwrap() as u128; let mut transact_msg = Xcm(vec![ ClearOrigin, @@ -443,23 +444,17 @@ fn transfer_with_transact_to_reserve_sibling() { ClearOrigin, Transact { origin_type: SovereignAccount, - require_weight_at_most: 4_000, + require_weight_at_most: dest_weight, call: vec![].into(), }, ]); - let transact_msg_weight = FixedWeigher::weight(&mut transact_msg).unwrap(); - println!("transact_msg_weight: {:?} \n", transact_msg_weight); + let transact_msg_weight = ParaWeigher::weight(&mut transact_msg).unwrap() as u128; let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); - // Token transfer --> 50_000 - 40 = 49_960. - // Weight is 40 because the token transfer has 4 messages each with 10 weight. - // Transact message --> 10_000 - 4_060 = 5_940 refunded. - // Weight is 4060 because there are 6 instructions * 10 weight + the 4000 - // required_weight_at_most in the Transact instruction - // Finally --> 50_000 - 40 - 4_060 = 45_900. assert_eq!( ParaTokens::free_balance(CurrencyId::B, &user_sovereign_account), - 50_000u128 - token_transfer_weight as u128 - transact_msg_weight as u128 + // AllTokensAreCreatedEqualToWeight means 1 to 1 mapping of the weight to fee + transfer_amount - token_transfer_weight - transact_msg_weight ); }); } @@ -481,19 +476,22 @@ fn transfer_with_transact_to_non_reserve_sibling() { let encoded_call: Vec = remark.encode().into(); let bounded_vec = BoundedVec::try_from(encoded_call).unwrap(); + let transfer_amount = 50_000u128; + let transact_fee_amount = 10_000u128; + let dest_weight = 4_000; + ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( Some(ALICE).into(), CurrencyId::R, - 50_000, + transfer_amount, 2, // PARA_B_ID, - 4_000, - bounded_vec.clone(), - // Will cost at 3_000 - 10_000 + dest_weight, + bounded_vec, + transact_fee_amount )); - assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 51_000); + assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), transfer_amount + 1_000); }); ParaB::execute_with(|| { @@ -512,10 +510,9 @@ fn transfer_with_transact_to_non_reserve_sibling() { .into(); let mut token_transfer_msg = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); - let token_transfer_weight = FixedWeigher::weight(&mut token_transfer_msg).unwrap(); - println!("token_transfer_weight: {:?} \n", token_transfer_weight); + let token_transfer_weight = ParaWeigher::weight(&mut token_transfer_msg).unwrap() as u128; let mut token_transfer_msg_on_relay = Xcm(vec![ClearOrigin, ClearOrigin, ClearOrigin, ClearOrigin]); - let token_transfer_weight_on_relay = RelayWeigher::weight(&mut token_transfer_msg_on_relay).unwrap(); + let token_transfer_weight_on_relay = RelayWeigher::weight(&mut token_transfer_msg_on_relay).unwrap() as u128; let mut transact_msg = Xcm(vec![ ClearOrigin, @@ -525,31 +522,19 @@ fn transfer_with_transact_to_non_reserve_sibling() { ClearOrigin, Transact { origin_type: SovereignAccount, - require_weight_at_most: 4_000, + require_weight_at_most: dest_weight, call: vec![].into(), }, ]); - let transact_msg_weight = FixedWeigher::weight(&mut transact_msg).unwrap(); - println!("transact_msg_weight: {:?} \n", transact_msg_weight); + let transact_msg_weight = ParaWeigher::weight(&mut transact_msg).unwrap() as u128; let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); - // Token transfer --> 50_000 - 40 = 49_960. - // Weight is 40 because the token transfer has 4 messages each with 10 weight. - // Transact message --> 10_000 - 4_060 = 5_940 refunded. - // Weight is 4060 because there are 6 instructions * 10 weight + the 4000 - // required_weight_at_most in the Transact instruction - // Finally --> 50_000 - 40 - 4_060 = 45_900. + assert_eq!( ParaTokens::free_balance(CurrencyId::R, &user_sovereign_account), - 50_000u128 - - token_transfer_weight_on_relay as u128 - - token_transfer_weight as u128 - - transact_msg_weight as u128 + // AllTokensAreCreatedEqualToWeight means 1 to 1 mapping of the weight to fee + transfer_amount - token_transfer_weight_on_relay - token_transfer_weight - transact_msg_weight ); - - // assert_eq!(ParaTokens::free_balance(CurrencyId::R, - // &sibling_a_account()), 50_000); - // assert_eq!(ParaTokens::free_balance(CurrencyId::R, &BOB), 460); }); } From 8ce4eab63d69266225c671bb04a9fc1ba647b50a Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 15:35:13 +0300 Subject: [PATCH 13/15] Make MaxEncodedDataLen a pallet constant, not configurable Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 31 +++++++++++--------------- xtokens/src/mock/para.rs | 6 ++--- xtokens/src/mock/para_relative_view.rs | 2 -- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index b996592f3..72b943957 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -58,6 +58,11 @@ enum TransferKind { } use TransferKind::*; +// Used for BoundedVector and has to be u32 because of Decode trait bounds. +// Since it can't be less than u32, opted not to make it a configurable +// component. +type MaxEncodedDataLen = ConstU32<256>; + #[frame_support::pallet] pub mod module { use super::*; @@ -119,9 +124,6 @@ pub mod module { type ReserveProvider: Reserve; type XcmSender: SendXcm; - - #[pallet::constant] - type MaxTransactSize: Get; } #[pallet::event] @@ -181,7 +183,6 @@ pub mod module { /// MinXcmFee not registered for certain reserve location MinXcmFeeNotDefined, SendFailure, - TransactTooLarge, } #[pallet::hooks] @@ -356,12 +357,9 @@ pub mod module { currency_id: T::CurrencyId, dest_id: u32, dest_weight: Weight, - encoded_call_data: BoundedVec, + encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { - // TODO: make the limit u8 or hard-code the constant in the ORML code - ensure!(T::MaxTransactSize::get() <= 256u32, Error::::TransactTooLarge); - let who = ensure_signed(origin)?; Self::do_transact(who, currency_id, transact_fee, dest_id, dest_weight, encoded_call_data) @@ -375,12 +373,9 @@ pub mod module { amount: T::Balance, dest_chain_id: u32, dest_weight: Weight, - encoded_call_data: BoundedVec, + encoded_call_data: BoundedVec, transact_fee: T::Balance, ) -> DispatchResult { - // TODO: make the limit u8 or hard-code the constant in the ORML code - ensure!(T::MaxTransactSize::get() <= 256u32, Error::::TransactTooLarge); - let who = ensure_signed(origin)?; Self::do_transfer_with_transact( @@ -436,7 +431,7 @@ pub mod module { transact_fee_amount: T::Balance, dest_chain_id: u32, dest_weight: Weight, - encoded_call_data: BoundedVec, + encoded_call_data: BoundedVec, ) -> DispatchResult { let transact_fee_location: MultiLocation = T::CurrencyIdConvert::convert(transact_currency_id) .ok_or(Error::::NotCrossChainTransferableCurrency)?; @@ -444,7 +439,7 @@ pub mod module { let origin_location_interior = T::AccountIdToMultiLocation::convert(who).interior; let dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); let refund_recipient = T::SelfLocation::get(); - Self::send_transact( + Self::send_transact_message( transact_fee_location, transact_fee_amount, dest_chain_location, @@ -463,7 +458,7 @@ pub mod module { amount: T::Balance, dest_chain_id: u32, dest_weight: Weight, - encoded_call_data: BoundedVec, + encoded_call_data: BoundedVec, transact_fee_amount: T::Balance, ) -> DispatchResult { ensure!(!amount.is_zero(), Error::::ZeroAmount); @@ -495,7 +490,7 @@ pub mod module { )?; let dest_chain_location = dest_chain_location.chain_part().ok_or(Error::::InvalidDest)?; - Self::send_transact( + Self::send_transact_message( transact_fee_location, transact_fee_amount, dest_chain_location, @@ -508,13 +503,13 @@ pub mod module { Ok(()) } - fn send_transact( + fn send_transact_message( transact_fee_location: MultiLocation, transact_fee_amount: T::Balance, dest_chain_location: MultiLocation, origin_location_interior: Junctions, dest_weight: Weight, - encoded_call_data: BoundedVec, + encoded_call_data: BoundedVec, refund_recipient: MultiLocation, ) -> DispatchResult { let ancestry = T::LocationInverter::ancestry(); diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index d559392f5..0b8a9889e 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -196,7 +196,8 @@ impl> ShouldExecute for AllowTransactFrom { } let next_instruction = iter.next().ok_or(())?; match next_instruction { - // TODO: can at least check the length of the encoded vec, need a PR in polkadot for that + // TODO: can at least check the length of the encoded vec, need a PR in polkadot for that. + // Also the allowed length could be configurable. // TODO: if possible decode the call and match against a configurable set of allowed calls Transact { .. } => (), _ => return Err(()), @@ -238,7 +239,6 @@ impl WeightTrader for AllTokensAreCreatedEqualToWeight { .next() .expect("Payment must be something; qed") .0; - let required = MultiAsset { id: asset_id.clone(), fun: Fungible(weight as u128), @@ -253,7 +253,6 @@ impl WeightTrader for AllTokensAreCreatedEqualToWeight { } let unused = payment.checked_sub(required).map_err(|_| XcmError::TooExpensive)?; - Ok(unused) } @@ -392,7 +391,6 @@ impl orml_xtokens::Config for Runtime { type MaxAssetsForTransfer = MaxAssetsForTransfer; type ReserveProvider = AbsoluteReserveProvider; type XcmSender = XcmRouter; - type MaxTransactSize = ConstU32<256>; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_relative_view.rs b/xtokens/src/mock/para_relative_view.rs index 2487cefe6..1a2922827 100644 --- a/xtokens/src/mock/para_relative_view.rs +++ b/xtokens/src/mock/para_relative_view.rs @@ -326,7 +326,6 @@ impl Convert> for RelativeCurrencyIdConvert { parameter_types! { pub SelfLocation: MultiLocation = MultiLocation::here(); pub const MaxAssetsForTransfer: usize = 2; - pub const MaxTransactSize: u32 = 256; } match_types! { @@ -367,7 +366,6 @@ impl orml_xtokens::Config for Runtime { type MaxAssetsForTransfer = MaxAssetsForTransfer; type ReserveProvider = RelativeReserveProvider; type XcmSender = XcmRouter; - type MaxTransactSize = MaxTransactSize; } impl orml_xcm::Config for Runtime { From 424ed0ffa1b4e04f804415c6e391944959c824db Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 16:39:13 +0300 Subject: [PATCH 14/15] Documentaiton draft and clean up Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 320 ++++++++++++++++++++++++--------------- xtokens/src/mock/para.rs | 5 +- xtokens/src/tests.rs | 8 +- 3 files changed, 203 insertions(+), 130 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 72b943957..ae6271388 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -59,8 +59,10 @@ enum TransferKind { use TransferKind::*; // Used for BoundedVector and has to be u32 because of Decode trait bounds. -// Since it can't be less than u32, opted not to make it a configurable -// component. +// Since the BoundedVector is used for extrinsic arguments, it is required to +// implement Encode/Decode, and it can't be less than u32, as that is the +// smallest type that implements the required traits. Therefore it is not a +// configurable pallet type to avoid huge transact messages being attempted. type MaxEncodedDataLen = ConstU32<256>; #[frame_support::pallet] @@ -123,6 +125,9 @@ pub mod module { /// configured to accept absolute or relative paths for self tokens type ReserveProvider: Reserve; + /// Used to bypass the xcm-executor for sending message including + /// Transact instruction that need to use specific descended location + /// for origin type XcmSender: SendXcm; } @@ -350,21 +355,90 @@ pub mod module { Self::do_transfer_multicurrencies(who, currencies, fee_item, dest, dest_weight) } + /// Transfer native currencies and use them to execute a Transact + /// instruction on the destination. + /// + /// Sends a XCM message that will descend to a user sovereign account on + /// the destination chain, created from the multilocation of the sender + /// from the perspective of the destination chain, usually by hashing. + /// From that origin some assets will be withdrawn to buy execution time + /// for a local extrinsic call. That call will be decoded from the + /// encoded call data in a `Transact` instruction. The call will then be + /// dispatched with the user sovereign account as origin. Any surplus + /// assets are deposited back into the user sovereign account. + /// + /// `dest_weight` is the weight for XCM execution on the dest chain, and + /// it would be charged from the transferred assets. If set below + /// requirements, the execution may fail and assets wouldn't be + /// received. + /// + /// `encoded_call_data` is the encoded call data of the extrinsic on the + /// destination chain, with a bounded length. + /// + /// `transact_fee_amount` is the amount of assets that will be withdrawn + /// to execute the Transact call. Those assets must be pre-funded to the + /// user sovereign account in order to succeed. + /// + /// It's a no-op if any error on local XCM execution or message sending. + /// Note sending assets out per se doesn't guarantee they would be + /// received. Receiving depends on if the XCM message could be delivered + /// by the network, and if the receiving chain would handle + /// messages correctly. #[pallet::weight(Pallet::::weight_of_send_transact())] #[transactional] pub fn transact( origin: OriginFor, - currency_id: T::CurrencyId, - dest_id: u32, + transact_currency_id: T::CurrencyId, + dest_chain_id: u32, dest_weight: Weight, encoded_call_data: BoundedVec, - transact_fee: T::Balance, + transact_fee_amount: T::Balance, ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_transact(who, currency_id, transact_fee, dest_id, dest_weight, encoded_call_data) + Self::do_transact( + who, + transact_currency_id, + transact_fee_amount, + dest_chain_id, + dest_weight, + encoded_call_data, + ) } + /// Transfer native currencies and use them to execute a Transact + /// instruction on the destination. + /// + /// Consists of sending two XCM messages: + /// 1. First message will transfer some assets to a user sovereign + /// account on the destination chain, created from the multilocation of + /// the sender from the perspective of the destination chain, usually by + /// hashing. + /// 2. Second message will descend to the user sovereign account origin + /// and will withdraw some of the transferred assets to buy execution + /// time for a local extrinsic call. That call will be decoded from the + /// encoded call data in a `Transact` instruction. The call will then be + /// dispatched with the user sovereign account as origin. Any surplus + /// assets are deposited back into the user sovereign account. + /// + /// `dest_weight` is the weight for XCM execution on the dest chain, and + /// it would be charged from the transferred assets. If set below + /// requirements, the execution may fail and assets wouldn't be + /// received. + /// + /// `encoded_call_data` is the encoded call data of the extrinsic on the + /// destination chain with a bounded length. + /// + /// `transact_fee_amount` is the amount of assets that will be withdrawn + /// to execute the Transact call. Those assets must be less than or + /// equal to the transferred amount minus any other execution costs of + /// the instructions, like the initial deposit of the assets. + /// + /// It's a no-op if any error on local XCM execution or message sending. + /// Note sending assets out per se doesn't guarantee they would be + /// received. Receiving depends on if the XCM message could be delivered + /// by the network, and if the receiving chain would handle + /// messages correctly. #[pallet::weight(Pallet::::weight_of_transfer_with_transact(currency_id.clone(), *amount, *dest_chain_id))] #[transactional] pub fn transfer_with_transact( @@ -425,6 +499,123 @@ pub mod module { } impl Pallet { + fn do_transfer( + who: T::AccountId, + currency_id: T::CurrencyId, + amount: T::Balance, + dest: MultiLocation, + dest_weight: Weight, + ) -> DispatchResult { + let location: MultiLocation = + T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; + + ensure!(!amount.is_zero(), Error::::ZeroAmount); + ensure!( + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation + ); + + let asset: MultiAsset = (location, amount.into()).into(); + Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight, None) + } + + fn do_transfer_with_fee( + who: T::AccountId, + currency_id: T::CurrencyId, + amount: T::Balance, + fee: T::Balance, + dest: MultiLocation, + dest_weight: Weight, + ) -> DispatchResult { + let location: MultiLocation = + T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; + + ensure!(!amount.is_zero(), Error::::ZeroAmount); + ensure!(!fee.is_zero(), Error::::ZeroFee); + ensure!( + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation + ); + + let asset = (location.clone(), amount.into()).into(); + let fee_asset: MultiAsset = (location, fee.into()).into(); + + // Push contains saturated addition, so we should be able to use it safely + let mut assets = MultiAssets::new(); + assets.push(asset); + assets.push(fee_asset.clone()); + + Self::do_transfer_multiassets(who, assets, fee_asset, dest, dest_weight, None) + } + + fn do_transfer_multiasset( + who: T::AccountId, + asset: MultiAsset, + dest: MultiLocation, + dest_weight: Weight, + ) -> DispatchResult { + Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight, None) + } + + fn do_transfer_multiasset_with_fee( + who: T::AccountId, + asset: MultiAsset, + fee: MultiAsset, + dest: MultiLocation, + dest_weight: Weight, + ) -> DispatchResult { + // Push contains saturated addition, so we should be able to use it safely + let mut assets = MultiAssets::new(); + assets.push(asset); + assets.push(fee.clone()); + + Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight, None)?; + + Ok(()) + } + + fn do_transfer_multicurrencies( + who: T::AccountId, + currencies: Vec<(T::CurrencyId, T::Balance)>, + fee_item: u32, + dest: MultiLocation, + dest_weight: Weight, + ) -> DispatchResult { + ensure!( + currencies.len() <= T::MaxAssetsForTransfer::get(), + Error::::TooManyAssetsBeingSent + ); + ensure!( + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation + ); + + let mut assets = MultiAssets::new(); + + // Lets grab the fee amount and location first + let (fee_currency_id, fee_amount) = currencies + .get(fee_item as usize) + .ok_or(Error::::AssetIndexNonExistent)?; + + for (currency_id, amount) in ¤cies { + let location: MultiLocation = T::CurrencyIdConvert::convert(currency_id.clone()) + .ok_or(Error::::NotCrossChainTransferableCurrency)?; + ensure!(!amount.is_zero(), Error::::ZeroAmount); + + // Push contains saturated addition, so we should be able to use it safely + assets.push((location, (*amount).into()).into()) + } + + // We construct the fee now, since getting it from assets wont work as assets + // sorts it + let fee_location: MultiLocation = T::CurrencyIdConvert::convert(fee_currency_id.clone()) + .ok_or(Error::::NotCrossChainTransferableCurrency)?; + + let fee: MultiAsset = (fee_location, (*fee_amount).into()).into(); + + Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight, None) + } + fn do_transact( who: T::AccountId, transact_currency_id: T::CurrencyId, @@ -548,123 +739,6 @@ pub mod module { Ok(()) } - fn do_transfer( - who: T::AccountId, - currency_id: T::CurrencyId, - amount: T::Balance, - dest: MultiLocation, - dest_weight: Weight, - ) -> DispatchResult { - let location: MultiLocation = - T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; - - ensure!(!amount.is_zero(), Error::::ZeroAmount); - ensure!( - T::MultiLocationsFilter::contains(&dest), - Error::::NotSupportedMultiLocation - ); - - let asset: MultiAsset = (location, amount.into()).into(); - Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight, None) - } - - fn do_transfer_with_fee( - who: T::AccountId, - currency_id: T::CurrencyId, - amount: T::Balance, - fee: T::Balance, - dest: MultiLocation, - dest_weight: Weight, - ) -> DispatchResult { - let location: MultiLocation = - T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; - - ensure!(!amount.is_zero(), Error::::ZeroAmount); - ensure!(!fee.is_zero(), Error::::ZeroFee); - ensure!( - T::MultiLocationsFilter::contains(&dest), - Error::::NotSupportedMultiLocation - ); - - let asset = (location.clone(), amount.into()).into(); - let fee_asset: MultiAsset = (location, fee.into()).into(); - - // Push contains saturated addition, so we should be able to use it safely - let mut assets = MultiAssets::new(); - assets.push(asset); - assets.push(fee_asset.clone()); - - Self::do_transfer_multiassets(who, assets, fee_asset, dest, dest_weight, None) - } - - fn do_transfer_multiasset( - who: T::AccountId, - asset: MultiAsset, - dest: MultiLocation, - dest_weight: Weight, - ) -> DispatchResult { - Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight, None) - } - - fn do_transfer_multiasset_with_fee( - who: T::AccountId, - asset: MultiAsset, - fee: MultiAsset, - dest: MultiLocation, - dest_weight: Weight, - ) -> DispatchResult { - // Push contains saturated addition, so we should be able to use it safely - let mut assets = MultiAssets::new(); - assets.push(asset); - assets.push(fee.clone()); - - Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight, None)?; - - Ok(()) - } - - fn do_transfer_multicurrencies( - who: T::AccountId, - currencies: Vec<(T::CurrencyId, T::Balance)>, - fee_item: u32, - dest: MultiLocation, - dest_weight: Weight, - ) -> DispatchResult { - ensure!( - currencies.len() <= T::MaxAssetsForTransfer::get(), - Error::::TooManyAssetsBeingSent - ); - ensure!( - T::MultiLocationsFilter::contains(&dest), - Error::::NotSupportedMultiLocation - ); - - let mut assets = MultiAssets::new(); - - // Lets grab the fee amount and location first - let (fee_currency_id, fee_amount) = currencies - .get(fee_item as usize) - .ok_or(Error::::AssetIndexNonExistent)?; - - for (currency_id, amount) in ¤cies { - let location: MultiLocation = T::CurrencyIdConvert::convert(currency_id.clone()) - .ok_or(Error::::NotCrossChainTransferableCurrency)?; - ensure!(!amount.is_zero(), Error::::ZeroAmount); - - // Push contains saturated addition, so we should be able to use it safely - assets.push((location, (*amount).into()).into()) - } - - // We construct the fee now, since getting it from assets wont work as assets - // sorts it - let fee_location: MultiLocation = T::CurrencyIdConvert::convert(fee_currency_id.clone()) - .ok_or(Error::::NotCrossChainTransferableCurrency)?; - - let fee: MultiAsset = (fee_location, (*fee_amount).into()).into(); - - Self::do_transfer_multiassets(who, assets, fee, dest, dest_weight, None) - } - fn do_transfer_multiassets( who: T::AccountId, assets: MultiAssets, diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 0b8a9889e..ba6443e4d 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -109,13 +109,12 @@ parameter_types! { pub Ancestry: MultiLocation = Parachain(ParachainInfo::parachain_id().into()).into(); } -pub type MyAccount32Hash = Account32Hash; - +pub type ParaAccount32Hash = Account32Hash; pub type LocationToAccountId = ( ParentIsPreset, SiblingParachainConvertsVia, AccountId32Aliases, - MyAccount32Hash, + ParaAccount32Hash, ); pub type XcmOriginToCallOrigin = ( diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index d37965e26..5319bd9ea 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -5,7 +5,7 @@ use codec::Encode; use cumulus_primitives_core::ParaId; use frame_support::{assert_err, assert_noop, assert_ok, traits::Currency, BoundedVec}; use mock::{ - para::{MyAccount32Hash, ParaWeigher}, + para::{ParaAccount32Hash, ParaWeigher}, relay::RelayWeigher, *, }; @@ -367,7 +367,7 @@ fn transfer_with_transact_self_reserve_sibling() { ]); let transact_msg_weight = ParaWeigher::weight(&mut transact_msg).unwrap() as u128; - let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); + let user_sovereign_account = ParaAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); assert_eq!( ParaTokens::free_balance(CurrencyId::A, &user_sovereign_account), @@ -450,7 +450,7 @@ fn transfer_with_transact_to_reserve_sibling() { ]); let transact_msg_weight = ParaWeigher::weight(&mut transact_msg).unwrap() as u128; - let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); + let user_sovereign_account = ParaAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); assert_eq!( ParaTokens::free_balance(CurrencyId::B, &user_sovereign_account), // AllTokensAreCreatedEqualToWeight means 1 to 1 mapping of the weight to fee @@ -528,7 +528,7 @@ fn transfer_with_transact_to_non_reserve_sibling() { ]); let transact_msg_weight = ParaWeigher::weight(&mut transact_msg).unwrap() as u128; - let user_sovereign_account = MyAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); + let user_sovereign_account = ParaAccount32Hash::convert_ref(&user_multilocaiton).unwrap(); assert_eq!( ParaTokens::free_balance(CurrencyId::R, &user_sovereign_account), From 3e927d0c9f261ad4e306447b3fefb979818c7f34 Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev Date: Wed, 13 Jul 2022 17:27:23 +0300 Subject: [PATCH 15/15] Add a separate argument for transact_dest_weight. More cleanup Signed-off-by: Georgi Zlatarev --- xtokens/src/lib.rs | 56 ++++++++++++++++++++++++-------------------- xtokens/src/tests.rs | 30 ++++++++++++++---------- 2 files changed, 48 insertions(+), 38 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index ae6271388..175f508b5 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -367,9 +367,9 @@ pub mod module { /// dispatched with the user sovereign account as origin. Any surplus /// assets are deposited back into the user sovereign account. /// - /// `dest_weight` is the weight for XCM execution on the dest chain, and - /// it would be charged from the transferred assets. If set below - /// requirements, the execution may fail and assets wouldn't be + /// `transact_dest_weight` is the weight for XCM execution on the dest + /// chain, and it would be charged from the transferred assets. If set + /// below requirements, the execution may fail and assets wouldn't be /// received. /// /// `encoded_call_data` is the encoded call data of the extrinsic on the @@ -390,7 +390,7 @@ pub mod module { origin: OriginFor, transact_currency_id: T::CurrencyId, dest_chain_id: u32, - dest_weight: Weight, + transact_dest_weight: Weight, encoded_call_data: BoundedVec, transact_fee_amount: T::Balance, ) -> DispatchResult { @@ -401,7 +401,7 @@ pub mod module { transact_currency_id, transact_fee_amount, dest_chain_id, - dest_weight, + transact_dest_weight, encoded_call_data, ) } @@ -421,9 +421,9 @@ pub mod module { /// dispatched with the user sovereign account as origin. Any surplus /// assets are deposited back into the user sovereign account. /// - /// `dest_weight` is the weight for XCM execution on the dest chain, and - /// it would be charged from the transferred assets. If set below - /// requirements, the execution may fail and assets wouldn't be + /// `transact_dest_weight` is the weight for XCM execution on the dest + /// chain, and it would be charged from the transferred assets. If set + /// below requirements, the execution may fail and assets wouldn't be /// received. /// /// `encoded_call_data` is the encoded call data of the extrinsic on the @@ -439,27 +439,30 @@ pub mod module { /// received. Receiving depends on if the XCM message could be delivered /// by the network, and if the receiving chain would handle /// messages correctly. - #[pallet::weight(Pallet::::weight_of_transfer_with_transact(currency_id.clone(), *amount, *dest_chain_id))] + #[pallet::weight(Pallet::::weight_of_transfer_with_transact(currency_id.clone(), *transfer_amount, *dest_chain_id))] #[transactional] pub fn transfer_with_transact( origin: OriginFor, currency_id: T::CurrencyId, - amount: T::Balance, + transfer_amount: T::Balance, dest_chain_id: u32, - dest_weight: Weight, + transfer_dest_weight: Weight, encoded_call_data: BoundedVec, - transact_fee: T::Balance, + transact_fee_amount: T::Balance, + transact_dest_weight: Weight, ) -> DispatchResult { let who = ensure_signed(origin)?; + ensure!(transfer_amount >= transact_fee_amount, Error::::FeeNotEnough); Self::do_transfer_with_transact( who, currency_id, - amount, + transfer_amount, dest_chain_id, - dest_weight, + transfer_dest_weight, encoded_call_data, - transact_fee, + transact_fee_amount, + transact_dest_weight, ) } @@ -621,7 +624,7 @@ pub mod module { transact_currency_id: T::CurrencyId, transact_fee_amount: T::Balance, dest_chain_id: u32, - dest_weight: Weight, + transact_dest_weight: Weight, encoded_call_data: BoundedVec, ) -> DispatchResult { let transact_fee_location: MultiLocation = T::CurrencyIdConvert::convert(transact_currency_id) @@ -635,7 +638,7 @@ pub mod module { transact_fee_amount, dest_chain_location, origin_location_interior, - dest_weight, + transact_dest_weight, encoded_call_data, refund_recipient, )?; @@ -646,13 +649,14 @@ pub mod module { fn do_transfer_with_transact( who: T::AccountId, currency_id: T::CurrencyId, - amount: T::Balance, + transfer_amount: T::Balance, dest_chain_id: u32, - dest_weight: Weight, + transfer_dest_weight: Weight, encoded_call_data: BoundedVec, transact_fee_amount: T::Balance, + transact_dest_weight: Weight, ) -> DispatchResult { - ensure!(!amount.is_zero(), Error::::ZeroAmount); + ensure!(!transfer_amount.is_zero(), Error::::ZeroAmount); let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); let mut dest_chain_location: MultiLocation = (1, Parachain(dest_chain_id)).into(); @@ -668,7 +672,7 @@ pub mod module { let transact_fee_location: MultiLocation = T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; - let transfer_asset: MultiAsset = (transact_fee_location.clone(), amount.into()).into(); + let transfer_asset: MultiAsset = (transact_fee_location.clone(), transfer_amount.into()).into(); let mut override_recipient = T::SelfLocation::get(); let _ = override_recipient.append_with(origin_location_interior.clone()); Self::do_transfer_multiassets( @@ -676,7 +680,7 @@ pub mod module { vec![transfer_asset.clone()].into(), transfer_asset, dest_chain_location.clone(), - dest_weight, + transfer_dest_weight, Some(override_recipient.clone()), )?; @@ -686,7 +690,7 @@ pub mod module { transact_fee_amount, dest_chain_location, origin_location_interior, - dest_weight, + transact_dest_weight, encoded_call_data, override_recipient, )?; @@ -699,7 +703,7 @@ pub mod module { transact_fee_amount: T::Balance, dest_chain_location: MultiLocation, origin_location_interior: Junctions, - dest_weight: Weight, + transact_dest_weight: Weight, encoded_call_data: BoundedVec, refund_recipient: MultiLocation, ) -> DispatchResult { @@ -718,12 +722,12 @@ pub mod module { WithdrawAsset(transact_fee_assets), BuyExecution { fees: transact_fee_asset, - weight_limit: WeightLimit::Limited(dest_weight), + weight_limit: WeightLimit::Limited(transact_dest_weight), }, Transact { // SovereignAccount of the user, not the chain origin_type: OriginKind::SovereignAccount, - require_weight_at_most: dest_weight, + require_weight_at_most: transact_dest_weight, call: encoded_call_data.into_inner().into(), }, RefundSurplus, diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 5319bd9ea..5a40804d8 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -320,7 +320,8 @@ fn transfer_with_transact_self_reserve_sibling() { let transfer_amount = 50_000u128; let transact_fee_amount = 10_000u128; - let dest_weight = 4_000; + let transfer_dest_weight = 4_000; + let transact_dest_weight = 4_000; ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( @@ -328,9 +329,10 @@ fn transfer_with_transact_self_reserve_sibling() { CurrencyId::A, transfer_amount, 2, // PARA_B_ID, - dest_weight, + transfer_dest_weight, bounded_vec, - transact_fee_amount + transact_fee_amount, + transact_dest_weight, )); assert_eq!(ParaTokens::free_balance(CurrencyId::A, &ALICE), transfer_amount); @@ -361,7 +363,7 @@ fn transfer_with_transact_self_reserve_sibling() { ClearOrigin, Transact { origin_type: SovereignAccount, - require_weight_at_most: dest_weight, + require_weight_at_most: transact_dest_weight, call: vec![].into(), }, ]); @@ -397,7 +399,8 @@ fn transfer_with_transact_to_reserve_sibling() { let transfer_amount = 50_000u128; let transact_fee_amount = 10_000u128; - let dest_weight = 4_000; + let transfer_dest_weight = 4_000; + let transact_dest_weight = 4_000; ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( @@ -405,9 +408,10 @@ fn transfer_with_transact_to_reserve_sibling() { CurrencyId::B, transfer_amount, 2, // PARA_B_ID, - dest_weight, + transfer_dest_weight, bounded_vec.clone(), - transact_fee_amount + transact_fee_amount, + transact_dest_weight, )); assert_eq!(ParaTokens::free_balance(CurrencyId::B, &ALICE), transfer_amount); @@ -444,7 +448,7 @@ fn transfer_with_transact_to_reserve_sibling() { ClearOrigin, Transact { origin_type: SovereignAccount, - require_weight_at_most: dest_weight, + require_weight_at_most: transact_dest_weight, call: vec![].into(), }, ]); @@ -478,7 +482,8 @@ fn transfer_with_transact_to_non_reserve_sibling() { let transfer_amount = 50_000u128; let transact_fee_amount = 10_000u128; - let dest_weight = 4_000; + let transfer_dest_weight = 4_000; + let transact_dest_weight = 4_000; ParaA::execute_with(|| { assert_ok!(ParaXTokens::transfer_with_transact( @@ -486,9 +491,10 @@ fn transfer_with_transact_to_non_reserve_sibling() { CurrencyId::R, transfer_amount, 2, // PARA_B_ID, - dest_weight, + transfer_dest_weight, bounded_vec, - transact_fee_amount + transact_fee_amount, + transact_dest_weight, )); assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), transfer_amount + 1_000); @@ -522,7 +528,7 @@ fn transfer_with_transact_to_non_reserve_sibling() { ClearOrigin, Transact { origin_type: SovereignAccount, - require_weight_at_most: dest_weight, + require_weight_at_most: transact_dest_weight, call: vec![].into(), }, ]);