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.

Sign up for GitHub

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

[Audit] Q-3 - Usage of saturating_sub vs checked_sub #321

Merged
merged 3 commits into from
Jan 1, 2024
Merged

[Audit] Q-3 - Usage of saturating_sub vs checked_sub #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f732b56
replace-saturating-sub
crypto-vincent Dec 24, 2023
36e32db
nit
crypto-vincent Dec 24, 2023
c9d32e5
Merge branch 'develop' into vbrunet/2023_12_23-audit-q3
crypto-vincent Dec 31, 2023
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
28 changes: 11 additions & 17 deletions programs/uxd/src/instructions/credix_lp/collect_profits_of_credix_lp_depository.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::error::UxdError;
use crate::events::CollectProfitsOfCredixLpDepositoryEvent;
use crate::state::controller::Controller;
use crate::state::credix_lp_depository::CredixLpDepository;
use crate::utils::checked_as_u64;
use crate::utils::compute_decrease;
use crate::utils::compute_increase;
use crate::utils::compute_shares_amount_for_value_floor;
Expand Down Expand Up @@ -178,37 +179,30 @@ pub(crate) fn handler(ctx: Context<CollectProfitsOfCredixLpDepository>) -> Resul
)?;

// How much collateral can we withdraw as profits
let profits_value: u128 = {
let profits_value = {
// Compute the set of liabilities owed to the users
let liabilities_value: u128 = ctx
.accounts
.depository
.load()?
.redeemable_amount_under_management;
let liabilities_value = checked_as_u64(
ctx.accounts
.depository
.load()?
.redeemable_amount_under_management,
)?;
msg!(
"[collect_profits_of_credix_lp_depository:liabilities_value:{}]",
liabilities_value
);
// Compute the set of assets owned in the LP
let assets_value: u128 = owned_shares_value_before.into();
let assets_value = owned_shares_value_before;
msg!(
"[collect_profits_of_credix_lp_depository:assets_value:{}]",
assets_value
);
// Compute the amount of profits that we can safely withdraw
assets_value
.checked_sub(liabilities_value)
.ok_or(UxdError::MathOverflow)?
assets_value.saturating_sub(liabilities_value)
};
msg!(
"[collect_profits_of_credix_lp_depository:profits_value:{}]",
profits_value
);

// Assumes and enforce a collateral/redeemable 1:1 relationship on purpose
let collateral_amount_before_precision_loss: u64 = u64::try_from(profits_value)
.ok()
.ok_or(UxdError::MathOverflow)?;
let collateral_amount_before_precision_loss: u64 = profits_value;
msg!(
"[collect_profits_of_credix_lp_depository:collateral_amount_before_precision_loss:{}]",
collateral_amount_before_precision_loss
Expand Down
13 changes: 3 additions & 10 deletions ...src/instructions/credix_lp/rebalance_create_withdraw_request_from_credix_lp_depository.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use crate::state::mercurial_vault_depository::MercurialVaultDepository;
use crate::utils::calculate_router_depositories_target_redeemable_amount;
use crate::utils::checked_add;
use crate::utils::checked_as_u64;
use crate::utils::checked_sub;
use crate::utils::compute_value_for_shares_amount_floor;
use crate::validate_is_program_frozen;
use crate::CONTROLLER_NAMESPACE;
Expand Down Expand Up @@ -183,7 +182,7 @@ pub(crate) fn handler(
total_shares_supply,
total_shares_value,
)?;
checked_sub(owned_shares_value, redeemable_amount_under_management)?
owned_shares_value.saturating_sub(redeemable_amount_under_management)
};

let overflow_value = {
Expand All @@ -196,14 +195,8 @@ pub(crate) fn handler(
&ctx.accounts.alloyx_vault_depository,
)?
.credix_lp_depository_target_redeemable_amount;
if redeemable_amount_under_management < redeemable_amount_under_management_target_amount {
0
} else {
checked_sub(
redeemable_amount_under_management,
redeemable_amount_under_management_target_amount,
)?
}
redeemable_amount_under_management
.saturating_sub(redeemable_amount_under_management_target_amount)
};

// ---------------------------------------------------------------------
Expand Down
16 changes: 4 additions & 12 deletions ...src/instructions/credix_lp/rebalance_redeem_withdraw_request_from_credix_lp_depository.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,8 @@ pub(crate) fn handler(
.redeemable_amount_under_management,
)?;

let profits_collateral_amount = checked_sub(
owned_shares_value_before,
redeemable_amount_under_management,
)?;
let profits_collateral_amount =
owned_shares_value_before.saturating_sub(redeemable_amount_under_management);
msg!(
"[rebalance_redeem_withdraw_request_from_credix_lp_depository:profits_collateral_amount:{}]",
profits_collateral_amount
Expand All @@ -283,14 +281,8 @@ pub(crate) fn handler(
&ctx.accounts.alloyx_vault_depository,
)?
.credix_lp_depository_target_redeemable_amount;
if redeemable_amount_under_management < redeemable_amount_under_management_target_amount {
0
} else {
checked_sub(
redeemable_amount_under_management,
redeemable_amount_under_management_target_amount,
)?
}
redeemable_amount_under_management
.saturating_sub(redeemable_amount_under_management_target_amount)
};
msg!(
"[rebalance_redeem_withdraw_request_from_credix_lp_depository:overflow_value:{}]",
Expand Down
4 changes: 1 addition & 3 deletions programs/uxd/src/instructions/mercurial/collect_profits_of_mercurial_vault_depository.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,7 @@ impl<'info> CollectProfitsOfMercurialVaultDepository<'info> {
.ok()
.ok_or(UxdError::MathOverflow)?;

Ok(owned_lp_tokens_value
.checked_sub(redeemable_amount_under_management)
.ok_or(UxdError::MathOverflow)?)
Ok(owned_lp_tokens_value.saturating_sub(redeemable_amount_under_management))
}
}

Expand Down
31 changes: 10 additions & 21 deletions programs/uxd/src/utils/calculate_depositories_mint_collateral_amount.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,13 @@ pub fn calculate_depositories_mint_collateral_amount(
.iter()
.map(|depository| {
if !depository.directly_mintable {
return Ok(0);
return 0;
}
if depository.target_redeemable_amount <= depository.redeemable_amount_under_management
{
return Ok(0);
}
checked_sub(
depository.target_redeemable_amount,
depository.redeemable_amount_under_management,
)
depository
.target_redeemable_amount
.saturating_sub(depository.redeemable_amount_under_management)
})
.collect::<Result<Vec<u64>>>()?;
.collect::<Vec<u64>>();

let total_under_target_redeemable_amount =
calculate_depositories_sum_value(&depositories_under_target_redeemable_amount)?;
Expand All @@ -63,23 +58,17 @@ pub fn calculate_depositories_mint_collateral_amount(
.iter()
.map(|depository| {
if !depository.directly_mintable {
return Ok(0);
return 0;
}
let depository_after_target_redeemable_amount = std::cmp::max(
depository.redeemable_amount_under_management,
depository.target_redeemable_amount,
);
if depository.redeemable_amount_under_management_cap
<= depository_after_target_redeemable_amount
{
return Ok(0);
}
checked_sub(
depository.redeemable_amount_under_management_cap,
depository_after_target_redeemable_amount,
)
depository
.redeemable_amount_under_management_cap
.saturating_sub(depository_after_target_redeemable_amount)
})
.collect::<Result<Vec<u64>>>()?;
.collect::<Vec<u64>>();

let total_under_cap_redeemable_amount =
calculate_depositories_sum_value(&depositories_under_cap_redeemable_amount)?;
Expand Down
23 changes: 9 additions & 14 deletions programs/uxd/src/utils/calculate_depositories_redeemable_amount.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,13 @@ pub fn calculate_depositories_redeemable_amount(
.iter()
.map(|depository| {
if !depository.directly_redeemable {
return Ok(0);
return 0;
}
if depository.redeemable_amount_under_management <= depository.target_redeemable_amount
{
return Ok(0);
}
checked_sub(
depository.redeemable_amount_under_management,
depository.target_redeemable_amount,
)
depository
.redeemable_amount_under_management
.saturating_sub(depository.target_redeemable_amount)
})
.collect::<Result<Vec<u64>>>()?;
.collect::<Vec<u64>>();

let total_over_target_redeemable_amount =
calculate_depositories_sum_value(&depositories_over_target_redeemable_amount)?;
Expand All @@ -63,14 +58,14 @@ pub fn calculate_depositories_redeemable_amount(
.iter()
.map(|depository| {
if !depository.directly_redeemable {
return Ok(0);
return 0;
}
Ok(std::cmp::min(
std::cmp::min(
depository.redeemable_amount_under_management,
depository.target_redeemable_amount,
))
)
})
.collect::<Result<Vec<u64>>>()?;
.collect::<Vec<u64>>();

let total_under_target_redeemable_amount =
calculate_depositories_sum_value(&depositories_under_target_redeemable_amount)?;
Expand Down
20 changes: 4 additions & 16 deletions programs/uxd/src/utils/calculate_depositories_target_redeemable_amount.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,10 @@ pub fn calculate_depositories_target_redeemable_amount(
)
.map(
|(depository_raw_target_redeemable_amount, depository_hard_cap_amount)| {
if depository_raw_target_redeemable_amount <= depository_hard_cap_amount {
return Ok(0);
}
checked_sub(
*depository_raw_target_redeemable_amount,
*depository_hard_cap_amount,
)
depository_raw_target_redeemable_amount.saturating_sub(*depository_hard_cap_amount)
},
)
.collect::<Result<Vec<u64>>>()?;
.collect::<Vec<u64>>();

// Compute the amount of space available under the cap in each depository
let depositories_available_amount = std::iter::zip(
Expand All @@ -94,16 +88,10 @@ pub fn calculate_depositories_target_redeemable_amount(
)
.map(
|(depository_raw_target_redeemable_amount, depository_hard_cap_amount)| {
if depository_raw_target_redeemable_amount >= depository_hard_cap_amount {
return Ok(0);
}
checked_sub(
*depository_hard_cap_amount,
*depository_raw_target_redeemable_amount,
)
depository_hard_cap_amount.saturating_sub(*depository_raw_target_redeemable_amount)
},
)
.collect::<Result<Vec<u64>>>()?;
.collect::<Vec<u64>>();

// ---------------------------------------------------------------------
// -- Phase 3
Expand Down
Loading