From 4921569f79dba4630f7a1a61890915b3f3beb1ee Mon Sep 17 00:00:00 2001
From: Shaun Wang <spxwang@gmail.com>
Date: Tue, 18 May 2021 19:30:07 +1200
Subject: [PATCH] Replace custom arithmetic error. (#495)

---
 authority/src/lib.rs   | 10 ++++------
 authority/src/tests.rs |  2 +-
 nft/src/lib.rs         |  8 +++-----
 nft/src/tests.rs       |  4 ++--
 tokens/src/lib.rs      | 14 ++++----------
 tokens/src/tests.rs    |  4 ++--
 vesting/src/lib.rs     | 14 ++++++--------
 vesting/src/tests.rs   |  4 ++--
 8 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/authority/src/lib.rs b/authority/src/lib.rs
index ef4aec2..4a02eeb 100644
--- a/authority/src/lib.rs
+++ b/authority/src/lib.rs
@@ -31,7 +31,7 @@ use frame_support::{
 use frame_system::pallet_prelude::*;
 use sp_runtime::{
 	traits::{CheckedSub, Dispatchable, Saturating},
-	DispatchError, DispatchResult, RuntimeDebug,
+	ArithmeticError, DispatchError, DispatchResult, RuntimeDebug,
 };
 use sp_std::prelude::*;
 
@@ -163,8 +163,6 @@ pub mod module {
 
 	#[pallet::error]
 	pub enum Error<T> {
-		/// Calculation overflow.
-		Overflow,
 		/// Failed to schedule a task.
 		FailedToSchedule,
 		/// Failed to cancel a task.
@@ -234,12 +232,12 @@ pub mod module {
 
 			let id = NextTaskIndex::<T>::mutate(|id| -> sp_std::result::Result<ScheduleTaskIndex, DispatchError> {
 				let current_id = *id;
-				*id = id.checked_add(1).ok_or(Error::<T>::Overflow)?;
+				*id = id.checked_add(1).ok_or(ArithmeticError::Overflow)?;
 				Ok(current_id)
 			})?;
 			let now = frame_system::Pallet::<T>::block_number();
 			let delay = match when {
-				DispatchTime::At(x) => x.checked_sub(&now).ok_or(Error::<T>::Overflow)?,
+				DispatchTime::At(x) => x.checked_sub(&now).ok_or(ArithmeticError::Overflow)?,
 				DispatchTime::After(x) => x,
 			};
 			let schedule_origin = if with_delayed_origin {
@@ -278,7 +276,7 @@ pub mod module {
 		) -> DispatchResultWithPostInfo {
 			let now = frame_system::Pallet::<T>::block_number();
 			let new_delay = match when {
-				DispatchTime::At(x) => x.checked_sub(&now).ok_or(Error::<T>::Overflow)?,
+				DispatchTime::At(x) => x.checked_sub(&now).ok_or(ArithmeticError::Overflow)?,
 				DispatchTime::After(x) => x,
 			};
 			let dispatch_at = match when {
diff --git a/authority/src/tests.rs b/authority/src/tests.rs
index 9777f02..013cde7 100644
--- a/authority/src/tests.rs
+++ b/authority/src/tests.rs
@@ -123,7 +123,7 @@ fn schedule_dispatch_after_work() {
 		run_to_block(1);
 		assert_eq!(
 			Authority::schedule_dispatch(Origin::root(), DispatchTime::At(0), 0, true, Box::new(call.clone())),
-			Err(Error::<Runtime>::Overflow.into())
+			Err(ArithmeticError::Overflow.into())
 		);
 
 		assert_ok!(Authority::schedule_dispatch(
diff --git a/nft/src/lib.rs b/nft/src/lib.rs
index ba6b831..16cff62 100644
--- a/nft/src/lib.rs
+++ b/nft/src/lib.rs
@@ -25,7 +25,7 @@ use codec::{Decode, Encode};
 use frame_support::{ensure, pallet_prelude::*, Parameter};
 use sp_runtime::{
 	traits::{AtLeast32BitUnsigned, CheckedAdd, CheckedSub, MaybeSerializeDeserialize, Member, One, Zero},
-	DispatchError, DispatchResult, RuntimeDebug,
+	ArithmeticError, DispatchError, DispatchResult, RuntimeDebug,
 };
 use sp_std::vec::Vec;
 
@@ -103,8 +103,6 @@ pub mod module {
 		ClassNotFound,
 		/// The operator is not the owner of the token and has no permission
 		NoPermission,
-		/// Arithmetic calculation overflow
-		NumOverflow,
 		/// Can not destroy class
 		/// Total issuance is not 0
 		CannotDestroyClass,
@@ -236,7 +234,7 @@ impl<T: Config> Pallet<T> {
 				info.total_issuance = info
 					.total_issuance
 					.checked_add(&One::one())
-					.ok_or(Error::<T>::NumOverflow)?;
+					.ok_or(ArithmeticError::Overflow)?;
 				Ok(())
 			})?;
 
@@ -263,7 +261,7 @@ impl<T: Config> Pallet<T> {
 				info.total_issuance = info
 					.total_issuance
 					.checked_sub(&One::one())
-					.ok_or(Error::<T>::NumOverflow)?;
+					.ok_or(ArithmeticError::Overflow)?;
 				Ok(())
 			})?;
 
diff --git a/nft/src/tests.rs b/nft/src/tests.rs
index bc2bf5b..1e2654e 100644
--- a/nft/src/tests.rs
+++ b/nft/src/tests.rs
@@ -55,7 +55,7 @@ fn mint_should_fail() {
 		});
 		assert_noop!(
 			NonFungibleTokenModule::mint(&BOB, CLASS_ID, vec![1], ()),
-			Error::<Runtime>::NumOverflow
+			ArithmeticError::Overflow,
 		);
 
 		NextTokenId::<Runtime>::mutate(CLASS_ID, |id| *id = <Runtime as Config>::TokenId::max_value());
@@ -136,7 +136,7 @@ fn burn_should_fail() {
 		});
 		assert_noop!(
 			NonFungibleTokenModule::burn(&BOB, (CLASS_ID, TOKEN_ID)),
-			Error::<Runtime>::NumOverflow
+			ArithmeticError::Overflow,
 		);
 	});
 }
diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs
index 51ae72e..c757c0e 100644
--- a/tokens/src/lib.rs
+++ b/tokens/src/lib.rs
@@ -61,7 +61,7 @@ use sp_runtime::{
 		AccountIdConversion, AtLeast32BitUnsigned, Bounded, CheckedAdd, CheckedSub, MaybeSerializeDeserialize, Member,
 		Saturating, StaticLookup, Zero,
 	},
-	DispatchError, DispatchResult, RuntimeDebug,
+	ArithmeticError, DispatchError, DispatchResult, RuntimeDebug,
 };
 use sp_std::{
 	convert::{Infallible, TryFrom, TryInto},
@@ -189,10 +189,6 @@ pub mod module {
 	pub enum Error<T> {
 		/// The balance is too low
 		BalanceTooLow,
-		/// This operation will cause balance to overflow
-		BalanceOverflow,
-		/// This operation will cause total issuance to overflow
-		TotalIssuanceOverflow,
 		/// Cannot convert Amount into Balance type
 		AmountIntoBalanceFailed,
 		/// Failed because liquidity restrictions due to locking
@@ -528,7 +524,7 @@ impl<T: Config> MultiCurrency<T::AccountId> for Pallet<T> {
 		let from_balance = Self::free_balance(currency_id, from);
 		let to_balance = Self::free_balance(currency_id, to)
 			.checked_add(&amount)
-			.ok_or(Error::<T>::BalanceOverflow)?;
+			.ok_or(ArithmeticError::Overflow)?;
 		// Cannot underflow because ensure_can_withdraw check
 		Self::set_free_balance(currency_id, from, from_balance - amount);
 		Self::set_free_balance(currency_id, to, to_balance);
@@ -545,9 +541,7 @@ impl<T: Config> MultiCurrency<T::AccountId> for Pallet<T> {
 		}
 
 		TotalIssuance::<T>::try_mutate(currency_id, |total_issuance| -> DispatchResult {
-			*total_issuance = total_issuance
-				.checked_add(&amount)
-				.ok_or(Error::<T>::TotalIssuanceOverflow)?;
+			*total_issuance = total_issuance.checked_add(&amount).ok_or(ArithmeticError::Overflow)?;
 
 			Self::set_free_balance(currency_id, who, Self::free_balance(currency_id, who) + amount);
 
@@ -932,7 +926,7 @@ where
 		let currency_id = GetCurrencyId::get();
 		let new_total = Pallet::<T>::free_balance(currency_id, who)
 			.checked_add(&value)
-			.ok_or(Error::<T>::TotalIssuanceOverflow)?;
+			.ok_or(ArithmeticError::Overflow)?;
 		Pallet::<T>::set_free_balance(currency_id, who, new_total);
 
 		Ok(Self::PositiveImbalance::new(value))
diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs
index 4e87788..02a4097 100644
--- a/tokens/src/tests.rs
+++ b/tokens/src/tests.rs
@@ -345,7 +345,7 @@ fn deposit_should_work() {
 
 			assert_noop!(
 				Tokens::deposit(DOT, &ALICE, Balance::max_value()),
-				Error::<Runtime>::TotalIssuanceOverflow,
+				ArithmeticError::Overflow,
 			);
 		});
 }
@@ -961,7 +961,7 @@ fn currency_adapter_transferring_too_high_value_should_not_panic() {
 				u64::max_value(),
 				ExistenceRequirement::AllowDeath
 			),
-			Error::<Runtime>::BalanceOverflow,
+			ArithmeticError::Overflow,
 		);
 
 		assert_eq!(
diff --git a/vesting/src/lib.rs b/vesting/src/lib.rs
index 2d3db16..57abc6d 100644
--- a/vesting/src/lib.rs
+++ b/vesting/src/lib.rs
@@ -37,7 +37,7 @@ use frame_support::{
 use frame_system::{ensure_root, ensure_signed, pallet_prelude::*};
 use sp_runtime::{
 	traits::{AtLeast32Bit, CheckedAdd, Saturating, StaticLookup, Zero},
-	DispatchResult, RuntimeDebug,
+	ArithmeticError, DispatchResult, RuntimeDebug,
 };
 use sp_std::{
 	cmp::{Eq, PartialEq},
@@ -144,8 +144,6 @@ pub mod module {
 		ZeroVestingPeriod,
 		/// Number of vests is zero
 		ZeroVestingPeriodCount,
-		/// Arithmetic calculation overflow
-		NumOverflow,
 		/// Insufficient amount of balance to lock
 		InsufficientBalanceToLock,
 		/// This account have too many vesting schedules
@@ -303,7 +301,7 @@ impl<T: Config> Pallet<T> {
 
 		let total_amount = Self::locked_balance(to)
 			.checked_add(&schedule_amount)
-			.ok_or(Error::<T>::NumOverflow)?;
+			.ok_or(ArithmeticError::Overflow)?;
 
 		T::Currency::transfer(from, to, schedule_amount, ExistenceRequirement::AllowDeath)?;
 		T::Currency::set_lock(VESTING_LOCK_ID, to, total_amount, WithdrawReasons::all());
@@ -312,7 +310,7 @@ impl<T: Config> Pallet<T> {
 	}
 
 	fn do_update_vesting_schedules(who: &T::AccountId, schedules: Vec<VestingScheduleOf<T>>) -> DispatchResult {
-		let total_amount = schedules.iter().try_fold::<_, _, Result<BalanceOf<T>, Error<T>>>(
+		let total_amount = schedules.iter().try_fold::<_, _, Result<BalanceOf<T>, DispatchError>>(
 			Zero::zero(),
 			|acc_amount, schedule| {
 				let amount = Self::ensure_valid_vesting_schedule(schedule)?;
@@ -331,12 +329,12 @@ impl<T: Config> Pallet<T> {
 	}
 
 	/// Returns `Ok(amount)` if valid schedule, or error.
-	fn ensure_valid_vesting_schedule(schedule: &VestingScheduleOf<T>) -> Result<BalanceOf<T>, Error<T>> {
+	fn ensure_valid_vesting_schedule(schedule: &VestingScheduleOf<T>) -> Result<BalanceOf<T>, DispatchError> {
 		ensure!(!schedule.period.is_zero(), Error::<T>::ZeroVestingPeriod);
 		ensure!(!schedule.period_count.is_zero(), Error::<T>::ZeroVestingPeriodCount);
-		ensure!(schedule.end().is_some(), Error::<T>::NumOverflow);
+		ensure!(schedule.end().is_some(), ArithmeticError::Overflow);
 
-		let total = schedule.total_amount().ok_or(Error::<T>::NumOverflow)?;
+		let total = schedule.total_amount().ok_or(ArithmeticError::Overflow)?;
 
 		ensure!(total >= T::MinVestedTransfer::get(), Error::<T>::AmountLow);
 
diff --git a/vesting/src/tests.rs b/vesting/src/tests.rs
index 1b9f98f..79ffc8d 100644
--- a/vesting/src/tests.rs
+++ b/vesting/src/tests.rs
@@ -172,7 +172,7 @@ fn vested_transfer_fails_if_overflow() {
 		};
 		assert_noop!(
 			Vesting::vested_transfer(Origin::signed(ALICE), BOB, schedule),
-			Error::<Runtime>::NumOverflow
+			ArithmeticError::Overflow,
 		);
 
 		let another_schedule = VestingSchedule {
@@ -183,7 +183,7 @@ fn vested_transfer_fails_if_overflow() {
 		};
 		assert_noop!(
 			Vesting::vested_transfer(Origin::signed(ALICE), BOB, another_schedule),
-			Error::<Runtime>::NumOverflow
+			ArithmeticError::Overflow,
 		);
 	});
 }
-- 
GitLab