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

Key Management and Config File for Validator Onboarding #1990

Merged
merged 43 commits into from
Nov 9, 2023

Conversation

dailinsubjam
Copy link
Contributor

@dailinsubjam dailinsubjam commented Nov 3, 2023

This PR:

  • Added a struct for ValidatorConfigFile and ValidatorConfig.
  • Added a flow from reading config seed of file to generating a node's own key pairs.
  • Make sure key generation only appear in test initialization or config initialization.
  • Added a script to generate key pair. The steps are in Scripts for generating key pair #1971.

Designs:

  • The config file for validators contains two parts: seed & node_id. They together will server as a final seed for key generation. They can be used either for input of scripts for key generation or validator's config for test. The reason that it's not directly private key & public key is that the form of the keys are very complex (seeconfig/ValidatorConfigExample for reference), it would be better to generate it rather than read it from config file, and also our HotshotConfig does contain the secret key originally for signning, so it's fine.
  • An parameter added for gen_launcher like gen_launcher::<TestTypes, MemoryImpl>(0) is to identify node_id for key pair (pk & sk) generation, since build_system_handle() in crates/testing/src/task_helpers.rs needs launcher with different node id.

Main review parts: orchestrator/src/config.rs, crates/types/src/lib.rs.

dailinsubjam and others added 30 commits October 12, 2023 15:47
…eer_validators

set other nodes' default pub key and stake value during initialization
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ dailinsubjam
❌ Sishan Long


Sishan Long seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@dailinsubjam dailinsubjam changed the title Key Management for Validator + Config Files Key Management and Config File for Validator Onboarding Nov 3, 2023
@dailinsubjam dailinsubjam linked an issue Nov 3, 2023 that may be closed by this pull request
4 tasks
bfish713
bfish713 previously approved these changes Nov 8, 2023
Copy link
Collaborator

@bfish713 bfish713 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM!

@bfish713 bfish713 self-requested a review November 9, 2023 14:45
Copy link
Collaborator

@rob-maron rob-maron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me !

@@ -54,12 +54,9 @@ impl BLSPrivKey {
}
}

#[allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this as long as it still works as intended ! I wonder why we implemented it this way to begin with

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also wondering about that. In the beginning the implementation on this was very long ago, and later implementation just follow that. There are more discussions on whether we want to keep this on zullip.

let config_file_as_string: String = fs::read_to_string(config_file.as_str())
.unwrap_or_else(|_| panic!("Could not read config file located at {config_file}"));
let config_toml: NetworkConfigFile =
toml::from_str::<NetworkConfigFile>(&config_file_as_string)
let config_toml: NetworkConfigFile<TYPES::SignatureKey> =
Copy link
Collaborator

@rob-maron rob-maron Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is clean! Love it

@dailinsubjam dailinsubjam merged commit a5fcb77 into develop Nov 9, 2023
4 of 6 checks passed
@dailinsubjam dailinsubjam deleted the sishan/merge_validator_onboarding_after_pull branch November 9, 2023 21:17
@dailinsubjam dailinsubjam self-assigned this Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validator Onboarding Macro Task
4 participants