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

Validators can prevent their removal by closing their fee account #400

Open
ruuda opened this issue Sep 1, 2021 · 1 comment
Open

Validators can prevent their removal by closing their fee account #400

ruuda opened this issue Sep 1, 2021 · 1 comment

Comments

@ruuda
Copy link
Contributor

ruuda commented Sep 1, 2021

As reported by Neodyme:

  • The token program has a CloseAccount instruction. This can be used by the account owner to delete this account. (And maybe afterwards use the same address for something else). In your case this could be an issue, if the developer or treasury account are deleted, because the fee-transferral will fail, which will DOS your contract. Question: who will control the treasury account?
  • Since you pay out validator-rewards asyncronously, there is no DOS issue when closing reward accounts. But it can be used to prevent a validator from being removed (as he will be stuck with an internal balance > 0 which nobody can withdraw)

For the developer and treasury account, we control them, so we just shouldn’t close them. But for the validator fee account, a validator can indeed close the fee account, and this would prevent removal.

This is not a problem for the Lido logic, because:

  • We still prevent delegations to the validator.
  • We can still unstake everything from the validator.

So the validator will be effectively removed, it’s just that the entry will remain in the Solido state and we can’t perform the final cleanup. I think it is fine to live with this for now, because it is mostly a cosmetic issue, and I prefer to keep the code as simple as possible over introducing complications for handling a hypothetical scenario.

If this does become problematic in the future, we can deploy an upgrade and either:

  • Allow the manager to remove even if there are unclaimed fees.
  • Allow the manager to change the fee address.
@enriquefynn
Copy link
Member

enriquefynn commented Sep 3, 2021

Allow the manager to remove even if there are unclaimed fees.

I think this is the best solution, and it doesn't make things more complicated.

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

No branches or pull requests

2 participants