From 68ac6589a29283b9268e986c8e0a8769125fef0a Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Thu, 13 Oct 2022 17:35:59 +0200 Subject: [PATCH] feat: configurable nomination limit --- crates/nomination/src/benchmarking.rs | 18 ++++++ crates/nomination/src/default_weights.rs | 11 ++++ crates/nomination/src/ext.rs | 7 --- crates/nomination/src/lib.rs | 32 ++++++++-- crates/nomination/src/tests.rs | 5 ++ crates/vault-registry/src/lib.rs | 43 +------------ .../tests/mock/nomination_testing_utils.rs | 6 ++ standalone/runtime/tests/test_issue.rs | 1 - standalone/runtime/tests/test_nomination.rs | 61 ++++++------------- 9 files changed, 86 insertions(+), 98 deletions(-) diff --git a/crates/nomination/src/benchmarking.rs b/crates/nomination/src/benchmarking.rs index abef33e3b7..e60dddd375 100644 --- a/crates/nomination/src/benchmarking.rs +++ b/crates/nomination/src/benchmarking.rs @@ -54,6 +54,11 @@ benchmarks! { set_nomination_enabled { }: _(RawOrigin::Root, true) + set_nomination_limit { + let vault_id = get_vault_id::(); + let amount = 100u32.into(); + }: _(RawOrigin::Signed(vault_id.account_id), vault_id.currencies.clone(), amount) + opt_in_to_nomination { setup_exchange_rate::(); >::set(true); @@ -81,6 +86,13 @@ benchmarks! { >::set(true); let vault_id = get_vault_id::(); + + Nomination::::set_nomination_limit( + RawOrigin::Signed(vault_id.account_id.clone()).into(), + vault_id.currencies.clone(), + (1u32 << 31).into() + ).unwrap(); + mint_collateral::(&vault_id.account_id, (1u32 << 31).into()); register_vault::(vault_id.clone()); @@ -102,6 +114,12 @@ benchmarks! { >::insert(&vault_id, true); + Nomination::::set_nomination_limit( + RawOrigin::Signed(vault_id.account_id.clone()).into(), + vault_id.currencies.clone(), + (1u32 << 31).into() + ).unwrap(); + let nominator: T::AccountId = account("Nominator", 0, 0); mint_collateral::(&nominator, (1u32 << 31).into()); let amount = 100u32.into(); diff --git a/crates/nomination/src/default_weights.rs b/crates/nomination/src/default_weights.rs index ec6d3c8adf..718b5c5a87 100644 --- a/crates/nomination/src/default_weights.rs +++ b/crates/nomination/src/default_weights.rs @@ -39,6 +39,7 @@ pub trait WeightInfo { fn opt_out_of_nomination() -> Weight; fn deposit_collateral() -> Weight; fn withdraw_collateral() -> Weight; + fn set_nomination_limit() -> Weight; } /// Weights for nomination using the Substrate node and recommended hardware. @@ -127,6 +128,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(21 as Weight)) .saturating_add(T::DbWeight::get().writes(10 as Weight)) } + // Storage: Nomination Nominationlimit (r:0 w:1) + fn set_nomination_limit() -> Weight { + (2_835_000 as Weight) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } } // For backwards compatibility and tests @@ -214,5 +220,10 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(21 as Weight)) .saturating_add(RocksDbWeight::get().writes(10 as Weight)) } + // Storage: Nomination NominationLimit (r:0 w:1) + fn set_nomination_limit() -> Weight { + (2_835_000 as Weight) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } } diff --git a/crates/nomination/src/ext.rs b/crates/nomination/src/ext.rs index 5c63118a68..92efbf02c3 100644 --- a/crates/nomination/src/ext.rs +++ b/crates/nomination/src/ext.rs @@ -36,13 +36,6 @@ pub(crate) mod vault_registry { >::is_allowed_to_withdraw_collateral(vault_id, amount) } - pub fn get_max_nominatable_collateral( - vault_collateral: &Amount, - currency_pair: &DefaultVaultCurrencyPair, - ) -> Result, DispatchError> { - >::get_max_nominatable_collateral(vault_collateral, currency_pair) - } - pub fn try_increase_total_backing_collateral( currency_pair: &DefaultVaultCurrencyPair, amount: &Amount, diff --git a/crates/nomination/src/lib.rs b/crates/nomination/src/lib.rs index 0812938731..262bf22bc2 100644 --- a/crates/nomination/src/lib.rs +++ b/crates/nomination/src/lib.rs @@ -89,7 +89,7 @@ pub mod pallet { /// Nomination is not enabled. VaultNominationDisabled, /// Nomination would overburden Vault. - DepositViolatesMaxNominationRatio, + NominationExceedsLimit, /// Vault cannot withdraw. CollateralizationTooLow, } @@ -106,6 +106,11 @@ pub mod pallet { #[pallet::storage] pub(super) type Vaults = StorageMap<_, Blake2_128Concat, DefaultVaultId, bool, ValueQuery>; + /// The maximum amount of collateral to be nominated for a given vault. + #[pallet::storage] + pub(super) type NominationLimit = + StorageMap<_, Blake2_128Concat, DefaultVaultId, BalanceOf, ValueQuery>; + #[pallet::genesis_config] pub struct GenesisConfig { pub is_nomination_enabled: bool, @@ -199,6 +204,21 @@ pub mod pallet { Self::_withdraw_collateral(&vault_id, &nominator_id, amount, index.unwrap_or_default())?; Ok(().into()) } + + #[pallet::weight(::WeightInfo::set_nomination_limit())] + #[transactional] + pub fn set_nomination_limit( + origin: OriginFor, + currency_pair: DefaultVaultCurrencyPair, + limit: BalanceOf, + ) -> DispatchResultWithPostInfo { + ext::security::ensure_parachain_status_running::()?; + let account_id = ensure_signed(origin)?; + let vault_id = VaultId::new(account_id, currency_pair.collateral, currency_pair.wrapped); + + NominationLimit::::insert(vault_id, limit); + Ok(().into()) + } } } @@ -256,14 +276,12 @@ impl Pallet { let amount = Amount::new(amount, vault_id.collateral_currency()); - let vault_backing_collateral = ext::vault_registry::get_backing_collateral::(vault_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 = - ext::vault_registry::get_max_nominatable_collateral::(&vault_backing_collateral, &vault_id.currencies)?; + let max_nominatable_collateral = Self::get_nomination_limit(vault_id); ensure!( new_nominated_collateral.le(&max_nominatable_collateral)?, - Error::::DepositViolatesMaxNominationRatio + Error::::NominationExceedsLimit ); // Withdraw all vault rewards first, to prevent the nominator from withdrawing past rewards @@ -335,6 +353,10 @@ impl Pallet { let vault_actual_collateral = ext::vault_registry::compute_collateral::(vault_id)?; vault_backing_collateral.checked_sub(&vault_actual_collateral) } + pub fn get_nomination_limit(vault_id: &DefaultVaultId) -> Amount { + let limit = NominationLimit::::get(vault_id); + Amount::new(limit, vault_id.collateral_currency()) + } pub fn get_nominator_collateral( vault_id: &DefaultVaultId, diff --git a/crates/nomination/src/tests.rs b/crates/nomination/src/tests.rs index 938fe01e89..c5b0ffcf1e 100644 --- a/crates/nomination/src/tests.rs +++ b/crates/nomination/src/tests.rs @@ -23,6 +23,11 @@ fn should_deposit_against_valid_vault() { ext::vault_registry::get_backing_collateral::.mock_safe(|_| MockResult::Return(Ok(collateral(10000)))); ext::vault_registry::compute_collateral::.mock_safe(|_| MockResult::Return(Ok(collateral(10000)))); assert_ok!(Nomination::_opt_in_to_nomination(&ALICE)); + assert_ok!(Nomination::set_nomination_limit( + Origin::signed(ALICE.account_id), + ALICE.currencies, + 100 + )); assert_ok!(Nomination::_deposit_collateral(&ALICE, &BOB.account_id, 100)); }) } diff --git a/crates/vault-registry/src/lib.rs b/crates/vault-registry/src/lib.rs index 37b350170e..b205882639 100644 --- a/crates/vault-registry/src/lib.rs +++ b/crates/vault-registry/src/lib.rs @@ -580,7 +580,7 @@ pub mod pallet { VaultNotBelowLiquidationThreshold, /// Deposit address could not be generated with the given public key. InvalidPublicKey, - /// The Max Nomination Ratio would be exceeded. + /// Deprecated error. TODO: remove when releasing a breaking runtime upgrade MaxNominationRatioViolation, /// The collateral ceiling would be exceeded for the vault's currency. CurrencyCeilingExceeded, @@ -905,26 +905,9 @@ impl Pallet { Self::is_allowed_to_withdraw_collateral(vault_id, amount)?, Error::::InsufficientCollateral ); - ensure!( - Self::is_max_nomination_ratio_preserved(vault_id, amount)?, - Error::::MaxNominationRatioViolation - ); Self::force_withdraw_collateral(vault_id, amount) } - pub fn is_max_nomination_ratio_preserved( - vault_id: &DefaultVaultId, - amount: &Amount, - ) -> Result { - let vault_collateral = Self::compute_collateral(vault_id)?; - let backing_collateral = Self::get_backing_collateral(vault_id)?; - let current_nomination = backing_collateral.checked_sub(&vault_collateral)?; - let new_vault_collateral = vault_collateral.checked_sub(&amount)?; - let max_nomination_after_withdrawal = - Self::get_max_nominatable_collateral(&new_vault_collateral, &vault_id.currencies)?; - Ok(current_nomination.le(&max_nomination_after_withdrawal)?) - } - /// Checks if the vault would be above the secure threshold after withdrawing collateral pub fn is_allowed_to_withdraw_collateral( vault_id: &DefaultVaultId, @@ -1858,30 +1841,6 @@ impl Pallet { Ok(Amount::new(amount, vault_id.currencies.collateral)) } - pub fn get_max_nomination_ratio( - currency_pair: &DefaultVaultCurrencyPair, - ) -> Result, DispatchError> { - // MaxNominationRatio = (SecureCollateralThreshold / PremiumRedeemThreshold) - 1) - // It denotes the maximum amount of collateral that can be nominated to a particular Vault. - // Its effect is to minimise the impact on collateralization of nominator withdrawals. - let secure_collateral_threshold = - Self::secure_collateral_threshold(currency_pair).ok_or(Error::::ThresholdNotSet)?; - let premium_redeem_threshold = - Self::premium_redeem_threshold(currency_pair).ok_or(Error::::ThresholdNotSet)?; - Ok(secure_collateral_threshold - .checked_div(&premium_redeem_threshold) - .ok_or(ArithmeticError::Underflow)? - .checked_sub(&UnsignedFixedPoint::::one()) - .ok_or(ArithmeticError::Underflow)?) - } - - pub fn get_max_nominatable_collateral( - vault_collateral: &Amount, - currency_pair: &DefaultVaultCurrencyPair, - ) -> Result, DispatchError> { - vault_collateral.rounded_mul(Self::get_max_nomination_ratio(currency_pair)?) - } - /// Private getters and setters fn get_collateral_ceiling(currency_pair: &DefaultVaultCurrencyPair) -> Result, DispatchError> { diff --git a/standalone/runtime/tests/mock/nomination_testing_utils.rs b/standalone/runtime/tests/mock/nomination_testing_utils.rs index fa9173f8e5..e6c9e928e1 100644 --- a/standalone/runtime/tests/mock/nomination_testing_utils.rs +++ b/standalone/runtime/tests/mock/nomination_testing_utils.rs @@ -7,6 +7,7 @@ pub const VAULT: [u8; 32] = BOB; pub const DEFAULT_BACKING_COLLATERAL: Balance = 1_000_000; pub const DEFAULT_NOMINATION: Balance = 20_000; +pub const DEFAULT_NOMINATION_LIMIT: Balance = 1_000_000; pub const DEFAULT_VAULT_UNBONDING_PERIOD: u32 = 100; pub const DEFAULT_NOMINATOR_UNBONDING_PERIOD: u32 = 50; @@ -38,6 +39,11 @@ pub fn nomination_opt_in(vault_id: &DefaultVaultId) -> DispatchResultWi pub fn assert_nomination_opt_in(vault_id: &VaultId) { assert_ok!(nomination_opt_in(vault_id)); + assert_ok!(Call::Nomination(NominationCall::set_nomination_limit { + currency_pair: vault_id.currencies.clone(), + limit: DEFAULT_NOMINATION_LIMIT + }) + .dispatch(origin_of(vault_id.account_id.clone()))); } pub fn nomination_opt_out(vault_id: &DefaultVaultId) -> DispatchResultWithPostInfo { diff --git a/standalone/runtime/tests/test_issue.rs b/standalone/runtime/tests/test_issue.rs index 8d7ea72039..1de6eb3d5d 100644 --- a/standalone/runtime/tests/test_issue.rs +++ b/standalone/runtime/tests/test_issue.rs @@ -1,7 +1,6 @@ mod mock; use currency::Amount; -use frame_support::assert_err; use mock::{assert_eq, issue_testing_utils::*, *}; fn test_with(execute: impl Fn(VaultId) -> R) { diff --git a/standalone/runtime/tests/test_nomination.rs b/standalone/runtime/tests/test_nomination.rs index 19cea0dfc7..fb5095033a 100644 --- a/standalone/runtime/tests/test_nomination.rs +++ b/standalone/runtime/tests/test_nomination.rs @@ -2,7 +2,6 @@ mod mock; use currency::Amount; use mock::{assert_eq, nomination_testing_utils::*, *}; -use sp_runtime::traits::{CheckedDiv, CheckedSub}; fn test_with(execute: impl Fn(VaultId) -> R) { let test_with = |currency_id, wrapped_id| { @@ -251,7 +250,7 @@ mod spec_based_tests { amount: 100000000000000000000000 }) .dispatch(origin_of(account_of(USER))), - NominationError::DepositViolatesMaxNominationRatio + NominationError::NominationExceedsLimit ); assert_ok!(Call::Nomination(NominationCall::deposit_collateral { vault_id: vault_id.clone(), @@ -460,16 +459,26 @@ fn integration_test_vaults_cannot_withdraw_nominated_collateral() { } #[test] -fn integration_test_nominated_collateral_cannot_exceed_max_nomination_ratio() { +fn integration_test_nominated_collateral_cannot_exceed_nomination_limit() { test_with_nomination_enabled_and_vault_opted_in(|vault_id| { + assert_ok!(Call::Nomination(NominationCall::deposit_collateral { + vault_id: vault_id.clone(), + amount: DEFAULT_NOMINATION_LIMIT - 100, + }) + .dispatch(origin_of(account_of(USER)))); assert_noop!( - nominate_collateral( - &vault_id, - account_of(USER), - default_backing_collateral(vault_id.collateral_currency()) - ), - NominationError::DepositViolatesMaxNominationRatio + Call::Nomination(NominationCall::deposit_collateral { + vault_id: vault_id.clone(), + amount: 101, + }) + .dispatch(origin_of(account_of(CAROL))), + NominationError::NominationExceedsLimit ); + assert_ok!(Call::Nomination(NominationCall::deposit_collateral { + vault_id: vault_id.clone(), + amount: 100, + }) + .dispatch(origin_of(account_of(CAROL)))); }); } @@ -560,22 +569,6 @@ fn integration_test_nomination_fee_distribution() { test_with_nomination_enabled(|_| {}); } -#[test] -fn integration_test_maximum_nomination_ratio_calculation() { - test_with_nomination_enabled_and_vault_opted_in(|vault_id| { - let expected_nomination_ratio = FixedU128::checked_from_rational(150, 100) - .unwrap() - .checked_div(&FixedU128::checked_from_rational(135, 100).unwrap()) - .unwrap() - .checked_sub(&FixedU128::one()) - .unwrap(); - assert_eq!( - VaultRegistryPallet::get_max_nomination_ratio(&vault_id.currencies).unwrap(), - expected_nomination_ratio - ); - }) -} - #[test] fn integration_test_vault_opt_out_must_refund_nomination() { test_with_nomination_enabled_and_vault_opted_in(|vault_id| { @@ -626,24 +619,6 @@ fn integration_test_liquidating_a_vault_does_not_force_refund() { }) } -#[test] -fn integration_test_vault_withdrawal_cannot_exceed_max_nomination_taio() { - test_with_nomination_enabled_and_vault_opted_in(|vault_id| { - let max_nomination = VaultRegistryPallet::get_max_nominatable_collateral( - &default_backing_collateral(vault_id.collateral_currency()), - &vault_id.currencies, - ) - .unwrap(); - assert_nominate_collateral(&vault_id, account_of(USER), max_nomination); - - // Need to withdraw 10 units to account for rounding errors - assert_noop!( - withdraw_vault_collateral(&vault_id, Amount::new(10, vault_id.collateral_currency())), - VaultRegistryError::MaxNominationRatioViolation - ); - }) -} - #[test] fn integration_test_rewards_are_preserved_on_collateral_withdrawal() { test_with_nomination_enabled_and_vault_opted_in(|vault_id| {