Skip to content

program: move to lamport-based accounting #601

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

Open
2501babe wants to merge 9 commits into
base: main
Choose a base branch
from
Open

program: move to lamport-based accounting #601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e1b5eb6
program: move to lamport-based accounting
2501babe Apr 3, 2026
6043b2e
add test
2501babe Apr 8, 2026
a9d2929
stop being clever
2501babe Apr 8, 2026
2ddd51a
test extra lamports separately
2501babe Apr 10, 2026
439cf91
shuffle withdraw errors
2501babe Apr 10, 2026
ad02f8a
aaaAAAAAgit add src/error.rs!
2501babe Apr 10, 2026
f9b3fa7
test inactive
2501babe Apr 10, 2026
f213496
withdraw tests
2501babe Apr 10, 2026
450127e
typo
2501babe Apr 10, 2026
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
19 changes: 14 additions & 5 deletions program/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ pub enum SinglePoolError {

// 10
/// Not enough stake to cover the provided quantity of pool tokens.
/// (Generally this should not happen absent user error, but may if the
/// minimum delegation increases beyond 1 sol.)
/// This typically means the value exists in the pool as activating stake,
/// and an epoch is required for it to become available. Otherwise, it means
/// active stake in the on-ramp must be moved via `ReplenishPool`.
#[error("WithdrawalTooLarge")]
WithdrawalTooLarge,
/// Required signature is missing.
Expand Down Expand Up @@ -105,6 +106,10 @@ pub enum SinglePoolError {
/// is in an exceptional state, or because the on-ramp account should be refreshed.
#[error("ReplenishRequired")]
ReplenishRequired,
/// Withdrawal would render the pool stake account impossible to redelegate.
/// This can only occur if the Stake Program minimum delegation increases above 1 sol.
#[error("WithdrawalViolatesPoolRequirements")]
WithdrawalViolatesPoolRequirements,
}
impl From<SinglePoolError> for ProgramError {
fn from(e: SinglePoolError) -> Self {
Expand Down Expand Up @@ -137,8 +142,9 @@ impl ToStr for SinglePoolError {
"Error: Not enough pool tokens provided to withdraw stake worth one lamport.",
SinglePoolError::WithdrawalTooLarge =>
"Error: Not enough stake to cover the provided quantity of pool tokens. \
(Generally this should not happen absent user error, but may if the minimum delegation increases \
beyond 1 sol.)",
This typically means the value exists in the pool as activating stake, \
and an epoch is required for it to become available. Otherwise, it means \
active stake in the onramp must be moved via `ReplenishPool`.",
SinglePoolError::SignatureMissing => "Error: Required signature is missing.",
SinglePoolError::WrongStakeState => "Error: Stake account is not in the state expected by the program.",
SinglePoolError::ArithmeticOverflow => "Error: Unsigned subtraction crossed the zero.",
Expand All @@ -157,11 +163,14 @@ impl ToStr for SinglePoolError {
SinglePoolError::InvalidPoolOnRampAccount =>
"Error: Provided pool onramp account does not match address derived from the pool account.",
SinglePoolError::OnRampDoesntExist =>
"The onramp account for this pool does not exist; you must call `InitializePoolOnRamp` \
"Error: The onramp account for this pool does not exist; you must call `InitializePoolOnRamp` \
before you can perform this operation.",
SinglePoolError::ReplenishRequired =>
"Error: The present operation requires a `ReplenishPool` call, either because the pool stake account \
is in an exceptional state, or because the on-ramp account should be refreshed.",
SinglePoolError::WithdrawalViolatesPoolRequirements =>
"Error: Withdrawal would render the pool stake account impossible to redelegate. \
This can only occur if the Stake Program minimum delegation increases above 1 sol.",
}
}
}
173 changes: 117 additions & 56 deletions program/src/processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,33 +40,49 @@ use {
spl_token_interface::{self as spl_token, state::Mint},
};

/// Calculate pool tokens to mint, given outstanding token supply, pool active
/// stake, and deposit active stake
/// Determine the canonical value of the pool from its staked and stake-able lamports
fn pool_net_asset_value(
pool_stake_info: &AccountInfo,
pool_onramp_info: &AccountInfo,
rent: &Rent,
) -> u64 {
// these numbers should typically be equal, but might differ during StakeState upgrades
let pool_rent_exempt_reserve = rent.minimum_balance(pool_stake_info.data_len());
let onramp_rent_exempt_reserve = rent.minimum_balance(pool_onramp_info.data_len());

// NEV is all lamports in both accounts less rent
pool_stake_info
.lamports()
.saturating_add(pool_onramp_info.lamports())
.saturating_sub(pool_rent_exempt_reserve)
.saturating_sub(onramp_rent_exempt_reserve)
}

/// Calculate pool tokens to mint, given outstanding token supply, pool NEV, and deposit amount
fn calculate_deposit_amount(
pre_token_supply: u64,
pre_pool_stake: u64,
user_stake_to_deposit: u64,
pre_pool_nev: u64,
user_deposit_amount: u64,
) -> Option<u64> {
if pre_pool_stake == 0 || pre_token_supply == 0 {
Some(user_stake_to_deposit)
if pre_pool_nev == 0 || pre_token_supply == 0 {
Some(user_deposit_amount)
Comment on lines -50 to +68
Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note the calculate_deposit_amount() and calculate_withdraw_amount() changes are just renaming variables

} else {
u64::try_from(
(user_stake_to_deposit as u128)
(user_deposit_amount as u128)
.checked_mul(pre_token_supply as u128)?
.checked_div(pre_pool_stake as u128)?,
.checked_div(pre_pool_nev as u128)?,
)
.ok()
}
}

/// Calculate pool stake to return, given outstanding token supply, pool active
/// stake, and tokens to redeem
/// Calculate pool value to return, given outstanding token supply, pool NEV, and tokens to redeem
fn calculate_withdraw_amount(
pre_token_supply: u64,
pre_pool_stake: u64,
pre_pool_nev: u64,
user_tokens_to_burn: u64,
) -> Option<u64> {
let numerator = (user_tokens_to_burn as u128).checked_mul(pre_pool_stake as u128)?;
let numerator = (user_tokens_to_burn as u128).checked_mul(pre_pool_nev as u128)?;
let denominator = pre_token_supply as u128;
if numerator < denominator || denominator == 0 {
Some(0)
Expand Down Expand Up @@ -774,8 +790,8 @@ impl Processor {
let stake_config_info = next_account_info(account_info_iter)?;
let stake_program_info = next_account_info(account_info_iter)?;

let rent = Rent::get()?;
let stake_history = &StakeHistorySysvar(clock.epoch);
let minimum_delegation = stake::tools::get_minimum_delegation()?;
Comment on lines +793 to -778
Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes to replenish are mostly just style to align with the other functions, though moving get_minimum_delegation() after check_stake_program() is technically a correctness improvement


check_vote_account(vote_account_info)?;
check_pool_address(program_id, vote_account_info.key, pool_info.key)?;
Expand All @@ -791,8 +807,9 @@ impl Processor {
)?;
check_stake_program(stake_program_info.key)?;

let minimum_delegation = stake::tools::get_minimum_delegation()?;

// we expect these numbers to be equal but get them separately in case of future changes
let rent = Rent::get()?;
let pool_rent_exempt_reserve = rent.minimum_balance(pool_stake_info.data_len());
let onramp_rent_exempt_reserve = rent.minimum_balance(pool_onramp_info.data_len());

Expand Down Expand Up @@ -970,6 +987,7 @@ impl Processor {
let token_program_info = next_account_info(account_info_iter)?;
let stake_program_info = next_account_info(account_info_iter)?;

let rent = &Rent::get()?;
let stake_history = &StakeHistorySysvar(clock.epoch);

SinglePool::from_account_info(pool_info, program_id)?;
Expand Down Expand Up @@ -998,9 +1016,8 @@ impl Processor {
return Err(SinglePoolError::InvalidPoolStakeAccountUsage.into());
}

let (_, pool_stake_state) = get_stake_state(pool_stake_info)?;

let (pool_is_active, pool_is_activating) = {
let (pre_pool_stake, pool_is_active, pool_is_activating) = {
let (_, pool_stake_state) = get_stake_state(pool_stake_info)?;
let pool_stake_status = pool_stake_state
.delegation
.stake_activating_and_deactivating(
Expand All @@ -1010,6 +1027,7 @@ impl Processor {
);

(
pool_stake_state.delegation.stake,
is_stake_fully_active(&pool_stake_status),
is_stake_newly_activating(&pool_stake_status),
)
Expand All @@ -1030,10 +1048,10 @@ impl Processor {
unreachable!();
};

let pre_pool_stake = pool_stake_state.delegation.stake;
let pre_pool_lamports = pool_stake_info.lamports();
msg!("Available stake pre merge {}", pre_pool_stake);
// tokens for deposit are determined off the total stakeable value of both pool-owned accounts
let pre_total_nev = pool_net_asset_value(pool_stake_info, pool_onramp_info, rent);

let pre_user_lamports = user_stake_info.lamports();
let (user_stake_meta, user_stake_status) = match deserialize_stake(user_stake_info) {
Ok(StakeStateV2::Stake(meta, stake, _)) => (
meta,
Expand Down Expand Up @@ -1078,21 +1096,15 @@ impl Processor {
stake_history_info.clone(),
)?;

let (_, pool_stake_state) = get_stake_state(pool_stake_info)?;
let post_pool_stake = pool_stake_state.delegation.stake;
let post_pool_lamports = pool_stake_info.lamports();
msg!("Available stake post merge {}", post_pool_stake);

// stake lamports added, as a stake difference
let stake_added = post_pool_stake
// determine new stake lamports added by merge
let post_pool_stake = get_stake_amount(pool_stake_info)?;
let new_stake_added = post_pool_stake
.checked_sub(pre_pool_stake)
.ok_or(SinglePoolError::ArithmeticOverflow)?;
Comment on lines -1081 to 1103
Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new deposit math feels straightforward to me but this is an important thing to scrutinize

we have already asserted the pool is fully active or newly activating. then we get NEV ie stakeable lamport sum of both pool accounts. we assert user stake is in a happy state and merge. we simply get pool stake delegation delta by subtracting pre from post. and then user excess lamports are pre-user lamports minus new stake. we then use new stake to calculate pool tokens


// if there were excess lamports in the user-provided account, we return them
// this includes their rent-exempt reserve if the pool is fully active
let user_excess_lamports = post_pool_lamports
.checked_sub(pre_pool_lamports)
.and_then(|amount| amount.checked_sub(stake_added))
// return user lamports that were not added to stake
let user_excess_lamports = pre_user_lamports
.checked_sub(new_stake_added)
.ok_or(SinglePoolError::ArithmeticOverflow)?;
Comment on lines -1091 to 1108
Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is like the third version of the user_excess_lamports calculation and while it feels strange to calculate from the user rather than from the pool i can find no reason this isnt correct

Copy link
Copy Markdown
Contributor

@joncinque joncinque Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little strange, but we should be able to trust the stake program to do the right thing.

The only use-case that could be worrying is a multi-epoch activation, but those don't allow merges anyway.

Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of #586 we know via stake history the pool account is either 100% activating or 100% active, and the user account is 100% active (with pool active), or 100% activating or inactive (with pool activating), otherwise we abort before we call Merge. so this is perhaps less complicated than it feels and we dont rely on Merge to reject it

Copy link
Copy Markdown
Contributor

@joncinque joncinque Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's right, good point! Even more confidence then


// sanity check: the user stake account is empty
Expand All @@ -1101,8 +1113,9 @@ impl Processor {
}

// deposit amount is determined off stake added because we return excess lamports
let new_pool_tokens = calculate_deposit_amount(token_supply, pre_pool_stake, stake_added)
.ok_or(SinglePoolError::UnexpectedMathError)?;
let new_pool_tokens =
calculate_deposit_amount(token_supply, pre_total_nev, new_stake_added)
.ok_or(SinglePoolError::UnexpectedMathError)?;

if new_pool_tokens == 0 {
return Err(SinglePoolError::DepositTooSmall.into());
Expand Down Expand Up @@ -1152,9 +1165,13 @@ impl Processor {
let user_stake_info = next_account_info(account_info_iter)?;
let user_token_account_info = next_account_info(account_info_iter)?;
let clock_info = next_account_info(account_info_iter)?;
let clock = &Clock::from_account_info(clock_info)?;
let token_program_info = next_account_info(account_info_iter)?;
let stake_program_info = next_account_info(account_info_iter)?;

let rent = &Rent::get()?;
let stake_history = &StakeHistorySysvar(clock.epoch);

SinglePool::from_account_info(pool_info, program_id)?;

check_pool_stake_address(program_id, pool_info.key, pool_stake_info.key)?;
Expand All @@ -1181,26 +1198,73 @@ impl Processor {
return Err(SinglePoolError::InvalidPoolStakeAccountUsage.into());
}

// we deliberately do NOT validate the activation status of the pool account.
// neither snow nor rain nor warmup/cooldown nor validator delinquency prevents a user withdrawal
//
// NOTE this is fine for stake v4 but subtly wrong for stake v5 *if* the pool account was deactivated.
// stake v5 declines to (meaninglessly) adjust delegations of deactivated sources.
// this will (again) be correct with #581, which shifts to NEV accounting on lamports rather than stake.
// we should plan another SVSP release before stake v5 activation
let pre_pool_stake = get_stake_amount(pool_stake_info)?;
msg!("Available stake pre split {}", pre_pool_stake);

// withdraw amount is determined off stake just like deposit amount
let withdraw_stake = calculate_withdraw_amount(token_supply, pre_pool_stake, token_amount)
.ok_or(SinglePoolError::UnexpectedMathError)?;

if withdraw_stake == 0 {
if token_amount == 0 {
return Err(SinglePoolError::WithdrawalTooSmall.into());
}

// the second case should never be true, but its best to be sure
if withdraw_stake > pre_pool_stake || withdraw_stake == pool_stake_info.lamports() {
let minimum_delegation = stake::tools::get_minimum_delegation()?;

// tokens for withdraw are determined off the total stakeable value of both pool-owned accounts
let pre_total_nev = pool_net_asset_value(pool_stake_info, pool_onramp_info, rent);

// note we deliberately do NOT validate the activation status of the pool account.
// neither warmup/cooldown nor validator delinquency prevent a user withdrawal.
// however, because we calculate NEV 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_stake_state) = get_stake_state(pool_stake_info)?;
let pool_stake_status = pool_stake_state
.delegation
.stake_activating_and_deactivating(
clock.epoch,
stake_history,
PERPETUAL_NEW_WARMUP_COOLDOWN_RATE_EPOCH,
);

// if fully inactive, we split on lamports; otherwise, on all delegation.
// the stake program works off delegation in this way *even* for a partially deactivated stake
if pool_stake_status == StakeActivationStatus::default() {
Copy link
Copy Markdown
Member

@grod220 grod220 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get a test for this?

Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f9b3fa7

(
pool_stake_info
.lamports()
.saturating_sub(rent.minimum_balance(pool_stake_info.data_len())),
true,
)
} else {
(pool_stake_state.delegation.stake, false)
}
Comment on lines +1224 to +1235
Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is quite touchy but i believe it to be correct. if there is any activating, deactivating, or effective stake then the delegation is meaningful, and its full value is always used, so we calculate off delegation. for a fully inactive stake we must calculate off lamports

the withdraw process gets pool NEV and uses that to calculate the amount to pass to the stake program, which does not depend on stake delegation. "withdrawable value" is just a guard for friendly error messages

Copy link
Copy Markdown
Contributor

@joncinque joncinque Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct to me -- there's weirdness in the activating case, since in some situations we do treat those as normal lamports, like merge, but split doesn't do that.

};

// withdraw amount is determined off pool NEV just like deposit amount
let stake_to_withdraw =
calculate_withdraw_amount(token_supply, pre_total_nev, token_amount)
.ok_or(SinglePoolError::UnexpectedMathError)?;

// self-explanatory. we catch 0 deposit above so we only hit this if we rounded to 0
if stake_to_withdraw == 0 {
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());
}

// if the destination would be in any non-inactive state it must meet minimum delegation
if !pool_is_fully_inactive && stake_to_withdraw < minimum_delegation {
return Err(SinglePoolError::WithdrawalTooSmall.into());
}

// if we do not have enough value to service this withdrawal, the user must wait a `ReplenishPool` cycle.
// this does *not* mean the value isnt in the pool, merely that it is not duly splittable.
// this check should always come last to avoid returning it if the withdrawal is actually invalid
if stake_to_withdraw > withdrawable_value {
return Err(SinglePoolError::WithdrawalTooLarge.into());
Copy link
Copy Markdown
Member

@grod220 grod220 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about we go for a ReplenishRequired error here?

Copy link
Copy Markdown
Member

@grod220 grod220 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess a large donation could also prevent a withdraw here and delay it to the next epoch?

Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more importantly a large DepositSol when that is added in the next pr, its highly unlikely this would be hit any other way (youd need a huge transfer in or a pool with nearly no value, since mev rewards are such a small fraction of stake). i plan for sol deposit to require post hoc replenish

we discussed a bit here #46, i had the idea of capping sol deposits to a % of nev to prevent the most annoying case, a large deposit that is instantly withdrawn, delaying other withdrawals an epoch. but i think jon is right to just not overcomplicate things by introducing that

essentially the problem is unavoidable in some form or another since by introducing DepositSol the pool begins selling instant activation as a service. but i also noted this is probably not that big a deal since:

  • after realizing this i decided to charge 10x more for DepositSol
  • the service on offer is instant activation, not instant deactivation. you can imagine the market paying any price for the latter during a high volatility event, but its hard to imagine something that would make people say "shit!! xyz just happened!!! i need my sol to get more illiquid NOW!!!"

}
Comment on lines +1264 to +1269
Copy link
Copy Markdown
Contributor

@joncinque joncinque Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's worth outputting some message in this case, so that people know that their funds aren't locked, and what they need to do.

That could also be addressed in the future whenever there's a WithdrawSol instruction

Copy link
Copy Markdown
Member Author

@2501babe 2501babe Apr 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

439cf91 ok i do a few things here:

  • i changed the WithdrawalTooLarge message to explicitly describe this situation, since it turns out we dont use it for anything except the impossible case
  • i changed the impossible case to a new WithdrawalViolatesPoolRequirements error
  • i added a possible case: splitting such that the pool falls below minimum, returning WithdrawalViolatesPoolRequirements. this is a helpful message for activating/active but also guards the real case of splitting out value from an inactive pool that would violate the minimum such that new users have to "donate" to make it usable again
  • i added a tokens in == 0 error, this is just a convenience since we would hit the stake out == 0 error anyway, but less cases to think about


Expand All @@ -1221,7 +1285,7 @@ impl Processor {
pool_stake_info.clone(),
pool_stake_authority_info.clone(),
stake_authority_bump_seed,
withdraw_stake,
stake_to_withdraw,
user_stake_info.clone(),
)?;

Expand All @@ -1235,9 +1299,6 @@ impl Processor {
clock_info.clone(),
)?;

let post_pool_stake = get_stake_amount(pool_stake_info)?;
msg!("Available stake post split {}", post_pool_stake);

Ok(())
}

Expand Down Expand Up @@ -1543,7 +1604,7 @@ mod tests {
test_case::test_case,
};

// approximately 6%/yr assuking 146 epochs
// approximately 6%/yr assuming 146 epochs
const INFLATION_BASE_RATE: f64 = 0.0004;

#[derive(Clone, Debug, Default)]
Expand Down
Loading
Loading