Hierarchical Deterministic Wallet Implementation #85
Conversation
Bumps [pytest](https://github.com/pytest-dev/pytest) from 7.0.1 to 7.1.1. - [Release notes](https://github.com/pytest-dev/pytest/releases) - [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst) - [Commits](pytest-dev/pytest@7.0.1...7.1.1) --- updated-dependencies: - dependency-name: pytest dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
….1.1 Bump pytest from 7.0.1 to 7.1.1
Codecov ReportBase: 85.86% // Head: 85.76% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #85 +/- ##
==========================================
- Coverage 85.86% 85.76% -0.10%
==========================================
Files 22 22
Lines 2440 2445 +5
Branches 554 555 +1
==========================================
+ Hits 2095 2097 +2
- Misses 253 256 +3
Partials 92 92
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Thanks for working on this! I haven't tried your branch, but by looking through the code it looks nice and clean to me 👍 |
There was a problem hiding this comment.
Thanks Henry for raising this PR, this is really amazing! Looks pretty good in general. I added a few minor comments.
One thing to consider is to integrate this with the existing extended public key and signing key in PyCardano. I am thinking about adding a method to_extended_signing_key() in HDWallet. Don't feel the pressure to add it in this PR. We can do it in future PRs.
pycardano/hdwallet.py
Outdated
|
|
||
| return self | ||
|
|
||
| def from_mnemonic( |
There was a problem hiding this comment.
I guess this is better to be a classmethod?
There was a problem hiding this comment.
Thanks Jerry for taking the time to review! Yeah I was actually thinking of turning those into classmethods. I'll make the modifications.
pycardano/hdwallet.py
Outdated
| self._depth: int = 0 | ||
| self._index: int = 0 | ||
|
|
||
| def from_seed(self, seed: str) -> "HDWallet": |
There was a problem hiding this comment.
Same as from_mnemonic, this could be a classmethod.
pycardano/hdwallet.py
Outdated
| self._xprivate_key = (kL, kR) | ||
| self._public_key = A | ||
| self._chain_code = c | ||
|
|
||
| return self |
There was a problem hiding this comment.
It may be more intuitive to return a new instance of HDWallet rather than changing the current one and returning self, so people can keep keys derived from different paths at the same time. What do you think?
There was a problem hiding this comment.
This is a a good question. I was thinking of keeping derive_from_path and derive_from_index to have similar behavior - for user to call derive_from_index, they may call the function multiple times, so I was thinking of modifying the keys in place while keeping the extended keys intact. Definitely interested in chatting more on this and figure out the most intuitive implementation :)
test/pycardano/test_hdwallet.py
Outdated
| # Tests copied from: https://github.com/Emurgo/cardano-serialization-lib/blob/master/rust/src/address.rs | ||
|
|
||
| MNEMONIC_12 = "test walk nut penalty hip pave soap entry language right filter choice" | ||
| MNEMONIC_15 = "art forum devote street sure rather head chuckle guard poverty release quote oak craft enemy" |
There was a problem hiding this comment.
Does this support 24 words mnemonic?
There was a problem hiding this comment.
Yep it supports 24 words too and I tested it. I'll add in the test case.
|
One thing I forgot to mention, sorry.. since HDWallet was initially proposed in bip32, it may be a good idea to move this into the bip32 module here. |
|
The following changes were made:
|
HDWallet that supports generation of private keys, public keys and chain codes for Shelley Address. Changes include:
poycardano/hdwallet.py: HDWallet class definition and functions, including ingestion of seed, mnemonic words and entropy. Can derive keys based on index or derivation path.
test/pycardano/test_hdwallet: test and verify address generation from implementation. Test cases are copied from cardano-seriliazation-lib.
pyproject.toml/poetry.lock: add mnemonic and ecpy dependencies.