-
Notifications
You must be signed in to change notification settings - Fork 123
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
Refactor key derivation #1309
Merged
Merged
Refactor key derivation #1309
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
96c9add
add PrivateKey.fromSeed method and write tests for SLIP 10 compatibility
dikel 5671277
chore: add BIP39 test vector
dikel 4da8097
feat: add initial support for ECDSA derivation (currently not working)
dikel d52e896
feat: add support for ECDSA key derivation
dikel 25524ec
chore: small fixes
dikel d7688f2
chore: update CHANGELOG.md
dikel 33a874e
chore: fix code smell
dikel 0351dcb
chore: fix code smells
dikel 836122a
create Bip32Utils class
dikel 055c763
merge develop
dikel fa03bb9
chore: make the hardened bit a constant
dikel d57074c
chore: fix code smells
dikel 3aa2e7b
fix: small things based on feedback
dikel f09c91a
revert some changes
dikel 9ebaa3d
chore: add more test vectors for mnemonic
dikel 587e3fa
Merge develop branch
dikel 5c156be
fix code smells
dikel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it would make sense to have versions of these
toStandard
functions with default passphrase and indextoStandard(passphrase)
(using index 0)toStandard()
(using no passphrase and index 0)toStandard(index)
(using no passphrase) << this one might be overkill, thoughThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We thought about having multiple versions of the
toStandard
methods but we decided it's better to have only one, because it cannot be done in the other SDKs. JS and Go don't support overloading of methods.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fine with that logic, but this really needs to be folded into the standard proposal.
For example, I know the C++ SDK effort has already implemented the proposal as it currently exists, with default parameters.