[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):
#11443.

Fixes #10412

---------

Co-authored-by: clangenb <clangenb@users.noreply.github.com>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: clangenb

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

substrate/frame/assets/src/functions.rs

@@ -384,6 +384,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		}
 
 		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::<T, I>::remove(&id, &who);
 		} else {
 			debug_assert!(false, "refund did not result in dead account?!");
@@ -392,6 +396,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			return Ok(());
 		}
 
+		if !account.balance.is_zero() {
+			Self::deposit_event(Event::Burned {
+				asset_id: id.clone(),
+				owner: who.clone(),
+				balance: account.balance,
+			});
+		}
+
 		Asset::<T, I>::insert(&id, details);
 		// Executing a hook here is safe, since it is not in a `mutate`.
 		T::Freezer::died(id.clone(), &who);

substrate/frame/assets/src/lib.rs

@@ -2004,10 +2004,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				}
 			}
 
-			// 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!(

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;

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 { .. }))));
 	});
 }