Add Orchard versioned signatures and add ALGORITHM_BYTE to ik and IssueAuthSig#129
Add Orchard versioned signatures and add ALGORITHM_BYTE to ik and IssueAuthSig#129ConstanceBeguier merged 21 commits intozsa1from
Conversation
ConstanceBeguier
changed the title
ConstanceBeguier
changed the title
ConstanceBeguier
changed the title
PaulLaux
left a comment
PaulLaux
left a comment
There was a problem hiding this comment.
overall good. added comments
| #[serde_as] | ||
| #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] | ||
| pub(crate) struct VerSpendAuthSig { | ||
| pub(crate) sighash_info: Vec<u8>, | ||
| #[serde_as(as = "[_; 64]")] | ||
| pub(crate) signature: [u8; 64], | ||
| } |
There was a problem hiding this comment.
can we import the entire struct from Orchard?
If not, the struct should be properly documented
There was a problem hiding this comment.
It is not exactly the same struct than the struct in Orchard.
In Orchard the signature is a redpallas signature and not a [u8; 64].
My implementation is inspired by the Zip32Derivation struct which differs between Orchard and librustzcash/pczt.
| 0x01, 0x01, 0x52, 0x66, 0xf9, 0xef, 0xd5, 0xcf, 0x90, 0x7d, 0x79, 0x3d, 0xab, | ||
| 0x9b, 0x1c, 0xd6, 0x4f, 0x86, 0x6b, 0x61, 0xa2, 0x98, 0x59, 0x53, 0x93, 0xf0, | ||
| 0x71, 0x53, 0xa6, 0xb9, 0x17, 0x13, 0x61, 0xa2, 0x8a, 0x53, 0x51, 0xfc, 0x49, | ||
| 0x3d, 0x15, 0x4a, 0x75, 0xe1, 0x29, 0xfe, 0xac, 0x9b, 0x67, 0x58, 0x17, 0xb8, |
There was a problem hiding this comment.
this file is different from https://github.com/QED-it/zcash-test-vectors/blob/e4ecd3ca7de56a84f742182521c708a15043c3b8/test-vectors/rust/orchard_zsa_digests.rs#L22 (the zsa1 branch)
please verify
There was a problem hiding this comment.
The content is the same but the indentation makes it hard to see.
There was a problem hiding this comment.
I removed mod orchard_zsa_digests to reduce indentation.
| #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] | ||
| pub(crate) struct VerSpendAuthSig { | ||
| pub(crate) sighash_info: Vec<u8>, | ||
| #[serde_as(as = "[_; 64]")] |
There was a problem hiding this comment.
the only similar thing we have in this project is
/// The spend authorization signature.
///
/// This is set by the Signer.
#[serde_as(as = "Option<[_; 64]>")]
pub(crate) spend_auth_sig: Option<[u8; 64]>,Is this consistent with the semantics of Option<_> for spend_auth_sig?
There was a problem hiding this comment.
Now we have a spend auth sig which contains two different fields the sighash version and the signature.
So, spend_auth_sig contains an Option and VerSpendAuthSig contains the sighash version and the signature.
It is similar to pub(crate) zip32_derivation: Option<Zip32Derivation>
The attribute #[serde_as(as = "[_; 64]")] tells serde_with to serialize and deserialize the [u8; 64] field as a fixed-size array of 64 elements, since Serde alone does not natively support arrays larger than 32 elements.
| Ok(VersionedSig::<Signature<T>>::new( | ||
| SIGHASH_V0, | ||
| Signature::from(bytes), | ||
| )) | ||
| } |
There was a problem hiding this comment.
how do we ensure that this does not break v5?
There was a problem hiding this comment.
Alternatively, we can keep this function as is and use it just for v5.
There was a problem hiding this comment.
As discussed, we will keep my implementation.
2 participants
Add
SighashVersion(version and associated data) toas defined in ZIP-246
Add
ALGORITHM_BYTEtoikandIssueAuthSigas defined in ZIP-227.Update test vectors