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

Handle errors when resulting public or private key is invalid #120

Merged
merged 8 commits into from
Mar 7, 2023

Conversation

Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Mar 2, 2023

This handles cases where the resulting public or private key after derivation is invalid (e.g., >= n, point at infinity, etc.). Previously we did not handle these situations at all. The chance of this happening is extremely low, but we need to handle it regardless to be compliant with BIP-32 and SLIP-10.

Unfortunately BIP-32 and SLIP-10 have a different way of handling this situation, so I had to refactor some code to make this possible:

  • The SLIP10Node class now accepts a "specification" field (bip32 or slip10), which determines how to handle errors.
    • This is always set to bip32 for BIP44Node.
    • If no specification is provided, we default to bip32 for secp256k1, and slip10 for ed25519.
  • Key derivation is now done sort of recursively, in that if the derivation fails, the same function is called again with the updated parameters (as described by the specification).

Related to MM-02-005.

@Mrtenz Mrtenz requested a review from a team as a code owner March 2, 2023 12:19
jest.config.js Outdated Show resolved Hide resolved
src/derivers/bip32.test.ts Outdated Show resolved Hide resolved
@Mrtenz Mrtenz force-pushed the mrtenz/handle-derivation-errors branch from 2d5ee5d to 5caa7e7 Compare March 7, 2023 10:00
* Note that `ed25519` is not supported by BIP-32, so the `bip32`
* specification is not available for this curve.
*/
readonly specification: Specification;
Copy link
Member Author

Choose a reason for hiding this comment

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

As alternative to the specification field, we could change the derivation paths to accept slip10:${index}, but that requires some refactoring.

@Mrtenz Mrtenz force-pushed the mrtenz/handle-derivation-errors branch from 5caa7e7 to 1a39169 Compare March 7, 2023 13:20
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.

2 participants