Remove IncRefError, DecRefError and StoredMapError (paritytech#…

…9384)

All of them are a subset of `DispatchError` anyway, no need to have
special errors IMHO.

frame/assets/src/impl_stored_map.rs

@@ -29,7 +29,7 @@ impl<T: Config<I>, I: 'static> StoredMap<(T::AssetId, T::AccountId), T::Extra> f
 		}
 	}
 
-	fn try_mutate_exists<R, E: From<StoredMapError>>(
+	fn try_mutate_exists<R, E: From<DispatchError>>(
 		id_who: &(T::AssetId, T::AccountId),
 		f: impl FnOnce(&mut Option<T::Extra>) -> Result<R, E>,
 	) -> Result<R, E> {
@@ -46,11 +46,11 @@ impl<T: Config<I>, 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)
 		})

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}};

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<T: Config<I>, I: 'static> Pallet<T, I> {
 	pub fn mutate_account<R>(
 		who: &T::AccountId,
 		f: impl FnOnce(&mut AccountData<T::Balance>) -> R,
-	) -> Result<R, StoredMapError> {
-		Self::try_mutate_account(who, |a, _| -> Result<R, StoredMapError> { Ok(f(a)) })
+	) -> Result<R, DispatchError> {
+		Self::try_mutate_account(who, |a, _| -> Result<R, DispatchError> { Ok(f(a)) })
 	}
 
 	/// Mutate an account to some new value, or delete it entirely with `None`. Will enforce
@@ -843,7 +843,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	///
 	/// NOTE: LOW-LEVEL: This will not attempt to maintain total issuance. It is expected that
 	/// the caller will do this.
-	fn try_mutate_account<R, E: From<StoredMapError>>(
+	fn try_mutate_account<R, E: From<DispatchError>>(
 		who: &T::AccountId,
 		f: impl FnOnce(&mut AccountData<T::Balance>, bool) -> Result<R, E>,
 	) -> Result<R, E> {
@@ -867,7 +867,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	///
 	/// 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<R, E: From<StoredMapError>>(
+	fn try_mutate_account_with_dust<R, E: From<DispatchError>>(
 		who: &T::AccountId,
 		f: impl FnOnce(&mut AccountData<T::Balance>, bool) -> Result<R, E>,
 	) -> Result<(R, DustCleaner<T, I>), E> {
@@ -1449,7 +1449,7 @@ impl<T: Config<I>, I: 'static> Currency<T::AccountId> for Pallet<T, I> 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.

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<AccountId> {
 /// A simple, generic one-parameter event notifier/handler.
 pub trait HandleLifetime<T> {
 	/// 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<T> HandleLifetime<T> for () {}

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,15 +31,15 @@ pub trait StoredMap<K, T: Default> {
 
 	/// 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<R, E: From<StoredMapError>>(
+	fn try_mutate_exists<R, E: From<DispatchError>>(
 		k: &K,
 		f: impl FnOnce(&mut Option<T>) -> Result<R, E>,
 	) -> Result<R, E>;
 
 	// Everything past here has a default implementation.
 
 	/// Mutate the item.
-	fn mutate<R>(k: &K, f: impl FnOnce(&mut T) -> R) -> Result<R, StoredMapError> {
+	fn mutate<R>(k: &K, f: impl FnOnce(&mut T) -> R) -> Result<R, DispatchError> {
 		Self::mutate_exists(k, |maybe_account| match maybe_account {
 			Some(ref mut account) => f(account),
 			x @ None => {
@@ -57,15 +57,15 @@ pub trait StoredMap<K, T: Default> {
 	fn mutate_exists<R>(
 		k: &K,
 		f: impl FnOnce(&mut Option<T>) -> R,
-	) -> Result<R, StoredMapError> {
-		Self::try_mutate_exists(k, |x| -> Result<R, StoredMapError> { Ok(f(x)) })
+	) -> Result<R, DispatchError> {
+		Self::try_mutate_exists(k, |x| -> Result<R, DispatchError> { 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<K, T> for StorageMapShim<S, L, K, T> {
 	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<R>(k: &K, f: impl FnOnce(&mut T) -> R) -> Result<R, StoredMapError> {
+	fn mutate<R>(k: &K, f: impl FnOnce(&mut T) -> R) -> Result<R, DispatchError> {
 		if !S::contains_key(&k) {
 			L::created(k)?;
 		}
 		Ok(S::mutate(k, f))
 	}
-	fn mutate_exists<R>(k: &K, f: impl FnOnce(&mut Option<T>) -> R) -> Result<R, StoredMapError> {
+	fn mutate_exists<R>(k: &K, f: impl FnOnce(&mut Option<T>) -> R) -> Result<R, DispatchError> {
 		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<R, E: From<StoredMapError>>(
+	fn try_mutate_exists<R, E: From<DispatchError>>(
 		k: &K,
 		f: impl FnOnce(&mut Option<T>) -> Result<R, E>,
 	) -> Result<R, E> {

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<T: Config> Pallet<T> {
 	pub fn account_exists(who: &T::AccountId) -> bool {
 		Account::<T>::contains_key(who)
@@ -1085,7 +1071,7 @@ impl<T: Config> Pallet<T> {
 	/// 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<DecRefStatus, DecRefError> {
+	pub fn dec_providers(who: &T::AccountId) -> Result<DecRefStatus, DispatchError> {
 		Account::<T>::try_mutate_exists(who, |maybe_account| {
 			if let Some(mut account) = maybe_account.take() {
 				if account.providers == 0 {
@@ -1105,7 +1091,7 @@ impl<T: Config> Pallet<T> {
 					}
 					(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<T: Config> Pallet<T> {
 	/// 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::<T>::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<T: Config> Pallet<T> {
 /// Event handler which registers a provider when created.
 pub struct Provider<T>(PhantomData<T>);
 impl<T: Config> HandleLifetime<T::AccountId> for Provider<T> {
-	fn created(t: &T::AccountId) -> Result<(), StoredMapError> {
+	fn created(t: &T::AccountId) -> Result<(), DispatchError> {
 		Pallet::<T>::inc_providers(t);
 		Ok(())
 	}
-	fn killed(t: &T::AccountId) -> Result<(), StoredMapError> {
-		Pallet::<T>::dec_providers(t)
-			.map(|_| ())
-			.or_else(|e| match e {
-				DecRefError::ConsumerRemaining => Err(StoredMapError::ConsumerRemaining),
-			})
+	fn killed(t: &T::AccountId) -> Result<(), DispatchError> {
+		Pallet::<T>::dec_providers(t).map(|_| ())
 	}
 }
 
 /// Event handler which registers a self-sufficient when created.
 pub struct SelfSufficient<T>(PhantomData<T>);
 impl<T: Config> HandleLifetime<T::AccountId> for SelfSufficient<T> {
-	fn created(t: &T::AccountId) -> Result<(), StoredMapError> {
+	fn created(t: &T::AccountId) -> Result<(), DispatchError> {
 		Pallet::<T>::inc_sufficients(t);
 		Ok(())
 	}
-	fn killed(t: &T::AccountId) -> Result<(), StoredMapError> {
+	fn killed(t: &T::AccountId) -> Result<(), DispatchError> {
 		Pallet::<T>::dec_sufficients(t);
 		Ok(())
 	}
@@ -1588,13 +1570,10 @@ impl<T: Config> HandleLifetime<T::AccountId> for SelfSufficient<T> {
 /// Event handler which registers a consumer when created.
 pub struct Consumer<T>(PhantomData<T>);
 impl<T: Config> HandleLifetime<T::AccountId> for Consumer<T> {
-	fn created(t: &T::AccountId) -> Result<(), StoredMapError> {
+	fn created(t: &T::AccountId) -> Result<(), DispatchError> {
 		Pallet::<T>::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::<T>::dec_consumers(t);
 		Ok(())
 	}
@@ -1623,7 +1602,7 @@ impl<T: Config> StoredMap<T::AccountId, T::AccountData> for Pallet<T> {
 		Account::<T>::get(k).data
 	}
 
-	fn try_mutate_exists<R, E: From<StoredMapError>>(
+	fn try_mutate_exists<R, E: From<DispatchError>>(
 		k: &T::AccountId,
 		f: impl FnOnce(&mut Option<T::AccountData>) -> Result<R, E>,
 	) -> Result<R, E> {
@@ -1635,10 +1614,9 @@ impl<T: Config> StoredMap<T::AccountId, T::AccountData> for Pallet<T> {
 		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...
 				}
 			}

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);

primitives/runtime/src/lib.rs

@@ -516,15 +516,6 @@ impl From<crate::traits::BadOrigin> for DispatchError {
 	}
 }
 
-impl From<crate::traits::StoredMapError> 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))]

primitives/runtime/src/traits.rs

@@ -152,25 +152,6 @@ impl From<BadOrigin> 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<StoredMapError> 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;