diff --git a/program/src/processor.rs b/program/src/processor.rs index 99450c6c..b9aa0aeb 100644 --- a/program/src/processor.rs +++ b/program/src/processor.rs @@ -69,15 +69,16 @@ fn calculate_deposit_amount( pre_pool_nav: u64, user_deposit_amount: u64, ) -> Option { - if pre_pool_nav == 0 || pre_token_supply == 0 { - Some(user_deposit_amount) - } else { + if pre_pool_nav > 0 && pre_token_supply > 0 { u64::try_from( (user_deposit_amount as u128) .checked_mul(pre_token_supply as u128)? .checked_div(pre_pool_nav as u128)?, ) .ok() + } else { + // this is unreachable for real pools but exists to satisfy unit tests + Some(user_deposit_amount) } } @@ -89,10 +90,10 @@ fn calculate_withdraw_amount( ) -> Option { let numerator = (user_tokens_to_burn as u128).checked_mul(pre_pool_nav as u128)?; let denominator = pre_token_supply as u128; - if numerator < denominator || denominator == 0 { - Some(0) - } else { + if numerator >= denominator && denominator > 0 { u64::try_from(numerator.checked_div(denominator)?).ok() + } else { + Some(0) } } @@ -1219,7 +1220,7 @@ impl Processor { // neither warmup/cooldown nor validator delinquency prevent a user withdrawal. // however, because we calculate NAV from all lamports in both pool accounts, // but can only split stake from the main account (unless inactive), we must determine whether this is possible - let (withdrawable_value, pool_is_fully_inactive) = { + let (pool_account_stake_value, pool_is_fully_inactive) = { let (_, pool_stake_state) = get_stake_state(pool_stake_info)?; let pool_stake_status = pool_stake_state .delegation @@ -1243,6 +1244,14 @@ impl Processor { } }; + let withdrawable_value = pool_account_stake_value.saturating_sub(minimum_delegation); + + // the main pool account must *always* meet minimum delegation, even if it is inactive. + // this error is currently impossible to hit and exists to protect pools if minimum delegation rises above 1sol + if withdrawable_value == 0 { + return Err(SinglePoolError::WithdrawalViolatesPoolRequirements.into()); + } + // onramp must exist. this does not create an edge case where withdrawals may be blocked, // because we also require the onramp to exist for deposits match deserialize_stake(pool_onramp_info) { @@ -1263,12 +1272,6 @@ impl Processor { return Err(SinglePoolError::WithdrawalTooSmall.into()); } - // the pool must *always* meet minimum delegation, even if it is inactive. - // this error is currently impossible to hit and exists to protect pools if minimum delegation rises above 1sol - if withdrawable_value.saturating_sub(stake_to_withdraw) < minimum_delegation { - return Err(SinglePoolError::WithdrawalViolatesPoolRequirements.into()); - } - // this is impossible but we guard explicitly because it would put the pool in an unrecoverable state if stake_to_withdraw == pool_stake_info.lamports() { return Err(SinglePoolError::WithdrawalViolatesPoolRequirements.into()); diff --git a/program/tests/withdraw_stake.rs b/program/tests/withdraw_stake.rs index 64e7c769..e9ddeabf 100644 --- a/program/tests/withdraw_stake.rs +++ b/program/tests/withdraw_stake.rs @@ -463,7 +463,7 @@ async fn fail_disallowed_withdraw(stake_version: StakeProgramVersion) { .process_transaction(transaction) .await .unwrap_err(); - check_error(e, SinglePoolError::WithdrawalViolatesPoolRequirements); + check_error(e, SinglePoolError::WithdrawalTooLarge); // slash the pool percentage that one token represents let mut mint_account = get_account(&mut context.banks_client, &accounts.mint).await;