Skip to content

Commit

Permalink
Replace bitmask with bitflags (paritytech#7159)
Browse files Browse the repository at this point in the history
Signed-off-by: koushiro <[email protected]>
  • Loading branch information
koushiro authored Oct 29, 2020
1 parent 995111a commit 179e02b
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 80 deletions.
8 changes: 1 addition & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ use frame_support::{
StorageValue, Parameter, decl_event, decl_storage, decl_module, decl_error, ensure,
traits::{
Currency, OnKilledAccount, OnUnbalanced, TryDrop, StoredMap,
WithdrawReason, WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement,
WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement,
Imbalance, SignedImbalance, ReservableCurrency, Get, ExistenceRequirement::KeepAlive,
ExistenceRequirement::AllowDeath, IsDeadAccount, BalanceStatus as Status,
}
Expand Down Expand Up @@ -292,9 +292,9 @@ pub enum Reasons {

impl From<WithdrawReasons> for Reasons {
fn from(r: WithdrawReasons) -> Reasons {
if r == WithdrawReasons::from(WithdrawReason::TransactionPayment) {
if r == WithdrawReasons::from(WithdrawReasons::TRANSACTION_PAYMENT) {
Reasons::Fee
} else if r.contains(WithdrawReason::TransactionPayment) {
} else if r.contains(WithdrawReasons::TRANSACTION_PAYMENT) {
Reasons::All
} else {
Reasons::Misc
Expand Down Expand Up @@ -1011,7 +1011,7 @@ impl<T: Trait<I>, I: Instance> Currency<T::AccountId> for Module<T, I> where
Self::ensure_can_withdraw(
transactor,
value,
WithdrawReason::Transfer.into(),
WithdrawReasons::TRANSFER,
from_account.free,
)?;

Expand Down Expand Up @@ -1170,7 +1170,7 @@ impl<T: Trait<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I>
Self::account(who).free
.checked_sub(&value)
.map_or(false, |new_balance|
Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve.into(), new_balance).is_ok()
Self::ensure_can_withdraw(who, value, WithdrawReasons::RESERVE, new_balance).is_ok()
)
}

Expand All @@ -1187,7 +1187,7 @@ impl<T: Trait<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I>
Self::try_mutate_account(who, |account, _| -> DispatchResult {
account.free = account.free.checked_sub(&value).ok_or(Error::<T, I>::InsufficientBalance)?;
account.reserved = account.reserved.checked_add(&value).ok_or(Error::<T, I>::Overflow)?;
Self::ensure_can_withdraw(&who, value.clone(), WithdrawReason::Reserve.into(), account.free)
Self::ensure_can_withdraw(&who, value.clone(), WithdrawReasons::RESERVE, account.free)
})?;

Self::deposit_event(RawEvent::Reserved(who.clone(), value));
Expand Down Expand Up @@ -1303,7 +1303,7 @@ where
amount: T::Balance,
reasons: WithdrawReasons,
) {
if amount.is_zero() || reasons.is_none() { return }
if amount.is_zero() || reasons.is_empty() { return }
let mut new_lock = Some(BalanceLock { id, amount, reasons: reasons.into() });
let mut locks = Self::locks(who).into_iter()
.filter_map(|l| if l.id == id { new_lock.take() } else { Some(l) })
Expand All @@ -1322,7 +1322,7 @@ where
amount: T::Balance,
reasons: WithdrawReasons,
) {
if amount.is_zero() || reasons.is_none() { return }
if amount.is_zero() || reasons.is_empty() { return }
let mut new_lock = Some(BalanceLock { id, amount, reasons: reasons.into() });
let mut locks = Self::locks(who).into_iter().filter_map(|l|
if l.id == id {
Expand Down
14 changes: 7 additions & 7 deletions frame/balances/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ macro_rules! decl_tests {
use frame_support::{
assert_noop, assert_ok, assert_err,
traits::{
LockableCurrency, LockIdentifier, WithdrawReason, WithdrawReasons,
LockableCurrency, LockIdentifier, WithdrawReasons,
Currency, ReservableCurrency, ExistenceRequirement::AllowDeath, StoredMap
}
};
Expand Down Expand Up @@ -133,7 +133,7 @@ macro_rules! decl_tests {
#[test]
fn combination_locking_should_work() {
<$ext_builder>::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, u64::max_value(), WithdrawReasons::none());
Balances::set_lock(ID_1, &1, u64::max_value(), WithdrawReasons::empty());
Balances::set_lock(ID_2, &1, 0, WithdrawReasons::all());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
});
Expand Down Expand Up @@ -168,7 +168,7 @@ macro_rules! decl_tests {
.build()
.execute_with(|| {
pallet_transaction_payment::NextFeeMultiplier::put(Multiplier::saturating_from_integer(1));
Balances::set_lock(ID_1, &1, 10, WithdrawReason::Reserve.into());
Balances::set_lock(ID_1, &1, 10, WithdrawReasons::RESERVE);
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),
Error::<$test, _>::LiquidityRestrictions
Expand All @@ -192,7 +192,7 @@ macro_rules! decl_tests {
1,
).is_ok());

Balances::set_lock(ID_1, &1, 10, WithdrawReason::TransactionPayment.into());
Balances::set_lock(ID_1, &1, 10, WithdrawReasons::TRANSACTION_PAYMENT);
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
assert_ok!(<Balances as ReservableCurrency<_>>::reserve(&1, 1));
assert!(<ChargeTransactionPayment<$test> as SignedExtension>::pre_dispatch(
Expand Down Expand Up @@ -237,17 +237,17 @@ macro_rules! decl_tests {
#[test]
fn lock_reasons_extension_should_work() {
<$ext_builder>::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, 10, WithdrawReason::Transfer.into());
Balances::set_lock(ID_1, &1, 10, WithdrawReasons::TRANSFER);
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
Error::<$test, _>::LiquidityRestrictions
);
Balances::extend_lock(ID_1, &1, 10, WithdrawReasons::none());
Balances::extend_lock(ID_1, &1, 10, WithdrawReasons::empty());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
Error::<$test, _>::LiquidityRestrictions
);
Balances::extend_lock(ID_1, &1, 10, WithdrawReason::Reserve.into());
Balances::extend_lock(ID_1, &1, 10, WithdrawReasons::RESERVE);
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
Error::<$test, _>::LiquidityRestrictions
Expand Down
6 changes: 3 additions & 3 deletions frame/contracts/src/rent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
use sp_std::prelude::*;
use sp_io::hashing::blake2_256;
use frame_support::storage::child;
use frame_support::traits::{Currency, ExistenceRequirement, Get, OnUnbalanced, WithdrawReason};
use frame_support::traits::{Currency, ExistenceRequirement, Get, OnUnbalanced, WithdrawReasons};
use frame_support::StorageMap;
use pallet_contracts_primitives::{ContractAccessError, RentProjection, RentProjectionResult};
use sp_runtime::traits::{Bounded, CheckedDiv, CheckedMul, SaturatedConversion, Saturating, Zero};
Expand Down Expand Up @@ -54,7 +54,7 @@ impl<T: Trait> OutstandingAmount<T> {
if let Ok(imbalance) = T::Currency::withdraw(
account,
self.amount,
WithdrawReason::Fee.into(),
WithdrawReasons::FEE,
ExistenceRequirement::KeepAlive,
) {
// This should never fail. However, let's err on the safe side.
Expand Down Expand Up @@ -192,7 +192,7 @@ fn consider_case<T: Trait>(
let can_withdraw_rent = T::Currency::ensure_can_withdraw(
account,
dues_limited,
WithdrawReason::Fee.into(),
WithdrawReasons::FEE,
free_balance.saturating_sub(dues_limited),
)
.is_ok();
Expand Down
8 changes: 4 additions & 4 deletions frame/democracy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ use frame_support::{
decl_module, decl_storage, decl_event, decl_error, ensure, Parameter,
weights::{Weight, DispatchClass, Pays},
traits::{
Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get,
Currency, ReservableCurrency, LockableCurrency, WithdrawReasons, LockIdentifier, Get,
OnUnbalanced, BalanceStatus, schedule::{Named as ScheduleNamed, DispatchTime}, EnsureOrigin
},
dispatch::DispatchResultWithPostInfo,
Expand Down Expand Up @@ -1278,7 +1278,7 @@ impl<T: Trait> Module<T> {
DEMOCRACY_ID,
who,
vote.balance(),
WithdrawReason::Transfer.into()
WithdrawReasons::TRANSFER
);
ReferendumInfoOf::<T>::insert(ref_index, ReferendumInfo::Ongoing(status));
Ok(())
Expand Down Expand Up @@ -1410,7 +1410,7 @@ impl<T: Trait> Module<T> {
DEMOCRACY_ID,
&who,
balance,
WithdrawReason::Transfer.into()
WithdrawReasons::TRANSFER
);
Ok(votes)
})?;
Expand Down Expand Up @@ -1461,7 +1461,7 @@ impl<T: Trait> Module<T> {
if lock_needed.is_zero() {
T::Currency::remove_lock(DEMOCRACY_ID, who);
} else {
T::Currency::set_lock(DEMOCRACY_ID, who, lock_needed, WithdrawReason::Transfer.into());
T::Currency::set_lock(DEMOCRACY_ID, who, lock_needed, WithdrawReasons::TRANSFER);
}
}

Expand Down
4 changes: 2 additions & 2 deletions frame/elections-phragmen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ use frame_support::{
traits::{
BalanceStatus, ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get,
InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency,
WithdrawReason, WithdrawReasons,
WithdrawReasons,
},
weights::Weight,
};
Expand Down Expand Up @@ -365,7 +365,7 @@ decl_module! {
T::ModuleId::get(),
&who,
locked_balance,
WithdrawReasons::except(WithdrawReason::TransactionPayment),
WithdrawReasons::except(WithdrawReasons::TRANSACTION_PAYMENT),
);

Voting::<T>::insert(&who, (locked_balance, votes));
Expand Down
4 changes: 2 additions & 2 deletions frame/elections/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use frame_support::{
weights::{Weight, DispatchClass},
traits::{
Currency, ExistenceRequirement, Get, LockableCurrency, LockIdentifier, BalanceStatus,
OnUnbalanced, ReservableCurrency, WithdrawReason, WithdrawReasons, ChangeMembers,
OnUnbalanced, ReservableCurrency, WithdrawReasons, ChangeMembers,
}
};
use codec::{Encode, Decode};
Expand Down Expand Up @@ -871,7 +871,7 @@ impl<T: Trait> Module<T> {
let imbalance = T::Currency::withdraw(
&who,
T::VotingFee::get(),
WithdrawReason::Fee.into(),
WithdrawReasons::FEE,
ExistenceRequirement::KeepAlive,
)?;
T::BadVoterIndex::on_unbalanced(imbalance);
Expand Down
6 changes: 3 additions & 3 deletions frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ mod tests {
use frame_support::{
parameter_types,
weights::{Weight, RuntimeDbWeight, IdentityFee, WeightToFeePolynomial},
traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons, WithdrawReason},
traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons},
};
use frame_system::{Call as SystemCall, ChainContext, LastRuntimeUpgradeInfo};
use pallet_balances::Call as BalancesCall;
Expand Down Expand Up @@ -950,7 +950,7 @@ mod tests {
Digest::default(),
));

if lock == WithdrawReasons::except(WithdrawReason::TransactionPayment) {
if lock == WithdrawReasons::except(WithdrawReasons::TRANSACTION_PAYMENT) {
assert!(Executive::apply_extrinsic(xt).unwrap().is_ok());
// tx fee has been deducted.
assert_eq!(<pallet_balances::Module<Runtime>>::total_balance(&1), 111 - fee);
Expand All @@ -965,7 +965,7 @@ mod tests {
};

execute_with_lock(WithdrawReasons::all());
execute_with_lock(WithdrawReasons::except(WithdrawReason::TransactionPayment));
execute_with_lock(WithdrawReasons::except(WithdrawReasons::TRANSACTION_PAYMENT));
}

#[test]
Expand Down
3 changes: 1 addition & 2 deletions frame/support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ frame-support-procedural = { version = "2.0.0", default-features = false, path =
paste = "0.1.6"
once_cell = { version = "1", default-features = false, optional = true }
sp-state-machine = { version = "0.8.0", optional = true, path = "../../primitives/state-machine" }
bitmask = { version = "0.5.0", default-features = false }
bitflags = "1.2"
impl-trait-for-tuples = "0.1.3"
smallvec = "1.4.1"

Expand All @@ -43,7 +43,6 @@ sp-api = { version = "2.0.0", default-features = false, path = "../../primitives
default = ["std"]
std = [
"once_cell",
"bitmask/std",
"serde",
"sp-io/std",
"codec/std",
Expand Down
3 changes: 0 additions & 3 deletions frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@
/// Export ourself as `frame_support` to make tests happy.
extern crate self as frame_support;

#[macro_use]
extern crate bitmask;

#[doc(hidden)]
pub use sp_tracing;

Expand Down
57 changes: 27 additions & 30 deletions frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use sp_runtime::{
use crate::dispatch::Parameter;
use crate::storage::StorageMap;
use crate::weights::Weight;
use bitflags::bitflags;
use impl_trait_for_tuples::impl_for_tuples;

/// Re-expected for the macro.
Expand Down Expand Up @@ -1184,58 +1185,54 @@ pub trait VestingSchedule<AccountId> {
fn remove_vesting_schedule(who: &AccountId);
}

bitmask! {
bitflags! {
/// Reasons for moving funds out of an account.
#[derive(Encode, Decode)]
pub mask WithdrawReasons: i8 where

/// Reason for moving funds out of an account.
#[derive(Encode, Decode)]
flags WithdrawReason {
pub struct WithdrawReasons: i8 {
/// In order to pay for (system) transaction costs.
TransactionPayment = 0b00000001,
const TRANSACTION_PAYMENT = 0b00000001;
/// In order to transfer ownership.
Transfer = 0b00000010,
const TRANSFER = 0b00000010;
/// In order to reserve some funds for a later return or repatriation.
Reserve = 0b00000100,
const RESERVE = 0b00000100;
/// In order to pay some other (higher-level) fees.
Fee = 0b00001000,
const FEE = 0b00001000;
/// In order to tip a validator for transaction inclusion.
Tip = 0b00010000,
const TIP = 0b00010000;
}
}

pub trait Time {
type Moment: AtLeast32Bit + Parameter + Default + Copy;

fn now() -> Self::Moment;
}

/// Trait to deal with unix time.
pub trait UnixTime {
/// Return duration since `SystemTime::UNIX_EPOCH`.
fn now() -> core::time::Duration;
}

impl WithdrawReasons {
/// Choose all variants except for `one`.
///
/// ```rust
/// # use frame_support::traits::{WithdrawReason, WithdrawReasons};
/// # use frame_support::traits::WithdrawReasons;
/// # fn main() {
/// assert_eq!(
/// WithdrawReason::Fee | WithdrawReason::Transfer | WithdrawReason::Reserve | WithdrawReason::Tip,
/// WithdrawReasons::except(WithdrawReason::TransactionPayment),
/// WithdrawReasons::FEE | WithdrawReasons::TRANSFER | WithdrawReasons::RESERVE | WithdrawReasons::TIP,
/// WithdrawReasons::except(WithdrawReasons::TRANSACTION_PAYMENT),
/// );
/// # }
/// ```
pub fn except(one: WithdrawReason) -> WithdrawReasons {
let mut mask = Self::all();
mask.toggle(one);
mask
pub fn except(one: WithdrawReasons) -> WithdrawReasons {
let mut flags = Self::all();
flags.toggle(one);
flags
}
}

pub trait Time {
type Moment: AtLeast32Bit + Parameter + Default + Copy;

fn now() -> Self::Moment;
}

/// Trait to deal with unix time.
pub trait UnixTime {
/// Return duration since `SystemTime::UNIX_EPOCH`.
fn now() -> core::time::Duration;
}

/// Trait for type that can handle incremental changes to a set of account IDs.
pub trait ChangeMembers<AccountId: Clone + Ord> {
/// A number of members `incoming` just joined the set and replaced some `outgoing` ones. The
Expand Down
Loading

0 comments on commit 179e02b

Please sign in to comment.