Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove minimum collateral check for nominator while withdrawing #1118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/nomination/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ pub(crate) mod vault_registry {
<vault_registry::Pallet<T>>::compute_collateral(vault_id)
}

pub fn is_allowed_to_withdraw_collateral<T: crate::Config>(
pub fn is_nominator_allowed_to_withdraw_collateral<T: crate::Config>(
vault_id: &DefaultVaultId<T>,
amount: &Amount<T>,
) -> Result<bool, DispatchError> {
<vault_registry::Pallet<T>>::is_allowed_to_withdraw_collateral(vault_id, amount)
<vault_registry::Pallet<T>>::is_nominator_allowed_to_withdraw_collateral(vault_id, amount)
}

pub fn try_increase_total_backing_collateral<T: crate::Config>(
Expand Down
7 changes: 5 additions & 2 deletions crates/nomination/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl<T: Config> Pallet<T> {
// we can only withdraw nominated collateral if the vault is still
// above the secure threshold for issued + to_be_issued tokens
ensure!(
ext::vault_registry::is_allowed_to_withdraw_collateral::<T>(vault_id, &amount)?,
ext::vault_registry::is_nominator_allowed_to_withdraw_collateral::<T>(vault_id, &amount)?,
Error::<T>::CannotWithdrawCollateral
);

Expand Down Expand Up @@ -332,7 +332,10 @@ impl<T: Config> Pallet<T> {
ensure!(Self::is_opted_in(&vault_id), Error::<T>::VaultNotOptedInToNomination);
let total_nominated_collateral = Self::get_total_nominated_collateral(&vault_id)?;
ensure!(
ext::vault_registry::is_allowed_to_withdraw_collateral::<T>(&vault_id, &total_nominated_collateral)?,
ext::vault_registry::is_nominator_allowed_to_withdraw_collateral::<T>(
&vault_id,
&total_nominated_collateral
)?,
Error::<T>::CollateralizationTooLow
);

Expand Down
4 changes: 2 additions & 2 deletions crates/replace/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ pub(crate) mod vault_registry {
<vault_registry::Pallet<T>>::force_withdraw_collateral(vault_id, amount)
}

pub fn is_allowed_to_withdraw_collateral<T: crate::Config>(
pub fn is_vault_allowed_to_withdraw_collateral<T: crate::Config>(
vault_id: &DefaultVaultId<T>,
amount: &Amount<T>,
) -> Result<bool, DispatchError> {
<vault_registry::Pallet<T>>::is_allowed_to_withdraw_collateral(vault_id, amount)
<vault_registry::Pallet<T>>::is_vault_allowed_to_withdraw_collateral(vault_id, amount)
}

pub fn calculate_collateral<T: crate::Config>(
Expand Down
2 changes: 1 addition & 1 deletion crates/replace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ impl<T: Config> Pallet<T> {
// if the new_vault locked additional collateral especially for this replace,
// release it if it does not cause them to be undercollateralized
if !ext::vault_registry::is_vault_liquidated::<T>(&new_vault_id)?
&& ext::vault_registry::is_allowed_to_withdraw_collateral::<T>(&new_vault_id, &collateral)?
&& ext::vault_registry::is_vault_allowed_to_withdraw_collateral::<T>(&new_vault_id, &collateral)?
{
ext::vault_registry::force_withdraw_collateral::<T>(&new_vault_id, &collateral)?;
}
Expand Down
3 changes: 2 additions & 1 deletion crates/replace/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ mod cancel_replace_tests {
ext::vault_registry::is_vault_liquidated::<Test>.mock_safe(|_| MockResult::Return(Ok(false)));
ext::vault_registry::cancel_replace_tokens::<Test>.mock_safe(|_, _, _| MockResult::Return(Ok(())));
ext::vault_registry::transfer_funds::<Test>.mock_safe(|_, _, _| MockResult::Return(Ok(())));
ext::vault_registry::is_allowed_to_withdraw_collateral::<Test>.mock_safe(|_, _| MockResult::Return(Ok(false)));
ext::vault_registry::is_vault_allowed_to_withdraw_collateral::<Test>
.mock_safe(|_, _| MockResult::Return(Ok(false)));
}

#[test]
Expand Down
46 changes: 44 additions & 2 deletions crates/vault-registry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -831,9 +831,51 @@ impl<T: Config> Pallet<T> {
}

/// Checks if the vault would be above the secure threshold after withdrawing collateral
///
/// # Arguments
/// * `vault_id` - The identifier of the vault.
/// * `amount` - The amount of collateral to be withdrawn.
///
/// # Returns
/// Returns `Ok(true)` if the vault is allowed to withdraw the collateral, otherwise returns `Ok(false)`.
/// Returns `Err` in case of an error.
pub fn is_vault_allowed_to_withdraw_collateral(
vault_id: &DefaultVaultId<T>,
amount: &Amount<T>,
) -> Result<bool, DispatchError> {
Self::is_allowed_to_withdraw_collateral(vault_id, amount, false)
}

/// Checks if a nominator is allowed to withdraw collateral.
///
/// # Arguments
/// * `vault_id` - The identifier of the vault.
/// * `amount` - The amount of collateral to be withdrawn.
///
/// # Returns
/// Returns `Ok(true)` if the nominator is allowed to withdraw the collateral, otherwise returns `Ok(false)`.
/// Returns `Err` in case of an error.
pub fn is_nominator_allowed_to_withdraw_collateral(
vault_id: &DefaultVaultId<T>,
amount: &Amount<T>,
) -> Result<bool, DispatchError> {
Self::is_allowed_to_withdraw_collateral(vault_id, amount, true)
}

/// Checks if an entity is allowed to withdraw collateral.
///
/// # Arguments
/// * `vault_id` - The identifier of the vault.
/// * `amount` - The amount of collateral to be withdrawn.
/// * `is_nominator` - A boolean indicating whether the entity is a nominator.
///
/// # Returns
/// Returns `Ok(true)` if the entity is allowed to withdraw the collateral, otherwise returns `Ok(false)`.
/// Returns `Err` in case of an error.
pub fn is_allowed_to_withdraw_collateral(
vault_id: &DefaultVaultId<T>,
amount: &Amount<T>,
is_nominator: bool,
) -> Result<bool, DispatchError> {
let vault = Self::get_rich_vault_from_id(vault_id)?;

Expand All @@ -842,9 +884,9 @@ impl<T: Config> Pallet<T> {
Err(x) if x == ArithmeticError::Underflow.into() => return Ok(false),
Err(x) => return Err(x),
};

ensure!(
new_collateral.is_zero()
is_nominator
|| new_collateral.is_zero()
|| new_collateral.ge(&Self::get_minimum_collateral_vault(vault_id.currencies.collateral))?,
Error::<T>::InsufficientVaultCollateralAmount
);
Expand Down
6 changes: 3 additions & 3 deletions crates/vault-registry/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,17 @@ fn should_check_withdraw_collateral() {

// should allow withdraw all
assert_ok!(
VaultRegistry::is_allowed_to_withdraw_collateral(&DEFAULT_ID, &amount(DEFAULT_COLLATERAL)),
VaultRegistry::is_vault_allowed_to_withdraw_collateral(&DEFAULT_ID, &amount(DEFAULT_COLLATERAL)),
true,
);
// should allow withdraw above minimum
assert_ok!(
VaultRegistry::is_allowed_to_withdraw_collateral(&DEFAULT_ID, &amount(DEFAULT_COLLATERAL / 4)),
VaultRegistry::is_vault_allowed_to_withdraw_collateral(&DEFAULT_ID, &amount(DEFAULT_COLLATERAL / 4)),
true,
);
// should not allow withdraw above zero, below minimum
assert_err!(
VaultRegistry::is_allowed_to_withdraw_collateral(&DEFAULT_ID, &amount(DEFAULT_COLLATERAL / 4 * 3)),
VaultRegistry::is_vault_allowed_to_withdraw_collateral(&DEFAULT_ID, &amount(DEFAULT_COLLATERAL / 4 * 3)),
TestError::InsufficientVaultCollateralAmount,
);
});
Expand Down