Merge pull request #781 from sander2/refactor/remove-vault-collateral

[BREAKING] Refactor: remove vault collateral interface
interlay · Nov 29, 2022 · 63b485b · 63b485b
2 parents 47bfef0 + 140335e
commit 63b485b
Show file tree

Hide file tree

Showing 27 changed files with 154 additions and 460 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/issue/Cargo.toml b/crates/issue/Cargo.toml
@@ -31,6 +31,7 @@ fee = { path = "../fee", default-features = false }
 security = { path = "../security", default-features = false }
 currency = { path = "../currency", default-features = false }
 vault-registry = { path = "../vault-registry", default-features = false }
+nomination = { path = "../nomination", default-features = false }
 
 primitives = { package = "interbtc-primitives", path = "../../primitives", default-features = false }
 
@@ -74,6 +75,7 @@ std = [
 	"security/std",
 	"currency/std",
 	"vault-registry/std",
+	"nomination/std",
 
     "primitives/std",
 

diff --git a/crates/issue/src/mock.rs b/crates/issue/src/mock.rs
@@ -45,6 +45,7 @@ frame_support::construct_runtime!(
         Fee: fee::{Pallet, Call, Config<T>, Storage},
         Staking: staking::{Pallet, Storage, Event<T>},
         Currency: currency::{Pallet},
+        Nomination: nomination::{Pallet, Call, Config, Storage, Event<T>},
     }
 );
 
@@ -150,6 +151,12 @@ impl vault_registry::Config for Test {
     type Balance = Balance;
     type WeightInfo = ();
     type GetGriefingCollateralCurrencyId = GetNativeCurrencyId;
+    type NominationApi = Nomination;
+}
+
+impl nomination::Config for Test {
+    type RuntimeEvent = RuntimeEvent;
+    type WeightInfo = ();
 }
 
 pub struct CurrencyConvert;

diff --git a/crates/nomination/Cargo.toml b/crates/nomination/Cargo.toml
@@ -31,6 +31,7 @@ reward = { path = "../reward", default-features = false }
 staking = { path = "../staking", default-features = false }
 
 primitives = { package = "interbtc-primitives", path = "../../primitives", default-features = false }
+traits = { path = "../../crates/traits", default-features = false }
 
 # Orml dependencies
 orml-tokens = { git = "https://github.com/open-web3-stack/open-runtime-module-library", rev = "3fcd3cf9e63fe80fd9671912833a900ba09d1cc0", default-features = false, optional = true }

diff --git a/crates/nomination/src/lib.rs b/crates/nomination/src/lib.rs
@@ -245,9 +245,10 @@ impl<T: Config> Pallet<T> {
                 Error::<T>::CannotWithdrawCollateral
             );
 
-            ensure!(Self::is_nomination_enabled(), Error::<T>::VaultNominationDisabled);
-            ensure!(Self::is_opted_in(vault_id)?, Error::<T>::VaultNotOptedInToNomination);
-
+            if &vault_id.account_id != nominator_id {
+                ensure!(Self::is_nomination_enabled(), Error::<T>::VaultNominationDisabled);
+                ensure!(Self::is_opted_in(vault_id)?, Error::<T>::VaultNotOptedInToNomination);
+            }
             ext::vault_registry::decrease_total_backing_collateral(&vault_id.currencies, &amount)?;
         }
 
@@ -271,25 +272,28 @@ impl<T: Config> Pallet<T> {
         nominator_id: &T::AccountId,
         amount: BalanceOf<T>,
     ) -> DispatchResult {
-        ensure!(Self::is_nomination_enabled(), Error::<T>::VaultNominationDisabled);
-        ensure!(Self::is_opted_in(vault_id)?, Error::<T>::VaultNotOptedInToNomination);
-
         let amount = Amount::new(amount, vault_id.collateral_currency());
 
-        let total_nominated_collateral = Self::get_total_nominated_collateral(vault_id)?;
-        let new_nominated_collateral = total_nominated_collateral.checked_add(&amount)?;
-        let max_nominatable_collateral = Self::get_nomination_limit(vault_id);
-        ensure!(
-            new_nominated_collateral.le(&max_nominatable_collateral)?,
-            Error::<T>::NominationExceedsLimit
-        );
+        if &vault_id.account_id != nominator_id {
+            let total_nominated_collateral = Self::get_total_nominated_collateral(vault_id)?;
+            let new_nominated_collateral = total_nominated_collateral.checked_add(&amount)?;
+            let max_nominatable_collateral = Self::get_nomination_limit(vault_id);
+
+            ensure!(Self::is_nomination_enabled(), Error::<T>::VaultNominationDisabled);
+            ensure!(Self::is_opted_in(vault_id)?, Error::<T>::VaultNotOptedInToNomination);
+
+            ensure!(
+                new_nominated_collateral.le(&max_nominatable_collateral)?,
+                Error::<T>::NominationExceedsLimit
+            );
+            amount.transfer(&nominator_id, &vault_id.account_id)?;
+        }
 
         // Withdraw all vault rewards first, to prevent the nominator from withdrawing past rewards
         ext::fee::withdraw_all_vault_rewards::<T>(vault_id)?;
 
         // Deposit `amount` of stake into the vault staking pool
         ext::staking::deposit_stake::<T>(vault_id, nominator_id, amount.amount())?;
-        amount.transfer(&nominator_id, &vault_id.account_id)?;
         amount.lock_on(&vault_id.account_id)?;
         ext::vault_registry::try_increase_total_backing_collateral(&vault_id.currencies, &amount)?;
 
@@ -366,3 +370,9 @@ impl<T: Config> Pallet<T> {
         Ok(Amount::new(amount, vault_id.collateral_currency()))
     }
 }
+
+impl<T: Config> traits::NominationApi<DefaultVaultId<T>, Amount<T>> for Pallet<T> {
+    fn deposit_vault_collateral(vault_id: &DefaultVaultId<T>, amount: &Amount<T>) -> Result<(), DispatchError> {
+        Pallet::<T>::_deposit_collateral(vault_id, &vault_id.account_id, amount.amount())
+    }
+}
diff --git a/crates/nomination/src/mock.rs b/crates/nomination/src/mock.rs
@@ -155,6 +155,7 @@ impl vault_registry::Config for Test {
     type Balance = Balance;
     type WeightInfo = ();
     type GetGriefingCollateralCurrencyId = GetNativeCurrencyId;
+    type NominationApi = Nomination;
 }
 
 pub struct CurrencyConvert;

diff --git a/crates/redeem/Cargo.toml b/crates/redeem/Cargo.toml
@@ -44,6 +44,7 @@ frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch =
 reward = { path = "../reward" }
 staking = { path = "../staking" }
 currency = { path = "../currency", features = ["testing-utils"] }
+nomination = { path = "../nomination" }
 
 # Orml dependencies
 orml-tokens = { git = "https://github.com/open-web3-stack/open-runtime-module-library", rev = "3fcd3cf9e63fe80fd9671912833a900ba09d1cc0" }
@@ -73,6 +74,7 @@ std = [
 	"fee/std",
 	"security/std",
 	"vault-registry/std",
+	"nomination/std",
     "primitives/std",
 
 	"orml-tokens/std",

diff --git a/crates/redeem/src/mock.rs b/crates/redeem/src/mock.rs
@@ -46,6 +46,7 @@ frame_support::construct_runtime!(
         Fee: fee::{Pallet, Call, Config<T>, Storage},
         Staking: staking::{Pallet, Storage, Event<T>},
         Currency: currency::{Pallet},
+        Nomination: nomination::{Pallet, Call, Config, Storage, Event<T>},
     }
 );
 
@@ -146,6 +147,12 @@ impl vault_registry::Config for Test {
     type Balance = Balance;
     type WeightInfo = ();
     type GetGriefingCollateralCurrencyId = GetNativeCurrencyId;
+    type NominationApi = Nomination;
+}
+
+impl nomination::Config for Test {
+    type RuntimeEvent = RuntimeEvent;
+    type WeightInfo = ();
 }
 
 pub struct CurrencyConvert;

diff --git a/crates/replace/src/mock.rs b/crates/replace/src/mock.rs
@@ -177,6 +177,12 @@ impl vault_registry::Config for Test {
     type Balance = Balance;
     type WeightInfo = ();
     type GetGriefingCollateralCurrencyId = GetNativeCurrencyId;
+    type NominationApi = Nomination;
+}
+
+impl nomination::Config for Test {
+    type RuntimeEvent = RuntimeEvent;
+    type WeightInfo = ();
 }
 
 impl staking::Config for Test {
@@ -201,11 +207,6 @@ impl security::Config for Test {
     type RuntimeEvent = RuntimeEvent;
 }
 
-impl nomination::Config for Test {
-    type RuntimeEvent = RuntimeEvent;
-    type WeightInfo = ();
-}
-
 parameter_types! {
     pub const MinimumPeriod: Moment = 5;
 }

diff --git a/crates/traits/src/lib.rs b/crates/traits/src/lib.rs
@@ -19,3 +19,7 @@ impl ConvertToBigUint for u128 {
 pub trait OracleApi<Amount, CurrencyId> {
     fn convert(amount: &Amount, to: CurrencyId) -> Result<Amount, DispatchError>;
 }
+
+pub trait NominationApi<VaultId, Amount> {
+    fn deposit_vault_collateral(vault_id: &VaultId, amount: &Amount) -> Result<(), DispatchError>;
+}
diff --git a/crates/vault-registry/Cargo.toml b/crates/vault-registry/Cargo.toml
@@ -35,6 +35,7 @@ currency = { path = "../currency", default-features = false }
 reward = { path = "../reward", default-features = false }
 staking = { path = "../staking", default-features = false }
 primitives = { package = "interbtc-primitives", path = "../../primitives", default-features = false }
+traits = { path = "../../crates/traits", default-features = false }
 
 # Orml dependencies
 orml-tokens = { git = "https://github.com/open-web3-stack/open-runtime-module-library", rev = "3fcd3cf9e63fe80fd9671912833a900ba09d1cc0", default-features = false }
@@ -44,8 +45,8 @@ orml-traits = { git = "https://github.com/open-web3-stack/open-runtime-module-li
 mocktopus = "0.7.0"
 frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.31", default-features = false }
 currency = { path = "../currency", default-features = false, features = ["testing-utils"] }
-traits = { path = "../../crates/traits", default-features = false }
 pretty_assertions = "0.7.2"
+visibility = { version = "0.0.1" }
 
 [features]
 default = ["std"]

diff --git a/crates/vault-registry/src/benchmarking.rs b/crates/vault-registry/src/benchmarking.rs
@@ -58,26 +58,6 @@ benchmarks! {
         VaultRegistry::<T>::register_public_key(origin.clone().into(), public_key).unwrap();
     }: _(origin, vault_id.currencies.clone(), amount.into())
 
-    deposit_collateral {
-        let vault_id = get_vault_id::<T>();
-        mint_collateral::<T>(&vault_id.account_id, (1u32 << 31).into());
-        let amount = 100u32.into();
-        register_vault_with_collateral::<T>(vault_id.clone(), 100000000);
-        Oracle::<T>::_set_exchange_rate(get_collateral_currency_id::<T>(),
-            UnsignedFixedPoint::<T>::one()
-        ).unwrap();
-    }: _(RawOrigin::Signed(vault_id.account_id), vault_id.currencies.clone(), amount)
-
-    withdraw_collateral {
-        let vault_id = get_vault_id::<T>();
-        mint_collateral::<T>(&vault_id.account_id, (1u32 << 31).into());
-        let amount = 100u32.into();
-        register_vault_with_collateral::<T>(vault_id.clone(), 100000000);
-        Oracle::<T>::_set_exchange_rate(get_collateral_currency_id::<T>(),
-            UnsignedFixedPoint::<T>::one()
-        ).unwrap();
-    }: _(RawOrigin::Signed(vault_id.account_id), vault_id.currencies.clone(), amount)
-
     register_public_key {
         let vault_id = get_vault_id::<T>();
         mint_collateral::<T>(&vault_id.account_id, (1u32 << 31).into());

diff --git a/crates/vault-registry/src/lib.rs b/crates/vault-registry/src/lib.rs
@@ -65,6 +65,7 @@ use sp_std::{
     fmt::Debug,
     vec::Vec,
 };
+use traits::NominationApi;
 
 // value taken from https://github.com/substrate-developer-hub/recipes/blob/master/pallets/ocw-demo/src/lib.rs
 pub const UNSIGNED_TXS_PRIORITY: u64 = 100;
@@ -123,6 +124,8 @@ pub mod pallet {
         /// Currency used for griefing collateral, e.g. DOT.
         #[pallet::constant]
         type GetGriefingCollateralCurrencyId: Get<CurrencyId<Self>>;
+
+        type NominationApi: NominationApi<DefaultVaultId<Self>, Amount<Self>>;
     }
 
     #[pallet::hooks]
@@ -190,74 +193,6 @@ pub mod pallet {
             Ok(().into())
         }
 
-        /// Deposit collateral as a security against stealing the
-        /// Bitcoin locked with the caller.
-        ///
-        /// # Arguments
-        /// * `amount` - the amount of extra collateral to lock
-        #[pallet::weight(<T as Config>::WeightInfo::deposit_collateral())]
-        #[transactional]
-        pub fn deposit_collateral(
-            origin: OriginFor<T>,
-            currency_pair: DefaultVaultCurrencyPair<T>,
-            #[pallet::compact] amount: BalanceOf<T>,
-        ) -> DispatchResultWithPostInfo {
-            let account_id = ensure_signed(origin)?;
-
-            let vault_id = VaultId::new(account_id, currency_pair.collateral, currency_pair.wrapped);
-
-            let vault = Self::get_active_rich_vault_from_id(&vault_id)?;
-
-            let amount = Amount::new(amount, currency_pair.collateral);
-
-            Self::try_deposit_collateral(&vault_id, &amount)?;
-
-            Self::deposit_event(Event::<T>::DepositCollateral {
-                vault_id: vault.id(),
-                new_collateral: amount.amount(),
-                total_collateral: vault.get_total_collateral()?.amount(),
-                free_collateral: vault.get_free_collateral()?.amount(),
-            });
-            Ok(().into())
-        }
-
-        /// Withdraws `amount` of the collateral from the amount locked by
-        /// the vault corresponding to the origin account
-        /// The collateral left after withdrawal must be more
-        /// (free or used in collateral issued tokens) than MinimumCollateralVault
-        /// and above the SecureCollateralThreshold. Collateral that is currently
-        /// being used to back issued tokens remains locked until the Vault
-        /// is used for a redeem request (full release can take multiple redeem requests).
-        ///
-        /// # Arguments
-        /// * `amount` - the amount of collateral to withdraw
-        ///
-        /// # Errors
-        /// * `VaultNotFound` - if no vault exists for the origin account
-        /// * `InsufficientCollateralAvailable` - if the vault does not own enough collateral
-        #[pallet::weight(<T as Config>::WeightInfo::withdraw_collateral())]
-        #[transactional]
-        pub fn withdraw_collateral(
-            origin: OriginFor<T>,
-            currency_pair: DefaultVaultCurrencyPair<T>,
-            #[pallet::compact] amount: BalanceOf<T>,
-        ) -> DispatchResultWithPostInfo {
-            let account_id = ensure_signed(origin)?;
-
-            let vault_id = VaultId::new(account_id, currency_pair.collateral, currency_pair.wrapped);
-            let vault = Self::get_rich_vault_from_id(&vault_id)?;
-
-            let amount = Amount::new(amount, currency_pair.collateral);
-            Self::try_withdraw_collateral(&vault_id, &amount)?;
-
-            Self::deposit_event(Event::<T>::WithdrawCollateral {
-                vault_id: vault.id(),
-                withdrawn_amount: amount.amount(),
-                total_collateral: vault.get_total_collateral()?.amount(),
-            });
-            Ok(().into())
-        }
-
         /// Registers a new Bitcoin address for the vault.
         ///
         /// # Arguments
@@ -864,21 +799,7 @@ impl<T: Config> Pallet<T> {
     /// * `vault_id` - the id of the vault
     /// * `amount` - the amount of collateral
     pub fn try_deposit_collateral(vault_id: &DefaultVaultId<T>, amount: &Amount<T>) -> DispatchResult {
-        // ensure the vault is active
-        let _vault = Self::get_active_rich_vault_from_id(vault_id)?;
-
-        // will fail if collateral ceiling exceeded
-        Self::try_increase_total_backing_collateral(&vault_id.currencies, amount)?;
-        // will fail if free_balance is insufficient
-        amount.lock_on(&vault_id.account_id)?;
-
-        // withdraw first such that past rewards don't get changed by this deposit
-        ext::fee::withdraw_all_vault_rewards::<T>(vault_id)?;
-
-        // Deposit `amount` of stake in the pool
-        ext::staking::deposit_stake::<T>(vault_id, &vault_id.account_id, amount)?;
-
-        Ok(())
+        T::NominationApi::deposit_vault_collateral(&vault_id, &amount)
     }
 
     /// Withdraw an `amount` of collateral without checking collateralization
@@ -900,20 +821,6 @@ impl<T: Config> Pallet<T> {
         Ok(())
     }
 
-    /// Withdraw an `amount` of collateral, ensuring that the vault is sufficiently
-    /// over-collateralized
-    ///
-    /// # Arguments
-    /// * `vault_id` - the id of the vault
-    /// * `amount` - the amount of collateral
-    pub fn try_withdraw_collateral(vault_id: &DefaultVaultId<T>, amount: &Amount<T>) -> DispatchResult {
-        ensure!(
-            Self::is_allowed_to_withdraw_collateral(vault_id, amount)?,
-            Error::<T>::InsufficientCollateral
-        );
-        Self::force_withdraw_collateral(vault_id, amount)
-    }
-
     /// Checks if the vault would be above the secure threshold after withdrawing collateral
     pub fn is_allowed_to_withdraw_collateral(
         vault_id: &DefaultVaultId<T>,