MetaMask / key-tree

MIT License
49 stars 19 forks source link

Support Cardano key derivation according to CIP3-Icarus #158

Closed PeterBenc closed 6 months ago

PeterBenc commented 9 months ago

What is the current state of things and why does it need to change?

What is the solution your changes offer and how does it work?

For this reasons

A longer explanation, also present in the code

CIP-3 defines standards for deriving keys on Cardano.

Key attributes

Are there any issues or other links reviewers should consult to understand this pull request better? For instance:

All sources are referenced in the code, and explained quite extensively, namely

Reference implementations

Mrtenz commented 7 months ago

At first glance this looks great. I will be doing a proper review next week, thanks for your patience!

PeterBenc commented 6 months ago

@Mrtenz have you had time to look into cip3? PS: all relevant links are in the PR description :)

Mrtenz commented 6 months ago

@Mrtenz have you had time to look into cip3? PS: all relevant links are in the PR description :)

Not yet, sorry about that. Will make time to look into this more this week!

Mrtenz commented 6 months ago

Can you rebase your branch?

PeterBenc commented 6 months ago

@Mrtenz the last couple of issues should be resolved. Should I squash the commits or do you want to do it yourself?

Mrtenz commented 6 months ago

src/index.ts is missing coverage(?), and you need to run yarn lint:fix. Looks good to me other than that!

PeterBenc commented 6 months ago

Should be fixed

Should I squash the commits into some reasonable chunks, would that be relevant for audit too? do you want to do it yourself? Let me know :)

Mrtenz commented 6 months ago

No need to. We want to do an audit for some other smaller changes since the last audit anyway, so I will just squash when merging this PR.