Pallets: Assets destroy_accounts releases the deposit (paritytech#1…

…4443)

* assset accounts destroy releases the deposit

* enumerate

* Update frame/assets/src/functions.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* import defensive from frame_support

---------

Co-authored-by: Gavin Wood <gavin@parity.io>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: parity-processbot <>

frame/assets/src/functions.rs

@@ -18,7 +18,7 @@
 //! Functions for the Assets pallet.
 
 use super::*;
-use frame_support::{traits::Get, BoundedVec};
+use frame_support::{defensive, traits::Get, BoundedVec};
 
 #[must_use]
 pub(super) enum DeadConsequence {
@@ -760,11 +760,22 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				let mut details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
 				// Should only destroy accounts while the asset is in a destroying state
 				ensure!(details.status == AssetStatus::Destroying, Error::<T, I>::IncorrectStatus);
-
-				for (who, v) in Account::<T, I>::drain_prefix(&id) {
-					let _ = Self::dead_account(&who, &mut details, &v.reason, true);
-					dead_accounts.push(who);
-					if dead_accounts.len() >= (max_items as usize) {
+				for (i, (who, mut v)) in Account::<T, I>::iter_prefix(&id).enumerate() {
+					// unreserve the existence deposit if any
+					if let Some((depositor, deposit)) = v.reason.take_deposit_from() {
+						T::Currency::unreserve(&depositor, deposit);
+					} else if let Some(deposit) = v.reason.take_deposit() {
+						T::Currency::unreserve(&who, deposit);
+					}
+					if let Remove = Self::dead_account(&who, &mut details, &v.reason, false) {
+						Account::<T, I>::remove(&id, &who);
+						dead_accounts.push(who);
+					} else {
+						// deposit may have been released, need to update `Account`
+						Account::<T, I>::insert(&id, &who, v);
+						defensive!("destroy did not result in dead account?!");
+					}
+					if i + 1 >= (max_items as usize) {
 						break
 					}
 				}

frame/assets/src/tests.rs

@@ -1741,3 +1741,37 @@ fn weights_sane() {
 	let info = crate::Call::<Test>::finish_destroy { id: 10 }.get_dispatch_info();
 	assert_eq!(<() as crate::WeightInfo>::finish_destroy(), info.weight);
 }
+
+#[test]
+fn asset_destroy_refund_existence_deposit() {
+	new_test_ext().execute_with(|| {
+		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, false, 1));
+		Balances::make_free_balance_be(&1, 100);
+		let admin = 1;
+		let admin_origin = RuntimeOrigin::signed(admin);
+
+		let account2 = 2; // account with own deposit
+		let account3 = 3; // account with admin's deposit
+		Balances::make_free_balance_be(&account2, 100);
+
+		assert_eq!(Balances::reserved_balance(&account2), 0);
+		assert_eq!(Balances::reserved_balance(&account3), 0);
+		assert_eq!(Balances::reserved_balance(&admin), 0);
+
+		assert_ok!(Assets::touch(RuntimeOrigin::signed(account2), 0));
+		assert_ok!(Assets::touch_other(admin_origin.clone(), 0, account3));
+
+		assert_eq!(Balances::reserved_balance(&account2), 10);
+		assert_eq!(Balances::reserved_balance(&account3), 0);
+		assert_eq!(Balances::reserved_balance(&admin), 10);
+
+		assert_ok!(Assets::start_destroy(admin_origin.clone(), 0));
+		assert_ok!(Assets::destroy_accounts(admin_origin.clone(), 0));
+		assert_ok!(Assets::destroy_approvals(admin_origin.clone(), 0));
+		assert_ok!(Assets::finish_destroy(admin_origin.clone(), 0));
+
+		assert_eq!(Balances::reserved_balance(&account2), 0);
+		assert_eq!(Balances::reserved_balance(&account3), 0);
+		assert_eq!(Balances::reserved_balance(&admin), 0);
+	});
+}