Skip to content

Commit 162b9c5

Browse files
paritytech-release-backport-bot[bot]acatangiugithub-actions[bot]yrongbkontur
authored
[stable2603] Backport #11910 (#12476)
Backport #11910 into `stable2603` from acatangiu. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Adrian Catangiu <adrian@parity.io> Co-authored-by: Adrian Catangiu <adrian@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: ron <yrong1997@gmail.com> Co-authored-by: Branislav Kontur <bkontur@gmail.com> Co-authored-by: Egor_P <egor@parity.io>
1 parent a697dd4 commit 162b9c5

11 files changed

Lines changed: 504 additions & 107 deletions

File tree

bridges/snowbridge/primitives/beacon/src/bls.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub fn fast_aggregate_verify(
3636

3737
/// Decompress one public key into a point in G1.
3838
pub fn prepare_milagro_pubkey(pubkey: &PublicKey) -> Result<PublicKeyPrepared, BlsError> {
39-
PublicKeyPrepared::from_bytes_unchecked(&pubkey.0).map_err(|_| BlsError::InvalidPublicKey)
39+
PublicKeyPrepared::from_bytes(&pubkey.0).map_err(|_| BlsError::InvalidPublicKey)
4040
}
4141

4242
/// Prepare for G1 public keys.

bridges/snowbridge/runtime/runtime-common/src/v2/register_token.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use xcm::prelude::Location;
1717
pub struct ForeignAssetOwner<IsForeign, AssetInspect, AccountId, LocationToAccountId, L = Location>(
1818
core::marker::PhantomData<(IsForeign, AssetInspect, AccountId, LocationToAccountId, L)>,
1919
);
20+
2021
impl<
2122
IsForeign: ContainsPair<L, L>,
2223
AssetInspect: frame_support::traits::fungibles::roles::Inspect<AccountId>,
@@ -41,9 +42,10 @@ where
4142
return Err(origin);
4243
}
4344
let asset_location: Location = asset_location.clone().into();
44-
let owner = AssetInspect::owner(asset_location.into());
45+
let owner = AssetInspect::owner(asset_location.into()).ok_or_else(|| origin.clone())?;
4546
let location: Location = origin_location.clone().into();
46-
let from = LocationToAccountId::convert_location(&location);
47+
let from =
48+
LocationToAccountId::convert_location(&location).ok_or_else(|| origin.clone())?;
4749
if from != owner {
4850
return Err(origin);
4951
}
@@ -64,6 +66,7 @@ where
6466
pub struct LocalAssetOwner<MatchAssetId, AssetInspect, AccountId, AssetId, L = Location>(
6567
core::marker::PhantomData<(MatchAssetId, AssetInspect, AccountId, AssetId, L)>,
6668
);
69+
6770
impl<
6871
MatchAssetId: MaybeEquivalence<L, AssetId>,
6972
AssetInspect: frame_support::traits::fungibles::roles::Inspect<AccountId>,
@@ -84,8 +87,8 @@ where
8487
asset_location: &L,
8588
) -> Result<Self::Success, RuntimeOrigin> {
8689
let who = ensure_signed(origin.clone()).map_err(|_| origin.clone())?;
87-
let asset_id = MatchAssetId::convert(asset_location).ok_or(origin.clone())?;
88-
let owner = AssetInspect::owner(asset_id.into()).ok_or(origin.clone())?;
90+
let asset_id = MatchAssetId::convert(asset_location).ok_or_else(|| origin.clone())?;
91+
let owner = AssetInspect::owner(asset_id.into()).ok_or_else(|| origin.clone())?;
8992
if who != owner {
9093
return Err(origin);
9194
}

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/snowbridge_pallet_ethereum_client.rs

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
//! Autogenerated weights for `snowbridge_pallet_ethereum_client`
1717
//!
1818
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0
19-
//! DATE: 2025-02-21, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
19+
//! DATE: 2026-06-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
2020
//! WORST CASE MAP SIZE: `1000000`
21-
//! HOSTNAME: `d3b41be4aae8`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
21+
//! HOSTNAME: `fdfc92755265`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
2222
//! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: 1024
2323
2424
// Executed Command:
@@ -70,17 +70,17 @@ impl<T: frame_system::Config> snowbridge_pallet_ethereum_client::WeightInfo for
7070
// Proof Size summary in bytes:
7171
// Measured: `42`
7272
// Estimated: `3501`
73-
// Minimum execution time: 105_684_862_000 picoseconds.
74-
Weight::from_parts(105_818_568_000, 0)
73+
// Minimum execution time: 633_619_446_000 picoseconds.
74+
Weight::from_parts(634_122_187_000, 0)
7575
.saturating_add(Weight::from_parts(0, 3501))
7676
.saturating_add(T::DbWeight::get().reads(2))
7777
.saturating_add(T::DbWeight::get().writes(8))
7878
}
7979
/// Storage: `EthereumBeaconClient::OperatingMode` (r:1 w:0)
8080
/// Proof: `EthereumBeaconClient::OperatingMode` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`)
81-
/// Storage: `EthereumBeaconClient::LatestFinalizedBlockRoot` (r:1 w:0)
81+
/// Storage: `EthereumBeaconClient::LatestFinalizedBlockRoot` (r:1 w:1)
8282
/// Proof: `EthereumBeaconClient::LatestFinalizedBlockRoot` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
83-
/// Storage: `EthereumBeaconClient::FinalizedBeaconState` (r:1 w:0)
83+
/// Storage: `EthereumBeaconClient::FinalizedBeaconState` (r:1 w:1)
8484
/// Proof: `EthereumBeaconClient::FinalizedBeaconState` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`)
8585
/// Storage: `EthereumBeaconClient::NextSyncCommittee` (r:1 w:0)
8686
/// Proof: `EthereumBeaconClient::NextSyncCommittee` (`max_values`: Some(1), `max_size`: Some(92372), added: 92867, mode: `MaxEncodedLen`)
@@ -90,14 +90,19 @@ impl<T: frame_system::Config> snowbridge_pallet_ethereum_client::WeightInfo for
9090
/// Proof: `EthereumBeaconClient::ValidatorsRoot` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
9191
/// Storage: `EthereumBeaconClient::LatestSyncCommitteeUpdatePeriod` (r:1 w:0)
9292
/// Proof: `EthereumBeaconClient::LatestSyncCommitteeUpdatePeriod` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`)
93+
/// Storage: `EthereumBeaconClient::FinalizedBeaconStateIndex` (r:1 w:1)
94+
/// Proof: `EthereumBeaconClient::FinalizedBeaconStateIndex` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`)
95+
/// Storage: `EthereumBeaconClient::FinalizedBeaconStateMapping` (r:1 w:1)
96+
/// Proof: `EthereumBeaconClient::FinalizedBeaconStateMapping` (`max_values`: None, `max_size`: Some(36), added: 2511, mode: `MaxEncodedLen`)
9397
fn submit() -> Weight {
9498
// Proof Size summary in bytes:
95-
// Measured: `92738`
99+
// Measured: `92751`
96100
// Estimated: `93857`
97-
// Minimum execution time: 27_295_666_000 picoseconds.
98-
Weight::from_parts(27_351_060_000, 0)
101+
// Minimum execution time: 27_054_037_000 picoseconds.
102+
Weight::from_parts(27_108_878_000, 0)
99103
.saturating_add(Weight::from_parts(0, 93857))
100-
.saturating_add(T::DbWeight::get().reads(7))
104+
.saturating_add(T::DbWeight::get().reads(9))
105+
.saturating_add(T::DbWeight::get().writes(4))
101106
}
102107
/// Storage: `EthereumBeaconClient::OperatingMode` (r:1 w:0)
103108
/// Proof: `EthereumBeaconClient::OperatingMode` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`)
@@ -117,8 +122,8 @@ impl<T: frame_system::Config> snowbridge_pallet_ethereum_client::WeightInfo for
117122
// Proof Size summary in bytes:
118123
// Measured: `92738`
119124
// Estimated: `93857`
120-
// Minimum execution time: 133_134_525_000 picoseconds.
121-
Weight::from_parts(133_396_722_000, 0)
125+
// Minimum execution time: 655_531_071_000 picoseconds.
126+
Weight::from_parts(656_067_127_000, 0)
122127
.saturating_add(Weight::from_parts(0, 93857))
123128
.saturating_add(T::DbWeight::get().reads(7))
124129
.saturating_add(T::DbWeight::get().writes(2))

cumulus/primitives/utility/src/lib.rs

Lines changed: 148 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,10 +259,11 @@ impl<
259259
if outstanding_credit.peek().saturating_sub(refund_balance) >= minimum_balance {
260260
outstanding_credit.extract(refund_balance)
261261
} else {
262-
// If refunding would leave less than ED, we refund ED to ensure the
263-
// OnUnbalanced handler receives at least ED when this trader is dropped.
264-
// This prevents dust amounts that can't be properly handled.
265-
outstanding_credit.extract(minimum_balance)
262+
// Keep at least ED in outstanding credit for the OnUnbalanced drop
263+
// handler. Refund only the surplus above ED (zero if outstanding < ED).
264+
let keep = minimum_balance.min(outstanding_credit.peek());
265+
let refund_amount = outstanding_credit.peek().saturating_sub(keep);
266+
outstanding_credit.extract(refund_amount)
266267
}
267268
})?;
268269
// Subtract the refunded weight from existing weight.
@@ -1060,6 +1061,149 @@ mod test_trader {
10601061
_ => panic!("Expected fungible asset"),
10611062
}
10621063
}
1064+
1065+
#[test]
1066+
fn take_first_asset_trader_refund_keeps_ed_for_drop_handler() {
1067+
// Regression test: when refunding would leave less than ED in outstanding
1068+
// credit, `refund_weight` must refund `outstanding - ED` and keep ED for the
1069+
// `OnUnbalanced` drop handler.
1070+
use core::cell::Cell;
1071+
1072+
const ED: u128 = 10;
1073+
const BUY_FEE: u128 = 15;
1074+
const REFUND_FEE: u128 = 10;
1075+
1076+
type TestAccountId = u32;
1077+
type TestAssetId = Location;
1078+
type TestBalance = u128;
1079+
1080+
struct TestAssets;
1081+
impl MatchesFungibles<TestAssetId, TestBalance> for TestAssets {
1082+
fn matches_fungibles(a: &Asset) -> Result<(TestAssetId, TestBalance), Error> {
1083+
match a {
1084+
Asset { fun: Fungible(amount), id: AssetId(_) } => {
1085+
Ok((Location::new(0, [GeneralIndex(1)]), *amount))
1086+
},
1087+
_ => Err(Error::AssetNotHandled),
1088+
}
1089+
}
1090+
}
1091+
impl fungibles::Inspect<TestAccountId> for TestAssets {
1092+
type AssetId = TestAssetId;
1093+
type Balance = TestBalance;
1094+
fn total_issuance(_: Self::AssetId) -> Self::Balance {
1095+
0
1096+
}
1097+
fn minimum_balance(_: Self::AssetId) -> Self::Balance {
1098+
ED
1099+
}
1100+
fn balance(_: Self::AssetId, _: &TestAccountId) -> Self::Balance {
1101+
0
1102+
}
1103+
fn total_balance(_: Self::AssetId, _: &TestAccountId) -> Self::Balance {
1104+
0
1105+
}
1106+
fn reducible_balance(
1107+
_: Self::AssetId,
1108+
_: &TestAccountId,
1109+
_: Preservation,
1110+
_: Fortitude,
1111+
) -> Self::Balance {
1112+
0
1113+
}
1114+
fn can_deposit(
1115+
_: Self::AssetId,
1116+
_: &TestAccountId,
1117+
_: Self::Balance,
1118+
_: Provenance,
1119+
) -> DepositConsequence {
1120+
DepositConsequence::Success
1121+
}
1122+
fn can_withdraw(
1123+
_: Self::AssetId,
1124+
_: &TestAccountId,
1125+
_: Self::Balance,
1126+
) -> WithdrawConsequence<Self::Balance> {
1127+
WithdrawConsequence::Success
1128+
}
1129+
fn asset_exists(_: Self::AssetId) -> bool {
1130+
true
1131+
}
1132+
}
1133+
impl fungibles::Mutate<TestAccountId> for TestAssets {}
1134+
impl fungibles::Balanced<TestAccountId> for TestAssets {
1135+
type OnDropCredit = fungibles::DecreaseIssuance<TestAccountId, Self>;
1136+
type OnDropDebt = fungibles::IncreaseIssuance<TestAccountId, Self>;
1137+
}
1138+
impl fungibles::Unbalanced<TestAccountId> for TestAssets {
1139+
fn handle_dust(_: fungibles::Dust<TestAccountId, Self>) {}
1140+
fn write_balance(
1141+
_: Self::AssetId,
1142+
_: &TestAccountId,
1143+
_: Self::Balance,
1144+
) -> Result<Option<Self::Balance>, DispatchError> {
1145+
Ok(None)
1146+
}
1147+
fn set_total_issuance(_: Self::AssetId, _: Self::Balance) {}
1148+
}
1149+
1150+
// Charge BUY_FEE on the first call (during `buy_weight`), REFUND_FEE on the
1151+
// next (during `refund_weight`).
1152+
thread_local! {
1153+
static CALLS: Cell<u32> = const { Cell::new(0) };
1154+
static HANDLER_RECEIVED: Cell<u128> = const { Cell::new(0) };
1155+
}
1156+
struct FeeCharger;
1157+
impl ChargeWeightInFungibles<TestAccountId, TestAssets> for FeeCharger {
1158+
fn charge_weight_in_fungibles(
1159+
_: <TestAssets as fungibles::Inspect<TestAccountId>>::AssetId,
1160+
_: Weight,
1161+
) -> Result<<TestAssets as fungibles::Inspect<TestAccountId>>::Balance, XcmError> {
1162+
let n = CALLS.with(|c| {
1163+
let v = c.get();
1164+
c.set(v + 1);
1165+
v
1166+
});
1167+
Ok(if n == 0 { BUY_FEE } else { REFUND_FEE })
1168+
}
1169+
}
1170+
1171+
struct HandleFees;
1172+
impl OnUnbalancedT<fungibles::Credit<TestAccountId, TestAssets>> for HandleFees {
1173+
fn on_unbalanced(credit: fungibles::Credit<TestAccountId, TestAssets>) {
1174+
HANDLER_RECEIVED.with(|h| h.set(credit.peek()));
1175+
}
1176+
}
1177+
1178+
type Trader =
1179+
TakeFirstAssetTrader<TestAccountId, FeeCharger, TestAssets, TestAssets, HandleFees>;
1180+
let mut trader = <Trader as WeightTrader>::new();
1181+
let ctx = XcmContext { origin: None, message_id: XcmHash::default(), topic: None };
1182+
1183+
// `buy_weight` populates outstanding_credit with BUY_FEE = 15.
1184+
let payment = asset_to_holding((Here, BUY_FEE).into());
1185+
assert_ok!(trader.buy_weight(Weight::from_parts(1_000, 1_000), payment, &ctx));
1186+
1187+
// `refund_weight` would compute refund=10, but outstanding(15) - 10 = 5 < ED(10),
1188+
// so it must refund only `outstanding - ED` = 5 and keep ED for the handler.
1189+
let refund = trader
1190+
.refund_weight(Weight::from_parts(500, 500), &ctx)
1191+
.expect("non-zero refund");
1192+
let refund_assets: Vec<Asset> = refund.fungible_assets_iter().collect();
1193+
assert_eq!(refund_assets.len(), 1);
1194+
match &refund_assets[0] {
1195+
Asset { fun: Fungible(amount), .. } => {
1196+
assert_eq!(*amount, BUY_FEE - ED, "refund must be `outstanding - ED`, not ED")
1197+
},
1198+
_ => panic!("expected fungible refund"),
1199+
}
1200+
1201+
// Drop the trader; the handler must receive at least ED.
1202+
drop(trader);
1203+
HANDLER_RECEIVED.with(|h| {
1204+
assert_eq!(h.get(), ED, "OnUnbalanced drop handler must receive at least ED")
1205+
});
1206+
}
10631207
}
10641208

10651209
/// Implementation of `xcm_builder::EnsureDelivery` which helps to ensure delivery to the

polkadot/xcm/pallet-xcm/src/tests/mod.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,57 @@ fn execute_withdraw_to_deposit_works() {
409409
});
410410
}
411411

412+
/// Defense-in-depth: extrinsic failure leads to a full storage rollback, but even so, a
413+
/// partial-fail multi-asset `WithdrawAsset` must not leak a `Credit` imbalance across the
414+
/// transactional rollback and should preserve the `TotalIssuance == Σ balances` invariant.
415+
///
416+
/// The message withdraws the native asset (succeeds, producing a `NegativeImbalance`) and a
417+
/// foreign asset that `ALICE` does not hold (fails). The whole instruction must roll back,
418+
/// restoring `ALICE`'s balance _and_ leaving `TotalIssuance` untouched.
419+
#[test]
420+
fn partial_fail_withdraw_asset_preserves_total_issuance() {
421+
let balances = vec![(ALICE, INITIAL_BALANCE)];
422+
new_test_ext_with_balances(balances).execute_with(|| {
423+
let weight = BaseXcmWeight::get() * 2;
424+
let alice_location: Location = AccountId32 { network: None, id: ALICE.into() }.into();
425+
426+
let total_issuance_before = Balances::total_issuance();
427+
assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE);
428+
429+
// Native `Here` asset sorts first and withdraws successfully (producing a live
430+
// `NegativeImbalance`); the foreign asset (which `ALICE` holds none of) is withdrawn
431+
// second and fails, aborting the instruction.
432+
let assets: Assets = vec![Asset::from((Here, SEND_AMOUNT)), ForeignAsset::get()].into();
433+
let message = Xcm(vec![WithdrawAsset(assets)]);
434+
let mut hash = fake_message_hash(&message);
435+
436+
// Drive the executor directly: going through extrinsic masks the issue since extrinsic
437+
// fully reverts any storage changes.
438+
let outcome = XcmExecutor::<XcmConfig>::prepare_and_execute(
439+
alice_location,
440+
message,
441+
&mut hash,
442+
weight,
443+
weight, // weight credit, so `TakeWeightCredit` lets the message through the barrier
444+
);
445+
// The multi-asset withdraw must fail because `ALICE` holds none of the foreign asset.
446+
assert!(
447+
matches!(outcome, Outcome::Incomplete { .. }),
448+
"partial-fail withdraw must abort the instruction, got {:?}",
449+
outcome
450+
);
451+
452+
// Rollback must fully restore state: `ALICE`'s native balance is untouched
453+
assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE);
454+
// and total issuance is preserved.
455+
assert_eq!(
456+
Balances::total_issuance(),
457+
total_issuance_before,
458+
"TotalIssuance must be preserved after a partial-fail WithdrawAsset rollback"
459+
);
460+
});
461+
}
462+
412463
/// Test XCM authorized aliases.
413464
#[test]
414465
fn authorized_aliases_work() {

polkadot/xcm/xcm-executor/src/assets.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,24 @@ impl AssetsInHolding {
186186
)
187187
}
188188

189+
/// A consuming iterator that yields one [`AssetsInHolding`] per individual asset —
190+
/// each fungible imbalance and each non-fungible instance — by transferring ownership
191+
/// out of the underlying maps. No `AssetId` clones and no `BTreeMap` lookups, unlike
192+
/// the borrowing-iterator + `try_take` pattern.
193+
///
194+
/// Iteration order matches [`Self::assets_iter`] and [`Self::into_assets_iter`]:
195+
/// fungibles in sorted-by-`AssetId` order, then non-fungibles in sorted order.
196+
pub fn into_per_asset_holdings(self) -> impl Iterator<Item = AssetsInHolding> {
197+
let fungibles = self.fungible.into_iter().map(|(asset_id, accounting)| {
198+
AssetsInHolding::new_from_fungible_credit(asset_id, accounting)
199+
});
200+
let non_fungibles = self
201+
.non_fungible
202+
.into_iter()
203+
.map(|(class, instance)| AssetsInHolding::new_from_non_fungible(class, instance));
204+
fungibles.chain(non_fungibles)
205+
}
206+
189207
/// A borrowing iterator over all assets.
190208
pub fn assets_iter(&self) -> impl Iterator<Item = Asset> + '_ {
191209
self.fungible_assets_iter().chain(self.non_fungible_assets_iter())

0 commit comments

Comments
 (0)