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

Insufficient stake can cause staking pool to be internally inconsistent #77

Closed
bowenwang1996 opened this issue Jul 9, 2020 · 4 comments · Fixed by #78
Closed

Insufficient stake can cause staking pool to be internally inconsistent #77

bowenwang1996 opened this issue Jul 9, 2020 · 4 comments · Fixed by #78
Assignees
Labels
bug Something isn't working

Comments

@bowenwang1996
Copy link
Contributor

Since near/nearcore#2805, if someone tries to stake less than the minimum threshold, the staking transaction will fail. This causes issues with the staking pool. For example, when some large delegator wants to withdraw all their stake, the rest of the stake left in the staking pool might be less than the minimum threshold, in which cause the unstaking will fail because it tries to restake with what is left and this causes the contract to enter an inconsistent state because from the contract perspective, unstaking succeeds. A similar issue exists if someone delegates a small amount of tokens to the pool such that the total stake in the pool is below the minimum threshold. We should check the result of the staking action to prevent the contract from entering such inconsistent state.

@bowenwang1996 bowenwang1996 added the bug Something isn't working label Jul 9, 2020
@evgenykuzyakov
Copy link
Collaborator

Let's say the account has active stake of Y, it tries to stake X, which smaller than the threshold T.

There are a few ways to address it:

  1. Address it on nearcore side. Change result of the action to success, but instead unstake everything. Issues: it maybe confusing to the user, and it maybe not the intent.
  2. Address it on the staking pool side. Add a callback to each re-staking action to restake 0, in case the staking action fails.
  3. A hack on the staking pool side. Issue 2 promises, the first is to stake 0, the second is to stake X. Assuming they both executed in the same block (in order), only the 2nd one will be reported to the epoch manager.

We can also expose minimum threshold, but it doesn't save us if the staking action arrives to the next epoch.

Overall it seems the contract has to be updated to address this issue.

@bowenwang1996
Copy link
Contributor Author

It would be nice to avoid change to nearcore as we now have to make things backward compatible.

Address it on the staking pool side. Add a callback to each re-staking action to restake 0, in case the staking action fails.

Looks promising, but doesn't restake 0 still break the contract internals? Or does that mean we unstake for every delegator?

A hack on the staking pool side. Issue 2 promises, the first is to stake 0, the second is to stake X. Assuming they both executed in the same block (in order), only the 2nd one will be reported to the epoch manager.

Same concern as above (whether the contract will be consistent after this operation)

@evgenykuzyakov
Copy link
Collaborator

Anytime the contract can be kicked out due to extra stakes from other validations, so it's fine to have less locked amount that the intended amount. But it's an issue to think the contract unstaked when it didn't. Because withdrawal actions will start to fail.

@bowenwang1996
Copy link
Contributor Author

I see. Seems like option 2 makes the most sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants