Skip to content
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

xcm-executor: allow deposit of multiple assets if at least one of them satisfies ED #4460

Merged
Merged
Changes from 6 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2de5654
check if at least one of the transactions satisfy existential deposit
jpserrat May 14, 2024
b9643e5
refactor deposit failed due to ED to a helper function
jpserrat Jun 9, 2024
487cf05
minor fixes
jpserrat Jul 18, 2024
1d26287
remove unused import
jpserrat Jul 18, 2024
f2be9eb
Merge branch 'master' of github.com:paritytech/polkadot-sdk into jpse…
jpserrat Aug 3, 2024
a4eb3be
retry only if there is at least one sucessufull deposit
jpserrat Aug 4, 2024
52682e4
Merge branch 'master' of github.com:paritytech/polkadot-sdk into jpse…
acatangiu Aug 7, 2024
d0ae626
address comments
acatangiu Aug 7, 2024
1a89812
add prdoc
acatangiu Aug 7, 2024
5567e0c
emulated-tests: AH test assets not sufficient
acatangiu Aug 7, 2024
895bd62
Merge branch 'master' into jpserrat/depositing-multiple-assets
franciscoaguirre Aug 8, 2024
90906b1
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=ro…
Aug 8, 2024
521aab4
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=we…
Aug 8, 2024
235b86c
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=as…
Aug 8, 2024
c4b5495
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=as…
Aug 8, 2024
899a52b
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=br…
Aug 8, 2024
cd1942f
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=co…
Aug 8, 2024
e76eea7
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=co…
Aug 8, 2024
b068eac
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=br…
Aug 8, 2024
bba43d9
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=pe…
Aug 8, 2024
d576c4a
fix: people chains had odd weight files for xcm
franciscoaguirre Aug 8, 2024
bc0759e
fix: typos and missing functions
franciscoaguirre Aug 8, 2024
3d80dc8
Merge branch 'master' into jpserrat/depositing-multiple-assets
franciscoaguirre Aug 8, 2024
62a40a3
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=pe…
Aug 8, 2024
0036c93
Merge branch 'master' into jpserrat/depositing-multiple-assets
franciscoaguirre Aug 9, 2024
516bed8
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=pe…
Aug 9, 2024
636e06d
".git/.scripts/commands/bench/bench.sh" --subcommand=xcm --runtime=pe…
Aug 9, 2024
d5c9d4b
emulated tests: add regression test
acatangiu Aug 9, 2024
c9ae0c2
Merge branch 'master' into jpserrat/depositing-multiple-assets
franciscoaguirre Aug 9, 2024
22e50b2
fix test after new weights
acatangiu Aug 9, 2024
8b960a4
Merge branch 'master' of github.com:paritytech/polkadot-sdk into jpse…
acatangiu Aug 9, 2024
955f4ac
update prdoc
acatangiu Aug 9, 2024
e565f31
Merge branch 'master' into jpserrat/depositing-multiple-assets
franciscoaguirre Aug 9, 2024
fe142fa
fix people emulated tests after weights update
acatangiu Aug 9, 2024
948db98
fix(people): remove unnecessary import
franciscoaguirre Aug 9, 2024
e628819
Merge branch 'master' into jpserrat/depositing-multiple-assets
franciscoaguirre Aug 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 47 additions & 11 deletions polkadot/xcm/xcm-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,14 +858,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
let old_holding = self.holding.clone();
let result = Config::TransactionalProcessor::process(|| {
let deposited = self.holding.saturating_take(assets);
for asset in deposited.into_assets_iter() {
Config::AssetTransactor::deposit_asset(
&asset,
&beneficiary,
Some(&self.context),
)?;
}
Ok(())
self.deposit_assets_with_retry(&deposited, &beneficiary)
});
if Config::TransactionalProcessor::IS_TRANSACTIONAL && result.is_err() {
self.holding = old_holding;
Expand All @@ -890,9 +883,7 @@ impl<Config: config::Config> XcmExecutor<Config> {

// now take assets to deposit (excluding transport_fee)
let deposited = self.holding.saturating_take(assets);
for asset in deposited.assets_iter() {
Config::AssetTransactor::deposit_asset(&asset, &dest, Some(&self.context))?;
}
self.deposit_assets_with_retry(&deposited, &dest)?;
// Note that we pass `None` as `maybe_failed_bin` and drop any assets which
// cannot be reanchored because we have already called `deposit_asset` on all
// assets.
Expand Down Expand Up @@ -1282,4 +1273,49 @@ impl<Config: config::Config> XcmExecutor<Config> {
}),
}
}


/// Deposit `to_deposit` assets to `beneficiary`, without giving up on the first (transient) error, and
/// retrying once just in case one of the subsequently deposited assets satisfy some requirement.
///
/// Most common transient error is: `beneficiary` account does not yet exist and the first asset(s) in
/// the (sorted) list does not satisfy ED, but a subsequent one in the list does.
///
/// This function can write into storage and also return an error at the same time, it should always be
/// called within a transactional process.
fn deposit_assets_with_retry(
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
&mut self,
to_deposit: &AssetsInHolding,
beneficiary: &Location,
) -> Result<(), XcmError> {
let mut failed_deposits = Vec::with_capacity(to_deposit.len());
let mut first_error = None;

for asset in to_deposit.assets_iter() {
let asset_result =
Config::AssetTransactor::deposit_asset(&asset, &beneficiary, Some(&self.context));
// if deposit failed for asset, mark it for retry after depositing the others.
if asset_result.is_err() {
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
if first_error.is_none() {
first_error = Some(asset_result);
}
failed_deposits.push(asset);
}
}

acatangiu marked this conversation as resolved.
Show resolved Hide resolved
if failed_deposits.len() == to_deposit.len() && first_error.is_some() {
// all deposits failed, in this case return the first error as there is no point in retrying.
return first_error.unwrap();
}
acatangiu marked this conversation as resolved.
Show resolved Hide resolved

// retry previously failed deposits, this time short-circuiting on any error.
for asset in failed_deposits {
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
Config::AssetTransactor::deposit_asset(
&asset,
&beneficiary,
Some(&self.context),
)?;
}
Ok(())
}
}
Loading