From 674e73caf01bad33caf7a74e7f5f82db27a1003c Mon Sep 17 00:00:00 2001
From: Dmitry Novikov <novikov.dm.al@gmail.com>
Date: Mon, 29 Aug 2022 14:22:40 +0300
Subject: [PATCH] Bug fix of currencies implementation for `pallet-balances`
(#11875)
* Add test
* Fix the bug
* Add similar test for named reservable
* Extend test with "overflow" repatriation
* Expand test for `NamedReservableCurrency`
* Add notes about return values meaning
---
substrate/frame/balances/src/lib.rs | 6 ++++-
substrate/frame/balances/src/tests.rs | 35 +++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs
index 0cb32a4e3ec..0a453503664 100644
--- a/substrate/frame/balances/src/lib.rs
+++ b/substrate/frame/balances/src/lib.rs
@@ -1000,6 +1000,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Is a no-op if:
/// - the value to be moved is zero; or
/// - the `slashed` id equal to `beneficiary` and the `status` is `Reserved`.
+ ///
+ /// NOTE: returns actual amount of transferred value in `Ok` case.
fn do_transfer_reserved(
slashed: &T::AccountId,
beneficiary: &T::AccountId,
@@ -1013,7 +1015,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
if slashed == beneficiary {
return match status {
- Status::Free => Ok(Self::unreserve(slashed, value)),
+ Status::Free => Ok(value.saturating_sub(Self::unreserve(slashed, value))),
Status::Reserved => Ok(value.saturating_sub(Self::reserved_balance(slashed))),
}
}
@@ -1785,6 +1787,8 @@ where
/// Unreserve some funds, returning any amount that was unable to be unreserved.
///
/// Is a no-op if the value to be unreserved is zero or the account does not exist.
+ ///
+ /// NOTE: returns amount value which wasn't successfully unreserved.
fn unreserve(who: &T::AccountId, value: Self::Balance) -> Self::Balance {
if value.is_zero() {
return Zero::zero()
diff --git a/substrate/frame/balances/src/tests.rs b/substrate/frame/balances/src/tests.rs
index 8f5470ae3ca..a9e44f08597 100644
--- a/substrate/frame/balances/src/tests.rs
+++ b/substrate/frame/balances/src/tests.rs
@@ -528,6 +528,22 @@ macro_rules! decl_tests {
});
}
+ #[test]
+ fn transferring_reserved_balance_to_yourself_should_work() {
+ <$ext_builder>::default().build().execute_with(|| {
+ let _ = Balances::deposit_creating(&1, 110);
+ assert_ok!(Balances::reserve(&1, 50));
+ assert_ok!(Balances::repatriate_reserved(&1, &1, 50, Status::Free), 0);
+ assert_eq!(Balances::free_balance(1), 110);
+ assert_eq!(Balances::reserved_balance(1), 0);
+
+ assert_ok!(Balances::reserve(&1, 50));
+ assert_ok!(Balances::repatriate_reserved(&1, &1, 60, Status::Free), 10);
+ assert_eq!(Balances::free_balance(1), 110);
+ assert_eq!(Balances::reserved_balance(1), 0);
+ });
+ }
+
#[test]
fn transferring_reserved_balance_to_nonexistent_should_fail() {
<$ext_builder>::default().build().execute_with(|| {
@@ -1167,6 +1183,25 @@ macro_rules! decl_tests {
});
}
+ #[test]
+ fn reserved_named_to_yourself_should_work() {
+ <$ext_builder>::default().build().execute_with(|| {
+ let _ = Balances::deposit_creating(&1, 110);
+
+ let id = [1u8; 8];
+
+ assert_ok!(Balances::reserve_named(&id, &1, 50));
+ assert_ok!(Balances::repatriate_reserved_named(&id, &1, &1, 50, Status::Free), 0);
+ assert_eq!(Balances::free_balance(1), 110);
+ assert_eq!(Balances::reserved_balance_named(&id, &1), 0);
+
+ assert_ok!(Balances::reserve_named(&id, &1, 50));
+ assert_ok!(Balances::repatriate_reserved_named(&id, &1, &1, 60, Status::Free), 10);
+ assert_eq!(Balances::free_balance(1), 110);
+ assert_eq!(Balances::reserved_balance_named(&id, &1), 0);
+ });
+ }
+
#[test]
fn ensure_reserved_named_should_work() {
<$ext_builder>::default().build().execute_with(|| {
--
GitLab