Unbalanced and Balanced fungible conformance tests, and fungible fixes

prdoc/pr_1296.prdoc

@@ -0,0 +1,16 @@
+title: fungible fixes and more conformance tests
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      Adds conformance tests for the Balanced and Unbalanced fungible traits
+      Fixes Unbalanced::decrease_balance not respecting preservation
+      Fixes Balanced::pair possibly returning pairs of imbalances which do not cancel each other out. Method now returns a Result instead (breaking change).
+      Fixes Balances pallet active_issuance possible 'underflow'
+      Refactors the conformance test file structure to match the fungible file structure: tests for traits in regular.rs go into a test file named regular.rs, tests for traits in freezes.rs go into a test file named freezes.rs, etc.
+      Improve doc comments
+      Simplify macros
+
+crates:
+  - name: pallet-balances
+  - name: frame-support

substrate/frame/assets/src/tests/sets.rs

@@ -153,7 +153,7 @@ fn pair_from_set_types_works() {
 		assert_eq!(First::<Assets>::total_issuance(()), 100);
 		assert_eq!(First::<Assets>::total_issuance(()), Assets::total_issuance(asset1));
 
-		let (debt, credit) = First::<Assets>::pair((), 100);
+		let (debt, credit) = First::<Assets>::pair((), 100).unwrap();
 		assert_eq!(First::<Assets>::total_issuance(()), 100);
 		assert_eq!(debt.peek(), 100);
 		assert_eq!(credit.peek(), 100);

substrate/frame/balances/src/impl_fungible.rs

@@ -177,7 +177,10 @@ impl<T: Config<I>, I: 'static> fungible::Unbalanced<T::AccountId> for Pallet<T,
 	}
 
 	fn deactivate(amount: Self::Balance) {
-		InactiveIssuance::<T, I>::mutate(|b| b.saturating_accrue(amount));
+		InactiveIssuance::<T, I>::mutate(|b| {
+			// InactiveIssuance cannot be greater than TotalIssuance.
+			*b = b.saturating_add(amount).min(TotalIssuance::<T, I>::get());
+		});
 	}
 
 	fn reactivate(amount: Self::Balance) {

substrate/frame/balances/src/tests/fungible_conformance_tests.rs

@@ -19,28 +19,30 @@ use super::*;
 use frame_support::traits::fungible::{conformance_tests, Inspect, Mutate};
 use paste::paste;
 
-macro_rules! run_tests {
-    ($path:path, $ext_deposit:expr, $($name:ident),*) => {
+macro_rules! generate_tests {
+	// Handle a conformance test that requires special testing with and without a dust trap.
+    (dust_trap_variation, $base_path:path, $scope:expr, $trait:ident, $ext_deposit:expr, $($test_name:ident),*) => {
 		$(
 			paste! {
 				#[test]
-				fn [< $name _existential_deposit_ $ext_deposit _dust_trap_on >]() {
+				fn [<$trait _ $scope _ $test_name _existential_deposit_ $ext_deposit _dust_trap_on >]() {
+					// Some random trap account.
 					let trap_account = <Test as frame_system::Config>::AccountId::from(65174286u64);
 					let builder = ExtBuilder::default().existential_deposit($ext_deposit).dust_trap(trap_account);
 					builder.build_and_execute_with(|| {
 						Balances::set_balance(&trap_account, Balances::minimum_balance());
-						$path::$name::<
+						$base_path::$scope::$trait::$test_name::<
 							Balances,
 							<Test as frame_system::Config>::AccountId,
 						>(Some(trap_account));
 					});
 				}
 
 				#[test]
-				fn [< $name _existential_deposit_ $ext_deposit _dust_trap_off >]() {
+				fn [< $trait _ $scope _ $test_name _existential_deposit_ $ext_deposit _dust_trap_off >]() {
 					let builder = ExtBuilder::default().existential_deposit($ext_deposit);
 					builder.build_and_execute_with(|| {
-						$path::$name::<
+						$base_path::$scope::$trait::$test_name::<
 							Balances,
 							<Test as frame_system::Config>::AccountId,
 						>(None);
@@ -49,9 +51,37 @@ macro_rules! run_tests {
 			}
 		)*
 	};
-	($path:path, $ext_deposit:expr) => {
-		run_tests!(
-			$path,
+	// Regular conformance test
+    ($base_path:path, $scope:expr, $trait:ident, $ext_deposit:expr, $($test_name:ident),*) => {
+		$(
+			paste! {
+				#[test]
+				fn [< $trait _ $scope _ $test_name _existential_deposit_ $ext_deposit>]() {
+					let builder = ExtBuilder::default().existential_deposit($ext_deposit);
+					builder.build_and_execute_with(|| {
+						$base_path::$scope::$trait::$test_name::<
+							Balances,
+							<Test as frame_system::Config>::AccountId,
+						>();
+					});
+				}
+			}
+		)*
+	};
+	($base_path:path, $ext_deposit:expr) => {
+		// regular::mutate
+		generate_tests!(
+			dust_trap_variation,
+			$base_path,
+			regular,
+			mutate,
+			$ext_deposit,
+			transfer_expendable_dust
+		);
+		generate_tests!(
+			$base_path,
+			regular,
+			mutate,
 			$ext_deposit,
 			mint_into_success,
 			mint_into_overflow,
@@ -66,7 +96,6 @@ macro_rules! run_tests {
 			shelve_insufficient_funds,
 			transfer_success,
 			transfer_expendable_all,
-			transfer_expendable_dust,
 			transfer_protect_preserve,
 			set_balance_mint_success,
 			set_balance_burn_success,
@@ -79,10 +108,34 @@ macro_rules! run_tests {
 			reducible_balance_expendable,
 			reducible_balance_protect_preserve
 		);
+		// regular::unbalanced
+		generate_tests!(
+			$base_path,
+			regular,
+			unbalanced,
+			$ext_deposit,
+			write_balance,
+			decrease_balance_expendable,
+			decrease_balance_preserve,
+			increase_balance,
+			set_total_issuance,
+			deactivate_and_reactivate
+		);
+		// regular::balanced
+		generate_tests!(
+			$base_path,
+			regular,
+			balanced,
+			$ext_deposit,
+			issue_and_resolve_credit,
+			rescind_and_settle_debt,
+			deposit,
+			withdraw,
+			pair
+		);
 	};
 }
 
-run_tests!(conformance_tests::inspect_mutate, 1);
-run_tests!(conformance_tests::inspect_mutate, 2);
-run_tests!(conformance_tests::inspect_mutate, 5);
-run_tests!(conformance_tests::inspect_mutate, 1000);
+generate_tests!(conformance_tests, 1);
+generate_tests!(conformance_tests, 5);
+generate_tests!(conformance_tests, 1000);

substrate/frame/support/src/traits/tokens/fungible/conformance_tests/mod.rs

@@ -16,3 +16,4 @@
 // limitations under the License.
 
 pub mod inspect_mutate;
+pub mod regular;