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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance dkg.checkWrites() logic #1919

Closed
gsora opened this issue Mar 22, 2023 · 4 comments 路 Fixed by #1935
Closed

Enhance dkg.checkWrites() logic #1919

gsora opened this issue Mar 22, 2023 · 4 comments 路 Fixed by #1935
Assignees
Labels
enhancement New feature or request protocol Protocol Team tickets

Comments

@gsora
Copy link
Collaborator

gsora commented Mar 22, 2023

馃悶 Bug Report

Description

dkg.checkWrites() should be enhanced to prevent situations like the one described in point 4 of this document.

Previous work

#1915 tries to handle some of the edge cases at the end of the DKG process, although those should be embedded in dkg.checkWrites() since it runs before the process takes place.

@gsora gsora added the enhancement New feature or request label Mar 22, 2023
@gsora gsora self-assigned this Mar 22, 2023
@github-actions github-actions bot added the protocol Protocol Team tickets label Mar 22, 2023
@gsora
Copy link
Collaborator Author

gsora commented Mar 24, 2023

I would propose the following simple solution: if the data directory has anything except a file named charon-enr-private-key, error out and tell the user to clean up their data directory without deleting the private key.

Thoughts? @corverroos @OisinKyne

@corverroos
Copy link
Contributor

Yeah, sounds good. Please do not remove checks that check that files and directories can be created.

@gsora
Copy link
Collaborator Author

gsora commented Mar 27, 2023

Please do not remove checks that check that files and directories can be created.

Sure thing, I'll augment the flow with the added checks.

obol-bulldozer bot pushed a commit that referenced this issue Mar 27, 2023
This PR adds the `checkClearDataDir` function which checks that during the DKG process, the data directory is clear of previous run artifacts and edge cases:

 - `validator_keys` directory or file is not present
 - `cluster-lock.json` file is not present
 - `deposit-data.json` file is not present
 - `charon-enr-private-key` is present and it's a file

This way we should be safe against the edge cases we encountered in the Lido cluster set up last week.

category: bug
ticket: #1919
@gsora gsora linked a pull request Mar 27, 2023 that will close this issue
@gsora
Copy link
Collaborator Author

gsora commented Mar 27, 2023

Implemented.

@gsora gsora closed this as completed Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request protocol Protocol Team tickets
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants