From ec7655cc7b3449e263e3c284b9431c3db466cf37 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 28 Oct 2019 13:04:20 +0100 Subject: [PATCH] Tip payment is a different withdraw reason. (#3937) * Tip payment is a different withdraw reason. * Bump runtime version. * Test fix. * Fix lock type --- substrate/core/primitives/src/sr25519.rs | 3 --- substrate/srml/balances/src/lib.rs | 22 ++++++++-------- substrate/srml/contracts/src/exec.rs | 2 +- substrate/srml/contracts/src/gas.rs | 2 +- substrate/srml/contracts/src/rent.rs | 4 +-- substrate/srml/elections-phragmen/src/lib.rs | 4 +-- substrate/srml/elections/src/lib.rs | 2 +- substrate/srml/generic-asset/src/lib.rs | 25 +++++++++++-------- substrate/srml/support/src/traits.rs | 12 +++++---- substrate/srml/transaction-payment/src/lib.rs | 9 +++++-- substrate/srml/treasury/src/lib.rs | 4 +-- 11 files changed, 49 insertions(+), 40 deletions(-) diff --git a/substrate/core/primitives/src/sr25519.rs b/substrate/core/primitives/src/sr25519.rs index bb319b8221..ba7f480128 100644 --- a/substrate/core/primitives/src/sr25519.rs +++ b/substrate/core/primitives/src/sr25519.rs @@ -129,9 +129,6 @@ impl std::fmt::Display for Public { } } -#[cfg(not(feature = "std"))] -use core as std; - impl rstd::fmt::Debug for Public { #[cfg(feature = "std")] fn fmt(&self, f: &mut rstd::fmt::Formatter) -> rstd::fmt::Result { diff --git a/substrate/srml/balances/src/lib.rs b/substrate/srml/balances/src/lib.rs index 3c354da508..71f37cb8f8 100644 --- a/substrate/srml/balances/src/lib.rs +++ b/substrate/srml/balances/src/lib.rs @@ -824,13 +824,13 @@ where fn ensure_can_withdraw( who: &T::AccountId, _amount: T::Balance, - reason: WithdrawReason, + reasons: WithdrawReasons, new_balance: T::Balance, ) -> Result { - match reason { - WithdrawReason::Reserve | WithdrawReason::Transfer if Self::vesting_balance(who) > new_balance => - return Err("vesting balance too high to send value"), - _ => {} + if reasons.intersects(WithdrawReason::Reserve | WithdrawReason::Transfer) + && Self::vesting_balance(who) > new_balance + { + return Err("vesting balance too high to send value"); } let locks = Self::locks(who); if locks.is_empty() { @@ -842,7 +842,7 @@ where .all(|l| now >= l.until || new_balance >= l.amount - || !l.reasons.contains(reason) + || !l.reasons.intersects(reasons) ) { Ok(()) @@ -868,7 +868,7 @@ where if would_create && value < T::ExistentialDeposit::get() { return Err("value too low to create account"); } - Self::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer, new_from_balance)?; + Self::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer.into(), new_from_balance)?; // NOTE: total stake being stored in the same type means that this could never overflow // but better to be safe than sorry. @@ -893,7 +893,7 @@ where fn withdraw( who: &T::AccountId, value: Self::Balance, - reason: WithdrawReason, + reasons: WithdrawReasons, liveness: ExistenceRequirement, ) -> result::Result { let old_balance = Self::free_balance(who); @@ -907,7 +907,7 @@ where { return Err("payment would kill account") } - Self::ensure_can_withdraw(who, value, reason, new_balance)?; + Self::ensure_can_withdraw(who, value, reasons, new_balance)?; Self::set_free_balance(who, new_balance); Ok(NegativeImbalance::new(value)) } else { @@ -1014,7 +1014,7 @@ where Self::free_balance(who) .checked_sub(&value) .map_or(false, |new_balance| - Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve, new_balance).is_ok() + Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve.into(), new_balance).is_ok() ) } @@ -1028,7 +1028,7 @@ where return Err("not enough free funds") } let new_balance = b - value; - Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve, new_balance)?; + Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve.into(), new_balance)?; Self::set_reserved_balance(who, Self::reserved_balance(who) + value); Self::set_free_balance(who, new_balance); Ok(()) diff --git a/substrate/srml/contracts/src/exec.rs b/substrate/srml/contracts/src/exec.rs index af79c6c818..686b173ec7 100644 --- a/substrate/srml/contracts/src/exec.rs +++ b/substrate/srml/contracts/src/exec.rs @@ -642,7 +642,7 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( if would_create && value < ctx.config.existential_deposit { return Err("value too low to create account"); } - T::Currency::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer, new_from_balance)?; + T::Currency::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer.into(), new_from_balance)?; let new_to_balance = match to_balance.checked_add(&value) { Some(b) => b, diff --git a/substrate/srml/contracts/src/gas.rs b/substrate/srml/contracts/src/gas.rs index 08070916fb..64997fbe81 100644 --- a/substrate/srml/contracts/src/gas.rs +++ b/substrate/srml/contracts/src/gas.rs @@ -215,7 +215,7 @@ pub fn buy_gas( let imbalance = T::Currency::withdraw( transactor, cost, - WithdrawReason::Fee, + WithdrawReason::Fee.into(), ExistenceRequirement::KeepAlive )?; diff --git a/substrate/srml/contracts/src/rent.rs b/substrate/srml/contracts/src/rent.rs index ecc5f61031..6647f89631 100644 --- a/substrate/srml/contracts/src/rent.rs +++ b/substrate/srml/contracts/src/rent.rs @@ -119,7 +119,7 @@ fn try_evict_or_and_pay_rent( let can_withdraw_rent = T::Currency::ensure_can_withdraw( account, dues_limited, - WithdrawReason::Fee, + WithdrawReason::Fee.into(), balance.saturating_sub(dues_limited), ) .is_ok(); @@ -129,7 +129,7 @@ fn try_evict_or_and_pay_rent( let imbalance = T::Currency::withdraw( account, dues_limited, - WithdrawReason::Fee, + WithdrawReason::Fee.into(), ExistenceRequirement::KeepAlive, ) .expect( diff --git a/substrate/srml/elections-phragmen/src/lib.rs b/substrate/srml/elections-phragmen/src/lib.rs index f56c5a4ec3..e74ecd8521 100644 --- a/substrate/srml/elections-phragmen/src/lib.rs +++ b/substrate/srml/elections-phragmen/src/lib.rs @@ -83,7 +83,7 @@ use srml_support::{ decl_storage, decl_event, ensure, decl_module, dispatch, traits::{ Currency, Get, LockableCurrency, LockIdentifier, ReservableCurrency, WithdrawReasons, - ChangeMembers, OnUnbalanced, + ChangeMembers, OnUnbalanced, WithdrawReason } }; use system::{self, ensure_signed, ensure_root}; @@ -215,7 +215,7 @@ decl_module! { &who, locked_balance, T::BlockNumber::max_value(), - WithdrawReasons::all(), + WithdrawReasons::except(WithdrawReason::TransactionPayment), ); >::insert(&who, locked_balance); >::insert(&who, votes); diff --git a/substrate/srml/elections/src/lib.rs b/substrate/srml/elections/src/lib.rs index 87e29f3b14..1e2349440f 100644 --- a/substrate/srml/elections/src/lib.rs +++ b/substrate/srml/elections/src/lib.rs @@ -807,7 +807,7 @@ impl Module { let imbalance = T::Currency::withdraw( &who, T::VotingFee::get(), - WithdrawReason::Fee, + WithdrawReason::Fee.into(), ExistenceRequirement::KeepAlive, )?; T::BadVoterIndex::on_unbalanced(imbalance); diff --git a/substrate/srml/generic-asset/src/lib.rs b/substrate/srml/generic-asset/src/lib.rs index fdbb57f56f..38bff08e12 100644 --- a/substrate/srml/generic-asset/src/lib.rs +++ b/substrate/srml/generic-asset/src/lib.rs @@ -128,7 +128,7 @@ //! T::Currency::withdraw( //! transactor, //! amount, -//! WithdrawReason::TransactionPayment, +//! WithdrawReason::TransactionPayment.into(), //! ExistenceRequirement::KeepAlive, //! )?; //! // ... @@ -563,11 +563,16 @@ impl Module { /// Transfer some liquid free balance from one account to another. /// This will not emit the `Transferred` event. - pub fn make_transfer(asset_id: &T::AssetId, from: &T::AccountId, to: &T::AccountId, amount: T::Balance) -> Result { + pub fn make_transfer( + asset_id: &T::AssetId, + from: &T::AccountId, + to: &T::AccountId, + amount: T::Balance + ) -> Result { let new_balance = Self::free_balance(asset_id, from) .checked_sub(&amount) .ok_or_else(|| "balance too low to send amount")?; - Self::ensure_can_withdraw(asset_id, from, amount, WithdrawReason::Transfer, new_balance)?; + Self::ensure_can_withdraw(asset_id, from, amount, WithdrawReason::Transfer.into(), new_balance)?; if from != to { >::mutate(asset_id, from, |balance| *balance -= amount); @@ -734,7 +739,7 @@ impl Module { asset_id: &T::AssetId, who: &T::AccountId, _amount: T::Balance, - reason: WithdrawReason, + reasons: WithdrawReasons, new_balance: T::Balance, ) -> Result { if asset_id != &Self::staking_asset_id() { @@ -748,7 +753,7 @@ impl Module { let now = >::block_number(); if Self::locks(who) .into_iter() - .all(|l| now >= l.until || new_balance >= l.amount || !l.reasons.contains(reason)) + .all(|l| now >= l.until || new_balance >= l.amount || !l.reasons.intersects(reasons)) { Ok(()) } else { @@ -1098,22 +1103,22 @@ where fn ensure_can_withdraw( who: &T::AccountId, amount: Self::Balance, - reason: WithdrawReason, + reasons: WithdrawReasons, new_balance: Self::Balance, ) -> Result { - >::ensure_can_withdraw(&U::asset_id(), who, amount, reason, new_balance) + >::ensure_can_withdraw(&U::asset_id(), who, amount, reasons, new_balance) } fn withdraw( who: &T::AccountId, value: Self::Balance, - reason: WithdrawReason, + reasons: WithdrawReasons, _: ExistenceRequirement, // no existential deposit policy for generic asset ) -> result::Result { let new_balance = Self::free_balance(who) .checked_sub(&value) .ok_or_else(|| "account has too few funds")?; - Self::ensure_can_withdraw(who, value, reason, new_balance)?; + Self::ensure_can_withdraw(who, value, reasons, new_balance)?; >::set_free_balance(&U::asset_id(), who, new_balance); Ok(NegativeImbalance::new(value)) } @@ -1197,7 +1202,7 @@ where .checked_sub(&value) .map_or(false, |new_balance| >::ensure_can_withdraw( - &U::asset_id(), who, value, WithdrawReason::Reserve, new_balance + &U::asset_id(), who, value, WithdrawReason::Reserve.into(), new_balance ).is_ok() ) } diff --git a/substrate/srml/support/src/traits.rs b/substrate/srml/support/src/traits.rs index c58d60ffc9..b321aeaeb9 100644 --- a/substrate/srml/support/src/traits.rs +++ b/substrate/srml/support/src/traits.rs @@ -381,7 +381,7 @@ pub trait Currency { fn ensure_can_withdraw( who: &AccountId, _amount: Self::Balance, - reason: WithdrawReason, + reasons: WithdrawReasons, new_balance: Self::Balance, ) -> result::Result<(), &'static str>; @@ -459,7 +459,7 @@ pub trait Currency { fn withdraw( who: &AccountId, value: Self::Balance, - reason: WithdrawReason, + reasons: WithdrawReasons, liveness: ExistenceRequirement, ) -> result::Result; @@ -467,11 +467,11 @@ pub trait Currency { fn settle( who: &AccountId, value: Self::PositiveImbalance, - reason: WithdrawReason, + reasons: WithdrawReasons, liveness: ExistenceRequirement, ) -> result::Result<(), Self::PositiveImbalance> { let v = value.peek(); - match Self::withdraw(who, v, reason, liveness) { + match Self::withdraw(who, v, reasons, liveness) { Ok(opposite) => Ok(drop(value.offset(opposite))), _ => Err(value), } @@ -614,6 +614,8 @@ bitmask! { Reserve = 0b00000100, /// In order to pay some other (higher-level) fees. Fee = 0b00001000, + /// In order to tip a validator for transaction inclusion. + Tip = 0b00010000, } } @@ -630,7 +632,7 @@ impl WithdrawReasons { /// # use srml_support::traits::{WithdrawReason, WithdrawReasons}; /// # fn main() { /// assert_eq!( - /// WithdrawReason::Fee | WithdrawReason::Transfer | WithdrawReason::Reserve, + /// WithdrawReason::Fee | WithdrawReason::Transfer | WithdrawReason::Reserve | WithdrawReason::Tip, /// WithdrawReasons::except(WithdrawReason::TransactionPayment), /// ); /// # } diff --git a/substrate/srml/transaction-payment/src/lib.rs b/substrate/srml/transaction-payment/src/lib.rs index 5ad5c650af..6ebda53b72 100644 --- a/substrate/srml/transaction-payment/src/lib.rs +++ b/substrate/srml/transaction-payment/src/lib.rs @@ -171,11 +171,16 @@ impl SignedExtension for ChargeTransactionPayment len: usize, ) -> TransactionValidity { // pay any fees. - let fee = Self::compute_fee(len, info, self.0); + let tip = self.0; + let fee = Self::compute_fee(len, info, tip); let imbalance = match T::Currency::withdraw( who, fee, - WithdrawReason::TransactionPayment, + if tip.is_zero() { + WithdrawReason::TransactionPayment.into() + } else { + WithdrawReason::TransactionPayment | WithdrawReason::Tip + }, ExistenceRequirement::KeepAlive, ) { Ok(imbalance) => imbalance, diff --git a/substrate/srml/treasury/src/lib.rs b/substrate/srml/treasury/src/lib.rs index 668dcaca32..7c38115bfb 100644 --- a/substrate/srml/treasury/src/lib.rs +++ b/substrate/srml/treasury/src/lib.rs @@ -324,7 +324,7 @@ impl Module { if let Err(problem) = T::Currency::settle( &Self::account_id(), imbalance, - WithdrawReason::Transfer, + WithdrawReason::Transfer.into(), ExistenceRequirement::KeepAlive ) { print("Inconsistent state - couldn't settle imbalance for funds spent by treasury"); @@ -657,7 +657,7 @@ mod tests { >::on_finalize(4); assert_eq!(Treasury::pot(), 0); // Pot is emptied - assert_eq!(Balances::free_balance(&Treasury::account_id()), 1); // but the account is still there + assert_eq!(Balances::free_balance(&Treasury::account_id()), 1); // but the account is still there }); }