ZIP 244: Fix ill-defined commitments for shielded coinbase #587
Conversation
This file contains hidden or 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
In zcash#577 we altered ZIP 244 to have shielded signatures commit to the same data as transparent inputs, in transactions that contain transparent components. However, the edge case of shielded coinbase was not correctly handled; they contain both a consensus-required "dummy" transparent input, and binding signatures which would be required to commit to a `CTxOut` that does not exist. We resolve this by partially reverting one of the zcash#577 changes, by having S.2 for coinbase transactions be identical to T.2. This reverts binding signatures in coinbase transactions to effectively signing the transaction ID. At the same time, we also revert the same change for transactions with no transparent inputs but some transparent outputs; these also now revert to using the transaction ID for all shielded signatures (like fully-shielded transactions). The hardware wallet edge case does not apply here, as all input values are shielded and therefore directly committed to.
|
ACK |
This was referenced Jan 25, 2022
daira
added a commit
to zcash/zcash-test-vectors
that referenced
this pull request
Jan 27, 2022
Co-authored-by: Kris Nuttycombe <kris@nutty.land> Signed-off-by: Daira Hopwood <daira@jacaranda.org>
str4d
added a commit
to zcash/librustzcash
that referenced
this pull request
Feb 1, 2022
This corresponds to the ZIP 244 changes in zcash/zips#587. Closes #485.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
In #577 we altered ZIP 244 to have shielded signatures commit
to the same data as transparent inputs, in transactions that contain
transparent components. However, the edge case of shielded coinbase was
not correctly handled; they contain both a consensus-required "dummy"
transparent input, and binding signatures which would be required to
commit to a
CTxOutthat does not exist.We resolve this by partially reverting one of the #577 changes,
by having S.2 for coinbase transactions be identical to T.2. This reverts
binding signatures in coinbase transactions to effectively signing the
transaction ID.
At the same time, we also revert the same change for transactions with no
transparent inputs but some transparent outputs; these also now revert to
using the transaction ID for all shielded signatures (like fully-shielded
transactions). The hardware wallet edge case does not apply here, as all
input values are shielded and therefore directly committed to.