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

feat: add keyIdentifier.derive() #99

Merged
merged 2 commits into from
May 7, 2024

Conversation

sparten11740
Copy link
Contributor

Add derive which accepts a partial derivation path and returns a new KeyIdentifier instance that has a derivation path that is extended with the supplied path indices.

Captured from https://github.com/ExodusMovement/exodus-hydra/pull/6497/files#r1575088966

Comment on lines +7 to +10
const isDerivationPath = (derivationPath) =>
typeof derivationPath === 'object' &&
Symbol.toStringTag in derivationPath &&
derivationPath[Symbol.toStringTag]() === 'DerivationPath'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to avoid instanceof - maybe worth porting to key-utils

Copy link
Contributor

@feri42 feri42 left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Collaborator

@mvayngrib mvayngrib left a comment

Choose a reason for hiding this comment

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

utACK

backwards compatible with keyIdentifier POJOs right?

@sparten11740
Copy link
Contributor Author

backwards compatible with keyIdentifier POJOs right?

yes

@sparten11740
Copy link
Contributor Author

Just to make sure I will update this where it's most crucial (address provider) and see if it breaks any of the tests

@sparten11740 sparten11740 merged commit 691e1d4 into master May 7, 2024
7 checks passed
@sparten11740 sparten11740 deleted the sparten11740/feat/expandable-key-identifier branch May 7, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants