Update to `ik` and `issueAuthSig` encoding by vivek-arte · Pull Request #170 · QED-it/orchard

vivek-arte · 2025-07-07T12:06:40Z

This PR makes the updates to the encoding of the issuance validating key and the issuance authorization signature, as done in the specification in zcash/zips#1042, along with the further updates in zcash/zips#1048 and zcash/zips#1053.

The test vectors are updated in QED-it/zcash-test-vectors#31.

what-the-diff · 2025-07-07T12:07:13Z

PR Summary

Inclusion of a New Enum in keys.rs
This is an update to include a new enumeration called IssuanceAuthSigScheme. This defines the ZIP227 signature scheme, a new system of issuing authorization.
Structural Modification of IssuanceValidatingKey
An alteration has been made to the IssuanceValidatingKey structure, where a scheme field has been incorporated and its type has been defined as IssuanceAuthSigScheme.
Alteration in to_bytes Method of IssuanceValidatingKey
The to_bytes method of IssuanceValidatingKey has been adjusted such that it can serialize the key with a scheme byte prefix. As a result, it increments the byte array size from 32 to 33.
Modification in from_bytes Method
The from_bytes method in IssuanceValidatingKey has been updated to manage the scheme byte prefix during the process of deserialization.
Tests Aligned with New Byte Array Length
The tests have been adjusted to match up with the new byte array length of issuance validating keys, and their serialization format.
Adopted Changes in asset_digest Function
The asset_digest function is updated to accept an array of size 66 in place of 65. This change reflects the modifications related to the encoding structure.
Changes in encode_asset_id Array Size
The encode_asset_id array size has been modified from 65 to 66. This aids in accommodating the additional serialization changes essential for issuance keys.

Copilot

Pull Request Overview

This PR updates the encoding of the issuance validating key (ik) and issuance authorization signature (issueAuthSig) by changing their serialization format to include a leading algorithm identifier byte, following the specification changes in zcash/zips#1042.

Updated ik encoding from 32 to 33 bytes with a 0x00 prefix byte
Updated issueAuthSig encoding from 64 to 65 bytes with a 0x00 prefix byte
Modified test vectors to reflect the new encoding format

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/test_vectors/keys.rs	Updated all test vectors for `ik` field from 32 to 33 bytes with 0x00 prefix
src/test_vectors/issuance_auth_sig.rs	Updated test vectors for both `ik` and `sig` fields with new encoding format
src/test_vectors/asset_base.rs	Updated test vectors for `key` field and corresponding `asset_base` values
src/note/asset_base.rs	Modified `asset_digest` function and `AssetBase::derive` to handle 66-byte asset IDs
src/keys.rs	Added issuance signature scheme infrastructure and updated key/signature encoding
src/issuance.rs	Added `IssuanceAuthorizationSignature` struct with scheme-aware encoding
src/bundle/commitments.rs	Updated hash computation to use new signature byte encoding

Copilot · 2025-07-17T16:19:26Z

src/issuance.rs

    CannotBeFirstIssuance,

+    /// The generation of the Issuance Authorization Signature failed.
+    IssueAuthSigGenerationFailed, //TODO: VA: This does not propagate the schnorr::Error, fix it.


The TODO comment indicates incomplete error handling. Consider either implementing proper error propagation from schnorr::Error or removing the TODO if this is intentional for now.

Copilot · 2025-07-17T16:19:26Z

src/keys.rs

+    pub fn to_bytes(&self) -> Vec<u8> {
+        let mut bytes = vec![self.scheme as u8];
+        bytes.extend(self.key.to_bytes());


Returning Vec for key serialization is inconsistent with other key types that return fixed-size arrays. Consider returning a fixed-size array like [u8; 33] for better type safety and consistency.

Suggested change

pub fn to_bytes(&self) -> Vec<u8> {

let mut bytes = vec![self.scheme as u8];

bytes.extend(self.key.to_bytes());

pub fn to_bytes(&self) -> [u8; 33] {

let mut bytes = [0u8; 33];

bytes[0] = self.scheme as u8;

bytes[1..].copy_from_slice(&self.key.to_bytes());

Copilot · 2025-07-17T16:19:27Z

src/issuance.rs

+    pub fn to_bytes(&self) -> Vec<u8> {
+        let mut bytes = vec![self.scheme as u8];
+        bytes.extend(self.signature.to_bytes());


Similar to IssuanceValidatingKey, returning Vec instead of a fixed-size array reduces type safety. Consider returning [u8; 65] for consistency with the scheme's defined signature length.

Suggested change

pub fn to_bytes(&self) -> Vec<u8> {

let mut bytes = vec![self.scheme as u8];

bytes.extend(self.signature.to_bytes());

pub fn to_bytes(&self) -> [u8; 65] {

let mut bytes = [0u8; 65];

bytes[0] = self.scheme as u8;

bytes[1..].copy_from_slice(&self.signature.to_bytes());

ConstanceBeguier

To simplify the usage of different schemes, we could create a trait with at least a signature function and a verify_signature function.

ConstanceBeguier · 2025-07-18T07:44:32Z

src/issuance.rs

+    }
+
+    /// Constructs an `IssuanceAuthorizationSignature` from a byte array.
+    pub fn from_bytes(bytes: &[u8; 65]) -> Result<Self, Error> {


Replace [u8; 65] with [u8] or Vec because the length of the sig depends on the scheme

ConstanceBeguier · 2025-07-18T07:45:37Z

src/issuance.rs

+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct IssuanceAuthorizationSignature {
+    scheme: IssuanceAuthSigScheme,
+    signature: schnorr::Signature,


I think signature should be a [u8] or Vec because we would like to have a generic implementation which might not be a Schnorr signature

ConstanceBeguier · 2025-07-18T07:46:13Z

src/issuance.rs

+    }
+
+    /// Returns the signature.
+    pub fn signature(&self) -> &schnorr::Signature {


Idem
This function should return &[u8] or &Vec

ConstanceBeguier · 2025-07-18T07:46:34Z

src/issuance.rs

+        let scheme = IssuanceAuthSigScheme::from_key_algorithm_byte(bytes[0])
+            .ok_or(IssueBundleInvalidSignature)?;
+        let signature =
+            schnorr::Signature::try_from(&bytes[1..]).map_err(|_| IssueBundleInvalidSignature)?;


The signature might not be the Schnorr signature

ConstanceBeguier · 2025-07-18T07:47:20Z

src/issuance.rs


    /// Constructs a `Signed` from a byte array containing Schnorr signature bytes.
-    pub fn from_data(data: [u8; 64]) -> Self {
+    pub fn from_data(data: [u8; 65]) -> Self {


Replace [u8; 65] with [u8] or Vec because the length of the sig depends on the scheme

ConstanceBeguier · 2025-07-18T08:02:13Z

src/keys.rs

-pub struct IssuanceValidatingKey(schnorr::VerifyingKey);
+pub struct IssuanceValidatingKey {
+    scheme: IssuanceAuthSigScheme,
+    key: schnorr::VerifyingKey,


Replace schnorr::VerifyingKey by [u8] or Vec

ConstanceBeguier · 2025-07-18T08:09:30Z

src/note/asset_base.rs

 ///
 ///    [assetdigest]: https://zips.z.cash/zip-0227.html#specification-asset-identifier-asset-digest-and-asset-base
-pub fn asset_digest(asset_id: [u8; 65]) -> Blake2bHash {
+pub fn asset_digest(asset_id: [u8; 66]) -> Blake2bHash {


asset_id could not have a fixed length because ik has not a fixed length

… bit

…ssueAuthSig, along with an assert on the length

vivek-arte · 2025-08-27T16:17:57Z

Closed in favour of #182

vivek-arte marked this pull request as draft July 7, 2025 12:08

vivek-arte force-pushed the update_to_ik_and_issueauthsig_encoding branch from 61ecd79 to 8b371c5 Compare July 8, 2025 14:36

vivek-arte mentioned this pull request Jul 12, 2025

Update encodings for ik and issueAuthSig QED-it/zcash-test-vectors#31

Merged

vivek-arte force-pushed the update_to_ik_and_issueauthsig_encoding branch from 4a25598 to a8d34b1 Compare July 17, 2025 13:28

vivek-arte marked this pull request as ready for review July 17, 2025 13:28

vivek-arte requested a review from PaulLaux July 17, 2025 13:29

PaulLaux requested review from ConstanceBeguier and Copilot July 17, 2025 16:18

Copilot AI reviewed Jul 17, 2025

View reviewed changes

ConstanceBeguier reviewed Jul 18, 2025

View reviewed changes

PaulLaux marked this pull request as draft July 22, 2025 07:53

vivek-arte force-pushed the update_to_ik_and_issueauthsig_encoding branch from a8d34b1 to 9ce9c49 Compare July 29, 2025 08:53

vivek-arte added 8 commits August 3, 2025 11:23

making the change to the encoding of ik

f64345c

updating the issueAuthSig encoding to include the algorithm selection…

41d63b3

… bit

improving imports

b6e7623

updating test vectors and reverting stopgap changes

3b1781e

WIP to improve the signature scheme specification

7705f8c

changes to to_bytes and from_bytes

879b9a1

changes to to_bytes and from_bytes in signature

9433d1b

switching to Vec<u8> as the return value of to_bytes for the ik and i…

e84fdcd

…ssueAuthSig, along with an assert on the length

vivek-arte force-pushed the update_to_ik_and_issueauthsig_encoding branch from 9ce9c49 to e84fdcd Compare August 3, 2025 06:03

fixing clippy error

2e06e43

vivek-arte closed this Aug 27, 2025

-    pub fn to_bytes(&self) -> Vec<u8> {
-        let mut bytes = vec![self.scheme as u8];
-        bytes.extend(self.key.to_bytes());
+    pub fn to_bytes(&self) -> [u8; 33] {
+        let mut bytes = [0u8; 33];
+        bytes[0] = self.scheme as u8;
+        bytes[1..].copy_from_slice(&self.key.to_bytes());

Conversation

vivek-arte commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

what-the-diff bot commented Jul 7, 2025

PR Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

vivek-arte commented Aug 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vivek-arte commented Jul 7, 2025 •

edited

Loading

ConstanceBeguier left a comment •

edited

Loading