Return early when fees/balances/values are zero (#4628)

* Return early when fees/balances/values are zero

* Add docs about no-op

substrate/frame/balances/src/lib.rs

@@ -885,7 +885,10 @@ where
 		Self::free_balance(who) + Self::reserved_balance(who)
 	}
 
+	// Check if `value` amount of free balance can be slashed from `who`.
+	// Is a no-op if value to be slashed is zero.
 	fn can_slash(who: &T::AccountId, value: Self::Balance) -> bool {
+		if value.is_zero() { return true }
 		Self::free_balance(who) >= value
 	}
 
@@ -901,7 +904,10 @@ where
 		<FreeBalance<T, I>>::get(who)
 	}
 
+	// Burn funds from the total issuance, returning a positive imbalance for the amount burned.
+	// Is a no-op if amount to be burned is zero.
 	fn burn(mut amount: Self::Balance) -> Self::PositiveImbalance {
+		if amount.is_zero() { return PositiveImbalance::zero() }
 		<TotalIssuance<T, I>>::mutate(|issued| {
 			*issued = issued.checked_sub(&amount).unwrap_or_else(|| {
 				amount = *issued;
@@ -911,7 +917,11 @@ where
 		PositiveImbalance::new(amount)
 	}
 
+	// Create new funds into the total issuance, returning a negative imbalance
+	// for the amount issued.
+	// Is a no-op if amount to be issued it zero.
 	fn issue(mut amount: Self::Balance) -> Self::NegativeImbalance {
+		if amount.is_zero() { return NegativeImbalance::zero() }
 		<TotalIssuance<T, I>>::mutate(|issued|
 			*issued = issued.checked_add(&amount).unwrap_or_else(|| {
 				amount = Self::Balance::max_value() - *issued;
@@ -921,16 +931,21 @@ where
 		NegativeImbalance::new(amount)
 	}
 
+	// Ensure that an account can withdraw from their free balance given any existing withdrawal
+	// restrictions like locks and vesting balance.
+	// Is a no-op if amount to be withdrawn is zero.
+	//
 	// # <weight>
 	// Despite iterating over a list of locks, they are limited by the number of
 	// lock IDs, which means the number of runtime modules that intend to use and create locks.
 	// # </weight>
 	fn ensure_can_withdraw(
 		who: &T::AccountId,
-		_amount: T::Balance,
+		amount: T::Balance,
 		reasons: WithdrawReasons,
 		new_balance: T::Balance,
 	) -> DispatchResult {
+		if amount.is_zero() { return Ok(()) }
 		if reasons.intersects(WithdrawReason::Reserve | WithdrawReason::Transfer)
 			&& Self::vesting_balance(who) > new_balance
 		{
@@ -955,12 +970,15 @@ where
 		}
 	}
 
+	// Transfer some free balance from `transactor` to `dest`, respecting existence requirements.
+	// Is a no-op if value to be transferred is zero.
 	fn transfer(
 		transactor: &T::AccountId,
 		dest: &T::AccountId,
 		value: Self::Balance,
 		existence_requirement: ExistenceRequirement,
 	) -> DispatchResult {
+		if value.is_zero() { return Ok(()) }
 		let from_balance = Self::free_balance(transactor);
 		let to_balance = Self::free_balance(dest);
 		let would_create = to_balance.is_zero();
@@ -1001,12 +1019,16 @@ where
 		Ok(())
 	}
 
+	// Withdraw some free balance from an account, respecting existence requirements.
+	// Is a no-op if value to be withdrawn is zero.
 	fn withdraw(
 		who: &T::AccountId,
 		value: Self::Balance,
 		reasons: WithdrawReasons,
 		liveness: ExistenceRequirement,
 	) -> result::Result<Self::NegativeImbalance, DispatchError> {
+		if value.is_zero() { return Ok(NegativeImbalance::zero()); }
+
 		let old_balance = Self::free_balance(who);
 		if let Some(new_balance) = old_balance.checked_sub(&value) {
 			// if we need to keep the account alive...
@@ -1026,10 +1048,15 @@ where
 		}
 	}
 
+	// Slash an account, returning the negative imbalance created and any left over
+	// amount that could not be slashed.
+	// Is a no-op if value to be slashed is zero.
 	fn slash(
 		who: &T::AccountId,
 		value: Self::Balance
 	) -> (Self::NegativeImbalance, Self::Balance) {
+		if value.is_zero() { return (NegativeImbalance::zero(), Zero::zero()) }
+
 		let free_balance = Self::free_balance(who);
 		let free_slash = cmp::min(free_balance, value);
 
@@ -1049,10 +1076,14 @@ where
 		}
 	}
 
+	// Deposit some `value` into the free balance of an existing account.
+	// Is a no-op if the value to be deposited is zero.
 	fn deposit_into_existing(
 		who: &T::AccountId,
 		value: Self::Balance
 	) -> result::Result<Self::PositiveImbalance, DispatchError> {
+		if value.is_zero() { return Ok(PositiveImbalance::zero()) }
+
 		if Self::total_balance(who).is_zero() {
 			Err(Error::<T, I>::DeadAccount)?
 		}
@@ -1060,10 +1091,14 @@ where
 		Ok(PositiveImbalance::new(value))
 	}
 
+	// Deposit some `value` into the free balance of `who`, possibly creating a new account.
+	// Is a no-op if the value to be deposited is zero.
 	fn deposit_creating(
 		who: &T::AccountId,
 		value: Self::Balance,
 	) -> Self::PositiveImbalance {
+		if value.is_zero() { return Self::PositiveImbalance::zero() }
+
 		let (imbalance, _) = Self::make_free_balance_be(who, Self::free_balance(who) + value);
 		if let SignedImbalance::Positive(p) = imbalance {
 			p
@@ -1122,7 +1157,10 @@ impl<T: Trait<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I>
 where
 	T::Balance: MaybeSerializeDeserialize + Debug
 {
+	// Check if `who` can reserve `value` from their free balance.
+	// Is a no-op if value to be reserved is zero.
 	fn can_reserve(who: &T::AccountId, value: Self::Balance) -> bool {
+		if value.is_zero() { return true }
 		Self::free_balance(who)
 			.checked_sub(&value)
 			.map_or(false, |new_balance|
@@ -1134,7 +1172,10 @@ where
 		<ReservedBalance<T, I>>::get(who)
 	}
 
+	// Move `value` from the free balance from `who` to their reserved balance.
+	// Is a no-op if value to be reserved is zero.
 	fn reserve(who: &T::AccountId, value: Self::Balance) -> result::Result<(), DispatchError> {
+		if value.is_zero() { return Ok(()) }
 		let b = Self::free_balance(who);
 		if b < value {
 			Err(Error::<T, I>::InsufficientBalance)?
@@ -1146,7 +1187,10 @@ where
 		Ok(())
 	}
 
+	// Unreserve some funds, returning any amount that was unable to be unreserved.
+	// Is a no-op if the value to be unreserved is zero.
 	fn unreserve(who: &T::AccountId, value: Self::Balance) -> Self::Balance {
+		if value.is_zero() { return Zero::zero() }
 		let b = Self::reserved_balance(who);
 		let actual = cmp::min(b, value);
 		Self::set_free_balance(who, Self::free_balance(who) + actual);
@@ -1154,10 +1198,14 @@ where
 		value - actual
 	}
 
+	// Slash from reserved balance, returning the negative imbalance created,
+	// and any amount that was unable to be slashed.
+	// Is a no-op if the value to be slashed is zero.
 	fn slash_reserved(
 		who: &T::AccountId,
 		value: Self::Balance
 	) -> (Self::NegativeImbalance, Self::Balance) {
+		if value.is_zero() { return (NegativeImbalance::zero(), Zero::zero()) }
 		let b = Self::reserved_balance(who);
 		let slash = cmp::min(b, value);
 		// underflow should never happen, but it if does, there's nothing to be done here.
@@ -1165,11 +1213,14 @@ where
 		(NegativeImbalance::new(slash), value - slash)
 	}
 
+	// Move the reserved balance of one account into the free balance of another.
+	// Is a no-op if the value to be moved is zero.
 	fn repatriate_reserved(
 		slashed: &T::AccountId,
 		beneficiary: &T::AccountId,
 		value: Self::Balance,
 	) -> result::Result<Self::Balance, DispatchError> {
+		if value.is_zero() { return Ok (Zero::zero()) }
 		if Self::total_balance(beneficiary).is_zero() {
 			Err(Error::<T, I>::DeadAccount)?
 		}
@@ -1187,6 +1238,8 @@ where
 {
 	type Moment = T::BlockNumber;
 
+	// Set a lock on the balance of `who`.
+	// Is a no-op if lock amount is zero.
 	fn set_lock(
 		id: LockIdentifier,
 		who: &T::AccountId,
@@ -1194,6 +1247,7 @@ where
 		until: T::BlockNumber,
 		reasons: WithdrawReasons,
 	) {
+		if amount.is_zero() { return }
 		let now = <frame_system::Module<T>>::block_number();
 		let mut new_lock = Some(BalanceLock { id, amount, until, reasons });
 		let mut locks = Self::locks(who).into_iter().filter_map(|l|
@@ -1275,12 +1329,14 @@ where
 	///
 	/// If there already exists a vesting schedule for the given account, an `Err` is returned
 	/// and nothing is updated.
+	/// Is a no-op if the amount to be vested is zero.
 	fn add_vesting_schedule(
 		who: &T::AccountId,
 		locked: T::Balance,
 		per_block: T::Balance,
 		starting_block: T::BlockNumber
 	) -> DispatchResult {
+		if locked.is_zero() { return Ok(()) }
 		if <Vesting<T, I>>::exists(who) {
 			Err(Error::<T, I>::ExistingVestingSchedule)?
 		}

substrate/frame/transaction-payment/src/lib.rs

@@ -218,20 +218,23 @@ impl<T: Trait + Send + Sync> SignedExtension for ChargeTransactionPayment<T>
 		// pay any fees.
 		let tip = self.0;
 		let fee = Self::compute_fee(len as u32, info, tip);
-		let imbalance = match T::Currency::withdraw(
-			who,
-			fee,
-			if tip.is_zero() {
-				WithdrawReason::TransactionPayment.into()
-			} else {
-				WithdrawReason::TransactionPayment | WithdrawReason::Tip
-			},
-			ExistenceRequirement::KeepAlive,
-		) {
-			Ok(imbalance) => imbalance,
-			Err(_) => return InvalidTransaction::Payment.into(),
-		};
-		T::OnTransactionPayment::on_unbalanced(imbalance);
+		// Only mess with balances if fee is not zero.
+		if !fee.is_zero() {
+			let imbalance = match T::Currency::withdraw(
+				who,
+				fee,
+				if tip.is_zero() {
+					WithdrawReason::TransactionPayment.into()
+				} else {
+					WithdrawReason::TransactionPayment | WithdrawReason::Tip
+				},
+				ExistenceRequirement::KeepAlive,
+			) {
+				Ok(imbalance) => imbalance,
+				Err(_) => return InvalidTransaction::Payment.into(),
+			};
+			T::OnTransactionPayment::on_unbalanced(imbalance);
+		}
 
 		let mut r = ValidTransaction::default();
 		// NOTE: we probably want to maximize the _fee (of any type) per weight unit_ here, which