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

EIP-2335: Keystore #1071

Merged
merged 69 commits into from
May 11, 2020
Merged

EIP-2335: Keystore #1071

merged 69 commits into from
May 11, 2020

Conversation

paulhauner
Copy link
Member

@paulhauner paulhauner commented Apr 28, 2020 •
edited
Loading

Issue Addressed

Which issue # does this PR address?

Proposed Changes

Implements EIP-2335 for BLS keystores.

Builds upon @pawanjay176's PRs:

Additional Info

N/A

pawanjay176 and others added 28 commits January 8, 2020 00:21
@pawanjay176
Add test to understand flow of key storage
7a336c6
@pawanjay176
First commit
44e4e29
@pawanjay176
Committing to save trait stuff
4dd39e4
@pawanjay176
Working naive design
cc56ac8
@pawanjay176
Add keystore struct
f668763
@pawanjay176
Move keystore files into their own module
dfbb40a
@pawanjay176
Add serde (de)serialize_with magic
d6204a5
@pawanjay176
Add keystore test
9716c41
@pawanjay176
Fix tests
d801316
@pawanjay176
Add comments and minor fixes
30027b1
@pawanjay176
Pass optional params to to_keystore function
5f4dddf
@pawanjay176
Add path field to keystore
3fe10a7
@pawanjay176
Add function to read Keystore from file
3238325
@pawanjay176
Add test vectors and fix Version serialization
b78c9fd
@pawanjay176
Checksum params is empty object
7ebe307
@pawanjay176
Add public key to Keystore
9bb8647
@pawanjay176
Add function for saving keystore into file
f8e7f57
@pawanjay176
Deleted account_manager main.rs
2777b49
@pawanjay176
Move keystore module to validator_client
ae631d4
@pawanjay176
Add save_keystore method to validator_directory
9c366eb
@pawanjay176
Add load_keystore function. Minor refactorings
4e1a0d0
@pawanjay176
Fixed dependencies
a56e5a3
@pawanjay176
Address some review comments
3a93b98
@pawanjay176
Add Password newtype; derive Zeroize
1933b75
@pawanjay176
Fix test
7fbc971
@paulhauner
Merge branch 'master' into keystore
d420845
@paulhauner
Move keystore into own crate
fbb4984
@paulhauner
Merge branch 'master' into keystore
5b67bfd
@paulhauner paulhauner added work-in-progress PR is a work-in-progress may-sec-review labels Apr 28, 2020
@paulhauner
Copy link
Member Author

All comments addressed, thank you for the detailed review! ❤️

@paulhauner paulhauner changed the title Keystore EIP-2335: Keystore May 7, 2020
@paulhauner
Copy link
Member Author

I had to make a change to split apart and make-public the encrypt and decrypt functions so I could so some stuff upsteam, sorry about the noise.

@paulhauner
Copy link
Member Author

I also realized the Password struct is unnecessary, we can just always accept the password as a &[u8] without forcing the user to use our Zeroize wrapper. Sorry, I'm finding out this stuff as I implement upstream things.

@paulhauner paulhauner mentioned this pull request May 7, 2020
Merged
kirk-baird
kirk-baird requested changes May 11, 2020
View reviewed changes
Copy link
Member

@kirk-baird kirk-baird 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, a couple more parameters that I think we should check against.

eth2/utils/eth2_keystore/src/keystore.rs Outdated Show resolved Hide resolved
eth2/utils/eth2_keystore/src/keystore.rs Outdated Show resolved Hide resolved
eth2/utils/eth2_keystore/src/keystore.rs Show resolved Hide resolved
eth2/utils/eth2_keystore/tests/json.rs Show resolved Hide resolved
@paulhauner
Copy link
Member Author

All comments addressed, thank you!

kirk-baird
kirk-baird approved these changes May 11, 2020
View reviewed changes
Copy link
Member

@kirk-baird kirk-baird 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!!

Assuming all CI checks pass I'm happy :)

@paulhauner paulhauner merged commit f30271e into master May 11, 2020
@paulhauner paulhauner deleted the keystore branch May 11, 2020 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kirk-baird kirk-baird kirk-baird approved these changes

Assignees
No one assigned
Labels
ready-for-review The code is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@paulhauner @kirk-baird @pawanjay176