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

@@ -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()

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(|| {