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

hdkeychain: Use direct hashes and remove dcrutil dep #2086

Merged
merged 1 commit into from
Feb 17, 2020

Conversation

jrick
Copy link
Member

@jrick jrick commented Feb 17, 2020

This change replaces usage of chainhash (which is intended solely for
transaction and blocks) and dcrutil.Hash160 (used in scripts for
hashing redeem scripts and pubkeys) with direct calls to each hashing
function from dcrd's crypto subpackages. This improves clarity and
shortens the transitive package imports. Notably, dcrutil and
everything it depends on is no longer transitively imported when
importing the hdkeychain package.

@jrick
Copy link
Member Author

jrick commented Feb 17, 2020

Adding a hash160 function with the crypto packages could make sense too, but it's so small I don't see any reason to not copy it.

@jrick
Copy link
Member Author

jrick commented Feb 17, 2020

I'm also aware that dcrutil is still imported by test code in the hdkeychain package, but this has no effect on code importing hdkeychain, besides that it is another module version to consider when solving versions.

This change replaces usage of chainhash (which is intended solely for
transaction and blocks) and dcrutil.Hash160 (used in scripts for
hashing redeem scripts and pubkeys) with direct calls to each hashing
function from dcrd's crypto subpackages.  This improves clarity and
shortens the transitive package imports.  Notably, dcrutil and
everything it depends on is no longer transitively imported when
importing the hdkeychain package.
@davecgh davecgh merged commit 64fee51 into decred:master Feb 17, 2020
@jrick jrick deleted the hashes branch February 17, 2020 01:10
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.

None yet

2 participants