diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 62402c78630c7..b53638f79b653 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -153,15 +153,16 @@ mod tests; mod tests_local; mod tests_composite; +mod tests_both; mod benchmarking; use sp_std::prelude::*; -use sp_std::{cmp, result, mem, fmt::Debug, ops::BitOr, convert::Infallible}; +use sp_std::{cmp, result, mem, fmt::Debug, ops::BitOr, convert::Infallible, marker::PhantomData}; use codec::{Codec, Encode, Decode}; use frame_support::{ StorageValue, Parameter, decl_event, decl_storage, decl_module, decl_error, ensure, traits::{ - Currency, OnKilledAccount, OnUnbalanced, TryDrop, StoredMap, + Currency, OnKilledAccount, OnUnbalanced, TryDrop, StoredMap, Happened, WithdrawReason, WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement, Imbalance, SignedImbalance, ReservableCurrency, Get, ExistenceRequirement::KeepAlive, ExistenceRequirement::AllowDeath, IsDeadAccount, BalanceStatus as Status, @@ -237,6 +238,10 @@ decl_event!( /// Some balance was moved from the reserve of the first account to the second account. /// Final argument indicates the destination balance type. ReserveRepatriated(AccountId, AccountId, Balance, Status), + /// A new account was created. + NewAccount(AccountId), + /// An account was reaped. + KilledAccount(AccountId), } ); @@ -258,6 +263,8 @@ decl_error! { ExistingVestingSchedule, /// Beneficiary account must pre-exist DeadAccount, + /// The main account of beneficiary account must pre-exist + DeadMainAccount, } } @@ -388,11 +395,15 @@ decl_storage! { config(balances): Vec<(T::AccountId, T::Balance)>; // ^^ begin, length, amount liquid at genesis build(|config: &GenesisConfig| { - for (_, balance) in &config.balances { + for (who, balance) in &config.balances { assert!( *balance >= >::ExistentialDeposit::get(), "the balance of any account should always be more than existential deposit.", - ) + ); + assert!( + main_account_alive::(&who), + "the balance of any account's main account should always be alive.", + ); } for &(ref who, free) in config.balances.iter() { T::AccountStore::insert(who, AccountData { free, .. Default::default() }); @@ -660,14 +671,14 @@ impl, I: Instance> Module { let existed = Locks::::contains_key(who); if locks.is_empty() { Locks::::remove(who); - if existed { + if existed && T::AccountStore::is_system() { // TODO: use Locks::::hashed_key // https://github.com/paritytech/substrate/issues/4969 system::Module::::dec_ref(who); } } else { Locks::::insert(who, locks); - if !existed { + if !existed && T::AccountStore::is_system() { system::Module::::inc_ref(who); } } @@ -978,6 +989,7 @@ impl, I: Instance> Currency for Module where let ed = T::ExistentialDeposit::get(); ensure!(to_account.total() >= ed, Error::::ExistentialDeposit); + ensure!(main_account_alive::(&dest), Error::::DeadMainAccount); Self::ensure_can_withdraw( transactor, @@ -986,8 +998,12 @@ impl, I: Instance> Currency for Module where from_account.free, )?; - let allow_death = existence_requirement == ExistenceRequirement::AllowDeath; - let allow_death = allow_death && system::Module::::allow_death(transactor); + let mut allow_death = existence_requirement == ExistenceRequirement::AllowDeath; + if T::AccountStore::is_system() { + allow_death = allow_death && system::Module::::allow_death(transactor); + } else { + allow_death = allow_death && !Locks::::contains_key(transactor); + } ensure!(allow_death || from_account.free >= ed, Error::::KeepAlive); Ok(()) @@ -1062,6 +1078,7 @@ impl, I: Instance> Currency for Module where // bail if not yet created and this operation wouldn't be enough to create it. let ed = T::ExistentialDeposit::get(); ensure!(value >= ed || !is_new, Self::PositiveImbalance::zero()); + ensure!(main_account_alive::(who), Self::PositiveImbalance::zero()); // defensive only: overflow should never happen, however in case it does, then this // operation is a no-op. @@ -1243,8 +1260,10 @@ impl, I: Instance> ReservableCurrency for Module /// Implement `OnKilledAccount` to remove the local account, if using local account storage. /// /// NOTE: You probably won't need to use this! This only needs to be "wired in" to System module -/// if you're using the local balance storage. **If you're using the composite system account -/// storage (which is the default in most examples and tests) then there's no need.** +/// if you're using the local balance storage and you want to kill account from System module +/// by `suicide` call or you configure balances module with `CallKillAccount` of System module. +/// **If you're using the composite system account storage (which is the default in most examples +/// and tests) then there's no need.** impl, I: Instance> OnKilledAccount for Module { fn on_killed_account(who: &T::AccountId) { Account::::mutate_exists(who, |account| { @@ -1329,3 +1348,27 @@ impl, I: Instance> IsDeadAccount for Module wher !T::AccountStore::is_explicit(who) } } + +/// Event handler which calls on_created_account when it happens. +pub struct CallOnCreatedAccount(PhantomData<(T, I)>); +impl, I: Instance> Happened for CallOnCreatedAccount { + fn happened(who: &T::AccountId) { + // T::OnNewAccount::on_new_account(&who); + Module::::deposit_event(RawEvent::NewAccount(who.clone())); + system::Module::::inc_ref(who); + } +} + +/// Event handler which calls kill_account when it happens. +pub struct CallKillAccount(PhantomData<(T, I)>); +impl, I: Instance> Happened for CallKillAccount { + fn happened(who: &T::AccountId) { + // T::OnKilledAccount::on_killed_account(&who); + Module::::deposit_event(RawEvent::KilledAccount(who.clone())); + system::Module::::dec_ref(who); + } +} + +fn main_account_alive, I: Instance>(who: &T::AccountId) -> bool { + T::AccountStore::is_system() || as StoredMap>::is_explicit(who) +} diff --git a/frame/balances/src/tests_both.rs b/frame/balances/src/tests_both.rs new file mode 100644 index 0000000000000..7088d15afab01 --- /dev/null +++ b/frame/balances/src/tests_both.rs @@ -0,0 +1,290 @@ +// This file is part of Substrate. + +// Copyright (C) 2018-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Test utilities + +#![cfg(test)] + +use sp_runtime::{ + Perbill, + traits::IdentityLookup, + testing::Header, +}; +use sp_core::H256; +use sp_io; +use frame_support::{impl_outer_origin, impl_outer_event, parameter_types}; +use frame_support::traits::{Get, StorageMapShim}; +use frame_support::weights::Weight; +use std::cell::RefCell; +use crate::{GenesisConfig, Trait, tests::CallWithDispatchInfo}; +use frame_support::{ + assert_noop, assert_ok, + traits::{ + LockableCurrency, LockIdentifier, WithdrawReasons, + Currency, ExistenceRequirement::AllowDeath, + } +}; + +use frame_system as system; +impl_outer_origin!{ + pub enum Origin for Test {} +} + +mod balances { + pub use crate::Event; +} +mod balances0 { + pub type Event = crate::Event; +} + +impl_outer_event! { + pub enum Event for Test { + system, + balances, + balances0, + } +} + +thread_local! { + static EXISTENTIAL_DEPOSIT: RefCell = RefCell::new(0); +} + +pub struct ExistentialDeposit; +impl Get for ExistentialDeposit { + fn get() -> u64 { EXISTENTIAL_DEPOSIT.with(|v| *v.borrow()) } +} + +// Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. +#[derive(Clone, PartialEq, Eq, Debug)] +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(); +} +type AccountId = u64; +impl frame_system::Trait for Test { + type BaseCallFilter = (); + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Call = CallWithDispatchInfo; + type Hash = H256; + type Hashing = ::sp_runtime::traits::BlakeTwo256; + type AccountId = AccountId; + type Lookup = IdentityLookup; + type Header = Header; + type Event = Event; + type BlockHashCount = BlockHashCount; + type MaximumBlockWeight = MaximumBlockWeight; + type DbWeight = (); + type BlockExecutionWeight = (); + type ExtrinsicBaseWeight = (); + type MaximumExtrinsicWeight = MaximumBlockWeight; + type MaximumBlockLength = MaximumBlockLength; + type AvailableBlockRatio = AvailableBlockRatio; + type Version = (); + type ModuleToIndex = (); + type AccountData = super::AccountData; + type OnNewAccount = (); + type OnKilledAccount = (); +} +impl Trait for Test { + type Balance = u64; + type DustRemoval = (); + type Event = Event; + type ExistentialDeposit = ExistentialDeposit; + type AccountStore = system::Module; +} +impl Trait for Test { + type Balance = u64; + type DustRemoval = (); + type Event = Event; + type ExistentialDeposit = ExistentialDeposit; + type AccountStore = StorageMapShim< + super::Account, + crate::CallOnCreatedAccount, + crate::CallKillAccount, + AccountId, + super::AccountData + >; +} + +type System = system::Module; +type Balances = crate::Module; +type Balances0 = crate::Module; +const ID_1: LockIdentifier = *b"1 "; + +pub struct ExtBuilder { + existential_deposit: u64, + monied: bool, +} +impl Default for ExtBuilder { + fn default() -> Self { + Self { + existential_deposit: 1, + monied: false, + } + } +} +impl ExtBuilder { + pub fn existential_deposit(mut self, existential_deposit: u64) -> Self { + self.existential_deposit = existential_deposit; + self + } + pub fn monied(mut self, monied: bool) -> Self { + self.monied = monied; + self + } + pub fn set_associated_consts(&self) { + EXISTENTIAL_DEPOSIT.with(|v| *v.borrow_mut() = self.existential_deposit); + } + pub fn build(self) -> sp_io::TestExternalities { + self.set_associated_consts(); + let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + GenesisConfig:: { + balances: if self.monied { + vec![ + (1, 10 * self.existential_deposit), + (2, 20 * self.existential_deposit), + (3, 30 * self.existential_deposit), + (4, 40 * self.existential_deposit), + (5, 50 * self.existential_deposit), + (12, 10 * self.existential_deposit), + ] + } else { + vec![] + }, + }.assimilate_storage(&mut t).unwrap(); + + GenesisConfig:: { + balances: if self.monied { + vec![ + (1, 10 * self.existential_deposit), + (2, 20 * self.existential_deposit), + (3, 30 * self.existential_deposit), + (4, 40 * self.existential_deposit), + (12, 10 * self.existential_deposit), + ] + } else { + vec![] + }, + }.assimilate_storage(&mut t).unwrap(); + + let mut ext = sp_io::TestExternalities::new(t); + ext.execute_with(|| System::set_block_number(1)); + ext + } +} + +#[test] +fn two_instance_are_independent_to_lock() { + ExtBuilder::default().monied(true).existential_deposit(1).build().execute_with(|| { + assert_eq!(System::refs(&1), 1); + assert_eq!(Balances::free_balance(1), 10); + Balances::set_lock(ID_1, &1, 9, WithdrawReasons::all()); + assert_eq!(System::refs(&1), 2); + assert_noop!( + >::transfer(&1, &2, 5, AllowDeath), + crate::Error::::LiquidityRestrictions + ); + + assert_eq!(Balances0::locks(&1).len(), 0); + assert_ok!(Balances0::transfer(Origin::signed(1), 2, 5)); + assert_eq!(Balances0::total_balance(&1), 5); + Balances0::set_lock(ID_1, &1, 9, WithdrawReasons::all()); + Balances0::set_lock(ID_1, &1, 9, WithdrawReasons::all()); + assert_eq!(Balances0::locks(&1).len(), 1); + assert_eq!(System::refs(&1), 2); + assert_noop!( + >::transfer(&1, &2, 5, AllowDeath), + crate::Error::::LiquidityRestrictions + ); + Balances0::remove_lock(ID_1, &1); + assert_ok!(>::transfer(&1, &2, 5, AllowDeath)); + assert_eq!(System::refs(&1), 1); + assert_eq!(Balances0::free_balance(1), 0); + + Balances::remove_lock(ID_1, &1); + assert_eq!(System::refs(&1), 0); + assert_ok!(>::transfer(&1, &2, 5, AllowDeath)); + + let expect_events = vec![ + Event::balances0(crate::RawEvent::Transfer(1, 2, 5)), + Event::balances0(crate::RawEvent::KilledAccount(1)), + Event::balances0(crate::RawEvent::Transfer(1, 2, 5)), + Event::balances(crate::RawEvent::Transfer(1, 2, 5)), + ]; + assert_eq!(System::events().len(), expect_events.len()); + assert!(!System::events().iter().zip(expect_events).any(|(a, b)| a.event != b)); + }); +} + +#[test] +fn test_keep_alive_error() { + ExtBuilder::default().monied(true).existential_deposit(1).build().execute_with(|| { + assert_ok!(Balances::transfer(Origin::signed(12), 13, 5)); + assert_ok!(Balances0::transfer(Origin::signed(12), 13, 5)); + assert_eq!(System::refs(&13), 1); + assert_eq!(Balances0::locks(&1).len(), 0); + assert_eq!(Balances::total_balance(&13), 5); + assert_eq!(Balances::total_balance(&12), 5); + assert_eq!(Balances0::total_balance(&13), 5); + assert_eq!(Balances0::total_balance(&12), 5); + assert_noop!( + Balances::transfer(Origin::signed(13), 12, 5), + crate::Error::::KeepAlive, + ); + let expect_events = vec![ + Event::system(system::RawEvent::NewAccount(13)), + Event::balances(crate::RawEvent::Endowed(13, 5)), + Event::balances(crate::RawEvent::Transfer(12, 13, 5)), + Event::balances0(crate::RawEvent::NewAccount(13)), + Event::balances0(crate::RawEvent::Endowed(13, 5)), + Event::balances0(crate::RawEvent::Transfer(12, 13, 5)), + ]; + assert_eq!(System::events().len(), expect_events.len()); + assert!(!System::events().iter().zip(expect_events).any(|(a, b)| a.event != b)); + }); +} + +#[test] +fn cannot_create_account_with_dead_main_account() { + ExtBuilder::default().monied(true).existential_deposit(1).build().execute_with(|| { + assert_noop!( + Balances0::transfer(Origin::signed(1), 100, 5), + crate::Error::::DeadMainAccount, + ); + let _ = Balances0::deposit_creating(&100, 100); + assert_eq!(Balances::total_balance(&100), 0); + + assert_eq!(System::refs(&5), 0); + assert_eq!(Balances0::locks(&5).len(), 0); + assert_ok!(Balances0::transfer(Origin::signed(1), 5, 3)); + assert_eq!(System::refs(&5), 1); + assert_eq!(Balances0::locks(&5).len(), 0); + + let expect_events = vec![ + Event::balances0(crate::RawEvent::NewAccount(5)), + Event::balances0(crate::RawEvent::Endowed(5, 3)), + Event::balances0(crate::RawEvent::Transfer(1, 5, 3)), + ]; + assert_eq!(System::events().len(), expect_events.len()); + assert!(!System::events().iter().zip(expect_events).any(|(a, b)| a.event != b)); + }); +} diff --git a/frame/balances/src/tests_local.rs b/frame/balances/src/tests_local.rs index 54ab22af33c6c..d2ca884e1011f 100644 --- a/frame/balances/src/tests_local.rs +++ b/frame/balances/src/tests_local.rs @@ -94,6 +94,7 @@ impl frame_system::Trait for Test { } parameter_types! { pub const TransactionByteFee: u64 = 1; + pub const UsingSystemAccount: bool = true; } impl pallet_transaction_payment::Trait for Test { type Currency = Module; @@ -111,7 +112,8 @@ impl Trait for Test { super::Account, system::CallOnCreatedAccount, system::CallKillAccount, - u64, super::AccountData + u64, super::AccountData, + UsingSystemAccount, >; } diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index e0b2f256f0c78..0c1fa2bb5857e 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -321,6 +321,8 @@ pub trait StoredMap { fn insert(k: &K, t: T) { Self::mutate(k, |i| *i = t); } /// Remove the item or otherwise replace it with its default value; we don't care which. fn remove(k: &K); + /// Get whether the storage of System is being used. + fn is_system() -> bool { true } } /// A simple, generic one-parameter event notifier/handler. @@ -349,15 +351,17 @@ pub struct StorageMapShim< Created, Removed, K, - T ->(sp_std::marker::PhantomData<(S, Created, Removed, K, T)>); + T, + IsSystem: Get=(), +>(sp_std::marker::PhantomData<(S, Created, Removed, K, T, IsSystem)>); impl< S: StorageMap, Created: Happened, Removed: Happened, K: FullCodec, T: FullCodec, -> StoredMap for StorageMapShim { + IsSystem: Get, +> StoredMap for StorageMapShim { fn get(k: &K) -> T { S::get(k) } fn is_explicit(k: &K) -> bool { S::contains_key(k) } fn insert(k: &K, t: T) { @@ -408,6 +412,7 @@ impl< v }) } + fn is_system() -> bool { IsSystem::get() } } /// Something that can estimate at which block the next session rotation will happen. This should