diff --git a/substrate/frame/assets/README.md b/substrate/frame/assets/README.md index 44c4eedc31..f8583a5c91 100644 --- a/substrate/frame/assets/README.md +++ b/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(); diff --git a/substrate/frame/assets/src/functions.rs b/substrate/frame/assets/src/functions.rs index 13c92f781b..c6b5391cff 100644 --- a/substrate/frame/assets/src/functions.rs +++ b/substrate/frame/assets/src/functions.rs @@ -47,7 +47,7 @@ impl, I: 'static> Pallet { who: &T::AccountId, d: &mut AssetDetails>, ) -> Result { - let accounts = d.accounts.checked_add(1).ok_or(Error::::Overflow)?; + let accounts = d.accounts.checked_add(1).ok_or(ArithmeticError::Overflow)?; let is_sufficient = if d.is_sufficient { frame_system::Pallet::::inc_sufficients(who); d.sufficients += 1; @@ -162,7 +162,7 @@ impl, I: 'static> Pallet { id: T::AssetId, who: &T::AccountId, keep_alive: bool, - ) -> Result> { + ) -> Result { let details = Asset::::get(id).ok_or_else(|| Error::::Unknown)?; ensure!(!details.is_frozen, Error::::Frozen); @@ -173,7 +173,7 @@ impl, I: 'static> Pallet { // Frozen balance: account CANNOT be deleted let required = frozen .checked_add(&details.min_balance) - .ok_or(Error::::Overflow)?; + .ok_or(ArithmeticError::Overflow)?; account.balance.saturating_sub(required) } else { let is_provider = false; diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index e8dfd50f40..3a2b1a6ce2 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/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. diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index 35841c504a..0bfe43623c 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/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, I: 'static> Pallet { match status { Status::Free => to_account.free = to_account.free .checked_add(&actual) - .ok_or(Error::::Overflow)?, + .ok_or(ArithmeticError::Overflow)?, Status::Reserved => to_account.reserved = to_account.reserved .checked_add(&actual) - .ok_or(Error::::Overflow)?, + .ok_or(ArithmeticError::Overflow)?, } from_account.reserved -= actual; Ok(actual) @@ -1332,7 +1330,7 @@ impl, I: 'static> Currency for Pallet 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::::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::::ExistentialDeposit); @@ -1431,7 +1429,7 @@ impl, I: 'static> Currency for Pallet where Self::try_mutate_account(who, |account, is_new| -> Result { ensure!(!is_new, Error::::DeadAccount); - account.free = account.free.checked_add(&value).ok_or(Error::::Overflow)?; + account.free = account.free.checked_add(&value).ok_or(ArithmeticError::Overflow)?; Ok(PositiveImbalance::new(value)) }) } @@ -1554,7 +1552,7 @@ impl, I: 'static> ReservableCurrency for Pallet Self::try_mutate_account(who, |account, _| -> DispatchResult { account.free = account.free.checked_sub(&value).ok_or(Error::::InsufficientBalance)?; - account.reserved = account.reserved.checked_add(&value).ok_or(Error::::Overflow)?; + account.reserved = account.reserved.checked_add(&value).ok_or(ArithmeticError::Overflow)?; Self::ensure_can_withdraw(&who, value.clone(), WithdrawReasons::RESERVE, account.free) })?; diff --git a/substrate/frame/balances/src/tests.rs b/substrate/frame/balances/src/tests.rs index de12c39ede..39de133990 100644 --- a/substrate/frame/balances/src/tests.rs +++ b/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()); diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs index 351204bfcb..6fdff1aa5a 100644 --- a/substrate/frame/democracy/src/lib.rs +++ b/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 Module { 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::::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 Module { } } // Shouldn't be possible to fail, but we handle it gracefully. - status.tally.add(vote).ok_or(Error::::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 Module { Some(ReferendumInfo::Ongoing(mut status)) => { ensure!(matches!(scope, UnvoteScope::Any), Error::::NoPermission); // Shouldn't be possible to fail, but we handle it gracefully. - status.tally.remove(votes[i].1).ok_or(Error::::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); } diff --git a/substrate/frame/lottery/src/lib.rs b/substrate/frame/lottery/src/lib.rs index a37238a2d9..a7782de029 100644 --- a/substrate/frame/lottery/src/lib.rs +++ b/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 { - /// 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::::try_mutate(|lottery| -> DispatchResult { ensure!(lottery.is_none(), Error::::InProgress); let index = LotteryIndex::get(); - let new_index = index.checked_add(1).ok_or(Error::::Overflow)?; + let new_index = index.checked_add(1).ok_or(ArithmeticError::Overflow)?; let start = frame_system::Pallet::::block_number(); // Use new_index to more easily track everything with the current state. *lottery = Some(LotteryConfig { @@ -400,7 +398,7 @@ impl Module { ensure!(T::ValidateCall::validate_call(call), Error::::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::::Overflow)?; + let new_ticket_count = ticket_count.checked_add(1).ok_or(ArithmeticError::Overflow)?; // Try to update the participant status Participants::::try_mutate(&caller, |(lottery_index, participating_calls)| -> DispatchResult { let index = LotteryIndex::get(); diff --git a/substrate/frame/recovery/src/lib.rs b/substrate/frame/recovery/src/lib.rs index ceb2f5a688..cf81e7b033 100644 --- a/substrate/frame/recovery/src/lib.rs +++ b/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::::Overflow)?; + .ok_or(ArithmeticError::Overflow)?; let total_deposit = T::ConfigDepositBase::get() .checked_add(&friend_deposit) - .ok_or(Error::::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 = >::block_number(); let recoverable_block_number = active_recovery.created .checked_add(&recovery_config.delay_period) - .ok_or(Error::::Overflow)?; + .ok_or(ArithmeticError::Overflow)?; ensure!(recoverable_block_number <= current_block_number, Error::::DelayPeriod); // Make sure the threshold is met ensure!( diff --git a/substrate/frame/support/src/traits/tokens/fungible/balanced.rs b/substrate/frame/support/src/traits/tokens/fungible/balanced.rs index 19bdb4f245..1cd0fcf0ca 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/balanced.rs +++ b/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: Inspect { -> Result { 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)? } diff --git a/substrate/frame/support/src/traits/tokens/fungibles/balanced.rs b/substrate/frame/support/src/traits/tokens/fungibles/balanced.rs index efb21300bc..a1016f8c11 100644 --- a/substrate/frame/support/src/traits/tokens/fungibles/balanced.rs +++ b/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: Inspect { -> Result { 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)? } diff --git a/substrate/frame/support/src/traits/tokens/misc.rs b/substrate/frame/support/src/traits/tokens/misc.rs index 9871123abd..342c69c8bb 100644 --- a/substrate/frame/support/src/traits/tokens/misc.rs +++ b/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 { } impl WithdrawConsequence { - /// 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 { + pub fn into_result(self) -> Result { 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(()), }) } diff --git a/substrate/primitives/runtime/src/lib.rs b/substrate/primitives/runtime/src/lib.rs index 51b89d484e..0ae69e9398 100644 --- a/substrate/primitives/runtime/src/lib.rs +++ b/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 for &'static str { @@ -557,8 +555,6 @@ impl From 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 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 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 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 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() {