-
Notifications
You must be signed in to change notification settings - Fork 14
Conversation
pallets/withdraw-teleport/src/lib.rs
Outdated
// We can deposit funds since they were both withdrawn on origin. | ||
DepositAsset { assets: MultiAssetFilter::Definite(foreing_assets), beneficiary }, | ||
RefundSurplus, | ||
DepositAsset { assets: MultiAssetFilter::Definite(fee_asset), beneficiary }, |
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.
But you don't know how much of fee_asset
remains, since part of it was moved into fee payment. I think you need an AllOf
variant.
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.
Ah, I had the misconception that this MultiAssetFilter
deposited "whatever you have left of x asset" but it takes also in consideration the amount. Will fix, thanks.
// Burn the native asset. | ||
WithdrawAsset(assets.clone()), | ||
BuyExecution { fees: native_asset, weight_limit: Unlimited }, | ||
BurnAsset(assets), |
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.
You should actually burn the amount that's left after buying execution.
You could estimate this via the same mechanism that's used to turn weight into assets.
The actual amount used to pay for the weight is stored in the Trader
in the XCM config.
You could use that to assert your calculation is right.
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.
Indeed, and this also applies for the amount to be minted on AH on the sent ReceiveTeleportedAsset
as stated on the known issue here.
Will dig into this.
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.
Update: Fixed with SetFeesMode
, fees are paid from origin on-chain account funds so amount withdrawn is amount to be burned and minted on destination.
@joepetrowski Could you take another (hopefully final) look ? 🙏️ |
Co-authored-by: joe petrowski <[email protected]>
Implementation of
do_withdraw_and_teleport
call from #266 that also withdraws a derivative asset on origin chainFor the sake of simplicity on wording:
Origin chain: Trappist
Destiny chain: Asset Hub
Pros:
Users cannot longer exploit Trappist’s SovereignAccount on Asset Hub as they would need to have the same amount of funds as derivative on their Trappist's account than they would want to use for buying execution on Asset Hub.
ROCs used for buying execution on Asset Hub are not longer trapped and they are refunded as ROC on Asset Hub, not as the derivatives on Trappist.
Cons:
Users must have derivative of ROC on Trappist apart from the HOP that they want to teleport. This could be done through a simple
ReserveAssetTransfer
TODO:
Refund surplus as derivative on origin chain to origin account.
Known issue:
The amount of native asset that is minted through
ReceiveTeleportedAsset
does not take in consideration the amount used for buying execution on origin chain, therefore it is "free" from the origin chain native asset perspective.