stake-pool: Remove lamport minimum for validator removal by staker #3999
Conversation
stake-pool/program/src/processor.rs
Outdated
| let required_delegation = minimum_delegation(stake_minimum_delegation) | ||
| .saturating_mul(LAMPORT_BUFFER_FACTOR) | ||
| .saturating_add(meta.rent_exempt_reserve); | ||
| if stake.delegation.stake > required_delegation { |
There was a problem hiding this comment.
the intent of this and the similar calculation at 1172-1174 are very confusing to me
without this pr, required_lamports = MINSTAKE + RENT and required_delegation = MINSTAKE. the two checks essentially confirm (i flip the signs to indicate success conditions) that:
stake_lamports <= required_lamports: the stake account has at most the minimum non-stake lamports for rent-exemption (kinda sorta... this assumes we have exactlyMINSTAKEstake, but the only case where thats not true is if the variable is increased out from under us)stake.delegation.stake <= required_delegation: the stake account has at most the minimum stake
now required_lamports = 2(MINSTAKE + RENT) and required_delegation = 2MINSTAKE + RENT. multiplying the rent by two in the first calculation seems like a mistake, which makes the constraint looser by RENT than intended, in which case the two variables maybe should be equal
but is this actually what we want to check? i think we actually care that there are no loose lamports in the account, ie stake_lamports == stake.delegation.stake + RENT
for the second calculation, why do we add the rent to compare to the stake? we expect to see at most 2MINSTAKE stake, because the lamps for rent-exemption dont show up in this number. maybe im missing something because the comment indicates this is on purpose tho...
but stepping back a bit... do we even need any of this? does anything bad happen if you remove a validator with extra stake or extra lamports in it? it all just goes back to the reserve either way, doesnt it? or is this to incentivize redelegation? but this code predates the redelegate instruction? i guess when i think about it i dont know what any of its for
There was a problem hiding this comment.
now required_lamports = 2(MINSTAKE + RENT) and required_delegation = 2MINSTAKE + RENT. multiplying the rent by two in the first calculation seems like a mistake, which makes the constraint looser by RENT than intended, in which case the two variables maybe should be equal
I thought so too at first, but in order to decrease a validator's stake, you need to be able to split a minimum stake from it, while maintaining minimum stake in the validator stake account. So that means you need to split minimum_delegation + stake_rent, while maintaining minimum_delegation + stake_rent in the source account, which means it needed 2 * (minimum_delegation + stake_rent)
but stepping back a bit... do we even need any of this? does anything bad happen if you remove a validator with extra stake or extra lamports in it? it all just goes back to the reserve either way, doesnt it?
This is an excellent point. The checks were from the times when the staker would put up the lamports for rent-exemption + minimum_delegation, so that the staker couldn't reclaim more than they put in. Now that everything comes from the reserve, there's no reason to keep this so complicated, and instead simplify the check to "if there's transient stake, make sure it's deactivating"
joncinque
changed the title
|
Completely removed the check, thanks for noticing! |
…olana-labs#3999) * stake-pool: Relax lamport minimum for validator removal by staker * Completely remove the lamport check
Problem
Deactivating a validator requires reducing its active stake to the minimum delegation first. However, the funds all go back to the reserve anyway, so there's no reason to gate the removal to some lamport amount.
Solution
Remove the lamport check.
Fixes #3994