Add more asserts and debug_asserts (#8541)

* Add more asserts and debug_asserts

fixing #8106

* Remove assignments

* convert debug_assert to runtime assert

substrate/frame/balances/src/benchmarking.rs

@@ -44,7 +44,8 @@ benchmarks_instance_pallet! {
 		let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
 		let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, balance);
 
-		// Transfer `e - 1` existential deposits + 1 unit, which guarantees to create one account, and reap this user.
+		// Transfer `e - 1` existential deposits + 1 unit, which guarantees to create one account,
+		// and reap this user.
 		let recipient: T::AccountId = account("recipient", 0, SEED);
 		let recipient_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(recipient.clone());
 		let transfer_amount = existential_deposit.saturating_mul((ED_MULTIPLIER - 1).into()) + 1u32.into();

substrate/frame/balances/src/tests.rs

@@ -169,13 +169,13 @@ macro_rules! decl_tests {
 						&info_from_weight(1),
 						1,
 					).is_err());
-					assert!(<ChargeTransactionPayment<$test> as SignedExtension>::pre_dispatch(
+					assert_ok!(<ChargeTransactionPayment<$test> as SignedExtension>::pre_dispatch(
 						ChargeTransactionPayment::from(0),
 						&1,
 						CALL,
 						&info_from_weight(1),
 						1,
-					).is_ok());
+					));
 
 					Balances::set_lock(ID_1, &1, 10, WithdrawReasons::TRANSACTION_PAYMENT);
 					assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
@@ -394,7 +394,7 @@ macro_rules! decl_tests {
 		fn refunding_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = Balances::deposit_creating(&1, 42);
-				assert!(Balances::mutate_account(&1, |a| a.reserved = 69).is_ok());
+				assert_ok!(Balances::mutate_account(&1, |a| a.reserved = 69));
 				Balances::unreserve(&1, 69);
 				assert_eq!(Balances::free_balance(1), 111);
 				assert_eq!(Balances::reserved_balance(1), 0);
@@ -669,7 +669,9 @@ macro_rules! decl_tests {
 					assert_eq!(Balances::reserved_balance(1), 50);
 
 					// Reserve some free balance
-					let _ = Balances::slash(&1, 1);
+					let res = Balances::slash(&1, 1);
+					assert_eq!(res, (NegativeImbalance::new(1), 0));
+
 					// The account should be dead.
 					assert_eq!(Balances::free_balance(1), 0);
 					assert_eq!(Balances::reserved_balance(1), 0);
@@ -727,7 +729,8 @@ macro_rules! decl_tests {
 						]
 					);
 
-					let _ = Balances::slash(&1, 1);
+					let res = Balances::slash(&1, 1);
+					assert_eq!(res, (NegativeImbalance::new(1), 0));
 
 					assert_eq!(
 						events(),
@@ -756,7 +759,8 @@ macro_rules! decl_tests {
 						]
 					);
 
-					let _ = Balances::slash(&1, 100);
+					let res = Balances::slash(&1, 100);
+					assert_eq!(res, (NegativeImbalance::new(100), 0));
 
 					assert_eq!(
 						events(),

substrate/frame/balances/src/tests_local.rs

@@ -175,12 +175,14 @@ fn emit_events_with_no_existential_deposit_suicide_with_dust() {
 				]
 			);
 
-			let _ = Balances::slash(&1, 98);
+			let res = Balances::slash(&1, 98);
+			assert_eq!(res, (NegativeImbalance::new(98), 0));
 
 			// no events
 			assert_eq!(events(), []);
 
-			let _ = Balances::slash(&1, 1);
+			let res = Balances::slash(&1, 1);
+			assert_eq!(res, (NegativeImbalance::new(1), 0));
 
 			assert_eq!(
 				events(),

substrate/frame/balances/src/tests_reentrancy.rs

@@ -105,7 +105,7 @@ impl pallet_transaction_payment::Config for Test {
 pub struct OnDustRemoval;
 impl OnUnbalanced<NegativeImbalance<Test>> for OnDustRemoval {
 	fn on_nonzero_unbalanced(amount: NegativeImbalance<Test>) {
-		let _ = Balances::resolve_into_existing(&1, amount);
+		assert_ok!(Balances::resolve_into_existing(&1, amount));
 	}
 }
 parameter_types! {

substrate/frame/bounties/src/tests.rs

@@ -312,7 +312,7 @@ fn pot_underflow_should_not_diminish() {
 		<Treasury as OnInitialize<u64>>::on_initialize(2);
 		assert_eq!(Treasury::pot(), 100); // Pot hasn't changed
 
-		let _ = Balances::deposit_into_existing(&Treasury::account_id(), 100).unwrap();
+		assert_ok!(Balances::deposit_into_existing(&Treasury::account_id(), 100));
 		<Treasury as OnInitialize<u64>>::on_initialize(4);
 		assert_eq!(Balances::free_balance(3), 150); // Fund has been spent
 		assert_eq!(Treasury::pot(), 25); // Pot has finally changed
@@ -689,7 +689,8 @@ fn claim_handles_high_fee() {
 		<Treasury as OnInitialize<u64>>::on_initialize(5);
 
 		// make fee > balance
-		let _ = Balances::slash(&Bounties::bounty_account_id(0), 10);
+		let res = Balances::slash(&Bounties::bounty_account_id(0), 10);
+		assert_eq!(res.0.peek(), 10);
 
 		assert_ok!(Bounties::claim_bounty(Origin::signed(1), 0));
 

substrate/frame/contracts/src/wasm/mod.rs

@@ -254,7 +254,7 @@ mod tests {
 	use sp_core::H256;
 	use hex_literal::hex;
 	use sp_runtime::DispatchError;
-	use frame_support::{dispatch::DispatchResult, weights::Weight};
+	use frame_support::{assert_ok, dispatch::DispatchResult, weights::Weight};
 	use assert_matches::assert_matches;
 	use pallet_contracts_primitives::{ExecReturnValue, ReturnFlags, ExecError, ErrorOrigin};
 	use pretty_assertions::assert_eq;
@@ -597,12 +597,12 @@ mod tests {
 	#[test]
 	fn contract_transfer() {
 		let mut mock_ext = MockExt::default();
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_TRANSFER,
 			vec![],
 			&mut mock_ext,
 			&mut GasMeter::new(GAS_LIMIT),
-		).unwrap();
+		));
 
 		assert_eq!(
 			&mock_ext.transfers,
@@ -663,12 +663,12 @@ mod tests {
 	#[test]
 	fn contract_call() {
 		let mut mock_ext = MockExt::default();
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_CALL,
 			vec![],
 			&mut mock_ext,
 			&mut GasMeter::new(GAS_LIMIT),
-		).unwrap();
+		));
 
 		assert_eq!(
 			&mock_ext.transfers,
@@ -739,12 +739,12 @@ mod tests {
 	#[test]
 	fn contract_instantiate() {
 		let mut mock_ext = MockExt::default();
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_INSTANTIATE,
 			vec![],
 			&mut mock_ext,
 			&mut GasMeter::new(GAS_LIMIT),
-		).unwrap();
+		));
 
 		assert_matches!(
 			&mock_ext.instantiates[..],
@@ -851,12 +851,12 @@ mod tests {
 	#[test]
 	fn contract_call_limited_gas() {
 		let mut mock_ext = MockExt::default();
-		let _ = execute(
+		assert_ok!(execute(
 			&CODE_TRANSFER_LIMITED_GAS,
 			vec![],
 			&mut mock_ext,
 			&mut GasMeter::new(GAS_LIMIT),
-		).unwrap();
+		));
 
 		assert_eq!(
 			&mock_ext.transfers,
@@ -994,12 +994,12 @@ mod tests {
 
 	#[test]
 	fn caller() {
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_CALLER,
 			vec![],
 			MockExt::default(),
 			&mut GasMeter::new(GAS_LIMIT),
-		).unwrap();
+		));
 	}
 
 	/// calls `seal_address` and compares the result with the constant 69.
@@ -1047,12 +1047,12 @@ mod tests {
 
 	#[test]
 	fn address() {
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_ADDRESS,
 			vec![],
 			MockExt::default(),
 			&mut GasMeter::new(GAS_LIMIT),
-		).unwrap();
+		));
 	}
 
 	const CODE_BALANCE: &str = r#"
@@ -1099,12 +1099,12 @@ mod tests {
 	#[test]
 	fn balance() {
 		let mut gas_meter = GasMeter::new(GAS_LIMIT);
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_BALANCE,
 			vec![],
 			MockExt::default(),
 			&mut gas_meter,
-		).unwrap();
+		));
 	}
 
 	const CODE_GAS_PRICE: &str = r#"
@@ -1151,12 +1151,12 @@ mod tests {
 	#[test]
 	fn gas_price() {
 		let mut gas_meter = GasMeter::new(GAS_LIMIT);
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_GAS_PRICE,
 			vec![],
 			MockExt::default(),
 			&mut gas_meter,
-		).unwrap();
+		));
 	}
 
 	const CODE_GAS_LEFT: &str = r#"
@@ -1258,12 +1258,12 @@ mod tests {
 	#[test]
 	fn value_transferred() {
 		let mut gas_meter = GasMeter::new(GAS_LIMIT);
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_VALUE_TRANSFERRED,
 			vec![],
 			MockExt::default(),
 			&mut gas_meter,
-		).unwrap();
+		));
 	}
 
 	const CODE_RETURN_FROM_START_FN: &str = r#"
@@ -1346,12 +1346,12 @@ mod tests {
 	#[test]
 	fn now() {
 		let mut gas_meter = GasMeter::new(GAS_LIMIT);
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_TIMESTAMP_NOW,
 			vec![],
 			MockExt::default(),
 			&mut gas_meter,
-		).unwrap();
+		));
 	}
 
 	const CODE_MINIMUM_BALANCE: &str = r#"
@@ -1397,12 +1397,12 @@ mod tests {
 	#[test]
 	fn minimum_balance() {
 		let mut gas_meter = GasMeter::new(GAS_LIMIT);
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_MINIMUM_BALANCE,
 			vec![],
 			MockExt::default(),
 			&mut gas_meter,
-		).unwrap();
+		));
 	}
 
 	const CODE_TOMBSTONE_DEPOSIT: &str = r#"
@@ -1448,12 +1448,12 @@ mod tests {
 	#[test]
 	fn tombstone_deposit() {
 		let mut gas_meter = GasMeter::new(GAS_LIMIT);
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_TOMBSTONE_DEPOSIT,
 			vec![],
 			MockExt::default(),
 			&mut gas_meter,
-		).unwrap();
+		));
 	}
 
 	const CODE_RANDOM: &str = r#"
@@ -1637,12 +1637,12 @@ mod tests {
 	fn deposit_event() {
 		let mut mock_ext = MockExt::default();
 		let mut gas_meter = GasMeter::new(GAS_LIMIT);
-		let _ = execute(
+		assert_ok!(execute(
 			CODE_DEPOSIT_EVENT,
 			vec![],
 			&mut mock_ext,
 			&mut gas_meter
-		).unwrap();
+		));
 
 		assert_eq!(mock_ext.events, vec![
 			(vec![H256::repeat_byte(0x33)],

substrate/frame/election-provider-multi-phase/src/benchmarking.rs

@@ -208,7 +208,7 @@ frame_benchmarking::benchmarks! {
 		// assume a queued solution is stored, regardless of where it comes from.
 		<QueuedSolution<T>>::put(ready_solution);
 	}: {
-		let _ = <MultiPhase<T> as ElectionProvider<T::AccountId, T::BlockNumber>>::elect();
+		assert_ok!(<MultiPhase<T> as ElectionProvider<T::AccountId, T::BlockNumber>>::elect());
 	} verify {
 		assert!(<MultiPhase<T>>::queued_solution().is_none());
 		assert!(<DesiredTargets<T>>::get().is_none());

substrate/frame/election-provider-multi-phase/src/lib.rs

@@ -1411,7 +1411,7 @@ mod tests {
 			roll_to(30);
 			assert!(MultiPhase::current_phase().is_signed());
 
-			let _ = MultiPhase::elect().unwrap();
+			assert_ok!(MultiPhase::elect());
 
 			assert!(MultiPhase::current_phase().is_off());
 			assert!(MultiPhase::snapshot().is_none());
@@ -1434,7 +1434,7 @@ mod tests {
 			assert!(MultiPhase::current_phase().is_off());
 
 			// this module is now only capable of doing on-chain backup.
-			let _ = MultiPhase::elect().unwrap();
+			assert_ok!(MultiPhase::elect());
 
 			assert!(MultiPhase::current_phase().is_off());
 		});

substrate/frame/executive/src/lib.rs

@@ -553,25 +553,25 @@ mod tests {
 				#[weight = 100]
 				fn some_function(origin) {
 					// NOTE: does not make any different.
-					let _ = frame_system::ensure_signed(origin);
+					frame_system::ensure_signed(origin)?;
 				}
 				#[weight = (200, DispatchClass::Operational)]
 				fn some_root_operation(origin) {
-					let _ = frame_system::ensure_root(origin);
+					frame_system::ensure_root(origin)?;
 				}
 				#[weight = 0]
 				fn some_unsigned_message(origin) {
-					let _ = frame_system::ensure_none(origin);
+					frame_system::ensure_none(origin)?;
 				}
 
 				#[weight = 0]
 				fn allowed_unsigned(origin) {
-					let _ = frame_system::ensure_root(origin)?;
+					frame_system::ensure_root(origin)?;
 				}
 
 				#[weight = 0]
 				fn unallowed_unsigned(origin) {
-					let _ = frame_system::ensure_root(origin)?;
+					frame_system::ensure_root(origin)?;
 				}
 
 				// module hooks.

substrate/frame/recovery/src/lib.rs

@@ -621,7 +621,8 @@ decl_module! {
 			let active_recovery = <ActiveRecoveries<T>>::take(&who, &rescuer).ok_or(Error::<T>::NotStarted)?;
 			// Move the reserved funds from the rescuer to the rescued account.
 			// Acts like a slashing mechanism for those who try to maliciously recover accounts.
-			let _ = T::Currency::repatriate_reserved(&rescuer, &who, active_recovery.deposit, BalanceStatus::Free);
+			let res = T::Currency::repatriate_reserved(&rescuer, &who, active_recovery.deposit, BalanceStatus::Free);
+			debug_assert!(res.is_ok());
 			Self::deposit_event(RawEvent::RecoveryClosed(who, rescuer));
 		}
 

substrate/frame/society/src/lib.rs

@@ -997,7 +997,8 @@ decl_module! {
 						match kind {
 							BidKind::Deposit(deposit) => {
 								// Slash deposit and move it to the society account
-								let _ = T::Currency::repatriate_reserved(&who, &Self::account_id(), deposit, BalanceStatus::Free);
+								let res = T::Currency::repatriate_reserved(&who, &Self::account_id(), deposit, BalanceStatus::Free);
+								debug_assert!(res.is_ok());
 							}
 							BidKind::Vouch(voucher, _) => {
 								// Ban the voucher from vouching again

substrate/frame/staking/src/tests.rs

@@ -437,7 +437,8 @@ fn no_candidate_emergency_condition() {
 			<Staking as crate::Store>::MinimumValidatorCount::put(10);
 
 			// try to chill
-			let _ = Staking::chill(Origin::signed(10));
+			let res = Staking::chill(Origin::signed(10));
+			assert_ok!(res);
 
 			// trigger era
 			mock::start_active_era(1);

substrate/frame/tips/src/benchmarking.rs

@@ -23,7 +23,7 @@ use super::*;
 
 use frame_system::RawOrigin;
 use frame_benchmarking::{benchmarks, account, whitelisted_caller, impl_benchmark_test_suite};
-use sp_runtime::{traits::{Saturating}};
+use sp_runtime::traits::Saturating;
 
 use crate::Module as TipsMod;
 

substrate/frame/treasury/src/lib.rs

@@ -68,16 +68,16 @@ use serde::{Serialize, Deserialize};
 use sp_std::prelude::*;
 use frame_support::{decl_module, decl_storage, decl_event, ensure, print, decl_error};
 use frame_support::traits::{
-	Currency, Get, Imbalance, OnUnbalanced, ExistenceRequirement::{KeepAlive},
+	Currency, Get, Imbalance, OnUnbalanced, ExistenceRequirement::KeepAlive,
 	ReservableCurrency, WithdrawReasons
 };
 use sp_runtime::{Permill, ModuleId, RuntimeDebug, traits::{
 	Zero, StaticLookup, AccountIdConversion, Saturating
 }};
 use frame_support::weights::{Weight, DispatchClass};
-use frame_support::traits::{EnsureOrigin};
+use frame_support::traits::EnsureOrigin;
 use codec::{Encode, Decode};
-use frame_system::{ensure_signed};
+use frame_system::ensure_signed;
 pub use weights::WeightInfo;
 
 pub type BalanceOf<T, I=DefaultInstance> =
@@ -187,10 +187,7 @@ decl_storage! {
 			let account_id = <Module<T, I>>::account_id();
 			let min = T::Currency::minimum_balance();
 			if T::Currency::free_balance(&account_id) < min {
-				let _ = T::Currency::make_free_balance_be(
-					&account_id,
-					min,
-				);
+				let _ = T::Currency::make_free_balance_be(&account_id, min);
 			}
 		});
 	}