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.

Sign up for GitHub

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

Jump to bottom

Remove default values for critical flags #2341

Closed
3 tasks
corverroos opened this issue Jun 20, 2023 · 0 comments · Fixed by #2380
Closed
3 tasks

Remove default values for critical flags #2341

corverroos opened this issue Jun 20, 2023 · 0 comments · Fixed by #2380
Assignees
@gsora
Labels
protocol Protocol Team tickets

Comments

@corverroos
Copy link
Contributor

🎯 Problem to be solved

Users do not understand how default values of flags work, see https://discord.com/channels/849256203614945310/1084021363406274560/1120691357766398003.

In charon create cluster, the number of validators to add is a critical decision the user should make. Defaulting to 1 doesn't mean that the user actually wants 1 validator, especially if they then provide 4 addresses.

🛠️ Proposed solution

Remove the --num-validators default value and error if 0, requesting the user explicitly provide the value.

🧪 Tests

  • Tested by new automated unit/integration/smoke tests
  • Manually tested on core team/canary/test clusters
  • Manually tested on local compose simnet
@github-actions github-actions bot added the protocol Protocol Team tickets label Jun 20, 2023
@gsora gsora mentioned this issue Jun 30, 2023
Merged
obol-bulldozer bot pushed a commit that referenced this issue Jul 3, 2023
@gsora
cmd: simplify create cluster logic (#2380)
fef2ac7 
This PR refactors `create cluster` logic to make it slightly simpler.

New behaviors:
 - if `--split-existing-keys` is enabled, `--num-validators` cannot be set
 - `--num-validators` must be present unless `--split-existing-keys` is enabled, otherwise `create cluster` exits with error
 - split key generation and disk read
 - don't fill in with random keys when splitting: during `--split-existing-keys` before this PR, if the user specified a `--num-validators` greater than the amount of keys contained in `--split-keys-dir` Charon would fill in the blanks with random keys
 - if definition file validator amount and `--num-validators` don't match, the creation process stops

category: refactor
ticket: #2341
@gsora gsora linked a pull request Jul 12, 2023 that will close this issue
cmd: simplify create cluster logic #2380
Merged
@gsora gsora closed this as completed Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gsora gsora

Labels
protocol Protocol Team tickets
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cmd: simplify create cluster logic ObolNetwork/charon
2 participants
@gsora @corverroos