Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Assets pallet: Don't allow set_min_balance when sufficient #13510

Merged
merged 9 commits into from Mar 3, 2023
2 changes: 1 addition & 1 deletion frame/assets/src/benchmarking.rs
Expand Up @@ -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());
Expand Down
8 changes: 6 additions & 2 deletions frame/assets/src/lib.rs
Expand Up @@ -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
Expand Down
44 changes: 37 additions & 7 deletions frame/assets/src/tests.rs
Expand Up @@ -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);
});
Expand Down