-
Notifications
You must be signed in to change notification settings - Fork 573
Refactor using of crypto functions #1796
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
Conversation
00bb768
to
fe85e60
Compare
4e79577
to
f2a24ea
Compare
ef0c7d6
to
3e6f7aa
Compare
self.assertEqual( | ||
sample_wif_privsigningkey, | ||
highlevelcrypto.decodeWalletImportFormat( | ||
b'5K42shDERM5g7Kbi3JT5vsAWpXMqRhWZpX835M2pdSoqQQpJMYm')) |
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.
these binary data should also be a variable in samples.
def test_double_sha512(self): | ||
"""Reproduce the example on page 1 of the Specification""" | ||
self.assertEqual( | ||
highlevelcrypto.double_sha512(b'hello'), sample_double_sha512) |
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.
ideally b'hello'
should also be a variable in samples
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.
overall nice work, although I haven't investigated it in depth |
It seems that I misplaced (or misnamed) the signatures. I was going to regenerate them. And there is also a lot of unfinished changes here, mentioned in the PR description. |
66cb016
to
b1c1401
Compare
"""Generate keys from *passphrase* and *nonce* (encoded as varint)""" | ||
priv = hashlib.sha512(passphrase + nonce).digest()[:32] | ||
pub = pointMult(priv) | ||
return priv, pub |
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'm not sure if these functions should return the pair or only the private key.
not addresses, where it's supposed to be because it uses pyelliptic.arithmetic, addresses.decodeBase58() returns int which needs to be encoded. Defined encodeWalletImportFormat() and replaced all uses.
this implementation for deterministic keys requires a passphrase of type bytes
In this branch I'm gonna cover by tests all the crypto functions used in code and do a refactoring. Basic points:
highlevelcrypto
should import frompyelliptic
highlevelcrypto
should not depend on the app state e.g. modulestate
orBMConfigparser()
instance (the same will be done forprotocol
in Compatibility tests #1788)highlevelcrypto
should include hashes - double sha512, ripemd160pyelliptic
into the separate repo to show a possible way of addressing Clarification on backport of pyelliptic requested + external dependency unbundling #886