Prevent withrawing Initialized stake account to rent-exempt reserve #17366
Conversation
|
@t-nelson , I did examine all the places in the Stake program where we ensure resulting accounts have balances > rent-exempt-reserve, as we discussed. But I didn't see a great way to unify the logic. This seemed low churn and the most consistent with the withdraw handling for Delegated account. |
CriesofCarrots
added
the
automerge
Codecov Report
@@ Coverage Diff @@
## master #17366 +/- ##
=======================================
Coverage 82.7% 82.7%
=======================================
Files 424 424
Lines 118353 118402 +49
=======================================
+ Hits 97906 97957 +51
+ Misses 20447 20445 -2 |
(cherry picked from commit 91f2b61) # Conflicts: # programs/stake/src/stake_instruction.rs
|
I think a similar problem is faced with splitting accounts. To combat this - rather than go for the somewhat confusing reliance on credits_observed as a indirect proxy to check if balances are too small, one can ensure that split results in accounts with at least 1 SOL staked. Having at least 1 SOL staked would combat stake fragmentation too. Not sure if this direction is desirable and should be applied uniformly or would cause churn in current deployment. |
@jon-chuang I do not think it is possible to generate a 0-stake account via |
|
I think I've probably made a mistake - the 1 in the code is 1 lamport, not 1 SOL - there are plenty of stakers presently w/ < 1 SOL. Also you're probably right but I'll go and double check. |
…backport #17366) (#17370) * Prevent withrawing Initialized stake account to zero stake (#17366) (cherry picked from commit 91f2b61) # Conflicts: # programs/stake/src/stake_instruction.rs * Fix conflicts Co-authored-by: Tyera Eulberg <teulberg@gmail.com> Co-authored-by: Tyera Eulberg <tyera@solana.com>
|
Nice! I had been wondering about this for some time |
| @@ -1248,8 +1250,13 @@ impl<'a> StakeAccount for KeyedAccount<'a> { | |||
| StakeState::Initialized(meta) => { | |||
| meta.authorized | |||
| .check(&signers, StakeAuthorize::Withdrawer)?; | |||
| let reserve = if prevent_withdraw_to_zero { | |||
| checked_add(meta.rent_exempt_reserve, 1)? // stake accounts must have a balance > rent_exempt_reserve | |||
|
This PR has been automatically locked since there has not been any activity in past 14 days after it was merged. |
mergify
bot
removed
the
automerge
Problem
It is possible to end up with a Delegated stake account with 0 delegated stake by withdrawing an Initialized account to exactly the rent-exempt reserve and then delegating. This is confusing to users (is it activating? can it be redelegated?), and defies our intention that every stake account have more lamports than the rent-exempt reserve.
Summary of Changes
Bump
reservefor Initialized stake accounts by 1 lamportAlso flesh out testing of withdrawals from Initialized accounts