Skip to content
This repository was archived by the owner on Nov 16, 2021. It is now read-only.

Replace hasher_Double with HASHER_*D #145

Merged
merged 2 commits into from
Apr 3, 2018
Merged

Replace hasher_Double with HASHER_*D #145

merged 2 commits into from
Apr 3, 2018

Conversation

saleemrashid
Copy link
Contributor

This allows us to finely control when to use a single hash or a double hash in
various places. For example, Bitcoin signatures use double SHA256, but Decred
signatures use a single BLAKE256. However, both use double hashes for Base58.

@prusnak
Copy link
Member

prusnak commented Apr 3, 2018

Looks nice. Will you be sending also PRs to trezor-mcu and trezor-core?

@saleemrashid
Copy link
Contributor Author

@prusnak See trezor/trezor-mcu#274. From what I can tell, TREZOR Core doesn't yet use Hasher, so it shouldn't be affected by this.

@prusnak
Copy link
Member

prusnak commented Apr 3, 2018 via email

@prusnak
Copy link
Member

prusnak commented Apr 3, 2018

Please fix Travis build failures.

test_speed.c:168:72: error: too few arguments to function call, expected 6, have 5
                ecdsa_get_address(node.public_key, HASHER_SHA2, 0, addr, sizeof(addr));
test_speed.c:178:106: error: too few arguments to function call, expected 9, have 8
                hdnode_public_ckd_address_optimized(&pub, root.chain_code, i, 0, HASHER_SHA2, addr, sizeof(addr), false);

Not sure if this one is the only one.

This allows us to finely control when to use a single hash or a double hash in
various places. For example, Bitcoin signatures use double SHA256, but Decred
signatures use a single BLAKE256. However, both use double hashes for Base58.
@saleemrashid
Copy link
Contributor Author

Fixed build failures, including OpenSSL 1.1 test failure.

@prusnak prusnak merged commit b904365 into trezor:master Apr 3, 2018
@prusnak
Copy link
Member

prusnak commented Apr 3, 2018

Thank you!

@saleemrashid saleemrashid deleted the decred branch April 3, 2018 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants