diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index 4fcda02c4f..10451aca15 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/substrate/frame/balances/src/lib.rs @@ -982,7 +982,7 @@ impl, I: Instance> Currency for Module where /// Slash a target account `who`, 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. + /// Is a no-op if `value` to be slashed is zero or the account does not exist. /// /// NOTE: `slash()` prefers free balance, but assumes that reserve balance can be drawn /// from in extreme circumstances. `can_slash()` should be used prior to `slash()` to avoid having @@ -993,6 +993,7 @@ impl, I: Instance> Currency for Module where value: Self::Balance ) -> (Self::NegativeImbalance, Self::Balance) { if value.is_zero() { return (NegativeImbalance::zero(), Zero::zero()) } + if Self::is_dead_account(&who) { return (NegativeImbalance::zero(), value) } Self::mutate_account(who, |account| { let free_slash = cmp::min(account.free, value); @@ -1146,9 +1147,10 @@ impl, I: Instance> ReservableCurrency for Module Self::Balance { if value.is_zero() { return Zero::zero() } + if Self::is_dead_account(&who) { return value } let actual = Self::mutate_account(who, |account| { let actual = cmp::min(account.reserved, value); @@ -1166,12 +1168,13 @@ impl, I: Instance> ReservableCurrency for Module (Self::NegativeImbalance, Self::Balance) { if value.is_zero() { return (NegativeImbalance::zero(), Zero::zero()) } + if Self::is_dead_account(&who) { return (NegativeImbalance::zero(), value) } Self::mutate_account(who, |account| { // underflow should never happen, but it if does, there's nothing to be done here. diff --git a/substrate/frame/balances/src/tests.rs b/substrate/frame/balances/src/tests.rs index 728bf036bb..1c120272dd 100644 --- a/substrate/frame/balances/src/tests.rs +++ b/substrate/frame/balances/src/tests.rs @@ -40,7 +40,7 @@ macro_rules! decl_tests { use crate::*; use sp_runtime::{FixedPointNumber, traits::{SignedExtension, BadOrigin}}; use frame_support::{ - assert_noop, assert_ok, assert_err, + assert_noop, assert_storage_noop, assert_ok, assert_err, traits::{ LockableCurrency, LockIdentifier, WithdrawReasons, Currency, ReservableCurrency, ExistenceRequirement::AllowDeath, StoredMap @@ -796,5 +796,28 @@ macro_rules! decl_tests { ); }); } + + #[test] + fn operations_on_dead_account_should_not_change_state() { + // These functions all use `mutate_account` which may introduce a storage change when + // the account never existed to begin with, and shouldn't exist in the end. + <$ext_builder>::default() + .existential_deposit(0) + .build() + .execute_with(|| { + assert!(!::AccountStore::is_explicit(&1337)); + + // Unreserve + assert_storage_noop!(assert_eq!(Balances::unreserve(&1337, 42), 42)); + // Reserve + assert_noop!(Balances::reserve(&1337, 42), Error::::InsufficientBalance); + // Slash Reserve + assert_storage_noop!(assert_eq!(Balances::slash_reserved(&1337, 42).1, 42)); + // Repatriate Reserve + assert_noop!(Balances::repatriate_reserved(&1337, &1338, 42, Status::Free), Error::::DeadAccount); + // Slash + assert_storage_noop!(assert_eq!(Balances::slash(&1337, 42).1, 42)); + }); + } } } diff --git a/substrate/frame/balances/src/weights.rs b/substrate/frame/balances/src/weights.rs index 1f7e1bec08..2b69c9c11d 100644 --- a/substrate/frame/balances/src/weights.rs +++ b/substrate/frame/balances/src/weights.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2020-2021 Parity Technologies (UK) Ltd. +// Copyright (C) 2021 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -15,9 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Weights for pallet_balances +//! Autogenerated weights for pallet_balances +//! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0 -//! DATE: 2020-10-27, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] +//! DATE: 2021-01-06, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: @@ -48,76 +49,63 @@ pub trait WeightInfo { fn set_balance_creating() -> Weight; fn set_balance_killing() -> Weight; fn force_transfer() -> Weight; - } /// Weights for pallet_balances using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { fn transfer() -> Weight { - (94_088_000 as Weight) + (100_698_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } fn transfer_keep_alive() -> Weight { - (64_828_000 as Weight) + (69_407_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } fn set_balance_creating() -> Weight { - (36_151_000 as Weight) + (38_489_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } fn set_balance_killing() -> Weight { - (45_505_000 as Weight) + (48_458_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) - } fn force_transfer() -> Weight { - (92_986_000 as Weight) + (99_320_000 as Weight) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) - } - } // For backwards compatibility and tests impl WeightInfo for () { fn transfer() -> Weight { - (94_088_000 as Weight) + (100_698_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } fn transfer_keep_alive() -> Weight { - (64_828_000 as Weight) + (69_407_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } fn set_balance_creating() -> Weight { - (36_151_000 as Weight) + (38_489_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } fn set_balance_killing() -> Weight { - (45_505_000 as Weight) + (48_458_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) - } fn force_transfer() -> Weight { - (92_986_000 as Weight) + (99_320_000 as Weight) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) - } - } diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index adea790a3f..1127afa9e8 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -499,6 +499,21 @@ macro_rules! assert_noop { } } +/// Evaluate any expression and assert that runtime storage has not been mutated +/// (i.e. expression is a storage no-operation). +/// +/// Used as `assert_storage_noop(expression_to_assert)`. +#[macro_export] +macro_rules! assert_storage_noop { + ( + $x:expr + ) => { + let h = $crate::storage_root(); + $x; + assert_eq!(h, $crate::storage_root()); + } +} + /// Assert an expression returns an error specified. /// /// Used as `assert_err!(expression_to_assert, expected_error_expression)`