Conversation
ConstanceBeguier
force-pushed
the
sighash_version
branch
from
0b60023 to
5d2a8df
Compare
ConstanceBeguier
force-pushed
the
sighash_version
branch
from
01d36ca to
527a70b
Compare
PaulLaux
left a comment
PaulLaux
left a comment
There was a problem hiding this comment.
Added questions,
Also, I'm concerned that this is not connected to the test-vectors we produce. Is there a simple way to connect to the test-vectors?
|
|
||
| /// No version (used for Sapling and TXv5 compatibility). | ||
| /// TXv5 does not require the sighash versioning bytes. | ||
| NoVersion = u8::MAX, |
There was a problem hiding this comment.
this looks unused.
Also, can you brifly explain the backward compatibility stratagy here?
There was a problem hiding this comment.
The NoVersion is used in librustzcash when we read V3/V4/V5 transactions.
The backward compatibility strategy is only done in librustzcash.
In sapling repo, all signatures (spend auth and binding) have a version.
In librustzcash, we manage backward compatibility by
- not writing signature version only for V6 transactions
- filling the version with NoVersion when reading signatures in V3/V4/V5 transactions
| let spend_auth_sig = spend_auth_sig | ||
| .as_ref() | ||
| .map(|(version, sig)| { | ||
| let version = sapling_sighash_version_from_u8(*version) | ||
| .ok_or(ParseError::InvalidSighashVersion)?; | ||
| let sig = redjubjub::Signature::from(*sig); | ||
| Ok(VerSpendAuthSig::new(version, sig)) | ||
| }) | ||
| .transpose()?; |
There was a problem hiding this comment.
is this backward compatible? If not, let's discuss
There was a problem hiding this comment.
It is not backward compatible because we add the version for every spend auth signature in PCZT
|
We will only modify librustzcash repo |
2 participants
Add
SighashInfo(version and associated data) to