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

Allow SetBalance to handle error when trying to kill acount with reference counter. #10826

Merged
merged 5 commits into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,25 +314,32 @@ pub mod pallet {
let new_free = if wipeout { Zero::zero() } else { new_free };
let new_reserved = if wipeout { Zero::zero() } else { new_reserved };

let (free, reserved) = Self::mutate_account(&who, |account| {
if new_free > account.free {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we adjusted the total issuance, while we could still bail out?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

mem::drop(PositiveImbalance::<T, I>::new(new_free - account.free));
} else if new_free < account.free {
mem::drop(NegativeImbalance::<T, I>::new(account.free - new_free));
}

if new_reserved > account.reserved {
mem::drop(PositiveImbalance::<T, I>::new(new_reserved - account.reserved));
} else if new_reserved < account.reserved {
mem::drop(NegativeImbalance::<T, I>::new(account.reserved - new_reserved));
}
// First we try to modify the account's balance to the forced balance.
let (old_free, old_reserved) = Self::mutate_account(&who, |account| {
let old_free = account.free;
let old_reserved = account.reserved;

account.free = new_free;
account.reserved = new_reserved;

(account.free, account.reserved)
(old_free, old_reserved)
})?;
Self::deposit_event(Event::BalanceSet { who, free, reserved });

// This will adjust the total issuance, which was not done by the `mutate_account`
// above.
if new_free > old_free {
mem::drop(PositiveImbalance::<T, I>::new(new_free - old_free));
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
} else if new_free < old_free {
mem::drop(NegativeImbalance::<T, I>::new(old_free - new_free));
}

if new_reserved > old_reserved {
mem::drop(PositiveImbalance::<T, I>::new(new_reserved - old_reserved));
} else if new_reserved < old_reserved {
mem::drop(NegativeImbalance::<T, I>::new(old_reserved - new_reserved));
}

Self::deposit_event(Event::BalanceSet { who, free: new_free, reserved: new_reserved });
Ok(().into())
}

Expand Down
24 changes: 24 additions & 0 deletions frame/balances/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1239,5 +1239,29 @@ macro_rules! decl_tests {
assert_eq!(Balances::free_balance(&3), 25);
});
}

#[test]
fn set_balance_handles_killing_account() {
<$ext_builder>::default().build().execute_with(|| {
let _ = Balances::deposit_creating(&1, 111);
assert_ok!(frame_system::Pallet::<Test>::inc_consumers(&1));
assert_noop!(
Balances::set_balance(Origin::root(), 1, 0, 0),
DispatchError::ConsumerRemaining,
);
});
}

#[test]
fn set_balance_handles_total_issuance() {
<$ext_builder>::default().build().execute_with(|| {
let old_total_issuance = Balances::total_issuance();
assert_ok!(Balances::set_balance(Origin::root(), 1337, 69, 42));
assert_eq!(Balances::total_issuance(), old_total_issuance + 69 + 42);
assert_eq!(Balances::total_balance(&1337), 69 + 42);
assert_eq!(Balances::free_balance(&1337), 69);
assert_eq!(Balances::reserved_balance(&1337), 42);
});
}
}
}