Add arithmetic dispatch errors. (#8726)

* Add arithmetic dispatch errors.

* Replace custom overflow errors.

* Replace custom underflow and division by zero errors.

* Replace overflow/underflow in token error.

* Add token and arithmetic errors in dispatch error equality test.

* Trigger CI.

substrate/frame/assets/README.md

@@ -71,6 +71,7 @@ Import the Assets module and types and derive your runtime's configuration trait
 use pallet_assets as assets;
 use frame_support::{decl_module, dispatch, ensure};
 use frame_system::ensure_signed;
+use sp_runtime::ArithmeticError;
 
 pub trait Config: assets::Config { }
 
@@ -84,7 +85,7 @@ decl_module! {
 			const COUNT_AIRDROP_RECIPIENTS: u64 = 2;
 			const TOKENS_FIXED_SUPPLY: u64 = 100;
 
-			ensure!(!COUNT_AIRDROP_RECIPIENTS.is_zero(), "Divide by zero error.");
+			ensure!(!COUNT_AIRDROP_RECIPIENTS.is_zero(), ArithmeticError::DivisionByZero);
 
 			let asset_id = Self::next_asset_id();
 

substrate/frame/assets/src/functions.rs

@@ -47,7 +47,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		who: &T::AccountId,
 		d: &mut AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T, I>>,
 	) -> Result<bool, DispatchError> {
-		let accounts = d.accounts.checked_add(1).ok_or(Error::<T, I>::Overflow)?;
+		let accounts = d.accounts.checked_add(1).ok_or(ArithmeticError::Overflow)?;
 		let is_sufficient = if d.is_sufficient {
 			frame_system::Pallet::<T>::inc_sufficients(who);
 			d.sufficients += 1;
@@ -162,7 +162,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		id: T::AssetId,
 		who: &T::AccountId,
 		keep_alive: bool,
-	) -> Result<T::Balance, Error<T, I>> {
+	) -> Result<T::Balance, DispatchError> {
 		let details = Asset::<T, I>::get(id).ok_or_else(|| Error::<T, I>::Unknown)?;
 		ensure!(!details.is_frozen, Error::<T, I>::Frozen);
 
@@ -173,7 +173,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			// Frozen balance: account CANNOT be deleted
 			let required = frozen
 				.checked_add(&details.min_balance)
-				.ok_or(Error::<T, I>::Overflow)?;
+				.ok_or(ArithmeticError::Overflow)?;
 			account.balance.saturating_sub(required)
 		} else {
 			let is_provider = false;

substrate/frame/assets/src/lib.rs

@@ -140,7 +140,7 @@ pub use types::*;
 
 use sp_std::{prelude::*, borrow::Borrow};
 use sp_runtime::{
-	RuntimeDebug, TokenError, traits::{
+	RuntimeDebug, TokenError, ArithmeticError, traits::{
 		AtLeast32BitUnsigned, Zero, StaticLookup, Saturating, CheckedSub, CheckedAdd, Bounded,
 		StoredMapError,
 	}
@@ -326,8 +326,6 @@ pub mod pallet {
 		BadWitness,
 		/// Minimum balance should be non-zero.
 		MinBalanceZero,
-		/// A mint operation lead to an overflow.
-		Overflow,
 		/// No provider reference exists to allow a non-zero balance of a non-self-sufficient asset.
 		NoProvider,
 		/// Invalid metadata given.

substrate/frame/balances/src/lib.rs

@@ -171,7 +171,7 @@ use frame_support::{
 #[cfg(feature = "std")]
 use frame_support::traits::GenesisBuild;
 use sp_runtime::{
-	RuntimeDebug, DispatchResult, DispatchError,
+	RuntimeDebug, DispatchResult, DispatchError, ArithmeticError,
 	traits::{
 		Zero, AtLeast32BitUnsigned, StaticLookup, CheckedAdd, CheckedSub,
 		MaybeSerializeDeserialize, Saturating, Bounded, StoredMapError,
@@ -402,8 +402,6 @@ pub mod pallet {
 		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
@@ -909,10 +907,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 						match status {
 							Status::Free => to_account.free = to_account.free
 								.checked_add(&actual)
-								.ok_or(Error::<T, I>::Overflow)?,
+								.ok_or(ArithmeticError::Overflow)?,
 							Status::Reserved => to_account.reserved = to_account.reserved
 								.checked_add(&actual)
-								.ok_or(Error::<T, I>::Overflow)?,
+								.ok_or(ArithmeticError::Overflow)?,
 						}
 						from_account.reserved -= actual;
 						Ok(actual)
@@ -1332,7 +1330,7 @@ impl<T: Config<I>, I: 'static> Currency<T::AccountId> for Pallet<T, I> where
 
 						// NOTE: total stake being stored in the same type means that this could never overflow
 						// but better to be safe than sorry.
-						to_account.free = to_account.free.checked_add(&value).ok_or(Error::<T, I>::Overflow)?;
+						to_account.free = to_account.free.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
 
 						let ed = T::ExistentialDeposit::get();
 						ensure!(to_account.total() >= ed, Error::<T, I>::ExistentialDeposit);
@@ -1431,7 +1429,7 @@ impl<T: Config<I>, I: 'static> Currency<T::AccountId> for Pallet<T, I> where
 
 		Self::try_mutate_account(who, |account, is_new| -> Result<Self::PositiveImbalance, DispatchError> {
 			ensure!(!is_new, Error::<T, I>::DeadAccount);
-			account.free = account.free.checked_add(&value).ok_or(Error::<T, I>::Overflow)?;
+			account.free = account.free.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
 			Ok(PositiveImbalance::new(value))
 		})
 	}
@@ -1554,7 +1552,7 @@ impl<T: Config<I>, I: 'static> ReservableCurrency<T::AccountId> for Pallet<T, I>
 
 		Self::try_mutate_account(who, |account, _| -> DispatchResult {
 			account.free = account.free.checked_sub(&value).ok_or(Error::<T, I>::InsufficientBalance)?;
-			account.reserved = account.reserved.checked_add(&value).ok_or(Error::<T, I>::Overflow)?;
+			account.reserved = account.reserved.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
 			Self::ensure_can_withdraw(&who, value.clone(), WithdrawReasons::RESERVE, account.free)
 		})?;
 

substrate/frame/balances/src/tests.rs

@@ -24,7 +24,7 @@ macro_rules! decl_tests {
 	($test:ty, $ext_builder:ty, $existential_deposit:expr) => {
 
 		use crate::*;
-		use sp_runtime::{FixedPointNumber, traits::{SignedExtension, BadOrigin}};
+		use sp_runtime::{ArithmeticError, FixedPointNumber, traits::{SignedExtension, BadOrigin}};
 		use frame_support::{
 			assert_noop, assert_storage_noop, assert_ok, assert_err, StorageValue,
 			traits::{
@@ -523,7 +523,7 @@ macro_rules! decl_tests {
 
 				assert_err!(
 					Balances::transfer(Some(1).into(), 2, u64::max_value()),
-					Error::<$test, _>::Overflow,
+					ArithmeticError::Overflow,
 				);
 
 				assert_eq!(Balances::free_balance(1), u64::max_value());

substrate/frame/democracy/src/lib.rs

@@ -154,7 +154,7 @@
 
 use sp_std::prelude::*;
 use sp_runtime::{
-	DispatchResult, DispatchError, RuntimeDebug,
+	DispatchResult, DispatchError, ArithmeticError, RuntimeDebug,
 	traits::{Zero, Hash, Dispatchable, Saturating, Bounded},
 };
 use codec::{Encode, Decode, Input};
@@ -510,10 +510,6 @@ decl_error! {
 		NoPermission,
 		/// The account is already delegating.
 		AlreadyDelegating,
-		/// An unexpected integer overflow occurred.
-		Overflow,
-		/// An unexpected integer underflow occurred.
-		Underflow,
 		/// Too high a balance was provided that the account cannot afford.
 		InsufficientFunds,
 		/// The account is not currently delegating.
@@ -1252,7 +1248,7 @@ impl<T: Config> Module<T> {
 				match votes.binary_search_by_key(&ref_index, |i| i.0) {
 					Ok(i) => {
 						// Shouldn't be possible to fail, but we handle it gracefully.
-						status.tally.remove(votes[i].1).ok_or(Error::<T>::Underflow)?;
+						status.tally.remove(votes[i].1).ok_or(ArithmeticError::Underflow)?;
 						if let Some(approve) = votes[i].1.as_standard() {
 							status.tally.reduce(approve, *delegations);
 						}
@@ -1264,7 +1260,7 @@ impl<T: Config> Module<T> {
 					}
 				}
 				// Shouldn't be possible to fail, but we handle it gracefully.
-				status.tally.add(vote).ok_or(Error::<T>::Overflow)?;
+				status.tally.add(vote).ok_or(ArithmeticError::Overflow)?;
 				if let Some(approve) = vote.as_standard() {
 					status.tally.increase(approve, *delegations);
 				}
@@ -1300,7 +1296,7 @@ impl<T: Config> Module<T> {
 					Some(ReferendumInfo::Ongoing(mut status)) => {
 						ensure!(matches!(scope, UnvoteScope::Any), Error::<T>::NoPermission);
 						// Shouldn't be possible to fail, but we handle it gracefully.
-						status.tally.remove(votes[i].1).ok_or(Error::<T>::Underflow)?;
+						status.tally.remove(votes[i].1).ok_or(ArithmeticError::Underflow)?;
 						if let Some(approve) = votes[i].1.as_standard() {
 							status.tally.reduce(approve, *delegations);
 						}

substrate/frame/lottery/src/lib.rs

@@ -56,7 +56,7 @@ pub mod weights;
 
 use sp_std::prelude::*;
 use sp_runtime::{
-	DispatchError,
+	DispatchError, ArithmeticError,
 	traits::{AccountIdConversion, Saturating, Zero},
 };
 use frame_support::{
@@ -188,8 +188,6 @@ decl_event!(
 
 decl_error! {
 	pub enum Error for Module<T: Config> {
-		/// An overflow has occurred.
-		Overflow,
 		/// A lottery has not been configured.
 		NotConfigured,
 		/// A lottery is already in progress.
@@ -278,7 +276,7 @@ decl_module! {
 			Lottery::<T>::try_mutate(|lottery| -> DispatchResult {
 				ensure!(lottery.is_none(), Error::<T>::InProgress);
 				let index = LotteryIndex::get();
-				let new_index = index.checked_add(1).ok_or(Error::<T>::Overflow)?;
+				let new_index = index.checked_add(1).ok_or(ArithmeticError::Overflow)?;
 				let start = frame_system::Pallet::<T>::block_number();
 				// Use new_index to more easily track everything with the current state.
 				*lottery = Some(LotteryConfig {
@@ -400,7 +398,7 @@ impl<T: Config> Module<T> {
 		ensure!(T::ValidateCall::validate_call(call), Error::<T>::InvalidCall);
 		let call_index = Self::call_to_index(call)?;
 		let ticket_count = TicketsCount::get();
-		let new_ticket_count = ticket_count.checked_add(1).ok_or(Error::<T>::Overflow)?;
+		let new_ticket_count = ticket_count.checked_add(1).ok_or(ArithmeticError::Overflow)?;
 		// Try to update the participant status
 		Participants::<T>::try_mutate(&caller, |(lottery_index, participating_calls)| -> DispatchResult {
 			let index = LotteryIndex::get();

substrate/frame/recovery/src/lib.rs

@@ -154,7 +154,7 @@
 use sp_std::prelude::*;
 use sp_runtime::{
 	traits::{Dispatchable, SaturatedConversion, CheckedAdd, CheckedMul},
-	DispatchResult
+	DispatchResult, ArithmeticError,
 };
 use codec::{Encode, Decode};
 
@@ -313,8 +313,6 @@ decl_error! {
 		Threshold,
 		/// There are still active recovery attempts that need to be closed
 		StillActive,
-		/// There was an overflow in a calculation
-		Overflow,
 		/// This account is already set up for recovery
 		AlreadyProxy,
 		/// Some internal state is broken.
@@ -443,10 +441,10 @@ decl_module! {
 			// Total deposit is base fee + number of friends * factor fee
 			let friend_deposit = T::FriendDepositFactor::get()
 				.checked_mul(&friends.len().saturated_into())
-				.ok_or(Error::<T>::Overflow)?;
+				.ok_or(ArithmeticError::Overflow)?;
 			let total_deposit = T::ConfigDepositBase::get()
 				.checked_add(&friend_deposit)
-				.ok_or(Error::<T>::Overflow)?;
+				.ok_or(ArithmeticError::Overflow)?;
 			// Reserve the deposit
 			T::Currency::reserve(&who, total_deposit)?;
 			// Create the recovery configuration
@@ -581,7 +579,7 @@ decl_module! {
 			let current_block_number = <system::Pallet<T>>::block_number();
 			let recoverable_block_number = active_recovery.created
 				.checked_add(&recovery_config.delay_period)
-				.ok_or(Error::<T>::Overflow)?;
+				.ok_or(ArithmeticError::Overflow)?;
 			ensure!(recoverable_block_number <= current_block_number, Error::<T>::DelayPeriod);
 			// Make sure the threshold is met
 			ensure!(

substrate/frame/support/src/traits/tokens/fungible/balanced.rs

@@ -20,7 +20,7 @@
 
 use super::*;
 use sp_std::marker::PhantomData;
-use sp_runtime::{TokenError, traits::{CheckedAdd, Zero}};
+use sp_runtime::{TokenError, ArithmeticError, traits::{CheckedAdd, Zero}};
 use super::super::Imbalance as ImbalanceT;
 use crate::traits::misc::{SameOrOther, TryDrop};
 use crate::dispatch::{DispatchResult, DispatchError};
@@ -221,7 +221,7 @@ pub trait Unbalanced<AccountId>: Inspect<AccountId> {
 		-> Result<Self::Balance, DispatchError>
 	{
 		let old_balance = Self::balance(who);
-		let new_balance = old_balance.checked_add(&amount).ok_or(TokenError::Overflow)?;
+		let new_balance = old_balance.checked_add(&amount).ok_or(ArithmeticError::Overflow)?;
 		if new_balance < Self::minimum_balance() {
 			Err(TokenError::BelowMinimum)?
 		}

substrate/frame/support/src/traits/tokens/fungibles/balanced.rs

@@ -20,7 +20,7 @@
 
 use super::*;
 use sp_std::marker::PhantomData;
-use sp_runtime::{TokenError, traits::{Zero, CheckedAdd}};
+use sp_runtime::{ArithmeticError, TokenError, traits::{Zero, CheckedAdd}};
 use sp_arithmetic::traits::Saturating;
 use crate::dispatch::{DispatchError, DispatchResult};
 use crate::traits::misc::{SameOrOther, TryDrop};
@@ -236,7 +236,7 @@ pub trait Unbalanced<AccountId>: Inspect<AccountId> {
 		-> Result<Self::Balance, DispatchError>
 	{
 		let old_balance = Self::balance(asset, who);
-		let new_balance = old_balance.checked_add(&amount).ok_or(TokenError::Overflow)?;
+		let new_balance = old_balance.checked_add(&amount).ok_or(ArithmeticError::Overflow)?;
 		if new_balance < Self::minimum_balance(asset) {
 			Err(TokenError::BelowMinimum)?
 		}

substrate/frame/support/src/traits/tokens/misc.rs

@@ -20,7 +20,7 @@
 use codec::{Encode, Decode, FullCodec};
 use sp_core::RuntimeDebug;
 use sp_arithmetic::traits::{Zero, AtLeast32BitUnsigned};
-use sp_runtime::TokenError;
+use sp_runtime::{DispatchError, ArithmeticError, TokenError};
 
 /// One of a number of consequences of withdrawing a fungible from an account.
 #[derive(Copy, Clone, Eq, PartialEq)]
@@ -50,17 +50,17 @@ pub enum WithdrawConsequence<Balance> {
 }
 
 impl<Balance: Zero> WithdrawConsequence<Balance> {
-	/// Convert the type into a `Result` with `TokenError` as the error or the additional `Balance`
+	/// Convert the type into a `Result` with `DispatchError` as the error or the additional `Balance`
 	/// by which the account will be reduced.
-	pub fn into_result(self) -> Result<Balance, TokenError> {
+	pub fn into_result(self) -> Result<Balance, DispatchError> {
 		use WithdrawConsequence::*;
 		match self {
-			NoFunds => Err(TokenError::NoFunds),
-			WouldDie => Err(TokenError::WouldDie),
-			UnknownAsset => Err(TokenError::UnknownAsset),
-			Underflow => Err(TokenError::Underflow),
-			Overflow => Err(TokenError::Overflow),
-			Frozen => Err(TokenError::Frozen),
+			NoFunds => Err(TokenError::NoFunds.into()),
+			WouldDie => Err(TokenError::WouldDie.into()),
+			UnknownAsset => Err(TokenError::UnknownAsset.into()),
+			Underflow => Err(ArithmeticError::Underflow.into()),
+			Overflow => Err(ArithmeticError::Overflow.into()),
+			Frozen => Err(TokenError::Frozen.into()),
 			ReducedToZero(result) => Ok(result),
 			Success => Ok(Zero::zero()),
 		}
@@ -90,13 +90,13 @@ pub enum DepositConsequence {
 
 impl DepositConsequence {
 	/// Convert the type into a `Result` with `TokenError` as the error.
-	pub fn into_result(self) -> Result<(), TokenError> {
+	pub fn into_result(self) -> Result<(), DispatchError> {
 		use DepositConsequence::*;
 		Err(match self {
-			BelowMinimum => TokenError::BelowMinimum,
-			CannotCreate => TokenError::CannotCreate,
-			UnknownAsset => TokenError::UnknownAsset,
-			Overflow => TokenError::Overflow,
+			BelowMinimum => TokenError::BelowMinimum.into(),
+			CannotCreate => TokenError::CannotCreate.into(),
+			UnknownAsset => TokenError::UnknownAsset.into(),
+			Overflow => ArithmeticError::Overflow.into(),
 			Success => return Ok(()),
 		})
 	}

substrate/primitives/runtime/src/lib.rs

@@ -468,6 +468,8 @@ pub enum DispatchError {
 	NoProviders,
 	/// An error to do with tokens.
 	Token(TokenError),
+	/// An arithmetic error.
+	Arithmetic(ArithmeticError),
 }
 
 /// Result of a `Dispatchable` which contains the `DispatchResult` and additional information about
@@ -542,10 +544,6 @@ pub enum TokenError {
 	UnknownAsset,
 	/// Funds exist but are frozen.
 	Frozen,
-	/// An underflow would occur.
-	Underflow,
-	/// An overflow would occur.
-	Overflow,
 }
 
 impl From<TokenError> for &'static str {
@@ -557,8 +555,6 @@ impl From<TokenError> for &'static str {
 			TokenError::CannotCreate => "Account cannot be created",
 			TokenError::UnknownAsset => "The asset in question is unknown",
 			TokenError::Frozen => "Funds exist but are frozen",
-			TokenError::Underflow => "An underflow would occur",
-			TokenError::Overflow => "An overflow would occur",
 		}
 	}
 }
@@ -569,6 +565,34 @@ impl From<TokenError> for DispatchError {
 	}
 }
 
+/// Arithmetic errors.
+#[derive(Eq, PartialEq, Clone, Copy, Encode, Decode, Debug)]
+#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
+pub enum ArithmeticError {
+	/// Underflow.
+	Underflow,
+	/// Overflow.
+	Overflow,
+	/// Division by zero.
+	DivisionByZero,
+}
+
+impl From<ArithmeticError> for &'static str {
+	fn from(e: ArithmeticError) -> &'static str {
+		match e {
+			ArithmeticError::Underflow => "An underflow would occur",
+			ArithmeticError::Overflow => "An overflow would occur",
+			ArithmeticError::DivisionByZero => "Division by zero",
+		}
+	}
+}
+
+impl From<ArithmeticError> for DispatchError {
+	fn from(e: ArithmeticError) -> DispatchError {
+		Self::Arithmetic(e)
+	}
+}
+
 impl From<&'static str> for DispatchError {
 	fn from(err: &'static str) -> DispatchError {
 		Self::Other(err)
@@ -585,6 +609,7 @@ impl From<DispatchError> for &'static str {
 			DispatchError::ConsumerRemaining => "Consumer remaining",
 			DispatchError::NoProviders => "No providers",
 			DispatchError::Token(e) => e.into(),
+			DispatchError::Arithmetic(e) => e.into(),
 		}
 	}
 }
@@ -616,6 +641,10 @@ impl traits::Printable for DispatchError {
 			Self::Token(e) => {
 				"Token error: ".print();
 				<&'static str>::from(*e).print();
+			},
+			Self::Arithmetic(e) => {
+				"Arithmetic error: ".print();
+				<&'static str>::from(*e).print();
 			}
 		}
 	}
@@ -643,6 +672,7 @@ impl PartialEq for DispatchError {
 
 			(Token(l), Token(r)) => l == r,
 			(Other(l), Other(r)) => l == r,
+			(Arithmetic(l), Arithmetic(r)) => l == r,
 
 			(
 				Module { index: index_l, error: error_l, .. },
@@ -903,6 +933,15 @@ mod tests {
 			Module { index: 2, error: 1, message: None },
 			ConsumerRemaining,
 			NoProviders,
+			Token(TokenError::NoFunds),
+			Token(TokenError::WouldDie),
+			Token(TokenError::BelowMinimum),
+			Token(TokenError::CannotCreate),
+			Token(TokenError::UnknownAsset),
+			Token(TokenError::Frozen),
+			Arithmetic(ArithmeticError::Overflow),
+			Arithmetic(ArithmeticError::Underflow),
+			Arithmetic(ArithmeticError::DivisionByZero),
 		];
 		for (i, variant) in variants.iter().enumerate() {
 			for (j, other_variant) in variants.iter().enumerate() {