First-stage ZSA integration: apply upstream feedback & refactoring (#9560)

zebra-chain/src/orchard.rs

@@ -26,7 +26,5 @@ pub use note::{EncryptedNote, Note, Nullifier, WrappedNoteKey};
 pub use shielded_data::{AuthorizedAction, Flags, ShieldedData};
 pub use shielded_data_flavor::{OrchardVanilla, ShieldedDataFlavor};
 
-pub(crate) use shielded_data::ActionCommon;
-
 #[cfg(feature = "tx-v6")]
 pub use shielded_data_flavor::OrchardZSA;

zebra-chain/src/orchard/note/arbitrary.rs

@@ -2,13 +2,13 @@ use proptest::{collection::vec, prelude::*};
 
 use super::*;
 
-impl<const N: usize> Arbitrary for EncryptedNote<N> {
+impl<const SIZE: usize> Arbitrary for EncryptedNote<SIZE> {
     type Parameters = ();
 
     fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
-        (vec(any::<u8>(), N))
+        (vec(any::<u8>(), SIZE))
             .prop_map(|v| {
-                let mut bytes = [0; N];
+                let mut bytes = [0; SIZE];
                 bytes.copy_from_slice(v.as_slice());
                 Self(bytes)
             })

zebra-chain/src/orchard/note/ciphertexts.rs

@@ -10,38 +10,38 @@ use crate::serialization::{SerializationError, ZcashDeserialize, ZcashSerialize}
 ///
 /// Corresponds to the Orchard 'encCiphertext's
 #[derive(Deserialize, Serialize, Clone, Debug, Eq, PartialEq)]
-pub struct EncryptedNote<const N: usize>(#[serde(with = "BigArray")] pub(crate) [u8; N]);
+pub struct EncryptedNote<const SIZE: usize>(#[serde(with = "BigArray")] pub(crate) [u8; SIZE]);
 
-impl<const N: usize> From<[u8; N]> for EncryptedNote<N> {
-    fn from(bytes: [u8; N]) -> Self {
+impl<const SIZE: usize> From<[u8; SIZE]> for EncryptedNote<SIZE> {
+    fn from(bytes: [u8; SIZE]) -> Self {
         Self(bytes)
     }
 }
 
-impl<const N: usize> From<EncryptedNote<N>> for [u8; N] {
-    fn from(enc_ciphertext: EncryptedNote<N>) -> Self {
+impl<const SIZE: usize> From<EncryptedNote<SIZE>> for [u8; SIZE] {
+    fn from(enc_ciphertext: EncryptedNote<SIZE>) -> Self {
         enc_ciphertext.0
     }
 }
 
-impl<const N: usize> TryFrom<&[u8]> for EncryptedNote<N> {
+impl<const SIZE: usize> TryFrom<&[u8]> for EncryptedNote<SIZE> {
     type Error = std::array::TryFromSliceError;
 
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         Ok(Self(bytes.try_into()?))
     }
 }
 
-impl<const N: usize> ZcashSerialize for EncryptedNote<N> {
+impl<const SIZE: usize> ZcashSerialize for EncryptedNote<SIZE> {
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
         writer.write_all(&self.0[..])?;
         Ok(())
     }
 }
 
-impl<const N: usize> ZcashDeserialize for EncryptedNote<N> {
+impl<const SIZE: usize> ZcashDeserialize for EncryptedNote<SIZE> {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
-        let mut bytes = [0; N];
+        let mut bytes = [0; SIZE];
         reader.read_exact(&mut bytes[..])?;
         Ok(Self(bytes))
     }

zebra-chain/src/orchard/shielded_data.rs

@@ -20,6 +20,9 @@ use crate::{
     },
 };
 
+#[cfg(feature = "tx-v6")]
+use crate::orchard_zsa::compute_burn_value_commitment;
+
 #[cfg(feature = "tx-v6")]
 use orchard::{note::AssetBase, value::ValueSum};
 
@@ -87,12 +90,6 @@ impl<Flavor: ShieldedDataFlavor> ShieldedData<Flavor> {
         self.actions.actions()
     }
 
-    /// Return an iterator for the [`ActionCommon`] copy of the Actions in this
-    /// transaction, in the order they appear in it.
-    pub fn action_commons(&self) -> impl Iterator<Item = ActionCommon> + '_ {
-        self.actions.actions().map(|action| action.into())
-    }
-
     /// Collect the [`Nullifier`]s for this transaction.
     pub fn nullifiers(&self) -> impl Iterator<Item = &Nullifier> {
         self.actions().map(|action| &action.nullifier)
@@ -142,7 +139,7 @@ impl<Flavor: ShieldedDataFlavor> ShieldedData<Flavor> {
                 (ValueSum::default() + i64::from(self.value_balance)).unwrap(),
                 AssetBase::native(),
             );
-            let burn_value_commitment = self.burn.clone().into();
+            let burn_value_commitment = compute_burn_value_commitment(self.burn.as_ref());
             cv - cv_balance - burn_value_commitment
         };
 
@@ -260,29 +257,6 @@ impl<Flavor: ShieldedDataFlavor> AuthorizedAction<Flavor> {
     }
 }
 
-/// The common field used both in Vanilla actions and ZSA actions.
-pub struct ActionCommon {
-    /// A value commitment to net value of the input note minus the output note
-    pub cv: ValueCommitment,
-    /// The nullifier of the input note being spent.
-    pub nullifier: super::note::Nullifier,
-    /// The randomized validating key for spendAuthSig,
-    pub rk: reddsa::VerificationKeyBytes<SpendAuth>,
-    /// The x-coordinate of the note commitment for the output note.
-    pub cm_x: pallas::Base,
-}
-
-impl<Flavor: ShieldedDataFlavor> From<&Action<Flavor>> for ActionCommon {
-    fn from(action: &Action<Flavor>) -> Self {
-        Self {
-            cv: action.cv,
-            nullifier: action.nullifier,
-            rk: action.rk,
-            cm_x: action.cm_x,
-        }
-    }
-}
-
 /// The maximum number of orchard actions in a valid Zcash on-chain transaction V5.
 ///
 /// If a transaction contains more actions than can fit in maximally large block, it might be

zebra-chain/src/orchard/shielded_data_flavor.rs

@@ -14,10 +14,7 @@ pub use orchard::{note::AssetBase, orchard_flavor::OrchardZSA, value::NoteValue}
 use crate::serialization::{ZcashDeserialize, ZcashSerialize};
 
 #[cfg(feature = "tx-v6")]
-use crate::{
-    orchard::ValueCommitment,
-    orchard_zsa::{Burn, BurnItem, NoBurn},
-};
+use crate::orchard_zsa::{Burn, BurnItem, NoBurn};
 
 use super::note;
 
@@ -54,13 +51,10 @@ pub trait ShieldedDataFlavor: OrchardFlavor {
 
     /// A type representing a burn field for this protocol version.
     #[cfg(feature = "tx-v6")]
-    // FIXME: try to get rid
     type BurnType: Clone
         + Debug
-        + Default
         + ZcashDeserialize
         + ZcashSerialize
-        + Into<ValueCommitment>
         + AsRef<[BurnItem]>
         + for<'a> From<&'a [(AssetBase, NoteValue)]>
         + test_arbitrary::TestArbitrary;

zebra-chain/src/orchard_zsa.rs

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

zebra-chain/src/orchard_zsa/burn.rs

@@ -100,16 +100,6 @@ impl From<&[(AssetBase, NoteValue)]> for NoBurn {
     }
 }
 
-impl From<NoBurn> for ValueCommitment {
-    fn from(_burn: NoBurn) -> ValueCommitment {
-        ValueCommitment::new(
-            pallas::Scalar::zero(),
-            NoteValue::from_raw(0).into(),
-            AssetBase::native(),
-        )
-    }
-}
-
 impl AsRef<[BurnItem]> for NoBurn {
     fn as_ref(&self) -> &[BurnItem] {
         &[]
@@ -149,18 +139,6 @@ impl From<&[(AssetBase, NoteValue)]> for Burn {
     }
 }
 
-impl From<Burn> for ValueCommitment {
-    fn from(burn: Burn) -> ValueCommitment {
-        burn.0
-            .into_iter()
-            .map(|BurnItem(asset, amount)| {
-                // The trapdoor for the burn which is public is always zero.
-                ValueCommitment::new(pallas::Scalar::zero(), amount.into(), asset)
-            })
-            .sum()
-    }
-}
-
 impl AsRef<[BurnItem]> for Burn {
     fn as_ref(&self) -> &[BurnItem] {
         &self.0
@@ -186,3 +164,14 @@ impl ZcashDeserialize for Burn {
         ))
     }
 }
+
+/// Computes the value commitment for a list of burns.
+///
+/// For burns, the public trapdoor is always zero.
+pub(crate) fn compute_burn_value_commitment(burn: &[BurnItem]) -> ValueCommitment {
+    burn.iter()
+        .map(|&BurnItem(asset, amount)| {
+            ValueCommitment::new(pallas::Scalar::zero(), amount.into(), asset)
+        })
+        .sum()
+}

zebra-chain/src/transaction.rs

@@ -193,7 +193,7 @@ impl fmt::Display for Transaction {
         fmter.field("sprout_joinsplits", &self.joinsplit_count());
         fmter.field("sapling_spends", &self.sapling_spends_per_anchor().count());
         fmter.field("sapling_outputs", &self.sapling_outputs().count());
-        fmter.field("orchard_actions", &self.orchard_actions().count());
+        fmter.field("orchard_actions", &self.orchard_action_count());
 
         fmter.field("unmined_id", &self.unmined_id());
 
@@ -335,7 +335,7 @@ impl Transaction {
     pub fn has_shielded_inputs(&self) -> bool {
         self.joinsplit_count() > 0
             || self.sapling_spends_per_anchor().count() > 0
-            || (self.orchard_actions().count() > 0
+            || (self.orchard_action_count() > 0
                 && self
                     .orchard_flags()
                     .unwrap_or_else(orchard::Flags::empty)
@@ -353,7 +353,7 @@ impl Transaction {
     pub fn has_shielded_outputs(&self) -> bool {
         self.joinsplit_count() > 0
             || self.sapling_outputs().count() > 0
-            || (self.orchard_actions().count() > 0
+            || (self.orchard_action_count() > 0
                 && self
                     .orchard_flags()
                     .unwrap_or_else(orchard::Flags::empty)
@@ -362,7 +362,7 @@ impl Transaction {
 
     /// Does this transaction has at least one flag when we have at least one orchard action?
     pub fn has_enough_orchard_flags(&self) -> bool {
-        if self.version() < 5 || self.orchard_actions().count() == 0 {
+        if self.version() < 5 || self.orchard_action_count() == 0 {
             return true;
         }
         self.orchard_flags()
@@ -999,31 +999,29 @@ impl Transaction {
     // orchard
 
     /// Iterate over the [`orchard::Action`]s in this transaction.
-    pub fn orchard_actions(&self) -> Box<dyn Iterator<Item = orchard::ActionCommon> + '_> {
+    pub fn orchard_action_count(&self) -> usize {
         match self {
             Transaction::V1 { .. }
             | Transaction::V2 { .. }
             | Transaction::V3 { .. }
-            | Transaction::V4 { .. } => Box::new(std::iter::empty()),
+            | Transaction::V4 { .. } => 0,
 
             Transaction::V5 {
                 orchard_shielded_data,
                 ..
-            } => Box::new(
-                orchard_shielded_data
-                    .iter()
-                    .flat_map(orchard::ShieldedData::action_commons),
-            ),
+            } => orchard_shielded_data
+                .iter()
+                .flat_map(orchard::ShieldedData::actions)
+                .count(),
 
             #[cfg(feature = "tx-v6")]
             Transaction::V6 {
                 orchard_shielded_data,
                 ..
-            } => Box::new(
-                orchard_shielded_data
-                    .iter()
-                    .flat_map(orchard::ShieldedData::action_commons),
-            ),
+            } => orchard_shielded_data
+                .iter()
+                .flat_map(orchard::ShieldedData::actions)
+                .count(),
         }
     }
 

zebra-chain/src/transaction/unmined/zip317.rs

@@ -153,7 +153,7 @@ pub fn conventional_actions(transaction: &Transaction) -> u32 {
     let n_join_split = transaction.joinsplit_count();
     let n_spends_sapling = transaction.sapling_spends_per_anchor().count();
     let n_outputs_sapling = transaction.sapling_outputs().count();
-    let n_actions_orchard = transaction.orchard_actions().count();
+    let n_actions_orchard = transaction.orchard_action_count();
 
     let tx_in_logical_actions = div_ceil(tx_in_total_size, P2PKH_STANDARD_INPUT_SIZE);
     let tx_out_logical_actions = div_ceil(tx_out_total_size, P2PKH_STANDARD_OUTPUT_SIZE);

zebra-consensus/src/transaction/tests.rs

@@ -166,7 +166,7 @@ fn v5_transaction_with_no_inputs_fails_validation() {
     .find(|transaction| {
         transaction.inputs().is_empty()
             && transaction.sapling_spends_per_anchor().next().is_none()
-            && transaction.orchard_actions().next().is_none()
+            && transaction.orchard_action_count() == 0
             && transaction.joinsplit_count() == 0
             && (!transaction.outputs().is_empty() || transaction.sapling_outputs().next().is_some())
     })
@@ -800,7 +800,7 @@ fn v5_transaction_with_no_outputs_fails_validation() {
     .find(|transaction| {
         transaction.outputs().is_empty()
             && transaction.sapling_outputs().next().is_none()
-            && transaction.orchard_actions().next().is_none()
+            && transaction.orchard_action_count() == 0
             && transaction.joinsplit_count() == 0
             && (!transaction.inputs().is_empty()
                 || transaction.sapling_spends_per_anchor().next().is_some())
@@ -2795,8 +2795,7 @@ fn coinbase_outputs_are_decryptable_for_historical_blocks_for_network(
 
         // Check if the coinbase outputs are decryptable with an all-zero key.
         if heartwood_onward
-            && (coinbase_tx.sapling_outputs().count() > 0
-                || coinbase_tx.orchard_actions().count() > 0)
+            && (coinbase_tx.sapling_outputs().count() > 0 || coinbase_tx.orchard_action_count() > 0)
         {
             // We are only truly decrypting something if it's Heartwood-onward
             // and there are relevant outputs.
@@ -2808,7 +2807,7 @@ fn coinbase_outputs_are_decryptable_for_historical_blocks_for_network(
         // For remaining transactions, check if existing outputs are NOT decryptable
         // with an all-zero key, if applicable.
         for tx in block.transactions.iter().skip(1) {
-            let has_outputs = tx.sapling_outputs().count() > 0 || tx.orchard_actions().count() > 0;
+            let has_outputs = tx.sapling_outputs().count() > 0 || tx.orchard_action_count() > 0;
             if has_outputs && heartwood_onward {
                 tested_non_coinbase_txs += 1;
                 check::coinbase_outputs_are_decryptable(tx, &network, height).expect_err(