Update Balances Pallet for decl_error! (#4405)

* Update balances for `decl_error!`

* Update for new `decl_error`

* Fix staking tests

* Use `ok_or` over `match`
This commit is contained in:
Shawn Tabrizi
2019-12-20 00:23:20 +01:00
committed by GitHub
parent dfe87ec61e
commit 4fffe19c28
5 changed files with 120 additions and 76 deletions
+33 -19
View File
@@ -163,7 +163,7 @@ use sp_std::prelude::*;
use sp_std::{cmp, result, mem, fmt::Debug};
use codec::{Codec, Encode, Decode};
use frame_support::{
StorageValue, Parameter, decl_event, decl_storage, decl_module,
StorageValue, Parameter, decl_event, decl_storage, decl_module, decl_error,
traits::{
UpdateBalanceOutcome, Currency, OnFreeBalanceZero, OnUnbalanced, TryDrop,
WithdrawReason, WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement,
@@ -272,6 +272,27 @@ decl_event!(
}
);
decl_error! {
pub enum Error for Module<T: Trait<I>, I: Instance> {
/// Vesting balance too high to send value
VestingBalance,
/// Account liquidity restrictions prevent withdrawal
LiquidityRestrictions,
/// Got an overflow after adding
Overflow,
/// Balance too low to send value
InsufficientBalance,
/// Value too low to create account due to existential deposit
ExistentialDeposit,
/// Transfer/payment would kill account
KeepAlive,
/// A vesting schedule already exists for this account
ExistingVestingSchedule,
/// Beneficiary account must pre-exist
DeadAccount,
}
}
/// Struct to encode the vesting schedule of an individual account.
#[derive(Encode, Decode, Copy, Clone, PartialEq, Eq, RuntimeDebug)]
pub struct VestingSchedule<Balance, BlockNumber> {
@@ -390,6 +411,8 @@ decl_storage! {
decl_module! {
pub struct Module<T: Trait<I>, I: Instance = DefaultInstance> for enum Call where origin: T::Origin {
type Error = Error<T, I>;
/// The minimum amount required to keep an account open.
const ExistentialDeposit: T::Balance = T::ExistentialDeposit::get();
@@ -897,7 +920,7 @@ where
if reasons.intersects(WithdrawReason::Reserve | WithdrawReason::Transfer)
&& Self::vesting_balance(who) > new_balance
{
Err("vesting balance too high to send value")?
Err(Error::<T, I>::VestingBalance)?
}
let locks = Self::locks(who);
if locks.is_empty() {
@@ -914,7 +937,7 @@ where
{
Ok(())
} else {
Err("account liquidity restrictions prevent withdrawal".into())
Err(Error::<T, I>::LiquidityRestrictions.into())
}
}
@@ -928,31 +951,22 @@ where
let to_balance = Self::free_balance(dest);
let would_create = to_balance.is_zero();
let fee = if would_create { T::CreationFee::get() } else { T::TransferFee::get() };
let liability = match value.checked_add(&fee) {
Some(l) => l,
None => Err("got overflow after adding a fee to value")?,
};
let liability = value.checked_add(&fee).ok_or(Error::<T, I>::Overflow)?;
let new_from_balance = from_balance.checked_sub(&liability).ok_or(Error::<T, I>::InsufficientBalance)?;
let new_from_balance = match from_balance.checked_sub(&liability) {
None => Err("balance too low to send value")?,
Some(b) => b,
};
if would_create && value < T::ExistentialDeposit::get() {
Err("value too low to create account")?
Err(Error::<T, I>::ExistentialDeposit)?
}
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.
let new_to_balance = match to_balance.checked_add(&value) {
Some(b) => b,
None => Err("destination balance too high to receive value")?,
};
let new_to_balance = to_balance.checked_add(&value).ok_or(Error::<T, I>::Overflow)?;
if transactor != dest {
if existence_requirement == ExistenceRequirement::KeepAlive {
if new_from_balance < Self::minimum_balance() {
Err("transfer would kill account")?
Err(Error::<T, I>::KeepAlive)?
}
}
@@ -1026,7 +1040,7 @@ where
value: Self::Balance
) -> result::Result<Self::PositiveImbalance, DispatchError> {
if Self::total_balance(who).is_zero() {
Err("beneficiary account must pre-exist")?
Err(Error::<T, I>::DeadAccount)?
}
Self::set_free_balance(who, Self::free_balance(who) + value);
Ok(PositiveImbalance::new(value))
@@ -1143,7 +1157,7 @@ where
value: Self::Balance,
) -> result::Result<Self::Balance, DispatchError> {
if Self::total_balance(beneficiary).is_zero() {
Err("beneficiary account must pre-exist")?
Err(Error::<T, I>::DeadAccount)?
}
let b = Self::reserved_balance(slashed);
let slash = cmp::min(b, value);
+11 -11
View File
@@ -27,7 +27,7 @@ use crate::{GenesisConfig, Module, Trait};
use frame_system as system;
impl_outer_origin!{
pub enum Origin for Runtime {}
pub enum Origin for Test {}
}
thread_local! {
@@ -53,14 +53,14 @@ impl Get<u64> for CreationFee {
// Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted.
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct Runtime;
pub struct Test;
parameter_types! {
pub const BlockHashCount: u64 = 250;
pub const MaximumBlockWeight: Weight = 1024;
pub const MaximumBlockLength: u32 = 2 * 1024;
pub const AvailableBlockRatio: Perbill = Perbill::one();
}
impl frame_system::Trait for Runtime {
impl frame_system::Trait for Test {
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
@@ -82,15 +82,15 @@ parameter_types! {
pub const TransactionBaseFee: u64 = 0;
pub const TransactionByteFee: u64 = 1;
}
impl pallet_transaction_payment::Trait for Runtime {
type Currency = Module<Runtime>;
impl pallet_transaction_payment::Trait for Test {
type Currency = Module<Test>;
type OnTransactionPayment = ();
type TransactionBaseFee = TransactionBaseFee;
type TransactionByteFee = TransactionByteFee;
type WeightToFee = ConvertInto;
type FeeMultiplierUpdate = ();
}
impl Trait for Runtime {
impl Trait for Test {
type Balance = u64;
type OnFreeBalanceZero = ();
type OnNewAccount = ();
@@ -152,8 +152,8 @@ impl ExtBuilder {
}
pub fn build(self) -> sp_io::TestExternalities {
self.set_associated_consts();
let mut t = frame_system::GenesisConfig::default().build_storage::<Runtime>().unwrap();
GenesisConfig::<Runtime> {
let mut t = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
GenesisConfig::<Test> {
balances: if self.monied {
vec![
(1, 10 * self.existential_deposit),
@@ -179,10 +179,10 @@ impl ExtBuilder {
}
}
pub type System = frame_system::Module<Runtime>;
pub type Balances = Module<Runtime>;
pub type System = frame_system::Module<Test>;
pub type Balances = Module<Test>;
pub const CALL: &<Runtime as frame_system::Trait>::Call = &();
pub const CALL: &<Test as frame_system::Trait>::Call = &();
/// create a transaction info struct from weight. Handy to avoid building the whole struct.
pub fn info_from_weight(w: Weight) -> DispatchInfo {
+39 -39
View File
@@ -17,7 +17,7 @@
//! Tests for the module.
use super::*;
use mock::{Balances, ExtBuilder, Runtime, System, info_from_weight, CALL};
use mock::{Balances, ExtBuilder, Test, System, info_from_weight, CALL};
use sp_runtime::traits::{SignedExtension, BadOrigin};
use frame_support::{
assert_noop, assert_ok, assert_err,
@@ -38,7 +38,7 @@ fn basic_locking_should_work() {
Balances::set_lock(ID_1, &1, 9, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 5, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
});
}
@@ -94,17 +94,17 @@ fn lock_value_extension_should_work() {
Balances::set_lock(ID_1, &1, 5, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
Balances::extend_lock(ID_1, &1, 2, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
Balances::extend_lock(ID_1, &1, 8, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 3, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
});
}
@@ -119,11 +119,11 @@ fn lock_reasons_should_work() {
Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::Transfer.into());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
assert_ok!(<Balances as ReservableCurrency<_>>::reserve(&1, 1));
// NOTE: this causes a fee payment.
assert!(<ChargeTransactionPayment<Runtime> as SignedExtension>::pre_dispatch(
assert!(<ChargeTransactionPayment<Test> as SignedExtension>::pre_dispatch(
ChargeTransactionPayment::from(1),
&1,
CALL,
@@ -135,9 +135,9 @@ fn lock_reasons_should_work() {
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
assert_noop!(
<Balances as ReservableCurrency<_>>::reserve(&1, 1),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
assert!(<ChargeTransactionPayment<Runtime> as SignedExtension>::pre_dispatch(
assert!(<ChargeTransactionPayment<Test> as SignedExtension>::pre_dispatch(
ChargeTransactionPayment::from(1),
&1,
CALL,
@@ -148,7 +148,7 @@ fn lock_reasons_should_work() {
Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::TransactionPayment.into());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
assert_ok!(<Balances as ReservableCurrency<_>>::reserve(&1, 1));
assert!(<ChargeTransactionPayment<Runtime> as SignedExtension>::pre_dispatch(
assert!(<ChargeTransactionPayment<Test> as SignedExtension>::pre_dispatch(
ChargeTransactionPayment::from(1),
&1,
CALL,
@@ -164,7 +164,7 @@ fn lock_block_number_should_work() {
Balances::set_lock(ID_1, &1, 10, 2, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
System::set_block_number(2);
@@ -178,18 +178,18 @@ fn lock_block_number_extension_should_work() {
Balances::set_lock(ID_1, &1, 10, 2, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
Balances::extend_lock(ID_1, &1, 10, 1, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
System::set_block_number(2);
Balances::extend_lock(ID_1, &1, 10, 8, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 3, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
});
}
@@ -200,17 +200,17 @@ fn lock_reasons_extension_should_work() {
Balances::set_lock(ID_1, &1, 10, 10, WithdrawReason::Transfer.into());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
Balances::extend_lock(ID_1, &1, 10, 10, WithdrawReasons::none());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
Balances::extend_lock(ID_1, &1, 10, 10, WithdrawReason::Reserve.into());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
Error::<Test, _>::LiquidityRestrictions
);
});
}
@@ -227,7 +227,7 @@ fn default_indexing_on_new_accounts_should_not_work2() {
// ext_deposit is 10, value is 9, not satisfies for ext_deposit
assert_noop!(
Balances::transfer(Some(1).into(), 5, 9),
"value too low to create account",
Error::<Test, _>::ExistentialDeposit,
);
assert_eq!(Balances::is_dead_account(&5), true); // account 5 should not exist
assert_eq!(Balances::free_balance(&1), 100);
@@ -277,7 +277,7 @@ fn reward_should_work() {
assert_eq!(Balances::total_balance(&1), 10);
assert_ok!(Balances::deposit_into_existing(&1, 10).map(drop));
assert_eq!(Balances::total_balance(&1), 20);
assert_eq!(<TotalIssuance<Runtime>>::get(), 120);
assert_eq!(<TotalIssuance<Test>>::get(), 120);
});
}
@@ -379,7 +379,7 @@ fn balance_transfer_when_reserved_should_not_work() {
assert_ok!(Balances::reserve(&1, 69));
assert_noop!(
Balances::transfer(Some(1).into(), 2, 69),
"balance too low to send value",
Error::<Test, _>::InsufficientBalance,
);
});
}
@@ -412,7 +412,7 @@ fn slashing_balance_should_work() {
assert!(Balances::slash(&1, 69).1.is_zero());
assert_eq!(Balances::free_balance(&1), 0);
assert_eq!(Balances::reserved_balance(&1), 42);
assert_eq!(<TotalIssuance<Runtime>>::get(), 42);
assert_eq!(<TotalIssuance<Test>>::get(), 42);
});
}
@@ -424,7 +424,7 @@ fn slashing_incomplete_balance_should_work() {
assert_eq!(Balances::slash(&1, 69).1, 27);
assert_eq!(Balances::free_balance(&1), 0);
assert_eq!(Balances::reserved_balance(&1), 0);
assert_eq!(<TotalIssuance<Runtime>>::get(), 0);
assert_eq!(<TotalIssuance<Test>>::get(), 0);
});
}
@@ -447,7 +447,7 @@ fn slashing_reserved_balance_should_work() {
assert_eq!(Balances::slash_reserved(&1, 42).1, 0);
assert_eq!(Balances::reserved_balance(&1), 69);
assert_eq!(Balances::free_balance(&1), 0);
assert_eq!(<TotalIssuance<Runtime>>::get(), 69);
assert_eq!(<TotalIssuance<Test>>::get(), 69);
});
}
@@ -459,7 +459,7 @@ fn slashing_incomplete_reserved_balance_should_work() {
assert_eq!(Balances::slash_reserved(&1, 69).1, 27);
assert_eq!(Balances::free_balance(&1), 69);
assert_eq!(Balances::reserved_balance(&1), 0);
assert_eq!(<TotalIssuance<Runtime>>::get(), 69);
assert_eq!(<TotalIssuance<Test>>::get(), 69);
});
}
@@ -482,7 +482,7 @@ fn transferring_reserved_balance_to_nonexistent_should_fail() {
ExtBuilder::default().build().execute_with(|| {
let _ = Balances::deposit_creating(&1, 111);
assert_ok!(Balances::reserve(&1, 111));
assert_noop!(Balances::repatriate_reserved(&1, &2, 42), "beneficiary account must pre-exist");
assert_noop!(Balances::repatriate_reserved(&1, &2, 42), Error::<Test, _>::DeadAccount);
});
}
@@ -503,12 +503,12 @@ fn transferring_incomplete_reserved_balance_should_work() {
#[test]
fn transferring_too_high_value_should_not_panic() {
ExtBuilder::default().build().execute_with(|| {
<FreeBalance<Runtime>>::insert(1, u64::max_value());
<FreeBalance<Runtime>>::insert(2, 1);
<FreeBalance<Test>>::insert(1, u64::max_value());
<FreeBalance<Test>>::insert(2, 1);
assert_err!(
Balances::transfer(Some(1).into(), 2, u64::max_value()),
"destination balance too high to receive value",
Error::<Test, _>::Overflow,
);
assert_eq!(Balances::free_balance(&1), u64::max_value());
@@ -520,12 +520,12 @@ fn transferring_too_high_value_should_not_panic() {
fn account_create_on_free_too_low_with_other() {
ExtBuilder::default().existential_deposit(100).build().execute_with(|| {
let _ = Balances::deposit_creating(&1, 100);
assert_eq!(<TotalIssuance<Runtime>>::get(), 100);
assert_eq!(<TotalIssuance<Test>>::get(), 100);
// No-op.
let _ = Balances::deposit_creating(&2, 50);
assert_eq!(Balances::free_balance(&2), 0);
assert_eq!(<TotalIssuance<Runtime>>::get(), 100);
assert_eq!(<TotalIssuance<Test>>::get(), 100);
})
}
@@ -536,14 +536,14 @@ fn account_create_on_free_too_low() {
// No-op.
let _ = Balances::deposit_creating(&2, 50);
assert_eq!(Balances::free_balance(&2), 0);
assert_eq!(<TotalIssuance<Runtime>>::get(), 0);
assert_eq!(<TotalIssuance<Test>>::get(), 0);
})
}
#[test]
fn account_removal_on_free_too_low() {
ExtBuilder::default().existential_deposit(100).build().execute_with(|| {
assert_eq!(<TotalIssuance<Runtime>>::get(), 0);
assert_eq!(<TotalIssuance<Test>>::get(), 0);
// Setup two accounts with free balance above the existential threshold.
let _ = Balances::deposit_creating(&1, 110);
@@ -551,7 +551,7 @@ fn account_removal_on_free_too_low() {
assert_eq!(Balances::free_balance(&1), 110);
assert_eq!(Balances::free_balance(&2), 110);
assert_eq!(<TotalIssuance<Runtime>>::get(), 220);
assert_eq!(<TotalIssuance<Test>>::get(), 220);
// Transfer funds from account 1 of such amount that after this transfer
// the balance of account 1 will be below the existential threshold.
@@ -563,7 +563,7 @@ fn account_removal_on_free_too_low() {
assert_eq!(Balances::free_balance(&2), 130);
// Verify that TotalIssuance tracks balance removal when free balance is too low.
assert_eq!(<TotalIssuance<Runtime>>::get(), 130);
assert_eq!(<TotalIssuance<Test>>::get(), 130);
});
}
@@ -575,7 +575,7 @@ fn transfer_overflow_isnt_exploitable() {
assert_err!(
Balances::transfer(Some(1).into(), 5, evil_value),
"got overflow after adding a fee to value",
Error::<Test, _>::Overflow,
);
});
}
@@ -656,7 +656,7 @@ fn unvested_balance_should_not_transfer() {
assert_eq!(Balances::vesting_balance(&1), 45);
assert_noop!(
Balances::transfer(Some(1).into(), 2, 56),
"vesting balance too high to send value",
Error::<Test, _>::VestingBalance,
); // Account 1 cannot send more than vested amount
});
}
@@ -751,7 +751,7 @@ fn transfer_keep_alive_works() {
let _ = Balances::deposit_creating(&1, 100);
assert_err!(
Balances::transfer_keep_alive(Some(1).into(), 2, 100),
"transfer would kill account"
Error::<Test, _>::KeepAlive
);
assert_eq!(Balances::is_dead_account(&1), false);
assert_eq!(Balances::total_balance(&1), 100);
@@ -763,8 +763,8 @@ fn transfer_keep_alive_works() {
#[should_panic="the balance of any account should always be more than existential deposit."]
fn cannot_set_genesis_value_below_ed() {
mock::EXISTENTIAL_DEPOSIT.with(|v| *v.borrow_mut() = 11);
let mut t = frame_system::GenesisConfig::default().build_storage::<Runtime>().unwrap();
let _ = GenesisConfig::<Runtime> {
let mut t = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
let _ = GenesisConfig::<Test> {
balances: vec![(1, 10)],
vesting: vec![],
}.assimilate_storage(&mut t).unwrap();
+1 -1
View File
@@ -784,7 +784,7 @@ decl_event!(
);
decl_error! {
/// Error for the stacking module.
/// Error for the staking module.
pub enum Error for Module<T: Trait> {
/// Not a controller account.
NotController,
+36 -6
View File
@@ -20,7 +20,11 @@ use super::*;
use mock::*;
use sp_runtime::{assert_eq_error_rate, traits::{OnInitialize, BadOrigin}};
use sp_staking::offence::OffenceDetails;
use frame_support::{assert_ok, assert_noop, traits::{Currency, ReservableCurrency}};
use frame_support::{
assert_ok, assert_noop,
traits::{Currency, ReservableCurrency},
dispatch::DispatchError,
};
use substrate_test_utils::assert_eq_uvec;
#[test]
@@ -32,7 +36,11 @@ fn force_unstake_works() {
// Cant transfer
assert_noop!(
Balances::transfer(Origin::signed(11), 1, 10),
"account liquidity restrictions prevent withdrawal"
DispatchError::Module {
index: 0,
error: 1,
message: Some("LiquidityRestrictions"),
}
);
// Force unstake requires root.
assert_noop!(Staking::force_unstake(Origin::signed(11), 11), BadOrigin);
@@ -326,7 +334,14 @@ fn staking_should_work() {
Some(StakingLedger { stash: 3, total: 1500, active: 1500, unlocking: vec![] })
);
// e.g. it cannot spend more than 500 that it has free from the total 2000
assert_noop!(Balances::reserve(&3, 501), "account liquidity restrictions prevent withdrawal");
assert_noop!(
Balances::reserve(&3, 501),
DispatchError::Module {
index: 0,
error: 1,
message: Some("LiquidityRestrictions"),
}
);
assert_ok!(Balances::reserve(&3, 409));
});
}
@@ -817,7 +832,11 @@ fn cannot_transfer_staked_balance() {
// Confirm account 11 cannot transfer as a result
assert_noop!(
Balances::transfer(Origin::signed(11), 20, 1),
"account liquidity restrictions prevent withdrawal",
DispatchError::Module {
index: 0,
error: 1,
message: Some("LiquidityRestrictions"),
}
);
// Give account 11 extra free balance
@@ -842,7 +861,11 @@ fn cannot_transfer_staked_balance_2() {
// Confirm account 21 can transfer at most 1000
assert_noop!(
Balances::transfer(Origin::signed(21), 20, 1001),
"account liquidity restrictions prevent withdrawal",
DispatchError::Module {
index: 0,
error: 1,
message: Some("LiquidityRestrictions"),
}
);
assert_ok!(Balances::transfer(Origin::signed(21), 20, 1000));
});
@@ -859,7 +882,14 @@ fn cannot_reserve_staked_balance() {
// Confirm account 11 (via controller 10) is totally staked
assert_eq!(Staking::stakers(&11).own, 1000);
// Confirm account 11 cannot transfer as a result
assert_noop!(Balances::reserve(&11, 1), "account liquidity restrictions prevent withdrawal");
assert_noop!(
Balances::reserve(&11, 1),
DispatchError::Module {
index: 0,
error: 1,
message: Some("LiquidityRestrictions"),
}
);
// Give account 11 extra free balance
let _ = Balances::make_free_balance_be(&11, 10000);