change(pallet-assets-holder): revert died to make it an infallible …

…function

substrate/frame/assets-holder/src/impl_fungibles.rs

@@ -39,16 +39,22 @@ impl<T: Config<I>, I: 'static> BalanceOnHold<T::AssetId, T::AccountId, T::Balanc
 		BalancesOnHold::<T, I>::get(asset, who)
 	}
 
-	fn died(asset: T::AssetId, who: &T::AccountId) -> DispatchResult {
-		Holds::<T, I>::try_mutate(asset.clone(), who, |holds| {
-			for l in holds.iter() {
-				Self::burn_all_held(asset.clone(), &l.id, who, Precision::Exact, Fortitude::Force)?;
-			}
+	fn died(asset: T::AssetId, who: &T::AccountId) {
+		if pallet_assets::Pallet::<T, I>::can_withdraw(asset.clone(), who, Zero::zero()) ==
+			WithdrawConsequence::Success
+		{
+			defensive!(
+				Holds::<T, I>::get(asset.clone(), who).is_empty(),
+				"The list of Holds should be empty before allowing an account to die"
+			);
+			defensive!(
+				BalancesOnHold::<T, I>::get(asset.clone(), who).is_none(),
+				"The should not be a balance on hold before allowing to die"
+			);
+		}
 
-			*holds = BoundedVec::new();
-			BalancesOnHold::<T, I>::remove(asset.clone(), who);
-			Ok(())
-		})
+		Holds::<T, I>::remove(asset.clone(), who);
+		BalancesOnHold::<T, I>::remove(asset, who);
 	}
 }
 

substrate/frame/assets-holder/src/tests.rs

@@ -77,10 +77,21 @@ mod impl_balance_on_hold {
 	}
 
 	#[test]
+	#[should_panic = "The list of Holds should be empty before allowing an account to die"]
+	fn died_fails_if_holds_exist() {
+		new_test_ext(|| {
+			test_hold(DummyHoldReason::Governance, 1);
+			AssetsHolder::died(ASSET_ID, &WHO);
+		});
+	}
+
+	#[test]
+	#[should_panic = "The list of Holds should be empty before allowing an account to die"]
 	fn died_works() {
 		new_test_ext(|| {
 			test_hold(DummyHoldReason::Governance, 1);
-			assert_ok!(AssetsHolder::died(ASSET_ID, &WHO));
+			test_release(DummyHoldReason::Governance);
+			AssetsHolder::died(ASSET_ID, &WHO);
 			assert!(BalancesOnHold::<Test>::get(ASSET_ID, WHO).is_none());
 			assert!(Holds::<Test>::get(ASSET_ID, WHO).is_empty());
 		});

substrate/frame/assets/src/functions.rs

@@ -384,7 +384,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		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);
-		T::Holder::died(id, &who)?;
+		T::Holder::died(id, &who);
 		Ok(())
 	}
 
@@ -421,7 +421,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		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);
-		T::Holder::died(id, &who)?;
+		T::Holder::died(id, &who);
 		return Ok(())
 	}
 
@@ -590,7 +590,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		// Execute hook outside of `mutate`.
 		if let Some(Remove) = target_died {
 			T::Freezer::died(id.clone(), target);
-			T::Holder::died(id, target)?;
+			T::Holder::died(id, target);
 		}
 		Ok(actual)
 	}
@@ -615,7 +615,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			Self::transfer_and_die(id.clone(), source, dest, amount, maybe_need_admin, f)?;
 		if let Some(Remove) = died {
 			T::Freezer::died(id.clone(), source);
-			T::Holder::died(id, source)?;
+			T::Holder::died(id, source);
 		}
 		Ok(balance)
 	}
@@ -816,7 +816,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		for who in &dead_accounts {
 			T::Freezer::died(id.clone(), &who);
-			T::Holder::died(id.clone(), &who)?;
+			T::Holder::died(id.clone(), &who);
 		}
 
 		Self::deposit_event(Event::AccountsDestroyed {
@@ -978,7 +978,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		// Execute hook outside of `mutate`.
 		if let Some(Remove) = owner_died {
 			T::Freezer::died(id.clone(), owner);
-			T::Holder::died(id, owner)?;
+			T::Holder::died(id, owner);
 		}
 		Ok(())
 	}

substrate/frame/assets/src/mock.rs

@@ -125,9 +125,8 @@ impl BalanceOnHold<u32, u64, u64> for TestHolder {
 		Held::get().get(&(asset, *who)).cloned()
 	}
 
-	fn died(_asset: u32, _who: &u64) -> DispatchResult {
+	fn died(_asset: u32, _who: &u64) {
 		// TODO: Connect with existing hooks list
-		Ok(())
 	}
 }
 

substrate/frame/assets/src/types.rs

@@ -249,17 +249,18 @@ pub trait BalanceOnHold<AssetId, AccountId, Balance> {
 
 	/// Called after an account has been removed.
 	///
-	/// NOTE: It is possible that the asset does no longer exist when this hook is called.
-	fn died(asset: AssetId, who: &AccountId) -> DispatchResult;
+	/// It is expected that this method is called only when there is not balance
+	/// on hold. Otherwise, an account should not be removed.
+	///
+	/// NOTE: It is possible that the asset no longer exists when this hook is called.
+	fn died(asset: AssetId, who: &AccountId);
 }
 
 impl<AssetId, AccountId, Balance> BalanceOnHold<AssetId, AccountId, Balance> for () {
 	fn balance_on_hold(_: AssetId, _: &AccountId) -> Option<Balance> {
 		None
 	}
-	fn died(_: AssetId, _: &AccountId) -> DispatchResult {
-		Ok(())
-	}
+	fn died(_: AssetId, _: &AccountId) {}
 }
 
 #[derive(Copy, Clone, PartialEq, Eq)]