Repository: interlay/interbtc (language: rust)

Issue:
Use sp_runtime::ArithmeticError for math handling
Currently most pallets have duplicate definitions for `ArithmeticOverflow` and `ArithmeticUnderflow`, we can use [this enum](https://docs.substrate.io/rustdocs/latest/sp_runtime/enum.ArithmeticError.html) instead to kick some unnecessary code.


Discussion / hints:

Fix patch:
diff --git a/crates/currency/src/amount.rs b/crates/currency/src/amount.rs
index da9214db74..11154d81fa 100644
--- a/crates/currency/src/amount.rs
+++ b/crates/currency/src/amount.rs
@@ -11,7 +11,7 @@ use orml_traits::{MultiCurrency, MultiReservableCurrency};
 use primitives::TruncateFixedPointToInt;
 use sp_runtime::{
     traits::{CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, UniqueSaturatedInto, Zero},
-    FixedPointNumber,
+    ArithmeticError, FixedPointNumber,
 };
 use sp_std::{convert::TryInto, fmt::Debug};
 
@@ -81,7 +81,7 @@ mod math {
             self.amount.is_zero()
         }
 
-        fn checked_fn<F>(&self, other: &Self, f: F, err: Error<T>) -> Result<Self, DispatchError>
+        fn checked_fn<F>(&self, other: &Self, f: F, err: ArithmeticError) -> Result<Self, DispatchError>
         where
             F: Fn(&BalanceOf<T>, &BalanceOf<T>) -> Option<BalanceOf<T>>,
         {
@@ -100,7 +100,7 @@ mod math {
             self.checked_fn(
                 other,
                 <BalanceOf<T> as CheckedAdd>::checked_add,
-                Error::<T>::ArithmeticOverflow,
+                ArithmeticError::Overflow,
             )
         }
 
@@ -108,7 +108,7 @@ mod math {
             self.checked_fn(
                 other,
                 <BalanceOf<T> as CheckedSub>::checked_sub,
-                Error::<T>::ArithmeticUnderflow,
+                ArithmeticError::Underflow,
             )
         }
 
@@ -119,9 +119,7 @@ mod math {
         }
 
         pub fn checked_fixed_point_mul(&self, scalar: &UnsignedFixedPoint<T>) -> Result<Self, DispatchError> {
-            let amount = scalar
-                .checked_mul_int(self.amount)
-                .ok_or(Error::<T>::ArithmeticUnderflow)?;
+            let amount = scalar.checked_mul_int(self.amount).ok_or(ArithmeticError::Underflow)?;
             Ok(Self {
                 amount,
                 currency_id: self.currency_id,
@@ -136,9 +134,7 @@ mod math {
                 UnsignedFixedPoint::<T>::checked_from_integer(self.amount).ok_or(Error::<T>::TryIntoIntError)?;
 
             // do the multiplication
-            let product = self_fixed_point
-                .checked_mul(&scalar)
-                .ok_or(Error::<T>::ArithmeticOverflow)?;
+            let product = self_fixed_point.checked_mul(&scalar).ok_or(ArithmeticError::Overflow)?;
 
             // convert to inner
             let product_inner = UniqueSaturatedInto::<u128>::unique_saturated_into(product.into_inner());
@@ -147,11 +143,11 @@ mod math {
             let accuracy = UniqueSaturatedInto::<u128>::unique_saturated_into(UnsignedFixedPoint::<T>::accuracy());
             let amount = product_inner
                 .checked_add(accuracy)
-                .ok_or(Error::<T>::ArithmeticOverflow)?
+                .ok_or(ArithmeticError::Overflow)?
                 .checked_sub(1)
-                .ok_or(Error::<T>::ArithmeticUnderflow)?
+                .ok_or(ArithmeticError::Underflow)?
                 .checked_div(accuracy)
-                .ok_or(Error::<T>::ArithmeticUnderflow)?
+                .ok_or(ArithmeticError::Underflow)?
                 .try_into()
                 .map_err(|_| Error::<T>::TryIntoIntError)?;
 
@@ -165,7 +161,7 @@ mod math {
             let amount = UnsignedFixedPoint::<T>::checked_from_integer(self.amount)
                 .ok_or(Error::<T>::TryIntoIntError)?
                 .checked_div(&scalar)
-                .ok_or(Error::<T>::ArithmeticOverflow)?
+                .ok_or(ArithmeticError::Overflow)?
                 .truncate_to_inner()
                 .ok_or(Error::<T>::TryIntoIntError)?;
             Ok(Self {
@@ -217,11 +213,11 @@ mod math {
             let rounding_addition = UnsignedFixedPoint::<T>::checked_from_rational(1, 2).unwrap();
 
             let amount = UnsignedFixedPoint::<T>::checked_from_integer(self.amount)
-                .ok_or(Error::<T>::ArithmeticOverflow)?
+                .ok_or(ArithmeticError::Overflow)?
                 .checked_mul(&fraction)
-                .ok_or(Error::<T>::ArithmeticOverflow)?
+                .ok_or(ArithmeticError::Overflow)?
                 .checked_add(&rounding_addition)
-                .ok_or(Error::<T>::ArithmeticOverflow)?
+                .ok_or(ArithmeticError::Overflow)?
                 .truncate_to_inner()
                 .ok_or(Error::<T>::TryIntoIntError)?;
 
diff --git a/crates/currency/src/lib.rs b/crates/currency/src/lib.rs
index b03afe2229..2eb546c431 100644
--- a/crates/currency/src/lib.rs
+++ b/crates/currency/src/lib.rs
@@ -89,8 +89,6 @@ pub mod pallet {
 
     #[pallet::error]
     pub enum Error<T> {
-        ArithmeticOverflow,
-        ArithmeticUnderflow,
         TryIntoIntError,
         InvalidCurrency,
     }
diff --git a/crates/oracle/src/lib.rs b/crates/oracle/src/lib.rs
index 7f5bc242ea..8aef835552 100644
--- a/crates/oracle/src/lib.rs
+++ b/crates/oracle/src/lib.rs
@@ -42,7 +42,7 @@ use scale_info::TypeInfo;
 use security::{ErrorCode, StatusCode};
 use sp_runtime::{
     traits::{UniqueSaturatedInto, *},
-    FixedPointNumber,
+    ArithmeticError, FixedPointNumber,
 };
 use sp_std::{convert::TryInto, vec::Vec};
 
@@ -102,10 +102,6 @@ pub mod pallet {
         MissingExchangeRate,
         /// Unable to convert value
         TryIntoIntError,
-        /// Mathematical operation caused an overflow
-        ArithmeticOverflow,
-        /// Mathematical operation caused an underflow
-        ArithmeticUnderflow,
     }
 
     #[pallet::hooks]
@@ -334,7 +330,7 @@ impl<T: Config> Pallet<T> {
 
     pub fn wrapped_to_collateral(amount: BalanceOf<T>, currency_id: CurrencyId) -> Result<BalanceOf<T>, DispatchError> {
         let rate = Self::get_price(OracleKey::ExchangeRate(currency_id))?;
-        let converted = rate.checked_mul_int(amount).ok_or(Error::<T>::ArithmeticOverflow)?;
+        let converted = rate.checked_mul_int(amount).ok_or(ArithmeticError::Overflow)?;
         let result = converted.try_into().map_err(|_e| Error::<T>::TryIntoIntError)?;
         Ok(result)
     }
@@ -349,7 +345,7 @@ impl<T: Config> Pallet<T> {
         Ok(T::UnsignedFixedPoint::checked_from_integer(amount)
             .ok_or(Error::<T>::TryIntoIntError)?
             .checked_div(&rate)
-            .ok_or(Error::<T>::ArithmeticUnderflow)?
+            .ok_or(ArithmeticError::Underflow)?
             .truncate_to_inner()
             .ok_or(Error::<T>::TryIntoIntError)?
             .unique_saturated_into())
diff --git a/crates/refund/src/lib.rs b/crates/refund/src/lib.rs
index 0a55ac641a..8e252b028e 100644
--- a/crates/refund/src/lib.rs
+++ b/crates/refund/src/lib.rs
@@ -35,6 +35,7 @@ use frame_system::{ensure_root, ensure_signed};
 use oracle::OracleKey;
 pub use pallet::*;
 use sp_core::H256;
+use sp_runtime::ArithmeticError;
 use sp_std::{convert::TryInto, vec::Vec};
 use types::{BalanceOf, DefaultVaultId, RefundRequestExt};
 
@@ -81,8 +82,6 @@ pub mod pallet {
 
     #[pallet::error]
     pub enum Error<T> {
-        ArithmeticUnderflow,
-        ArithmeticOverflow,
         NoRefundFoundForIssueId,
         RefundIdNotFound,
         RefundCompleted,
@@ -316,7 +315,7 @@ impl<T: Config> Pallet<T> {
 
         let fee = satoshi_per_bytes
             .checked_mul_int(size)
-            .ok_or(Error::<T>::ArithmeticOverflow)?;
+            .ok_or(ArithmeticError::Overflow)?;
         let amount = fee.try_into().map_err(|_| Error::<T>::TryIntoIntError)?;
         Ok(Amount::new(amount, wrapped_currency))
     }
diff --git a/crates/vault-registry/src/lib.rs b/crates/vault-registry/src/lib.rs
index b45b411be3..2c7eb55fa6 100644
--- a/crates/vault-registry/src/lib.rs
+++ b/crates/vault-registry/src/lib.rs
@@ -58,7 +58,7 @@ use sp_runtime::traits::AtLeast32BitUnsigned;
 use sp_runtime::{
     traits::*,
     transaction_validity::{InvalidTransaction, TransactionSource, TransactionValidity, ValidTransaction},
-    FixedPointNumber, FixedPointOperand,
+    ArithmeticError, FixedPointNumber, FixedPointOperand,
 };
 use sp_std::{
     convert::{TryFrom, TryInto},
@@ -648,9 +648,6 @@ pub mod pallet {
         /// Vault attempted to set secure threshold below the global secure threshold
         ThresholdNotAboveGlobalThreshold,
 
-        // Unexpected errors that should never be thrown in normal operation
-        ArithmeticOverflow,
-        ArithmeticUnderflow,
         /// Unable to convert value
         TryIntoIntError,
     }
@@ -954,7 +951,7 @@ impl<T: Config> Pallet<T> {
 
         let new_collateral = match Self::get_backing_collateral(vault_id)?.checked_sub(&amount) {
             Ok(x) => x,
-            Err(x) if x == currency::Error::<T>::ArithmeticUnderflow.into() => return Ok(false),
+            Err(x) if x == ArithmeticError::Underflow.into() => return Ok(false),
             Err(x) => return Err(x),
         };
 
@@ -1683,9 +1680,9 @@ impl<T: Config> Pallet<T> {
 
         let amount = collateral
             .checked_mul(numerator)
-            .ok_or(Error::<T>::ArithmeticOverflow)?
+            .ok_or(ArithmeticError::Overflow)?
             .checked_div(denominator)
-            .ok_or(Error::<T>::ArithmeticUnderflow)?
+            .ok_or(ArithmeticError::Underflow)?
             .try_into()
             .map_err(|_| Error::<T>::TryIntoIntError)?;
         Ok(Amount::new(amount, currency))
@@ -1881,9 +1878,9 @@ impl<T: Config> Pallet<T> {
             Self::premium_redeem_threshold(currency_pair).ok_or(Error::<T>::ThresholdNotSet)?;
         Ok(secure_collateral_threshold
             .checked_div(&premium_redeem_threshold)
-            .ok_or(Error::<T>::ArithmeticUnderflow)?
+            .ok_or(ArithmeticError::Underflow)?
             .checked_sub(&UnsignedFixedPoint::<T>::one())
-            .ok_or(Error::<T>::ArithmeticUnderflow)?)
+            .ok_or(ArithmeticError::Underflow)?)
     }
 
     pub fn get_max_nominatable_collateral(
diff --git a/crates/vault-registry/src/mock.rs b/crates/vault-registry/src/mock.rs
index 37877572b1..40e95c759e 100644
--- a/crates/vault-registry/src/mock.rs
+++ b/crates/vault-registry/src/mock.rs
@@ -231,7 +231,6 @@ impl staking::Config for Test {
 
 pub type TestEvent = Event;
 pub type TestError = Error<Test>;
-pub type CurrencyError = currency::Error<Test>;
 pub type TokensError = orml_tokens::Error<Test>;
 
 pub struct ExtBuilder;
diff --git a/crates/vault-registry/src/types.rs b/crates/vault-registry/src/types.rs
index f897a1750a..b48dae2193 100644
--- a/crates/vault-registry/src/types.rs
+++ b/crates/vault-registry/src/types.rs
@@ -10,7 +10,10 @@ use frame_support::{
 pub use primitives::{VaultCurrencyPair, VaultId};
 use scale_info::TypeInfo;
 use sp_core::H256;
-use sp_runtime::traits::{CheckedAdd, CheckedSub, Zero};
+use sp_runtime::{
+    traits::{CheckedAdd, CheckedSub, Zero},
+    ArithmeticError,
+};
 use sp_std::{collections::btree_set::BTreeSet, vec::Vec};
 
 #[cfg(test)]
@@ -710,7 +713,7 @@ impl<T: Config> RichVault<T> {
             .data
             .issued_tokens
             .checked_add(&self.data.to_be_issued_tokens)
-            .ok_or(Error::<T>::ArithmeticOverflow)?;
+            .ok_or(ArithmeticError::Overflow)?;
         Ok(Amount::new(amount, self.wrapped_currency()))
     }
 
@@ -799,7 +802,7 @@ impl<T: Config> RichVault<T> {
             v.replace_collateral = v
                 .replace_collateral
                 .checked_add(&amount.amount())
-                .ok_or(Error::<T>::ArithmeticOverflow)?;
+                .ok_or(ArithmeticError::Overflow)?;
             Ok(())
         })
     }
@@ -809,7 +812,7 @@ impl<T: Config> RichVault<T> {
             v.replace_collateral = v
                 .replace_collateral
                 .checked_sub(&amount.amount())
-                .ok_or(Error::<T>::ArithmeticUnderflow)?;
+                .ok_or(ArithmeticError::Underflow)?;
             Ok(())
         })
     }
@@ -819,7 +822,7 @@ impl<T: Config> RichVault<T> {
             v.active_replace_collateral = v
                 .active_replace_collateral
                 .checked_add(&amount.amount())
-                .ok_or(Error::<T>::ArithmeticOverflow)?;
+                .ok_or(ArithmeticError::Overflow)?;
             Ok(())
         })
     }
@@ -829,7 +832,7 @@ impl<T: Config> RichVault<T> {
             v.active_replace_collateral = v
                 .active_replace_collateral
                 .checked_sub(&amount.amount())
-                .ok_or(Error::<T>::ArithmeticUnderflow)?;
+                .ok_or(ArithmeticError::Underflow)?;
             Ok(())
         })
     }
@@ -853,7 +856,7 @@ impl<T: Config> RichVault<T> {
             v.liquidated_collateral = v
                 .liquidated_collateral
                 .checked_add(&amount.amount())
-                .ok_or(Error::<T>::ArithmeticOverflow)?;
+                .ok_or(ArithmeticError::Overflow)?;
             Ok(())
         })
     }
@@ -863,7 +866,7 @@ impl<T: Config> RichVault<T> {
             v.liquidated_collateral = v
                 .liquidated_collateral
                 .checked_sub(&amount.amount())
-                .ok_or(Error::<T>::ArithmeticUnderflow)?;
+                .ok_or(ArithmeticError::Underflow)?;
             Ok(())
         })
     }


Test patch:
diff --git a/crates/vault-registry/src/tests.rs b/crates/vault-registry/src/tests.rs
index cf0f3ebb0d..36f2abc4c2 100644
--- a/crates/vault-registry/src/tests.rs
+++ b/crates/vault-registry/src/tests.rs
@@ -2,7 +2,7 @@ use crate::{
     ext,
     mock::*,
     types::{BalanceOf, UpdatableVault},
-    BtcPublicKey, CurrencySource, DefaultVaultId, DispatchError, Error, Vault, VaultStatus,
+    BtcPublicKey, CurrencySource, DefaultVaultId, DispatchError, Vault, VaultStatus,
 };
 use codec::Decode;
 use currency::Amount;
@@ -13,8 +13,12 @@ use primitives::{VaultCurrencyPair, VaultId};
 use security::Pallet as Security;
 use sp_arithmetic::{traits::One, FixedPointNumber, FixedU128};
 use sp_core::U256;
-use sp_runtime::offchain::{testing::TestTransactionPoolExt, TransactionPoolExt};
+use sp_runtime::{
+    offchain::{testing::TestTransactionPoolExt, TransactionPoolExt},
+    ArithmeticError,
+};
 use sp_std::convert::TryInto;
+
 type Event = crate::Event<Test>;
 
 fn vault_id(account_id: AccountId) -> VaultId<AccountId, CurrencyId> {
@@ -321,7 +325,7 @@ fn decrease_to_be_issued_tokens_fails_with_insufficient_tokens() {
         let id = create_sample_vault();
 
         let res = VaultRegistry::decrease_to_be_issued_tokens(&id, &wrapped(50));
-        assert_err!(res, CurrencyError::ArithmeticUnderflow);
+        assert_err!(res, ArithmeticError::Underflow);
     });
 }
 
@@ -349,7 +353,7 @@ fn issue_tokens_fails_with_insufficient_tokens() {
 
         assert_err!(
             VaultRegistry::issue_tokens(&id, &wrapped(50)),
-            CurrencyError::ArithmeticUnderflow
+            ArithmeticError::Underflow
         );
     });
 }
@@ -441,7 +445,7 @@ fn decrease_to_be_redeemed_tokens_fails_with_insufficient_tokens() {
         let id = create_sample_vault();
 
         let res = VaultRegistry::decrease_to_be_redeemed_tokens(&id, &wrapped(50));
-        assert_err!(res, CurrencyError::ArithmeticUnderflow);
+        assert_err!(res, ArithmeticError::Underflow);
     });
 }
 
@@ -474,7 +478,7 @@ fn decrease_tokens_fails_with_insufficient_tokens() {
         VaultRegistry::try_increase_to_be_issued_tokens(&id, &wrapped(50)).unwrap();
         assert_ok!(VaultRegistry::issue_tokens(&id, &wrapped(50)));
         let res = VaultRegistry::decrease_tokens(&id, &user_id, &wrapped(50));
-        assert_err!(res, CurrencyError::ArithmeticUnderflow);
+        assert_err!(res, ArithmeticError::Underflow);
     });
 }
 
@@ -504,7 +508,7 @@ fn redeem_tokens_fails_with_insufficient_tokens() {
         VaultRegistry::try_increase_to_be_issued_tokens(&id, &wrapped(50)).unwrap();
         assert_ok!(VaultRegistry::issue_tokens(&id, &wrapped(50)));
         let res = VaultRegistry::redeem_tokens(&id, &wrapped(50), &amount(0), &0);
-        assert_err!(res, CurrencyError::ArithmeticUnderflow);
+        assert_err!(res, ArithmeticError::Underflow);
     });
 }
 
@@ -546,7 +550,7 @@ fn redeem_tokens_premium_fails_with_insufficient_tokens() {
         VaultRegistry::try_increase_to_be_issued_tokens(&id, &wrapped(50)).unwrap();
         assert_ok!(VaultRegistry::issue_tokens(&id, &wrapped(50)));
         let res = VaultRegistry::redeem_tokens(&id, &wrapped(50), &amount(30), &user_id);
-        assert_err!(res, CurrencyError::ArithmeticUnderflow);
+        assert_err!(res, ArithmeticError::Underflow);
         assert_not_emitted!(Event::RedeemTokensPremium {
             vault_id: id,
             redeemed_amount: 50,
@@ -967,7 +971,7 @@ fn test_threshold_equivalent_to_legacy_calculation() {
         // calculate how many tokens should be maximally issued given the threshold
         let scaled_collateral_in_wrapped = collateral_in_wrapped
             .checked_mul(U256::from(10).pow(granularity.into()))
-            .ok_or(Error::<Test>::ArithmeticOverflow)?;
+            .ok_or(ArithmeticError::Overflow)?;
         let scaled_max_tokens = scaled_collateral_in_wrapped
             .checked_div(threshold.into())
             .unwrap_or(0.into());
@@ -1005,16 +1009,14 @@ fn test_get_required_collateral_threshold_equivalent_to_legacy_calculation_() {
         // Step 1: inverse of the scaling applied in calculate_max_wrapped_from_collateral_for_threshold
 
         // inverse of the div
-        let btc = btc
-            .checked_mul(threshold.into())
-            .ok_or(Error::<Test>::ArithmeticOverflow)?;
+        let btc = btc.checked_mul(threshold.into()).ok_or(ArithmeticError::Overflow)?;
 
         // To do the inverse of the multiplication, we need to do division, but
         // we need to round up. To round up (a/b), we need to do ((a+b-1)/b):
         let rounding_addition = U256::from(10).pow(granularity.into()) - U256::from(1);
         let btc = (btc + rounding_addition)
             .checked_div(U256::from(10).pow(granularity.into()))
-            .ok_or(Error::<Test>::ArithmeticUnderflow)?;
+            .ok_or(ArithmeticError::Underflow)?;
 
         // Step 2: convert the amount to collateral
         let amount_in_collateral = convert_to(Token(DOT), wrapped(btc.try_into()?))?;