From c132b6fb112fd5d9149da496b138e6428cd7c91d Mon Sep 17 00:00:00 2001 From: Constance Beguier Date: Thu, 5 Mar 2026 13:59:09 +0100 Subject: [PATCH 1/5] Apply Str4d's suggestions --- src/bundle/commitments/issuance.rs | 2 +- src/circuit.rs | 2 +- src/issuance/sighash_kind.rs | 4 +++- src/keys.rs | 1 - src/note.rs | 2 +- src/note/asset_base.rs | 2 +- src/primitives/zcash_note_encryption_domain.rs | 2 -- src/sighash_kind.rs | 4 +++- 8 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/bundle/commitments/issuance.rs b/src/bundle/commitments/issuance.rs index 22eb266eb..cb3dd7ce3 100644 --- a/src/bundle/commitments/issuance.rs +++ b/src/bundle/commitments/issuance.rs @@ -44,7 +44,7 @@ pub(crate) fn hash_issue_bundle_txid_data(bundle: &IssueBundle) ind.update(note.rseed().as_bytes()); } ia.update(ind.finalize().as_bytes()); - ia.update(&[u8::from(action.is_finalized())]); + ia.update(&[action.flags().to_byte()]); } h.update(ia.finalize().as_bytes()); h.finalize() diff --git a/src/circuit.rs b/src/circuit.rs index ad22b47e0..a80ebeadc 100644 --- a/src/circuit.rs +++ b/src/circuit.rs @@ -290,7 +290,7 @@ impl VerifyingKey { /// The proving key for the Orchard Action circuit. /// -/// In the current type system, this could be a verifying key for either +/// In the current type system, this could be a proving key for either /// the original Orchard Action circuit, or the OrchardZSA circuit. #[derive(Debug)] pub struct ProvingKey { diff --git a/src/issuance/sighash_kind.rs b/src/issuance/sighash_kind.rs index bea2a77e6..58fb00529 100644 --- a/src/issuance/sighash_kind.rs +++ b/src/issuance/sighash_kind.rs @@ -8,6 +8,8 @@ use alloc::vec::Vec; /// The kind of data that a sighash commits to. /// /// This is used to implement [sighash versioning] for issuance transactions. +/// +/// [sighashversioning]: https://zips.z.cash/zip-0246#sighash-versioning #[derive(Debug, Clone, Eq, PartialEq)] pub enum IssueSighashKind { /// The "default" sighash that commits to all effecting data of the transaction, as defined in @@ -48,7 +50,7 @@ pub type BIP340IssueAuthSig = IssueSig; /// /// This helper is only intended for use in tests. #[cfg(test)] -pub fn test_sighash_info_for_kind(kind: &IssueSighashKind) -> Vec { +pub(crate) fn test_sighash_info_for_kind(kind: &IssueSighashKind) -> Vec { match kind { IssueSighashKind::AllEffecting => vec![0], } diff --git a/src/keys.rs b/src/keys.rs index 1a9f2d473..9a2f24c77 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -1,7 +1,6 @@ //! Key structures for Orchard. use alloc::vec::Vec; -use core::fmt::Debug; use core2::io::{self, Read, Write}; use ::zip32::ChildIndex; diff --git a/src/note.rs b/src/note.rs index ad8af931b..e373cffd8 100644 --- a/src/note.rs +++ b/src/note.rs @@ -360,7 +360,7 @@ impl Note { } #[cfg(test)] - pub fn has_rho(&self) -> bool { + pub(crate) fn has_rho(&self) -> bool { self.rho.is_some() } diff --git a/src/note/asset_base.rs b/src/note/asset_base.rs index 67adcbcdb..de81e73e4 100644 --- a/src/note/asset_base.rs +++ b/src/note/asset_base.rs @@ -59,7 +59,7 @@ impl<'a> AssetId<'a> { } } - /// Derives the Asset Digest for the given ZSA asset. + /// Derives the Asset Digest for this ZSA asset. /// /// Defined in [ZIP-227: Issuance of Zcash Shielded Assets][assetdigest]. /// diff --git a/src/primitives/zcash_note_encryption_domain.rs b/src/primitives/zcash_note_encryption_domain.rs index 8fc2f234c..1ca91f28e 100644 --- a/src/primitives/zcash_note_encryption_domain.rs +++ b/src/primitives/zcash_note_encryption_domain.rs @@ -1,8 +1,6 @@ //! This module implements `Domain` and `BatchDomain` traits from the `zcash_note_encryption` //! crate and contains the common logic for `OrchardVanilla` and `OrchardZSA` flavors. -// Review hint: this file is largely derived from src/note_encryption.rs - use alloc::vec::Vec; use blake2b_simd::{Hash, Params}; use group::ff::PrimeField; diff --git a/src/sighash_kind.rs b/src/sighash_kind.rs index 22277b6d2..0fe7cc4a3 100644 --- a/src/sighash_kind.rs +++ b/src/sighash_kind.rs @@ -9,6 +9,8 @@ use alloc::vec::Vec; /// /// This is used to implement [sighash versioning] for transactions containing Orchard /// bundles. +/// +/// [sighashversioning]: https://zips.z.cash/zip-0246#sighash-versioning #[derive(Debug, Clone, Eq, PartialEq)] pub enum OrchardSighashKind { /// The "default" sighash that commits to all effecting data of the transaction, as defined in @@ -52,7 +54,7 @@ pub type OrchardBindingSig = OrchardSig; /// /// This helper is only intended for use in tests. #[cfg(test)] -pub fn test_sighash_info_for_kind(kind: &OrchardSighashKind) -> Vec { +pub(crate) fn test_sighash_info_for_kind(kind: &OrchardSighashKind) -> Vec { match kind { OrchardSighashKind::AllEffecting => vec![0], } From a58de391e8a7b1ba74064dde3b906757240ca852 Mon Sep 17 00:00:00 2001 From: Constance Beguier Date: Thu, 5 Mar 2026 14:27:28 +0100 Subject: [PATCH 2/5] Fix infinite loop in arb_zsa_asset_base --- src/note/asset_base.rs | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/note/asset_base.rs b/src/note/asset_base.rs index de81e73e4..0e41d2778 100644 --- a/src/note/asset_base.rs +++ b/src/note/asset_base.rs @@ -195,6 +195,10 @@ pub mod testing { use proptest::prelude::*; + use crate::constants::fixed_bases::ZSA_ASSET_BASE_PERSONALIZATION; + use group::Group; + use pasta_curves::{arithmetic::CurveExt, pallas}; + prop_compose! { /// Generate a uniformly distributed asset base. pub fn arb_asset_base() @@ -215,22 +219,15 @@ pub mod testing { /// it is sufficient to use a random asset digest. This allows generating a random /// `AssetBase` even when `zsa-issuance` feature is disabled. pub fn arb_zsa_asset_base()( - asset_digest in any::<[u8; 64]>(), + asset_digest in any::<[u8; 64]>().prop_filter("hash_to_curve must not be identity", |digest| { + // Reject the extremely unlikely case where `hash_to_curve` returns the + // identity point. `prop_filter` makes proptest discard such inputs and + // regenerate new ones instead of failing the test. + let asset_base = pallas::Point::hash_to_curve(ZSA_ASSET_BASE_PERSONALIZATION)(digest); + !bool::from(asset_base.is_identity()) + }) ) -> AssetBase { - use crate::constants::fixed_bases::ZSA_ASSET_BASE_PERSONALIZATION; - use group::Group; - use pasta_curves::{arithmetic::CurveExt, pallas}; - - let asset_base = loop { - let asset_base = - pallas::Point::hash_to_curve(ZSA_ASSET_BASE_PERSONALIZATION)(&asset_digest); - - // Extremely unlikely, but explicitly reject the identity point. - if bool::from(!asset_base.is_identity()) { - break asset_base; - } - }; - + let asset_base = pallas::Point::hash_to_curve(ZSA_ASSET_BASE_PERSONALIZATION)(&asset_digest); AssetBase(asset_base) } } From a4c284a25016985ffc6d907af1348fbacf14aa70 Mon Sep 17 00:00:00 2001 From: Constance Beguier Date: Thu, 5 Mar 2026 14:29:34 +0100 Subject: [PATCH 3/5] Assert sighash_kind is AllEffecting in Vanilla hash_bundle_auth_data method --- src/primitives/orchard_primitives_vanilla.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/primitives/orchard_primitives_vanilla.rs b/src/primitives/orchard_primitives_vanilla.rs index 4335ad325..6dfe2dcea 100644 --- a/src/primitives/orchard_primitives_vanilla.rs +++ b/src/primitives/orchard_primitives_vanilla.rs @@ -94,6 +94,12 @@ impl OrchardPrimitives for OrchardVanilla { /// Evaluate `orchard_auth_digest` for the bundle as defined in /// [ZIP-244: Transaction Identifier Non-Malleability][zip244] /// + /// # Panics + /// + /// Panics if any signature in the bundle uses a sighash kind different from + /// `OrchardSighashKind::AllEffecting`. In Orchard v5 transactions, this is the + /// only defined sighash kind. + /// /// [zip244]: https://zips.z.cash/zip-0244 fn hash_bundle_auth_data( bundle: &Bundle, @@ -102,8 +108,16 @@ impl OrchardPrimitives for OrchardVanilla { let mut h = hasher(ZCASH_ORCHARD_SIGS_HASH_PERSONALIZATION); h.update(bundle.authorization().proof().as_ref()); for action in bundle.actions().iter() { + assert_eq!( + *action.authorization().sighash_kind(), + OrchardSighashKind::AllEffecting + ); h.update(&<[u8; 64]>::from(action.authorization().sig())); } + assert_eq!( + *bundle.authorization().binding_signature().sighash_kind(), + OrchardSighashKind::AllEffecting + ); h.update(&<[u8; 64]>::from( bundle.authorization().binding_signature().sig(), )); From 9e4a166b62a0f1db0a084ba16f494e8ab0fbc31f Mon Sep 17 00:00:00 2001 From: Constance Beguier Date: Thu, 5 Mar 2026 15:08:58 +0100 Subject: [PATCH 4/5] Fix CI: pinned quote version for no_std --- .github/workflows/ci.yml | 1 + .github/workflows/lints-stable.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 60c30ea55..a4e54ec66 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,6 +84,7 @@ jobs: run: | cargo update -p unicode-ident --precise 1.0.12 cargo update -p syn --precise 2.0.114 + cargo update -p quote --precise 1.0.44 - name: Build no_std for target working-directory: ./ci-build run: cargo build --verbose --target ${{ matrix.target }} diff --git a/.github/workflows/lints-stable.yml b/.github/workflows/lints-stable.yml index bb212f995..9bf4c8ec6 100644 --- a/.github/workflows/lints-stable.yml +++ b/.github/workflows/lints-stable.yml @@ -61,6 +61,7 @@ jobs: run: | cargo update -p unicode-ident --precise 1.0.12 cargo update -p syn --precise 2.0.114 + cargo update -p quote --precise 1.0.44 - name: Clippy no_std for target working-directory: ./ci-build run: cargo clippy --verbose --target ${{ matrix.target }} From d7a520d7914921bb2ed3f2d0057844dc0ea2ae46 Mon Sep 17 00:00:00 2001 From: Constance Beguier Date: Fri, 6 Mar 2026 10:59:31 +0100 Subject: [PATCH 5/5] Fix typo --- src/issuance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/issuance.rs b/src/issuance.rs index 79496ba6c..c268ac5b6 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -829,7 +829,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { IssueActionNotFound => { - write!(f, "the requested IssueAction not exists in the bundle.") + write!(f, "the requested IssueAction does not exist in the bundle.") } IssueBundleIkMismatchAssetBase => { write!(