Skip to content
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.

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

stake-pool: Add / remove validators from the reserve #3714

Merged
merged 17 commits into from
Nov 21, 2022

Conversation

joncinque
Copy link
Contributor

@joncinque joncinque commented Oct 14, 2022

Problem

Currently, stake pool stakers add validators to the pool using a minimum delegation of ~0.003 SOL from their funds, rather than using the reserve. This was an easy decision at the outset, before there was even a reserve. The staker can recover those funds by removing validators from the pool.

When the minimum delegation jumps from 1 lamport to 1 SOL, the stake pool program must respect that minimum in order to keep stakes usable. This means that practically speaking, the pool is stealing 0.999 SOL from the depositors for each validator added to the pool. That stinks.

Solution

The solution is very simple on the outset, but involves some pretty big changes in the accounting of stake pools and states of the system. The overall gist is to add and remove validators from the pool reserve rather than some external funder. To make this work, we need the following:

  • BREAKING: force adding validators to the pool to come from the reserve
  • BREAKING: removing validators simply deactivates the stake account rather than splitting into a destination
  • update the stake pool accounting to not take the minimum into account, since all of the lamports into the account now belong to the pool. Before, it would ignore what was provided from the outside.

This is all well and good. But it opens up a new attack -- a malicious manager can lock depositor funds by creating loads of new stake accounts from the reserve, since they can't withdraw below the minimum delegation. Which means we have to:

  • give users the ability to remove validators from the pool if they can't withdraw from active / transient stakes. this way they can recover their funds from malicious managers

This opens up another attack vector! Validator stake accounts exist on a specific PDA which uses the validator vote account and the stake pool address as seeds. When a stake account is deleted, it actually sticks around for the duration of the transaction, and anyone can resurrect it if they want. Since now anyone can remove a stake pool's accounts under certain circumstances, a malicious user can resurrect the deleted stake account and make it impossible for that pool to ever re-add that validator. That means we have to:

  • BREAKING: add an additional seed parameter to the instruction which adds a validator to the pool. This repurposes unused data in the current validator list entries, splitting an unused u64 into two u32s. This is safe to do and doesn't break current pools, only the instruction is breaking.

After that, make sure this change is reflected everywhere:

  • Update the CLI
  • Update the Python bindings
  • Update the docs and scripts

Since the 1 SOL minimum delegation breaks stake pools, the choice here is to make some breaking changes for the benefit of depositors.

Note that this change will also be audited since it touches some sensitive code and could open up other unknown attack vectors. For example, it's still possible for a malicious pool owner to keep user funds in limbo by constantly rebalancing the minimum amount between validators.

Note 2: the commit history is intentional, and can be reviewed piece by piece. I had to go backwards for two little things that didn't work as expected.

Bonus: added token-2022 support!

@joncinque joncinque force-pushed the sp-reserve branch 2 times, most recently from 762f6a5 to 7c853f4 Compare October 18, 2022 03:18
@2501babe
Copy link
Member

i spent most of today reviewing existing staking and stake pool documentation, since i didnt have any familiarity with that already. kinda information overloaded now but should be ready to do this next week

@joncinque joncinque merged commit fa438bb into solana-labs:master Nov 21, 2022
@joncinque joncinque deleted the sp-reserve branch November 21, 2022 19:13
HaoranYi pushed a commit to HaoranYi/solana-program-library that referenced this pull request Jul 19, 2023
* stake-pool: Create validator stake accounts from reserve

* Update accounting to take minimum into account

* Refactor withdraw tests

* Add ability to remove validators during withdraw stake

* Add seed to validator stake account address derivation

* Update CLI with new instruction formats

* Update documentation and helper scripts for new funding

* Update Python bindings

* Try to fix flakey test

* Support token-2022

* Condense tests for CI

* Reduce huge pool limit post-rebase

* Avoid draining the whole reserve

* Restrict account extensions on allow-list

* Update comments to say "lamports"

* Fixup code comments and variable names

* Fix clippy in tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants