polkadot-sdk
polkadot-sdk copied to clipboard
[xcm] depositing multiple assets to inexistent account should work if at least one of them satisfies ED
Is there an existing issue?
- [X] I have searched the existing issues
Experiencing problems? Have you tried our Stack Exchange first?
- [X] This is not a support question.
Motivation
One should be able to send a bunch of assets over XCM to some previously inexistent account on the local chain or on some remote chain.
(inspired by https://substrate.stackexchange.com/a/11360/382)
Request
Context: destination/beneficiary account does not (yet) exist/have any assets.
- Sending a single asset that satisfies destination ED currently works.
- Sending multiple assets also work, if and only if the first asset in the list satisfies ED.
- Sending multiple assets where there's at least one that satisfies ED, but not the first in the list fails.
Please note, that the user cannot really control the order of assets in the list since they are automatically sorted by their id/location. So we cannot rely on 2 as a workaround.
3 should "just work", since at least one of the incoming assets will satisfy ED.
Solution
The problem is that asset depositing is done one asset at a time, and account ED is checked for each.
The implementation should treat "not enough ED" errors as transient unless all assets suffer the same.
Are you willing to help with this request?
Yes!
The implementation should treat "not enough ED" errors as transient unless all assets suffer the same.
so, throw iff all assets fail to deposit? for example, if n assets are to be deposited, and asset at index n-1 satisfied ED, what happens to all failed deposits prior to it? they should get deposited again?
The implementation should treat "not enough ED" errors as transient unless all assets suffer the same.
so, throw iff all assets fail to deposit? for example, if n assets are to be deposited, and asset at index n-1 satisfied ED, what happens to all failed deposits prior to it? they should get deposited again?
Yes, failed deposits because of ED can be queued for retry.
If at least one deposit succeeds (ED is now satisfied), the retry-queue can be processed knowing the transient error is now gone.
Hey @acatangiu, is something like this that you are looking for?
DepositAsset { assets, beneficiary } => {
let old_holding = self.holding.clone();
let result = Config::TransactionalProcessor::process(|| {
let deposited = self.holding.saturating_take(assets);
let mut all_failed = false;
let mut failed_deposits = Vec::new();
for asset in deposited.into_assets_iter() {
let asset_result = Config::AssetTransactor::deposit_asset(
&asset,
&beneficiary,
Some(&self.context),
);
if asset_result.is_ok() {
all_failed = false;
continue
}
failed_deposits.push(asset);
}
if all_failed {
return Err(Error)
}
for asset in failed_deposits {
Config::AssetTransactor::deposit_asset(
&asset,
&beneficiary,
Some(&self.context),
)?;
}
Ok(())
});
if Config::TransactionalProcessor::IS_TRANSACTIONAL && result.is_err() {
self.holding = old_holding;
}
result
},
In this code I'm not checking the error is "not enough ED", the deposit_asset returns an Error enum but I couldn't find an equivalent to "not enough ED", how can I check this? Can I use the "not enough ED" string?
For an error different from "not enough ED", the behaviour should remain the same that we have now, right?
why not have this code review on a PR?