Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Return early when fees/balances/values are zero (#4628)
Browse files Browse the repository at this point in the history 
* Return early when fees/balances/values are zero

* Add docs about no-op
  • Loading branch information
@shawntabrizi @gavofyork
shawntabrizi authored and gavofyork committed Jan 15, 2020
1 parent 5649259 commit 33b3c23
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 15 deletions.
58 changes: 57 additions & 1 deletion frame/balances/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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
{
Expand All @@ -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();
Expand Down Expand Up @@ -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...
Expand All @@ -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);

Expand All @@ -1049,21 +1076,29 @@ 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)?
}
Self::set_free_balance(who, Self::free_balance(who) + value);
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
Expand Down Expand Up @@ -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|
Expand All @@ -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)?
Expand All @@ -1146,30 +1187,40 @@ 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);
Self::set_reserved_balance(who, b - actual);
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.
Self::set_reserved_balance(who, b - slash);
(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)?
}
Expand All @@ -1187,13 +1238,16 @@ 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,
amount: T::Balance,
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|
Expand Down Expand Up @@ -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)?
}
Expand Down
31 changes: 17 additions & 14 deletions frame/transaction-payment/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 33b3c23

Please sign in to comment.