Assets pallet: Don't allow set_min_balance when sufficient (parityt…

…ech#13510)

* Assets pallet: Don't allow set_min_balance when sufficient

* fix

* fix benchmark

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_assets

* Update frame/assets/src/lib.rs

* fix

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_assets

---------

Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <git@kchr.de>

frame/assets/src/benchmarking.rs

@@ -472,7 +472,7 @@ benchmarks_instance_pallet! {
 	}
 
 	set_min_balance {
-		let (asset_id, caller, caller_lookup) = create_default_asset::<T, I>(true);
+		let (asset_id, caller, caller_lookup) = create_default_asset::<T, I>(false);
 	}: _(SystemOrigin::Signed(caller.clone()), asset_id, 50u32.into())
 	verify {
 		assert_last_event::<T, I>(Event::AssetMinBalanceChanged { asset_id: asset_id.into(), new_min_balance: 50u32.into() }.into());

frame/assets/src/lib.rs

@@ -1540,8 +1540,12 @@ pub mod pallet {
 			ensure!(origin == details.owner, Error::<T, I>::NoPermission);
 
 			let old_min_balance = details.min_balance;
-			// Ensure that either the new min_balance is less than old min_balance or there aren't
-			// any accounts holding the asset.
+			// If the asset is marked as sufficient it won't be allowed to
+			// change the min_balance.
+			ensure!(!details.is_sufficient, Error::<T, I>::NoPermission);
+
+			// Ensure that either the new min_balance is less than old
+			// min_balance or there aren't any accounts holding the asset.
 			ensure!(
 				min_balance < old_min_balance || details.accounts == 0,
 				Error::<T, I>::NoPermission

frame/assets/src/tests.rs

@@ -1101,23 +1101,53 @@ fn set_min_balance_should_work() {
 		Balances::make_free_balance_be(&1, 10);
 		assert_ok!(Assets::create(RuntimeOrigin::signed(1), id, 1, 30));
 
-		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), id, 1, 50));
-		// won't execute because there is an asset holder.
+		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), id, 1, 100));
+		// Won't execute because there is an asset holder.
 		assert_noop!(
 			Assets::set_min_balance(RuntimeOrigin::signed(1), id, 50),
 			Error::<Test>::NoPermission
 		);
 
-		// will execute because the new value of min_balance is less than the
-		// old value. 10 < 30
-		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), id, 1, 10));
+		// Force asset status to make this a sufficient asset.
+		assert_ok!(Assets::force_asset_status(
+			RuntimeOrigin::root(),
+			id,
+			1,
+			1,
+			1,
+			1,
+			30,
+			true,
+			false
+		));
 
-		assert_ok!(Assets::burn(RuntimeOrigin::signed(1), id, 1, 50));
+		// Won't execute because there is an account holding the asset and the asset is marked as
+		// sufficient.
 		assert_noop!(
-			Assets::set_min_balance(RuntimeOrigin::signed(2), id, 50),
+			Assets::set_min_balance(RuntimeOrigin::signed(1), id, 10),
 			Error::<Test>::NoPermission
 		);
 
+		// Make the asset not sufficient.
+		assert_ok!(Assets::force_asset_status(
+			RuntimeOrigin::root(),
+			id,
+			1,
+			1,
+			1,
+			1,
+			60,
+			false,
+			false
+		));
+
+		// Will execute because the new value of min_balance is less than the
+		// old value. 10 < 30
+		assert_ok!(Assets::set_min_balance(RuntimeOrigin::signed(1), id, 10));
+		assert_eq!(Asset::<Test>::get(id).unwrap().min_balance, 10);
+
+		assert_ok!(Assets::burn(RuntimeOrigin::signed(1), id, 1, 100));
+
 		assert_ok!(Assets::set_min_balance(RuntimeOrigin::signed(1), id, 50));
 		assert_eq!(Asset::<Test>::get(id).unwrap().min_balance, 50);
 	});