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

Also use librustzcash for signature hashes (sighash) in V4 transactions and earlier #2183

Closed
conradoplg opened this issue May 21, 2021 · 0 comments · Fixed by #2469
Closed
Labels
C-enhancement Category: This is an improvement

Comments

@conradoplg
Copy link
Collaborator

conradoplg commented May 21, 2021

Is your feature request related to a problem? Please describe.

Zebra has its own signature hash (sighash) implementation for V4 transactions and earlier. For V5, it will use librustzcash (#2051) . Therefore, we'll have two different code paths.

Describe the solution you'd like

Also use librustzcash for V4 and earlier.

Describe alternatives you've considered

We could do nothing.

Additional context

The downside of this idea is additional overhead of reserializing and deserializing with librustzcash. However, we already accepted that for V5 transactions, and it could be improved in this issue or in a later one (by e.g. converting to librustzcash while deserializing, and not when computing the signature hash, so it will only happen once).

#2051 already adds a test for comparing librustzcash sighashes with zebra sighashes, and it passes (with a small bug fix in librustzcash that is being discussed with its devs). Therefore we already have some assurance that this change won't break anything.

@conradoplg conradoplg added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels May 21, 2021
@dconnolly dconnolly changed the title Also use librustzcash for signature hashes in V4 transactions and earlier Also use librustzcash for signature hashes (sighash) in V4 transactions and earlier May 25, 2021
@teor2345 teor2345 added the P-Low label May 25, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label May 31, 2021
@mpguerra mpguerra added this to the 2021 Sprint 13 milestone Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants