-
Notifications
You must be signed in to change notification settings - Fork 721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix delivery fees issue in Kusama #3075
Changes from 3 commits
d637826
d53dde2
74a2d88
398b333
1c827db
9368692
74471d0
4051d85
bb15bed
ba59a14
67c7390
b3ea78a
b05a313
1ce2488
b88a5ca
6a85a54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -510,6 +510,21 @@ impl<Config: config::Config> XcmExecutor<Config> { | |
}, | ||
TransferReserveAsset { mut assets, dest, xcm } => { | ||
let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?; | ||
|
||
// we need to do this take/put cycle to solve wildcards and get exact assets to be | ||
// weighed | ||
let to_weigh = self.holding.saturating_take(assets.clone()); | ||
self.holding.subsume_assets(to_weigh.clone()); | ||
|
||
let mut message_to_weigh = | ||
vec![ReserveAssetDeposited(to_weigh.into()), ClearOrigin]; | ||
message_to_weigh.extend(xcm.0.clone().into_iter()); | ||
let (_, fee) = validate_send::<Config::XcmSender>(dest, Xcm(message_to_weigh))?; | ||
// set aside fee to be charged by XcmSender | ||
let parked_fee = self.holding.saturating_take(fee.into()); | ||
|
||
// now take assets to deposit (excluding parked_fee) | ||
let assets = self.holding.saturating_take(assets); | ||
// Take `assets` from the origin account (on-chain) and place into dest account. | ||
for asset in assets.inner() { | ||
Config::AssetTransactor::transfer_asset(asset, origin, &dest, &self.context)?; | ||
|
@@ -624,6 +639,19 @@ impl<Config: config::Config> XcmExecutor<Config> { | |
Ok(()) | ||
}, | ||
DepositReserveAsset { assets, dest, xcm } => { | ||
// we need to do this take/put cycle to solve wildcards and get exact assets to be | ||
// weighed | ||
let to_weigh = self.holding.saturating_take(assets.clone()); | ||
self.holding.subsume_assets(to_weigh.clone()); | ||
|
||
let mut message_to_weigh = | ||
vec![ReserveAssetDeposited(to_weigh.into()), ClearOrigin]; | ||
message_to_weigh.extend(xcm.0.clone().into_iter()); | ||
let (_, fee) = validate_send::<Config::XcmSender>(dest, Xcm(message_to_weigh))?; | ||
// set aside fee to be charged by XcmSender | ||
let parked_fee = self.holding.saturating_take(fee.into()); | ||
|
||
// now take assets to deposit (excluding parked_fee) | ||
let deposited = self.holding.saturating_take(assets); | ||
for asset in deposited.assets_iter() { | ||
Config::AssetTransactor::deposit_asset(&asset, &dest, Some(&self.context))?; | ||
|
@@ -633,10 +661,26 @@ impl<Config: config::Config> XcmExecutor<Config> { | |
let assets = Self::reanchored(deposited, &dest, None); | ||
let mut message = vec![ReserveAssetDeposited(assets), ClearOrigin]; | ||
message.extend(xcm.0.into_iter()); | ||
|
||
// put back parked_fee in holding register to be charged by XcmSender | ||
self.holding.subsume_assets(parked_fee); | ||
self.send(dest, Xcm(message), FeeReason::DepositReserveAsset)?; | ||
Ok(()) | ||
}, | ||
InitiateReserveWithdraw { assets, reserve, xcm } => { | ||
// we need to do this take/put cycle to solve wildcards and get exact assets to be | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is less of a problem since it happens on the origin chain, and not the intermediary chain. We could thus use JIT withdrawal to resolve this. However, we may want a consistent UX, which would require us to not use JIT withdrawal and remove the SetFeesMode instruction when executing the initial teleport or reserve asset transfer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The thing with jit withdrawal is that it's not expected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is definitely needed for InitiateReserveWithdraw too but not for the current scenario where the intermediary chain is the reserve - I say we keep it, but add a check if jit_withdraw is set - if it is, then we can skip it and let it withdraw from origin's account instead of holding (maybe we do this jit check for all of them, but honestly I would just deprecate JIT_WITHDRAW altogether, it just introduces extra complexity and error cases) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's true for we can't assume in executor that no one will use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @franciscoaguirre, just to confirm; this means that we can't just send now (as an example) USDT to a para chain from AssetHub. We also have to include KSM and AT LEAST the amount of the XCM delivery fee? And then whatever we put over the min XCM Delivery Fee amount will be delivered to the target account in the target chain? |
||
// weighed | ||
let to_weigh = self.holding.saturating_take(assets.clone()); | ||
self.holding.subsume_assets(to_weigh.clone()); | ||
|
||
let mut message_to_weigh = | ||
vec![WithdrawAsset(to_weigh.clone()), ClearOrigin]; | ||
message_to_weigh.extend(xcm.0.clone().into_iter()); | ||
let (_, fee) = validate_send::<Config::XcmSender>(dest, Xcm(message_to_weigh))?; | ||
// set aside fee to be charged by XcmSender | ||
let parked_fee = self.holding.saturating_take(fee.into()); | ||
|
||
// now take assets to deposit (excluding parked_fee) | ||
// Note that here we are able to place any assets which could not be reanchored | ||
// back into Holding. | ||
let assets = Self::reanchored( | ||
|
@@ -646,11 +690,25 @@ impl<Config: config::Config> XcmExecutor<Config> { | |
); | ||
let mut message = vec![WithdrawAsset(assets), ClearOrigin]; | ||
message.extend(xcm.0.into_iter()); | ||
|
||
// put back parked_fee in holding register to be charged by XcmSender | ||
self.holding.subsume_assets(parked_fee); | ||
self.send(reserve, Xcm(message), FeeReason::InitiateReserveWithdraw)?; | ||
Ok(()) | ||
}, | ||
InitiateTeleport { assets, dest, xcm } => { | ||
// We must do this first in order to resolve wildcards. | ||
let to_weigh = self.holding.saturating_take(assets); | ||
self.holding.subsume_assets(to_weigh.clone()); | ||
|
||
let mut message_to_weigh = | ||
vec![ReceiveTeleportedAsset(to_weigh.clone()), ClearOrigin]; | ||
message_to_weigh.extend(xcm.0.clone().into_iter()); | ||
let (_, fee) = validate_send::<Config::XcmSender>(dest, Xcm(message_to_weigh))?; | ||
// set aside fee to be charged by XcmSender | ||
let parked_fee = self.holding.saturating_take(fee.into()); | ||
|
||
// now take assets to deposit (excluding parked_fee) | ||
let assets = self.holding.saturating_take(assets); | ||
for asset in assets.assets_iter() { | ||
// We should check that the asset can actually be teleported out (for this to | ||
|
@@ -667,6 +725,9 @@ impl<Config: config::Config> XcmExecutor<Config> { | |
let assets = Self::reanchored(assets, &dest, None); | ||
let mut message = vec![ReceiveTeleportedAsset(assets), ClearOrigin]; | ||
message.extend(xcm.0.into_iter()); | ||
|
||
// put back parked_fee in holding register to be charged by XcmSender | ||
self.holding.subsume_assets(parked_fee); | ||
self.send(dest, Xcm(message), FeeReason::InitiateTeleport)?; | ||
Ok(()) | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
this can be extracted in some
fn extract_transport_fee()
or similar and reused