ZSA integration (step 6): Integrate Orchard ZSA Issuance note commitments with Shielded Data action commitments for V6 transactions

zebra-chain/src/block.rs

@@ -175,7 +175,7 @@ impl Block {
     }
 
     /// Access the [orchard note commitments](pallas::Base) from all transactions in this block.
-    pub fn orchard_note_commitments(&self) -> impl Iterator<Item = &pallas::Base> {
+    pub fn orchard_note_commitments(&self) -> impl Iterator<Item = pallas::Base> + '_ {
         self.transactions
             .iter()
             .flat_map(|transaction| transaction.orchard_note_commitments())

zebra-chain/src/block/arbitrary.rs

@@ -452,7 +452,7 @@ impl Block {
                                 sapling_tree.append(*sapling_note_commitment).unwrap();
                             }
                             for orchard_note_commitment in transaction.orchard_note_commitments() {
-                                orchard_tree.append(*orchard_note_commitment).unwrap();
+                                orchard_tree.append(orchard_note_commitment).unwrap();
                             }
                         }
                         new_transactions.push(Arc::new(transaction));

zebra-chain/src/orchard.rs

@@ -30,6 +30,3 @@ pub(crate) use shielded_data::ActionCommon;
 
 #[cfg(feature = "tx-v6")]
 pub use orchard_flavor_ext::OrchardZSA;
-
-#[cfg(feature = "tx-v6")]
-pub(crate) use crate::orchard_zsa::issuance::IssueData;

zebra-chain/src/orchard/orchard_flavor_ext.rs

@@ -12,7 +12,7 @@ use orchard::{note_encryption::OrchardDomainCommon, orchard_flavor};
 use crate::serialization::{ZcashDeserialize, ZcashSerialize};
 
 #[cfg(feature = "tx-v6")]
-use crate::orchard_zsa::burn::{Burn, NoBurn};
+use crate::orchard_zsa::{Burn, NoBurn};
 
 use super::note;
 

zebra-chain/src/orchard_zsa.rs

@@ -6,7 +6,8 @@ pub(crate) mod arbitrary;
 
 mod common;
 
-pub mod burn;
-pub mod issuance;
+mod burn;
+mod issuance;
 
-pub use burn::BurnItem;
+pub(crate) use burn::{Burn, NoBurn};
+pub(crate) use issuance::IssueData;

zebra-chain/src/orchard_zsa/issuance.rs

@@ -2,30 +2,35 @@
 
 use std::{fmt::Debug, io};
 
-use crate::{
-    block::MAX_BLOCK_BYTES,
-    serialization::{
-        zcash_serialize_bytes, zcash_serialize_empty_list, ReadZcashExt, SerializationError,
-        TrustedPreallocate, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize,
-    },
-};
-
 use nonempty::NonEmpty;
 
 // FIXME: needed for "as_bool" only - consider to implement as_bool locally
 use bitvec::macros::internal::funty::Fundamental;
 
 use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 
+use halo2::pasta::pallas;
+
+// For pallas::Base::from_repr only
+use group::ff::PrimeField;
+
 use orchard::{
     issuance::{IssueAction, IssueBundle, Signed},
     keys::IssuanceValidatingKey,
-    note::{RandomSeed, Rho},
+    note::{ExtractedNoteCommitment, RandomSeed, Rho},
     primitives::redpallas::{SigType, Signature, SpendAuth},
     value::NoteValue,
     Address, Note,
 };
 
+use crate::{
+    block::MAX_BLOCK_BYTES,
+    serialization::{
+        zcash_serialize_bytes, zcash_serialize_empty_list, ReadZcashExt, SerializationError,
+        TrustedPreallocate, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize,
+    },
+};
+
 use super::common::ASSET_BASE_SIZE;
 
 /// Wrapper for `IssueBundle` used in the context of Transaction V6. This allows the implementation of
@@ -39,6 +44,21 @@ impl From<IssueBundle<Signed>> for IssueData {
     }
 }
 
+impl IssueData {
+    pub(crate) fn note_commitments(&self) -> impl Iterator<Item = pallas::Base> + '_ {
+        self.0.actions().iter().flat_map(|action| {
+            action.notes().iter().map(|note| {
+                // FIXME: Make `ExtractedNoteCommitment::inner` public in `orchard` (this would
+                // eliminate the need for the workaround of converting `pallas::Base` from bytes
+                // here), or introduce a new public method in `orchard::issuance::IssueBundle` to
+                // retrieve note commitments directly from `orchard`.
+                pallas::Base::from_repr(ExtractedNoteCommitment::from(note.commitment()).to_bytes())
+                    .unwrap()
+            })
+        })
+    }
+}
+
 // Sizes of the serialized values for types in bytes (used for TrustedPreallocate impls)
 // FIXME: are those values correct (43, 32 etc.)?
 //const ISSUANCE_VALIDATING_KEY_SIZE: u64 = 32;

zebra-chain/src/parallel/tree.rs

@@ -73,7 +73,7 @@ impl NoteCommitmentTrees {
 
         let sprout_note_commitments: Vec<_> = block.sprout_note_commitments().cloned().collect();
         let sapling_note_commitments: Vec<_> = block.sapling_note_commitments().cloned().collect();
-        let orchard_note_commitments: Vec<_> = block.orchard_note_commitments().cloned().collect();
+        let orchard_note_commitments: Vec<_> = block.orchard_note_commitments().collect();
 
         let mut sprout_result = None;
         let mut sapling_result = None;

zebra-chain/src/transaction.rs

@@ -40,7 +40,7 @@ pub use unmined::{
 
 use crate::{
     amount::{Amount, Error as AmountError, NegativeAllowed, NonNegative},
-    block, orchard,
+    block, orchard, orchard_zsa,
     parameters::{ConsensusBranchId, NetworkUpgrade},
     primitives::{ed25519, Bctv14Proof, Groth16Proof},
     sapling,
@@ -53,65 +53,75 @@ use crate::{
     value_balance::{ValueBalance, ValueBalanceError},
 };
 
-// FIXME: doc this
-// Move down
-macro_rules! orchard_shielded_data_iter {
-    ($self:expr, $mapper:expr) => {
+/// Applies expression to different versions of `Transaction` to handle Orchard shielded data.
+macro_rules! match_orchard_shielded_data {
+    // Separate field sets and expressions for V5 and V6
+    ($self:expr, $pre_v5_expr:expr, { $( $v5_field:ident ),+ }, $v5_expr:expr, { $( $v6_field:ident ),+ }, $v6_expr:expr) => {
         match $self {
-            // No Orchard shielded data
             Transaction::V1 { .. }
             | Transaction::V2 { .. }
             | Transaction::V3 { .. }
-            | Transaction::V4 { .. } => Box::new(std::iter::empty()),
+            | Transaction::V4 { .. } => $pre_v5_expr,
 
-            Transaction::V5 {
-                orchard_shielded_data,
-                ..
-            } => Box::new(orchard_shielded_data.into_iter().flat_map($mapper)),
+            Transaction::V5 { $( $v5_field ),+, .. } => $v5_expr,
 
             #[cfg(feature = "tx-v6")]
-            Transaction::V6 {
-                orchard_shielded_data,
-                ..
-            } => Box::new(orchard_shielded_data.into_iter().flat_map($mapper)),
+            Transaction::V6 { $( $v6_field ),+, .. } => $v6_expr,
         }
     };
+
+    // Single field set and expression used for both V5 and V6
+    ($self:expr, $pre_v5_expr:expr, { $( $v5_v6_field:ident ),+ }, $v5_v6_expr:expr) => {
+        match_orchard_shielded_data!(
+            $self,
+            $pre_v5_expr,
+            { $( $v5_v6_field ),+ },
+            $v5_v6_expr,
+            { $( $v5_v6_field ),+ },
+            $v5_v6_expr
+        )
+    };
+}
+
+/// Creates an iterator over Orchard shielded data.
+///
+/// - `$self`: The `Transaction` instance.
+/// - `$mapper`: Function to apply to each data item.
+macro_rules! orchard_shielded_data_iter {
+    ($self:expr, $mapper:expr) => {
+        match_orchard_shielded_data!(
+            $self,
+            Box::new(std::iter::empty()),
+            { orchard_shielded_data },
+            Box::new(orchard_shielded_data.into_iter().flat_map($mapper))
+        )
+    };
 }
 
-// FIXME: doc this
-// Move down
+/// Retrieves a specific field from Orchard shielded data.
+///
+/// - `$self`: The `Transaction` instance.
+/// - `$field`: The field to access
 macro_rules! orchard_shielded_data_field {
     ($self:expr, $field:ident) => {
-        match $self {
-            // No Orchard shielded data
-            Transaction::V1 { .. }
-            | Transaction::V2 { .. }
-            | Transaction::V3 { .. }
-            | Transaction::V4 { .. } => None,
-
-            Transaction::V5 {
-                orchard_shielded_data,
-                ..
-            } => orchard_shielded_data.as_ref().map(|data| data.$field),
-
-            #[cfg(feature = "tx-v6")]
-            Transaction::V6 {
-                orchard_shielded_data,
-                ..
-            } => orchard_shielded_data.as_ref().map(|data| data.$field),
-        }
+        match_orchard_shielded_data!(
+            $self,
+            None,
+            { orchard_shielded_data },
+            orchard_shielded_data.as_ref().map(|d| d.$field)
+        )
     };
 }
 
-// FIXME:
-// Define the macro for including the V6 pattern
+/// Generates match patterns for `Transaction::V5` and `Transaction::V6`
 #[cfg(feature = "tx-v6")]
 macro_rules! tx_v5_and_v6 {
     { $($fields:tt)* } => {
         Transaction::V5 { $($fields)* } | Transaction::V6 { $($fields)* }
     };
 }
 
+/// Generates a match pattern for `Transaction::V5` only
 #[cfg(not(feature = "tx-v6"))]
 macro_rules! tx_v5_and_v6 {
     { $($fields:tt)* } => {
@@ -235,7 +245,7 @@ pub enum Transaction {
         orchard_shielded_data: Option<orchard::ShieldedData<orchard::OrchardZSA>>,
         /// The ZSA issuance data for this transaction, if any.
         #[cfg(feature = "tx-v6")]
-        orchard_zsa_issue_data: Option<orchard::IssueData>,
+        orchard_zsa_issue_data: Option<orchard_zsa::IssueData>,
     },
 }
 
@@ -1036,8 +1046,32 @@ impl Transaction {
 
     /// Access the note commitments in this transaction, if there are any,
     /// regardless of version.
-    pub fn orchard_note_commitments(&self) -> Box<dyn Iterator<Item = &pallas::Base> + '_> {
-        orchard_shielded_data_iter!(self, orchard::ShieldedData::note_commitments)
+    pub fn orchard_note_commitments(&self) -> Box<dyn Iterator<Item = pallas::Base> + '_> {
+        match_orchard_shielded_data!(
+            self,
+            Box::new(std::iter::empty()),
+            { orchard_shielded_data },
+            Box::new(
+                orchard_shielded_data
+                    .iter()
+                    .flat_map(orchard::ShieldedData::note_commitments)
+                    .cloned()
+            ),
+            { orchard_shielded_data, orchard_zsa_issue_data },
+            {
+                Box::new(
+                    orchard_shielded_data
+                        .iter()
+                        .flat_map(orchard::ShieldedData::note_commitments)
+                        .cloned()
+                        .chain(
+                            orchard_zsa_issue_data
+                              .iter()
+                              .flat_map(orchard_zsa::IssueData::note_commitments)
+                        )
+                )
+            }
+        )
     }
 
     /// Access the [`orchard::Flags`] in this transaction, if there is any,
@@ -1441,33 +1475,14 @@ impl Transaction {
     /// See `orchard_value_balance` for details.
     #[cfg(any(test, feature = "proptest-impl"))]
     pub fn orchard_value_balance_mut(&mut self) -> Option<&mut Amount<NegativeAllowed>> {
-        match self {
-            Transaction::V5 {
-                orchard_shielded_data: Some(orchard_shielded_data),
-                ..
-            } => Some(&mut orchard_shielded_data.value_balance),
-
-            #[cfg(feature = "tx-v6")]
-            Transaction::V6 {
-                orchard_shielded_data: Some(orchard_shielded_data),
-                ..
-            } => Some(&mut orchard_shielded_data.value_balance),
-
-            Transaction::V1 { .. }
-            | Transaction::V2 { .. }
-            | Transaction::V3 { .. }
-            | Transaction::V4 { .. }
-            | Transaction::V5 {
-                orchard_shielded_data: None,
-                ..
-            } => None,
-
-            #[cfg(feature = "tx-v6")]
-            Transaction::V6 {
-                orchard_shielded_data: None,
-                ..
-            } => None,
-        }
+        match_orchard_shielded_data!(
+            self,
+            None,
+            { orchard_shielded_data },
+            orchard_shielded_data
+                .as_mut()
+                .map(|shielded_data| &mut shielded_data.value_balance)
+        )
     }
 
     /// Returns the value balances for this transaction using the provided transparent outputs.

zebra-chain/src/transaction/arbitrary.rs

@@ -21,7 +21,7 @@ use crate::{
 };
 
 #[cfg(feature = "tx-v6")]
-use crate::orchard_zsa::issuance::IssueData;
+use crate::orchard_zsa::IssueData;
 
 use itertools::Itertools;
 

zebra-state/src/service/finalized_state/disk_format/upgrade/add_subtrees.rs

@@ -800,7 +800,6 @@ fn calculate_orchard_subtree(
         let orchard_note_commitments = block
             .orchard_note_commitments()
             .take(prev_remaining_notes)
-            .cloned()
             .collect();
 
         // This takes less than 1 second per tree, so we don't need to make it cancellable.