Skip to content

Commit

Permalink
Add more tolerance on withdrawal
Browse files Browse the repository at this point in the history
  • Loading branch information
joncinque committed Nov 29, 2022
1 parent 24fb559 commit d0992ea
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 12 deletions.
18 changes: 17 additions & 1 deletion stake-pool/program/src/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2551,7 +2551,23 @@ impl Processor {
}
StakeWithdrawSource::ValidatorRemoval => {
let split_from_lamports = stake_split_from.lamports();
let upper_bound = split_from_lamports.saturating_add(lamports_per_pool_token);
// The upper bound for reasonable tolerance is twice the lamports per
// pool token because we have two sources of rounding. The first happens
// when reducing the stake account to as close to the minimum as possible,
// and the second happens on this withdrawal.
//
// For example, if the pool token is extremely valuable, it might only
// be possible to reduce the stake account to a minimum of
// `stake_rent + minimum_delegation + lamports_per_pool_token - 1`.
//
// After that, the minimum amount of pool tokens to get to this amount
// may actually be worth
// `stake_rent + minimum_delegation + lamports_per_pool_token * 2 - 2`.
// We give an extra grace on this check of two lamports, which should be
// reasonable. At worst, it just means that a withdrawer is losing out
// on two lamports.
let upper_bound = split_from_lamports
.saturating_add(lamports_per_pool_token.saturating_mul(2));
if withdraw_lamports < split_from_lamports || withdraw_lamports > upper_bound {
msg!(
"Cannot withdraw a whole account worth {} lamports, \
Expand Down
25 changes: 14 additions & 11 deletions stake-pool/program/tests/withdraw_edge_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,10 @@ async fn fail_remove_validator() {
);
}

#[test_case(1; "1 to 1")]
#[test_case(5; "bigger multiple")]
#[test_case(11; "biggest multiple")]
#[test_case(0; "equal")]
#[test_case(5; "big")]
#[test_case(11; "bigger")]
#[test_case(29; "biggest")]
#[tokio::test]
async fn success_remove_validator(multiple: u64) {
let (
Expand Down Expand Up @@ -216,16 +217,20 @@ async fn success_remove_validator(multiple: u64) {

let rent = context.banks_client.get_rent().await.unwrap();
let stake_rent = rent.minimum_balance(std::mem::size_of::<stake::state::StakeState>());
let stake_pool = stake_pool_accounts
.get_stake_pool(&mut context.banks_client)
.await;
let lamports_per_pool_token = stake_pool.get_lamports_per_pool_token().unwrap();

// decrease all of stake except for 7 lamports, should be withdrawable
// decrease all of stake except for lamports_per_pool_token lamports, must be withdrawable
let error = stake_pool_accounts
.decrease_validator_stake(
&mut context.banks_client,
&context.payer,
&context.last_blockhash,
&validator_stake.stake_account,
&validator_stake.transient_stake_account,
deposit_info.stake_lamports + stake_rent - multiple,
deposit_info.stake_lamports + stake_rent - lamports_per_pool_token,
validator_stake.transient_stake_seed,
)
.await;
Expand Down Expand Up @@ -259,14 +264,12 @@ async fn success_remove_validator(multiple: u64) {
)
.await;
// make sure it's actually more than the minimum
assert_ne!(remaining_lamports, stake_rent + stake_minimum_delegation);
assert!(remaining_lamports > stake_rent + stake_minimum_delegation);

let stake_pool = stake_pool_accounts
.get_stake_pool(&mut context.banks_client)
.await;
// round up to force one more pool token than needed
// round up to force one more pool token if needed
let pool_tokens_post_fee =
remaining_lamports * stake_pool.pool_token_supply / stake_pool.total_lamports + 1;
(remaining_lamports * stake_pool.pool_token_supply + stake_pool.total_lamports - 1)
/ stake_pool.total_lamports;
let new_user_authority = Pubkey::new_unique();
let pool_tokens = stake_pool_accounts.calculate_inverse_withdrawal_fee(pool_tokens_post_fee);
let error = stake_pool_accounts
Expand Down

0 comments on commit d0992ea

Please sign in to comment.