stake-pool: Add "IncreaseAdditionalValidatorStake" instruction #3924
Conversation
joncinque
force-pushed
the
sp-reincrease
branch
from
4d5407a to
8d98aef
Compare
mergify
bot
dismissed
t-nelson’s stale review
Pull request has been modified.
stake-pool/program/src/processor.rs
Outdated
| if validator_stake_info.transient_stake_lamports > 0 { | ||
| // transient stake exists, try to merge from the source account | ||
| Self::stake_merge( | ||
| stake_pool_info.key, | ||
| source_stake_account_info.clone(), | ||
| withdraw_authority_info.clone(), | ||
| AUTHORITY_WITHDRAW, | ||
| stake_pool.stake_withdraw_bump_seed, | ||
| transient_stake_account_info.clone(), | ||
| clock_info.clone(), | ||
| stake_history_info.clone(), | ||
| stake_program_info.clone(), | ||
| )?; | ||
| } else { | ||
| // no transient stake, split | ||
| let transient_stake_bump_seed = check_transient_stake_address( | ||
| program_id, | ||
| stake_pool_info.key, | ||
| transient_stake_account_info.key, | ||
| vote_account_address, | ||
| transient_stake_seed, | ||
| )?; |
There was a problem hiding this comment.
we should move the call to check_transient_stake_address outside the if/else. as it stands, if validator_stake_info.transient_stake_lamports > 0 is true, transient_stake_account_info is never validated. the pool can be griefed by creating an unrelated stake account with the same lockup/authority to merge into instead, resulting in a miscalculation in validator_stake_info.transient_stake_lamports at the end of this function, which could have unknown downstream effects
also for the record today i learned that merging stake accounts requires them to have the same authority, the missing check baited me into partway writing a poc for loss of funds before i learned this fact 😅
edit: wait, im double stupid, i forgot until i started reviewing the decrease pr that this instruction requires a staker signature 😭
There was a problem hiding this comment.
Great catch, thanks! I don't think you could steal funds, but you could definitely mess up the pool management if you spoof this.
|
other than that, looks good! |
…a-labs#3924) * stake-pool: Add "IncreaseAdditionalValidatorStake" instruction * Address feedback * Always check transient stake account address
Problem
As brought up during the single validator manager proposal #3881, there's an annoying "attack" vector where a person puts in the minimum stake delegation at the start of an epoch and immediately increases stake on the validator, preventing later SOL deposits from being activated.
Solution
There's nothing technically stopping two activating stakes from merging, so introduce a new instruction on the stake pool program which allows for a second increase in an epoch.
The normal instruction splits into the transient account and then activates it, so this instruction: