Make transparent signatures commit to all input amounts & scripts. #472
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nuttycom
force-pushed
the
zip_244/hw_wallet_commitments
branch
2 times, most recently
from
cf8a31c
to
7fcad74
Compare
nuttycom
force-pushed
the
zip_244/hw_wallet_commitments
branch
5 times, most recently
from
3e984cf
to
f38f5a4
Compare
Codecov Report
@@ Coverage Diff @@
## master #472 +/- ##
==========================================
+ Coverage 50.20% 50.57% +0.37%
==========================================
Files 88 88
Lines 7962 8002 +40
==========================================
+ Hits 3997 4047 +50
+ Misses 3965 3955 -10
Continue to review full report at Codecov.
|
nuttycom
force-pushed
the
zip_244/hw_wallet_commitments
branch
3 times, most recently
from
c808f36
to
c66bb89
Compare
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
daira
reviewed
Jan 14, 2022
| @@ -5985,39 +5986,36 @@ pub mod zip_0244 { | |||
| 0xdc, 0x7f, 0x83, 0x46, 0xb2, 0x82, 0xec, 0x04, 0xb8, 0x40, 0xf2, 0x45, 0x1d, | |||
| 0x09, 0xc0, 0x86, 0x3c, 0xda, 0x6b, | |||
| ], | |||
There was a problem hiding this comment.
I have not checked the test data changes.
daira
requested changes
Jan 14, 2022
There was a problem hiding this comment.
https://github.com/zcash/librustzcash/pull/472/files/c66bb892baf7e54437be7341c163f40909a64b43#r785116088 (the incorrect comment in sighash_v5::transparent_sig_digest) is blocking.
nuttycom
force-pushed
the
zip_244/hw_wallet_commitments
branch
from
e666cb4
to
9baceba
Compare
|
The refactoring of array hashing needed a rustfmt, sorry about that. |
nuttycom
force-pushed
the
zip_244/hw_wallet_commitments
branch
from
9baceba
to
8cf5c8c
Compare
Transparent UTXOs are referred as pair of previous txid and index. In order to verify UTXO's amount in HWW (hardware wallet), whole previous transaction containing this UTXO must be streamed into the device. This increases complexity of signing process significantly. zcash/zips#574 identifies this problem and suggests a modification to ZIP-244 to resolve this issue, by adding three new fields to section S.2 of the signature hash.
Co-authored-by: Daira Hopwood <daira@jacaranda.org>
nuttycom
force-pushed
the
zip_244/hw_wallet_commitments
branch
from
8cf5c8c
to
eaa3ec5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the changes to ZIP 244 sighash rules specified in zcash/zips#577 .
Transparent UTXOs are referred to as a pair of previous txid and index. In order to verify a UTXO's amount, e.g. in a hardware wallet, under the previous rules the whole previous transaction containing this UTXO would have needed to be streamed into the device. This would have increased the complexity of signing process significantly, but is not necessary under the new rules.
Fixes #473