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

A Node Operator can circumvent DAO validator key approval #141

Open
skozin opened this issue Nov 15, 2020 · 4 comments
Open

A Node Operator can circumvent DAO validator key approval #141

skozin opened this issue Nov 15, 2020 · 4 comments
Labels
bug Something isn't working
Milestone

Comments

@skozin
Copy link
Member

skozin commented Nov 15, 2020

Adding validator keys is performed as follows:

  1. A node operator appends new keys to its set; the node operator's staking limit stays the same, making the added keys unusable yet.
  2. The DAO performs off-chain verification of the submitted keys: it checks that withdrawal credentials, deposit amounts, and signatures are correct and that there are no duplicate keys in the updated key set.
  3. The DAO votes to increase the node operator's limit to allow the added keys to be used by the pool.

However, a node operator can currently add keys that were not approved by the DAO using a simple trick:

  1. A node operator submits N correct keys.
  2. The DAO validates them and sets stakingLimit = N
  3. The node operator submits M <= N more (bad) keys and removes the first M keys.
  4. As a result, stakingLimit is still N, and the newly-added keys are allowed to be used in staking despite not being approved by the DAO.

Moreover, currently, key removal is implemented by copying the last key to the storage slot previously occupied by the removed key, so simply decreasing the staking limit won't help. We need to either shift all keys to the left or mark individual keys as validated instead of using an index pointer.

Failing test scenario: #142

@vshvsh
Copy link
Contributor

vshvsh commented Nov 15, 2020

I think we can leave it for the after-release. Node operators can technically make system behave erroneously, but they's a very convoluted process that can't be done by mistake, and NO set is permissioned, professional and has no immediate way to profit from that.

@vshvsh vshvsh modified the milestones: Testnet, after-release Nov 15, 2020
@skozin
Copy link
Member Author

skozin commented Nov 15, 2020

Yep, I agree that, currently, we have more important issues to fix.

@rkolpakov
Copy link

Closed at #478

@TheDZhon TheDZhon reopened this Mar 27, 2023
@TheDZhon
Copy link
Contributor

Decided to re-open the issue because some more sophisticated approaches still exist.

For instance, suppose that steps 2, 3, and 4 are reordered as 3, 4, 2 by leveraging Easy Track motions front-running.

It can be tolerable with the curated set of node operators.

Would require additional Easy Track factory for the upcoming validators sets, and also can be addressed using the https://eips.ethereum.org/EIPS/eip-2537 (expected in the Cancun hardfork)

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

No branches or pull requests

5 participants
@TheDZhon @skozin @vshvsh @rkolpakov and others