diff --git a/substrate/frame/assets/src/impl_stored_map.rs b/substrate/frame/assets/src/impl_stored_map.rs index 6e91e5c132..4c1ff1a0c6 100644 --- a/substrate/frame/assets/src/impl_stored_map.rs +++ b/substrate/frame/assets/src/impl_stored_map.rs @@ -29,7 +29,7 @@ impl, I: 'static> StoredMap<(T::AssetId, T::AccountId), T::Extra> f } } - fn try_mutate_exists>( + fn try_mutate_exists>( id_who: &(T::AssetId, T::AccountId), f: impl FnOnce(&mut Option) -> Result, ) -> Result { @@ -46,11 +46,11 @@ impl, I: 'static> StoredMap<(T::AssetId, T::AccountId), T::Extra> f if let Some(ref mut account) = maybe_account { account.extra = extra; } else { - Err(StoredMapError::NoProviders)?; + Err(DispatchError::NoProviders)?; } } else { // They want to delete it. Let this pass if the item never existed anyway. - ensure!(maybe_account.is_none(), StoredMapError::ConsumerRemaining); + ensure!(maybe_account.is_none(), DispatchError::ConsumerRemaining); } Ok(r) }) diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index 2531f14f45..5c1370ed28 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -141,10 +141,7 @@ pub use types::*; use sp_std::{prelude::*, borrow::Borrow, convert::TryInto}; use sp_runtime::{ TokenError, ArithmeticError, - traits::{ - AtLeast32BitUnsigned, Zero, StaticLookup, Saturating, CheckedSub, CheckedAdd, Bounded, - StoredMapError, - } + traits::{AtLeast32BitUnsigned, Zero, StaticLookup, Saturating, CheckedSub, CheckedAdd, Bounded} }; use codec::HasCompact; use frame_support::{ensure, dispatch::{DispatchError, DispatchResult}}; diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index 6e3a3b1e2f..7a092a75b2 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/substrate/frame/balances/src/lib.rs @@ -175,7 +175,7 @@ use sp_runtime::{ RuntimeDebug, DispatchResult, DispatchError, ArithmeticError, traits::{ Zero, AtLeast32BitUnsigned, StaticLookup, CheckedAdd, CheckedSub, - MaybeSerializeDeserialize, Saturating, Bounded, StoredMapError, + MaybeSerializeDeserialize, Saturating, Bounded, }, }; use frame_system as system; @@ -830,8 +830,8 @@ impl, I: 'static> Pallet { pub fn mutate_account( who: &T::AccountId, f: impl FnOnce(&mut AccountData) -> R, - ) -> Result { - Self::try_mutate_account(who, |a, _| -> Result { Ok(f(a)) }) + ) -> Result { + Self::try_mutate_account(who, |a, _| -> Result { Ok(f(a)) }) } /// Mutate an account to some new value, or delete it entirely with `None`. Will enforce @@ -843,7 +843,7 @@ impl, I: 'static> Pallet { /// /// NOTE: LOW-LEVEL: This will not attempt to maintain total issuance. It is expected that /// the caller will do this. - fn try_mutate_account>( + fn try_mutate_account>( who: &T::AccountId, f: impl FnOnce(&mut AccountData, bool) -> Result, ) -> Result { @@ -867,7 +867,7 @@ impl, I: 'static> Pallet { /// /// NOTE: LOW-LEVEL: This will not attempt to maintain total issuance. It is expected that /// the caller will do this. - fn try_mutate_account_with_dust>( + fn try_mutate_account_with_dust>( who: &T::AccountId, f: impl FnOnce(&mut AccountData, bool) -> Result, ) -> Result<(R, DustCleaner), E> { @@ -1449,7 +1449,7 @@ impl, I: 'static> Currency for Pallet where for attempt in 0..2 { match Self::try_mutate_account(who, - |account, _is_new| -> Result<(Self::NegativeImbalance, Self::Balance), StoredMapError> { + |account, _is_new| -> Result<(Self::NegativeImbalance, Self::Balance), DispatchError> { // Best value is the most amount we can slash following liveness rules. let best_value = match attempt { // First attempt we try to slash the full amount, and see if liveness issues happen. diff --git a/substrate/frame/support/src/traits/misc.rs b/substrate/frame/support/src/traits/misc.rs index 7ec29522cb..9cab2626cd 100644 --- a/substrate/frame/support/src/traits/misc.rs +++ b/substrate/frame/support/src/traits/misc.rs @@ -17,7 +17,7 @@ //! Smaller traits used in FRAME which don't need their own file. -use sp_runtime::traits::{StoredMapError, Block as BlockT}; +use sp_runtime::{traits::Block as BlockT, DispatchError}; use sp_arithmetic::traits::AtLeast32Bit; use crate::dispatch::Parameter; @@ -157,10 +157,10 @@ pub trait OnKilledAccount { /// A simple, generic one-parameter event notifier/handler. pub trait HandleLifetime { /// An account was created. - fn created(_t: &T) -> Result<(), StoredMapError> { Ok(()) } + fn created(_t: &T) -> Result<(), DispatchError> { Ok(()) } /// An account was killed. - fn killed(_t: &T) -> Result<(), StoredMapError> { Ok(()) } + fn killed(_t: &T) -> Result<(), DispatchError> { Ok(()) } } impl HandleLifetime for () {} diff --git a/substrate/frame/support/src/traits/stored_map.rs b/substrate/frame/support/src/traits/stored_map.rs index 10964541ab..0e1660df54 100644 --- a/substrate/frame/support/src/traits/stored_map.rs +++ b/substrate/frame/support/src/traits/stored_map.rs @@ -18,7 +18,7 @@ //! Traits and associated datatypes for managing abstract stored values. use codec::FullCodec; -use sp_runtime::traits::StoredMapError; +use sp_runtime::DispatchError; use crate::storage::StorageMap; use crate::traits::misc::HandleLifetime; @@ -31,7 +31,7 @@ pub trait StoredMap { /// Maybe mutate the item only if an `Ok` value is returned from `f`. Do nothing if an `Err` is /// returned. It is removed or reset to default value if it has been mutated to `None` - fn try_mutate_exists>( + fn try_mutate_exists>( k: &K, f: impl FnOnce(&mut Option) -> Result, ) -> Result; @@ -39,7 +39,7 @@ pub trait StoredMap { // Everything past here has a default implementation. /// Mutate the item. - fn mutate(k: &K, f: impl FnOnce(&mut T) -> R) -> Result { + fn mutate(k: &K, f: impl FnOnce(&mut T) -> R) -> Result { Self::mutate_exists(k, |maybe_account| match maybe_account { Some(ref mut account) => f(account), x @ None => { @@ -57,15 +57,15 @@ pub trait StoredMap { fn mutate_exists( k: &K, f: impl FnOnce(&mut Option) -> R, - ) -> Result { - Self::try_mutate_exists(k, |x| -> Result { Ok(f(x)) }) + ) -> Result { + Self::try_mutate_exists(k, |x| -> Result { Ok(f(x)) }) } /// Set the item to something new. - fn insert(k: &K, t: T) -> Result<(), StoredMapError> { Self::mutate(k, |i| *i = t) } + fn insert(k: &K, t: T) -> Result<(), DispatchError> { 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) -> Result<(), StoredMapError> { Self::mutate_exists(k, |x| *x = None) } + fn remove(k: &K) -> Result<(), DispatchError> { Self::mutate_exists(k, |x| *x = None) } } /// A shim for placing around a storage item in order to use it as a `StoredValue`. Ideally this @@ -87,27 +87,27 @@ impl< T: FullCodec + Default, > StoredMap for StorageMapShim { fn get(k: &K) -> T { S::get(k) } - fn insert(k: &K, t: T) -> Result<(), StoredMapError> { + fn insert(k: &K, t: T) -> Result<(), DispatchError> { if !S::contains_key(&k) { L::created(k)?; } S::insert(k, t); Ok(()) } - fn remove(k: &K) -> Result<(), StoredMapError> { + fn remove(k: &K) -> Result<(), DispatchError> { if S::contains_key(&k) { L::killed(&k)?; S::remove(k); } Ok(()) } - fn mutate(k: &K, f: impl FnOnce(&mut T) -> R) -> Result { + fn mutate(k: &K, f: impl FnOnce(&mut T) -> R) -> Result { if !S::contains_key(&k) { L::created(k)?; } Ok(S::mutate(k, f)) } - fn mutate_exists(k: &K, f: impl FnOnce(&mut Option) -> R) -> Result { + fn mutate_exists(k: &K, f: impl FnOnce(&mut Option) -> R) -> Result { S::try_mutate_exists(k, |maybe_value| { let existed = maybe_value.is_some(); let r = f(maybe_value); @@ -121,7 +121,7 @@ impl< Ok(r) }) } - fn try_mutate_exists>( + fn try_mutate_exists>( k: &K, f: impl FnOnce(&mut Option) -> Result, ) -> Result { diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index e3fa45e70f..1c16514750 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -78,7 +78,7 @@ use sp_runtime::{ self, CheckEqual, AtLeast32Bit, Zero, Lookup, LookupError, SimpleBitOps, Hash, Member, MaybeDisplay, BadOrigin, MaybeSerializeDeserialize, MaybeMallocSizeOf, StaticLookup, One, Bounded, - Dispatchable, AtLeast32BitUnsigned, Saturating, StoredMapError, BlockNumberProvider, + Dispatchable, AtLeast32BitUnsigned, Saturating, BlockNumberProvider, }, }; @@ -1025,20 +1025,6 @@ pub enum DecRefStatus { Exists, } -/// Some resultant status relevant to decrementing a provider reference. -#[derive(Eq, PartialEq, RuntimeDebug)] -pub enum DecRefError { - /// Account cannot have the last provider reference removed while there is a consumer. - ConsumerRemaining, -} - -/// Some resultant status relevant to incrementing a consumer reference. -#[derive(Eq, PartialEq, RuntimeDebug)] -pub enum IncRefError { - /// Account cannot introduce a consumer while there are no providers. - NoProviders, -} - impl Pallet { pub fn account_exists(who: &T::AccountId) -> bool { Account::::contains_key(who) @@ -1085,7 +1071,7 @@ impl Pallet { /// Decrement the provider reference counter on an account. /// /// This *MUST* only be done once for every time you called `inc_providers` on `who`. - pub fn dec_providers(who: &T::AccountId) -> Result { + pub fn dec_providers(who: &T::AccountId) -> Result { Account::::try_mutate_exists(who, |maybe_account| { if let Some(mut account) = maybe_account.take() { if account.providers == 0 { @@ -1105,7 +1091,7 @@ impl Pallet { } (1, c, _) if c > 0 => { // Cannot remove last provider if there are consumers. - Err(DecRefError::ConsumerRemaining) + Err(DispatchError::ConsumerRemaining) } (x, _, _) => { // Account will continue to exist as there is either > 1 provider or @@ -1191,12 +1177,12 @@ impl Pallet { /// Increment the reference counter on an account. /// /// The account `who`'s `providers` must be non-zero or this will return an error. - pub fn inc_consumers(who: &T::AccountId) -> Result<(), IncRefError> { + pub fn inc_consumers(who: &T::AccountId) -> Result<(), DispatchError> { Account::::try_mutate(who, |a| if a.providers > 0 { a.consumers = a.consumers.saturating_add(1); Ok(()) } else { - Err(IncRefError::NoProviders) + Err(DispatchError::NoProviders) }) } @@ -1559,27 +1545,23 @@ impl Pallet { /// Event handler which registers a provider when created. pub struct Provider(PhantomData); impl HandleLifetime for Provider { - fn created(t: &T::AccountId) -> Result<(), StoredMapError> { + fn created(t: &T::AccountId) -> Result<(), DispatchError> { Pallet::::inc_providers(t); Ok(()) } - fn killed(t: &T::AccountId) -> Result<(), StoredMapError> { - Pallet::::dec_providers(t) - .map(|_| ()) - .or_else(|e| match e { - DecRefError::ConsumerRemaining => Err(StoredMapError::ConsumerRemaining), - }) + fn killed(t: &T::AccountId) -> Result<(), DispatchError> { + Pallet::::dec_providers(t).map(|_| ()) } } /// Event handler which registers a self-sufficient when created. pub struct SelfSufficient(PhantomData); impl HandleLifetime for SelfSufficient { - fn created(t: &T::AccountId) -> Result<(), StoredMapError> { + fn created(t: &T::AccountId) -> Result<(), DispatchError> { Pallet::::inc_sufficients(t); Ok(()) } - fn killed(t: &T::AccountId) -> Result<(), StoredMapError> { + fn killed(t: &T::AccountId) -> Result<(), DispatchError> { Pallet::::dec_sufficients(t); Ok(()) } @@ -1588,13 +1570,10 @@ impl HandleLifetime for SelfSufficient { /// Event handler which registers a consumer when created. pub struct Consumer(PhantomData); impl HandleLifetime for Consumer { - fn created(t: &T::AccountId) -> Result<(), StoredMapError> { + fn created(t: &T::AccountId) -> Result<(), DispatchError> { Pallet::::inc_consumers(t) - .map_err(|e| match e { - IncRefError::NoProviders => StoredMapError::NoProviders - }) } - fn killed(t: &T::AccountId) -> Result<(), StoredMapError> { + fn killed(t: &T::AccountId) -> Result<(), DispatchError> { Pallet::::dec_consumers(t); Ok(()) } @@ -1623,7 +1602,7 @@ impl StoredMap for Pallet { Account::::get(k).data } - fn try_mutate_exists>( + fn try_mutate_exists>( k: &T::AccountId, f: impl FnOnce(&mut Option) -> Result, ) -> Result { @@ -1635,10 +1614,9 @@ impl StoredMap for Pallet { if !was_providing && is_providing { Self::inc_providers(k); } else if was_providing && !is_providing { - match Self::dec_providers(k) { - Err(DecRefError::ConsumerRemaining) => Err(StoredMapError::ConsumerRemaining)?, - Ok(DecRefStatus::Reaped) => return Ok(result), - Ok(DecRefStatus::Exists) => { + match Self::dec_providers(k)? { + DecRefStatus::Reaped => return Ok(result), + DecRefStatus::Exists => { // Update value as normal... } } diff --git a/substrate/frame/system/src/tests.rs b/substrate/frame/system/src/tests.rs index df28e2c118..77d4baee88 100644 --- a/substrate/frame/system/src/tests.rs +++ b/substrate/frame/system/src/tests.rs @@ -121,18 +121,18 @@ fn sufficient_cannot_support_consumer() { assert_eq!(System::inc_sufficients(&0), IncRefStatus::Created); System::inc_account_nonce(&0); assert_eq!(System::account_nonce(&0), 1); - assert_noop!(System::inc_consumers(&0), IncRefError::NoProviders); + assert_noop!(System::inc_consumers(&0), DispatchError::NoProviders); assert_eq!(System::inc_providers(&0), IncRefStatus::Existed); assert_ok!(System::inc_consumers(&0)); - assert_noop!(System::dec_providers(&0), DecRefError::ConsumerRemaining); + assert_noop!(System::dec_providers(&0), DispatchError::ConsumerRemaining); }); } #[test] fn provider_required_to_support_consumer() { new_test_ext().execute_with(|| { - assert_noop!(System::inc_consumers(&0), IncRefError::NoProviders); + assert_noop!(System::inc_consumers(&0), DispatchError::NoProviders); assert_eq!(System::inc_providers(&0), IncRefStatus::Created); System::inc_account_nonce(&0); @@ -143,7 +143,7 @@ fn provider_required_to_support_consumer() { assert_eq!(System::account_nonce(&0), 1); assert_ok!(System::inc_consumers(&0)); - assert_noop!(System::dec_providers(&0), DecRefError::ConsumerRemaining); + assert_noop!(System::dec_providers(&0), DispatchError::ConsumerRemaining); System::dec_consumers(&0); assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Reaped); diff --git a/substrate/primitives/runtime/src/lib.rs b/substrate/primitives/runtime/src/lib.rs index 9bc23be1e9..6ad721079f 100644 --- a/substrate/primitives/runtime/src/lib.rs +++ b/substrate/primitives/runtime/src/lib.rs @@ -516,15 +516,6 @@ impl From for DispatchError { } } -impl From for DispatchError { - fn from(e: crate::traits::StoredMapError) -> Self { - match e { - crate::traits::StoredMapError::ConsumerRemaining => Self::ConsumerRemaining, - crate::traits::StoredMapError::NoProviders => Self::NoProviders, - } - } -} - /// Description of what went wrong when trying to complete an operation on a token. #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode, Debug)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] diff --git a/substrate/primitives/runtime/src/traits.rs b/substrate/primitives/runtime/src/traits.rs index f03e1be2a5..4396c97598 100644 --- a/substrate/primitives/runtime/src/traits.rs +++ b/substrate/primitives/runtime/src/traits.rs @@ -152,25 +152,6 @@ impl From for &'static str { } } -/// Error that can be returned by our impl of `StoredMap`. -#[derive(Encode, Decode, RuntimeDebug)] -pub enum StoredMapError { - /// Attempt to create map value when it is a consumer and there are no providers in place. - NoProviders, - /// Attempt to anull/remove value when it is the last provider and there is still at - /// least one consumer left. - ConsumerRemaining, -} - -impl From for &'static str { - fn from(e: StoredMapError) -> &'static str { - match e { - StoredMapError::NoProviders => "No providers", - StoredMapError::ConsumerRemaining => "Consumer remaining", - } - } -} - /// An error that indicates that a lookup failed. #[derive(Encode, Decode, RuntimeDebug)] pub struct LookupError;