Enhance and cleanup ivk-to-bytes-visibility-downgrade branch (#81)

Added burn validation, fixes and minor additions. 
Bumped Rust version to 1.65

---------

Co-authored-by: alexeykoren <>
Co-authored-by: Dmitry Demin <dmitry@qed-it.com>
Co-authored-by: Paul <3682187+PaulLaux@users.noreply.github.com>

.circleci/config.yml

@@ -12,7 +12,7 @@ jobs:
     # Specify the execution environment. You can specify an image from Dockerhub or use one of our Convenience Images from CircleCI's Developer Hub.
     # See: https://circleci.com/docs/2.0/configuration-reference/#docker-machine-macos-windows-executor
     docker:
-      - image: cimg/rust:1.61.0
+      - image: cimg/rust:1.65.0
     # Add steps to the job
     # See: https://circleci.com/docs/2.0/configuration-reference/#steps
     steps:

.github/workflows/bench.yml

@@ -16,7 +16,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: 1.61.0
+          toolchain: 1.65.0
           override: true
       - name: Run benchmark
         run: cargo bench -- --output-format bencher | tee output.txt

.github/workflows/ci.yml

@@ -14,7 +14,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: 1.61.0
+          toolchain: 1.65.0
           override: true
       - name: Run tests
         uses: actions-rs/cargo@v1
@@ -46,7 +46,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: 1.61.0
+          toolchain: 1.65.0
           override: true
       # Build benchmarks to prevent bitrot
       - name: Build benchmarks
@@ -62,7 +62,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: 1.61.0
+          toolchain: 1.65.0
           override: true
       - name: Setup mdBook
         uses: peaceiris/actions-mdbook@v1
@@ -106,7 +106,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: 1.61.0
+          toolchain: 1.65.0
           override: true
       - name: cargo fetch
         uses: actions-rs/cargo@v1
@@ -129,7 +129,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: 1.61.0
+          toolchain: 1.65.0
           override: true
       - run: rustup component add rustfmt
       - uses: actions-rs/cargo@v1

.github/workflows/lints-stable.yml

@@ -5,19 +5,19 @@ on: pull_request
 
 jobs:
   clippy:
-    name: Clippy (1.61.0)
+    name: Clippy (1.65.0)
     timeout-minutes: 30
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: 1.61.0
+          toolchain: 1.65.0
           components: clippy
           override: true
       - name: Run Clippy
         uses: actions-rs/clippy-check@v1
         with:
-          name: Clippy (1.61.0)
+          name: Clippy (1.65.0)
           token: ${{ secrets.GITHUB_TOKEN }}
           args: --all-features --all-targets -- -D warnings

Cargo.toml

@@ -9,7 +9,7 @@ authors = [
     "Kris Nuttycombe <kris@electriccoin.co>",
 ]
 edition = "2021"
-rust-version = "1.61.0"
+rust-version = "1.65"
 description = "The Orchard shielded transaction protocol"
 license-file = "LICENSE-BOSL"
 repository = "https://github.com/zcash/orchard"
@@ -25,7 +25,7 @@ rustdoc-args = ["--cfg", "docsrs", "--html-in-header", "katex-header.html"]
 [dependencies]
 aes = "0.8"
 bitvec = "1"
-blake2b_simd = "1"
+blake2b_simd = "=1.0.1" # Last version required rust 1.66
 ff = "0.13"
 fpe = "0.6"
 group = { version = "0.13", features = ["wnaf-memuse"] }
@@ -35,39 +35,35 @@ hex = "0.4"
 lazy_static = "1"
 memuse = { version = "0.2.1", features = ["nonempty"] }
 pasta_curves = "0.5"
-tempfile = "= 3.5.0" # Last version required rust 1.63
 proptest = { version = "1.0.0", optional = true }
 rand = "0.8"
-reddsa = "=0.5.0" # Last version required rust 1.65
+reddsa = "0.5"
 nonempty = "0.7"
 serde = { version = "1.0", features = ["derive"] }
 subtle = "2.3"
 zcash_note_encryption = "0.4"
-incrementalmerkletree = "0.4"
+incrementalmerkletree = "0.5"
 
 # Logging
 tracing = "0.1"
 
 # Developer tooling dependencies
-image = { version = ">= 0.24, < 0.24.5", optional = true } # 0.24.5 has MSRV 1.61
-flate2 = ">= 1.0, <1.0.27" # Clippy issues in last version
+image = { version = "0.24", optional = true }
 plotters = { version = "0.3.0", optional = true }
 
 [dev-dependencies]
-bridgetree = "0.3"
-half = ">= 1.8, < 2.3"
-criterion = "0.3"
+bridgetree = "0.4"
+criterion = "0.4" # 0.5 depends on clap 4 which has MSRV 1.70
 halo2_gadgets = { git = "https://github.com/QED-it/halo2", branch = "zsa1", features = ["test-dependencies"] }
 hex = "0.4"
 proptest = "1.0.0"
 zcash_note_encryption = { version = "0.4", features = ["pre-zip-212"] }
-incrementalmerkletree = { version = "0.4", features = ["test-dependencies"] }
+incrementalmerkletree = { version = "0.5", features = ["test-dependencies"] }
 
 [target.'cfg(unix)'.dev-dependencies]
-hashbrown = ">= 0.12, <0.13"
-dashmap = ">= 5.4, <5.5"
-inferno = ">= 0.11, < 0.11.15"
-pprof = { version = "0.9", features = ["criterion", "flamegraph"] } # MSRV 1.56
+inferno = "0.11"
+clap = "=4.2.0" # Used by inferno. Last version required rust 1.70
+pprof = { version = "0.11", features = ["criterion", "flamegraph"] }
 
 [lib]
 bench = false
@@ -97,4 +93,4 @@ debug = true
 debug = true
 
 [patch.crates-io]
-zcash_note_encryption = { version = "0.4", git = "https://github.com/QED-it/librustzcash.git", tag = "orchard_zsa_0.5.0_compatible" }
+zcash_note_encryption = { version = "0.4", git = "https://github.com/QED-it/librustzcash.git", branch = "zsa1-zebra" }

README.md

@@ -1,7 +1,7 @@
 # orchard [![Crates.io](https://img.shields.io/crates/v/orchard.svg)](https://crates.io/crates/orchard) [![CI checks](https://github.com/QED-it/orchard/actions/workflows/ci.yml/badge.svg?branch=zsa1)](https://github.com/QED-it/orchard/actions/workflows/ci.yml)
 #
 
-Requires Rust 1.61+.
+Requires Rust 1.65+.
 
 ## Documentation
 

rust-toolchain.toml

@@ -1,3 +1,3 @@
 [toolchain]
-channel = "1.61.0"
+channel = "1.65.0"
 components = [ "clippy", "rustfmt" ]

src/builder.rs

@@ -464,9 +464,11 @@ impl Builder {
             .max()
             .cloned()
             .unwrap();
-        (num_actions < MIN_ACTIONS)
-            .then(|| MIN_ACTIONS - num_actions)
-            .unwrap_or(0)
+        if num_actions < MIN_ACTIONS {
+            MIN_ACTIONS - num_actions
+        } else {
+            0
+        }
     }
 
     /// Builds a bundle containing the given spent notes and recipients.

src/bundle.rs

@@ -1,6 +1,7 @@
 //! Structs related to bundles of Orchard actions.
 
 mod batch;
+pub mod burn_validation;
 pub mod commitments;
 
 pub use batch::BatchValidator;

src/bundle/burn_validation.rs

@@ -0,0 +1,146 @@
+//! Validating burn operations on asset bundles.
+//!
+//! The module provides a function `validate_bundle_burn` that can be used to validate the burn values for the bundle.
+//!
+use std::fmt;
+
+use crate::note::AssetBase;
+
+/// Possible errors that can occur during bundle burn validation.
+#[derive(Debug)]
+#[cfg_attr(test, derive(PartialEq, Eq))]
+pub enum BurnError {
+    /// Encountered a duplicate asset to burn.
+    DuplicateAsset,
+    /// Cannot burn a native asset.
+    NativeAsset,
+    /// Cannot burn an asset with a non-positive value.
+    NonPositiveAmount,
+}
+
+/// Validates burn for a bundle by ensuring each asset is unique, non-native, and has a positive value.
+///
+/// Each burn element is represented as a tuple of `AssetBase` and `i64` (value for the burn).
+///
+/// # Arguments
+///
+/// * `burn` - A vector of assets, where each asset is represented as a tuple of `AssetBase` and `i64` (value the burn).
+///
+/// # Errors
+///
+/// Returns a `BurnError` if:
+/// * Any asset in the `burn` vector is not unique (`BurnError::DuplicateAsset`).
+/// * Any asset in the `burn` vector is native (`BurnError::NativeAsset`).
+/// * Any asset in the `burn` vector has a non-positive value (`BurnError::NonPositiveAmount`).
+pub fn validate_bundle_burn(bundle_burn: &Vec<(AssetBase, i64)>) -> Result<(), BurnError> {
+    let mut asset_set = std::collections::HashSet::<&AssetBase>::new();
+
+    for (asset, value) in bundle_burn {
+        if !asset_set.insert(asset) {
+            return Err(BurnError::DuplicateAsset);
+        }
+        if asset.is_native().into() {
+            return Err(BurnError::NativeAsset);
+        }
+        if *value <= 0 {
+            return Err(BurnError::NonPositiveAmount);
+        }
+    }
+
+    Ok(())
+}
+
+impl fmt::Display for BurnError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            BurnError::DuplicateAsset => write!(f, "Encountered a duplicate asset to burn."),
+            BurnError::NativeAsset => write!(f, "Cannot burn a native asset."),
+            BurnError::NonPositiveAmount => {
+                write!(f, "Cannot burn an asset with a non-positive value.")
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Creates an item of bundle burn list for a given asset description and value.
+    ///
+    /// This function is deterministic and guarantees that each call with the same parameters
+    /// will return the same result. It achieves determinism by using a static `IssuanceKey`.
+    ///
+    /// # Arguments
+    ///
+    /// * `asset_desc` - The asset description string.
+    /// * `value` - The value for the burn.
+    ///
+    /// # Returns
+    ///
+    /// A tuple `(AssetBase, Amount)` representing the burn list item.
+    ///
+    pub fn get_burn_tuple(asset_desc: &str, value: i64) -> (AssetBase, i64) {
+        use crate::keys::{IssuanceAuthorizingKey, IssuanceKey, IssuanceValidatingKey};
+
+        let sk_iss = IssuanceKey::from_bytes([0u8; 32]).unwrap();
+        let isk: IssuanceAuthorizingKey = (&sk_iss).into();
+
+        (
+            AssetBase::derive(&IssuanceValidatingKey::from(&isk), asset_desc),
+            value,
+        )
+    }
+
+    #[test]
+    fn validate_bundle_burn_success() {
+        let bundle_burn = vec![
+            get_burn_tuple("Asset 1", 10),
+            get_burn_tuple("Asset 2", 20),
+            get_burn_tuple("Asset 3", 10),
+        ];
+
+        let result = validate_bundle_burn(&bundle_burn);
+
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn validate_bundle_burn_duplicate_asset() {
+        let bundle_burn = vec![
+            get_burn_tuple("Asset 1", 10),
+            get_burn_tuple("Asset 1", 20),
+            get_burn_tuple("Asset 3", 10),
+        ];
+
+        let result = validate_bundle_burn(&bundle_burn);
+
+        assert_eq!(result, Err(BurnError::DuplicateAsset));
+    }
+
+    #[test]
+    fn validate_bundle_burn_native_asset() {
+        let bundle_burn = vec![
+            get_burn_tuple("Asset 1", 10),
+            (AssetBase::native(), 20),
+            get_burn_tuple("Asset 3", 10),
+        ];
+
+        let result = validate_bundle_burn(&bundle_burn);
+
+        assert_eq!(result, Err(BurnError::NativeAsset));
+    }
+
+    #[test]
+    fn validate_bundle_burn_zero_value() {
+        let bundle_burn = vec![
+            get_burn_tuple("Asset 1", 10),
+            get_burn_tuple("Asset 2", 0),
+            get_burn_tuple("Asset 3", 10),
+        ];
+
+        let result = validate_bundle_burn(&bundle_burn);
+
+        assert_eq!(result, Err(BurnError::NonPositiveAmount));
+    }
+}

src/bundle/commitments.rs

@@ -96,6 +96,22 @@ pub fn hash_bundle_auth_empty() -> Blake2bHash {
     hasher(ZCASH_ORCHARD_SIGS_HASH_PERSONALIZATION).finalize()
 }
 
+/// Construct the commitment for an absent issue bundle as defined in
+/// [ZIP-227: Issuance of Zcash Shielded Assets][zip227]
+///
+/// [zip227]: https://qed-it.github.io/zips/zip-0227
+pub fn hash_issue_bundle_auth_empty() -> Blake2bHash {
+    hasher(ZCASH_ORCHARD_ZSA_ISSUE_PERSONALIZATION).finalize()
+}
+
+/// Construct the commitment for an absent issue bundle as defined in
+/// [ZIP-227: Issuance of Zcash Shielded Assets][zip227]
+///
+/// [zip227]: https://qed-it.github.io/zips/zip-0227
+pub fn hash_issue_bundle_txid_empty() -> Blake2bHash {
+    hasher(ZCASH_ORCHARD_ZSA_ISSUE_PERSONALIZATION).finalize()
+}
+
 /// Construct the commitment for the issue bundle
 pub(crate) fn hash_issue_bundle_txid_data<A: IssueAuth>(bundle: &IssueBundle<A>) -> Blake2bHash {
     let mut h = hasher(ZCASH_ORCHARD_ZSA_ISSUE_PERSONALIZATION);
@@ -119,14 +135,6 @@ pub(crate) fn hash_issue_bundle_txid_data<A: IssueAuth>(bundle: &IssueBundle<A>)
     h.finalize()
 }
 
-/// Construct the commitment for the absent issue bundle as defined in
-/// [ZIP-227: Issuance of Zcash Shielded Assets][zip227]
-///
-/// [zip227]: https://qed-it.github.io/zips/zip-0227
-pub fn hash_issue_bundle_txid_empty() -> Blake2bHash {
-    hasher(ZCASH_ORCHARD_ZSA_ISSUE_PERSONALIZATION).finalize()
-}
-
 /// Construct the commitment to the authorizing data of an
 /// authorized issue bundle
 pub(crate) fn hash_issue_bundle_auth_data(bundle: &IssueBundle<Signed>) -> Blake2bHash {
@@ -134,11 +142,3 @@ pub(crate) fn hash_issue_bundle_auth_data(bundle: &IssueBundle<Signed>) -> Blake
     h.update(&<[u8; 64]>::from(bundle.authorization().signature()));
     h.finalize()
 }
-
-/// Construct the commitment for an absent issue bundle as defined in
-/// [ZIP-227: Issuance of Zcash Shielded Assets][zip227]
-///
-/// [zip227]: https://qed-it.github.io/zips/zip-0227
-pub fn hash_issue_bundle_auth_empty() -> Blake2bHash {
-    hasher(ZCASH_ORCHARD_ZSA_ISSUE_SIG_PERSONALIZATION).finalize()
-}

src/circuit.rs

@@ -1333,9 +1333,9 @@ mod tests {
             w.write_all(&<[u8; 32]>::from(instance.rk.clone()))?;
             w.write_all(&instance.cmx.to_bytes())?;
             w.write_all(&[
-                if instance.enable_spend { 1 } else { 0 },
-                if instance.enable_output { 1 } else { 0 },
-                if instance.enable_zsa { 1 } else { 0 },
+                u8::from(instance.enable_spend),
+                u8::from(instance.enable_output),
+                u8::from(instance.enable_zsa),
             ])?;
 
             w.write_all(proof.as_ref())?;

src/issuance.rs

@@ -143,7 +143,7 @@ impl IssueAction {
                 // All assets should be derived correctly
                 note.asset()
                     .eq(&issue_asset)
-                    .then(|| ())
+                    .then_some(())
                     .ok_or(IssueBundleIkMismatchAssetBase)?;
 
                 // The total amount should not overflow
@@ -157,8 +157,13 @@ impl IssueAction {
     }
 
     /// Serialize `finalize` flag to a byte
+    #[allow(clippy::bool_to_int_with_if)]
     pub fn flags(&self) -> u8 {
-        self.finalize.then(|| 0b0000_0001).unwrap_or(0b0000_0000)
+        if self.finalize {
+            0b0000_0001
+        } else {
+            0b0000_0000
+        }
     }
 }
 
@@ -1395,14 +1400,27 @@ mod tests {
 #[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use crate::issuance::{IssueAction, IssueBundle, Prepared, Signed, Unauthorized};
-    use crate::keys::testing::{arb_issuance_authorizing_key, arb_issuance_validating_key};
+    use crate::keys::testing::arb_issuance_validating_key;
     use crate::note::asset_base::testing::zsa_asset_id;
     use crate::note::testing::arb_zsa_note;
+    use crate::primitives::redpallas::Signature;
     use nonempty::NonEmpty;
     use proptest::collection::vec;
     use proptest::prelude::*;
     use proptest::prop_compose;
-    use rand::{rngs::StdRng, SeedableRng};
+    use reddsa::orchard::SpendAuth;
+
+    prop_compose! {
+        /// Generate a uniformly distributed signature
+        pub(crate) fn arb_signature()(
+            half_bytes in prop::array::uniform32(prop::num::u8::ANY)
+        ) -> Signature<SpendAuth> {
+            // prop::array can only generate 32 elements max, so we duplicate it
+            let sig_bytes: [u8; 64] = [half_bytes, half_bytes].concat().try_into().unwrap();
+            let sig: Signature<SpendAuth> = Signature::from(sig_bytes);
+            sig
+        }
+    }
 
     prop_compose! {
         /// Generate an issue action
@@ -1462,17 +1480,14 @@ pub mod testing {
         (
             actions in vec(arb_issue_action("asset_desc".to_string()), n_actions),
             ik in arb_issuance_validating_key(),
-            isk in arb_issuance_authorizing_key(),
-            rng_seed in prop::array::uniform32(prop::num::u8::ANY),
-            fake_sighash in prop::array::uniform32(prop::num::u8::ANY)
+            fake_sig in arb_signature(),
         ) -> IssueBundle<Signed> {
-            let rng = StdRng::from_seed(rng_seed);
             let actions = NonEmpty::from_vec(actions).unwrap();
             IssueBundle {
                 ik,
                 actions,
-                authorization: Prepared { sighash: fake_sighash },
-            }.sign(rng, &isk).unwrap()
+                authorization: Signed { signature: fake_sig },
+            }
         }
     }
 }

src/primitives/redpallas.rs

@@ -101,7 +101,7 @@ impl<T: SigType> Eq for VerificationKey<T> {}
 
 impl<T: SigType> PartialOrd for VerificationKey<T> {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        <[u8; 32]>::from(self).partial_cmp(&<[u8; 32]>::from(other))
+        Some(self.cmp(other))
     }
 }
 

src/supply_info.rs

@@ -6,7 +6,7 @@ use crate::{issuance::Error, note::AssetBase, value::ValueSum};
 
 /// Represents the amount of an asset and its finalization status.
 #[derive(Debug, Clone, Copy)]
-#[cfg_attr(test, derive(PartialEq))]
+#[cfg_attr(test, derive(PartialEq, Eq))]
 pub struct AssetSupply {
     /// The amount of the asset.
     pub amount: ValueSum,
@@ -64,7 +64,7 @@ impl SupplyInfo {
         finalization_set.extend(
             self.assets
                 .iter()
-                .filter_map(|(asset, supply)| supply.is_finalized.then(|| asset)),
+                .filter_map(|(asset, supply)| supply.is_finalized.then_some(asset)),
         );
     }
 }

tests/zsa.rs

@@ -77,12 +77,12 @@ fn prepare_keys() -> Keychain {
 
 fn sign_issue_bundle(
     unauthorized: IssueBundle<Unauthorized>,
-    mut rng: OsRng,
+    rng: OsRng,
     isk: &IssuanceAuthorizingKey,
 ) -> IssueBundle<Signed> {
     let sighash = unauthorized.commitment().into();
     let proven = unauthorized.prepare(sighash);
-    proven.sign(&mut rng, isk).unwrap()
+    proven.sign(rng, isk).unwrap()
 }
 
 fn build_and_sign_bundle(
@@ -95,7 +95,7 @@ fn build_and_sign_bundle(
     let sighash = unauthorized.commitment().into();
     let proven = unauthorized.create_proof(pk, &mut rng).unwrap();
     proven
-        .apply_signatures(&mut rng, sighash, &[SpendAuthorizingKey::from(sk)])
+        .apply_signatures(rng, sighash, &[SpendAuthorizingKey::from(sk)])
         .unwrap()
 }
 
@@ -203,7 +203,7 @@ fn create_native_note(keys: &Keychain) -> Note {
         let unauthorized = builder.build(&mut rng).unwrap();
         let sighash = unauthorized.commitment().into();
         let proven = unauthorized.create_proof(keys.pk(), &mut rng).unwrap();
-        proven.apply_signatures(&mut rng, sighash, &[]).unwrap()
+        proven.apply_signatures(rng, sighash, &[]).unwrap()
     };
     let ivk = keys.fvk().to_ivk(Scope::External);
     let (native_note, _, _) = shielding_bundle