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

egnkey cli functionality #71

Merged
merged 42 commits into from
Aug 13, 2024
Merged

Conversation

ricomateo
Copy link
Contributor

Description
This PR adds the following CLI commands

  • egnaddrs
  • egnkey
    • generate
    • convert
    • derive-operator-id

@ricomateo ricomateo marked this pull request as ready for review August 8, 2024 19:36
@supernovahs
Copy link
Collaborator

this needs some changes after #69 is merged

Copy link
Collaborator

@supernovahs supernovahs left a comment

Choose a reason for hiding this comment

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

naming for ecdsa keystore

crates/eigen-cli/src/args.rs Outdated Show resolved Hide resolved
crates/eigen-cli/src/generate.rs Show resolved Hide resolved
use convert::store;
use eigen_crypto_bls::error::BlsError;
use generate::KeyGenerator;
use operator_id::derive_operator_id;
use thiserror::Error;
pub mod args;
Copy link
Collaborator

Choose a reason for hiding this comment

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

separate the module declarations form imports

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done!

Copy link
Collaborator

@supernovahs supernovahs left a comment

Choose a reason for hiding this comment

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

some small nits

crates/eigen-cli/src/lib.rs Show resolved Hide resolved
crates/eigen-cli/src/lib.rs Show resolved Hide resolved
crates/eigen-cli/src/generate.rs Show resolved Hide resolved
crates/eigen-cli/src/lib.rs Outdated Show resolved Hide resolved
@supernovahs
Copy link
Collaborator

also @dralves do we want the test naming convention to be test_name or just name?

because the tests above don't use test as prefix , while we are doing that in other crates .

@dralves
Copy link
Collaborator

dralves commented Aug 9, 2024

also @dralves do we want the test naming convention to be test_name or just name?

because the tests above don't use test as prefix , while we are doing that in other crates .

good call. let's keep it consistent

@ricomateo
Copy link
Contributor Author

@supernovahs @dralves I just fixed the test naming. Should we create a new PR fixing this issue in the other crates?

Copy link
Collaborator

@supernovahs supernovahs left a comment

Choose a reason for hiding this comment

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

^

@supernovahs
Copy link
Collaborator

@supernovahs @dralves I just fixed the test naming. Should we create a new PR fixing this issue in the other crates?

yup would be better to make that for another PR . I'll do that no worries

Copy link
Collaborator

@supernovahs supernovahs left a comment

Choose a reason for hiding this comment

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

^

@supernovahs
Copy link
Collaborator

Blocked by #69

@supernovahs
Copy link
Collaborator

@ricomateo this is now unblocked, has some conflicts from bls crate sorry

@supernovahs supernovahs merged commit 80921c9 into Layr-Labs:main Aug 13, 2024
1 check failed
@pablodeymo pablodeymo deleted the cmd-egnkey branch August 22, 2024 15:25
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.

5 participants