From 7d920296b65b538375afb25c73305d3fe4a57d03 Mon Sep 17 00:00:00 2001 From: clangenb <37865735+clangenb@users.noreply.github.com> Date: Wed, 25 Mar 2026 00:11:04 +0100 Subject: [PATCH] [pallet-assets] fix: decrement supply when refund burns balance (#11441) When a user calls `refund` with `allow_burn = true`, their token balance is destroyed, but the asset's total supply was never updated. This caused `total_issuance()` to overcount. The fix decrements supply and emits a `Burned` event, consistent with how every other burn path works. In production, burning path is rarely triggered. The fungibles trait interface always passes `allow_burn = false`, so only users manually submitting the refund extrinsic with the burn flag would hit it. Follow-up issue for migrating the discrepancy (observed on Westend): https://github.com/paritytech/polkadot-sdk/issues/11443. Fixes #10412 --------- Co-authored-by: clangenb Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> (cherry picked from commit 44f97882cb5c890a6d228df63499953f317348d0) --- prdoc/pr_11441.prdoc | 14 +++++++++++++ substrate/frame/assets/src/functions.rs | 12 ++++++++++++ substrate/frame/assets/src/lib.rs | 9 +++++---- substrate/frame/assets/src/mock.rs | 6 +++++- substrate/frame/assets/src/tests.rs | 26 +++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 prdoc/pr_11441.prdoc diff --git a/prdoc/pr_11441.prdoc b/prdoc/pr_11441.prdoc new file mode 100644 index 0000000000000..8f462b84194ba --- /dev/null +++ b/prdoc/pr_11441.prdoc @@ -0,0 +1,14 @@ +title: '[pallet-assets] fix: decrement supply when refund burns balance' +doc: +- audience: Runtime Dev + description: |- + When a user calls `refund` with `allow_burn = true`, their token balance is destroyed, but the asset's total supply was never updated. This caused `total_issuance()` to overcount. The fix decrements supply and emits a `Burned` event, consistent with how every other burn path works. + + In production, burning path is rarely triggered. The fungibles trait interface always passes `allow_burn = false`, so only users manually submitting the refund extrinsic with the burn flag would hit it. + + Follow-up issue for migrating the discrepancy (observed on Westend): https://github.com/paritytech/polkadot-sdk/issues/11443. + + Fixes #10412 +crates: +- name: pallet-assets + bump: patch diff --git a/substrate/frame/assets/src/functions.rs b/substrate/frame/assets/src/functions.rs index dc626b9d7e8f3..8633fb8e9fafd 100644 --- a/substrate/frame/assets/src/functions.rs +++ b/substrate/frame/assets/src/functions.rs @@ -384,6 +384,10 @@ impl, I: 'static> Pallet { } if let Remove = Self::dead_account(&who, &mut details, &account.reason, false) { + if !account.balance.is_zero() { + debug_assert!(details.supply >= account.balance, "supply < balance; qed"); + details.supply = details.supply.saturating_sub(account.balance); + } Account::::remove(&id, &who); } else { debug_assert!(false, "refund did not result in dead account?!"); @@ -392,6 +396,14 @@ impl, I: 'static> Pallet { return Ok(()); } + if !account.balance.is_zero() { + Self::deposit_event(Event::Burned { + asset_id: id.clone(), + owner: who.clone(), + balance: account.balance, + }); + } + Asset::::insert(&id, details); // Executing a hook here is safe, since it is not in a `mutate`. T::Freezer::died(id.clone(), &who); diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index 80576cd1a5d01..3ea6a82413a92 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -2024,10 +2024,11 @@ impl, I: 'static> Pallet { } } - // Using >= instead of == because the provided `do_refund` implementation destroys - // the account balance without decrementing the asset supply. This causes the - // tracked supply to permanently exceed the actual sum of balances + holds - // whenever a refund occurs. + // Using >= instead of == because the provided `do_refund` implementation + // historically destroyed the account balance without decrementing the asset + // supply. Although this has been fixed, existing on-chain state may still + // contain overcounted supply from prior refunds. + // TODO: add a migration to recalculate supply, then tighten this to `==`. ensure!(details.supply >= calculated_supply, "Asset supply mismatch"); ensure!(details.accounts == calculated_accounts, "Asset account count mismatch"); ensure!( diff --git a/substrate/frame/assets/src/mock.rs b/substrate/frame/assets/src/mock.rs index 94cc23f143cc8..581ff901690bc 100644 --- a/substrate/frame/assets/src/mock.rs +++ b/substrate/frame/assets/src/mock.rs @@ -173,7 +173,11 @@ pub(crate) fn set_balance_on_hold(asset: u32, who: u64, amount: u64) { pub(crate) fn clear_balance_on_hold(asset: u32, who: u64) { OnHold::mutate(|v| { - v.remove(&(asset, who)); + if let Some(amount) = v.remove(&(asset, who)) { + if amount > 0 { + assert_ok!(Assets::increase_balance(asset, &who, amount, |_| Ok(()))); + } + } }); } pub struct TestFreezer; diff --git a/substrate/frame/assets/src/tests.rs b/substrate/frame/assets/src/tests.rs index 531bfb672a63a..6c03737bfa948 100644 --- a/substrate/frame/assets/src/tests.rs +++ b/substrate/frame/assets/src/tests.rs @@ -164,9 +164,35 @@ fn refunding_asset_deposit_with_burn_should_work() { Balances::make_free_balance_be(&1, 100); assert_ok!(Assets::touch(RuntimeOrigin::signed(1), 0)); assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100)); + assert_eq!(Assets::total_supply(0), 100); assert_ok!(Assets::refund(RuntimeOrigin::signed(1), 0, true)); assert_eq!(Balances::reserved_balance(&1), 0); assert_eq!(Assets::balance(1, 0), 0); + assert_eq!(Assets::total_supply(0), 0); + System::assert_last_event(RuntimeEvent::Assets(crate::Event::Burned { + asset_id: 0, + owner: 1, + balance: 100, + })); + }); +} + +#[test] +fn refund_with_zero_balance_does_not_emit_burned() { + build_and_execute(|| { + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, false, 1)); + Balances::make_free_balance_be(&1, 100); + assert_ok!(Assets::touch(RuntimeOrigin::signed(1), 0)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100)); + assert_ok!(Assets::burn(RuntimeOrigin::signed(1), 0, 1, 100)); + assert_eq!(Assets::total_supply(0), 0); + + System::reset_events(); + assert_ok!(Assets::refund(RuntimeOrigin::signed(1), 0, false)); + assert_eq!(Assets::total_supply(0), 0); + assert!(System::events() + .iter() + .all(|e| !matches!(e.event, RuntimeEvent::Assets(crate::Event::Burned { .. })))); }); }