Refactor Sapling data and use it in V4 (#1946)

* start refactoring transaction v4 for transaction v5
- move ShieldedData to sapling
- add AnchorVariant
- rename shielded_data to sapling_shielded data in V4
- move value_balance into ShieldedData
- update prop tests for new structure

* add AnchorVariant to Spend
- make anchor types available from sapling crate
- update serialize

* change shielded_balances_match() arguments
* change variable name anchor to shared_anchor in ShieldedData
* fix empty value balance serialization
* use AnchorV in shielded spends
* Rename anchor to per_spend_anchor

* Use nullifiers function directly in non-finalized state
* Use self.value_balance instead of passing it as an argument

* Add missing fields to ShieldedData PartialEq
* Derive Copy for tag types

* Add doc comments for ShieldedData refactor
* Implement a per-spend anchor compatibility iterator

Co-authored-by: teor <teor@riseup.net>

Cargo.lock

@@ -1,5 +1,7 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
+version = 3
+
 [[package]]
 name = "abscissa_core"
 version = "0.5.2"
@@ -921,6 +923,20 @@ dependencies = [
  "syn 1.0.60",
 ]
 
+[[package]]
+name = "ed25519-zebra"
+version = "2.2.0"
+source = "git+https://github.com/ZcashFoundation/ed25519-zebra?rev=539fad040c443302775b0f508e616418825e6c22#539fad040c443302775b0f508e616418825e6c22"
+dependencies = [
+ "curve25519-dalek",
+ "hex",
+ "rand_core 0.6.2",
+ "serde",
+ "sha2",
+ "thiserror",
+ "zeroize",
+]
+
 [[package]]
 name = "ed25519-zebra"
 version = "2.2.0"
@@ -935,20 +951,6 @@ dependencies = [
  "thiserror",
 ]
 
-[[package]]
-name = "ed25519-zebra"
-version = "2.2.0"
-source = "git+https://github.com/ZcashFoundation/ed25519-zebra?rev=856c96500125e8dd38a525dad13dc64a0ac672cc#856c96500125e8dd38a525dad13dc64a0ac672cc"
-dependencies = [
- "curve25519-dalek",
- "hex",
- "rand_core 0.6.2",
- "serde",
- "sha2",
- "thiserror",
- "zeroize",
-]
-
 [[package]]
 name = "either"
 version = "1.6.1"
@@ -4070,7 +4072,7 @@ dependencies = [
  "chrono",
  "color-eyre",
  "displaydoc",
- "ed25519-zebra 2.2.0 (git+https://github.com/ZcashFoundation/ed25519-zebra?rev=856c96500125e8dd38a525dad13dc64a0ac672cc)",
+ "ed25519-zebra 2.2.0 (git+https://github.com/ZcashFoundation/ed25519-zebra?rev=539fad040c443302775b0f508e616418825e6c22)",
  "equihash",
  "futures 0.3.13",
  "hex",

zebra-chain/src/sapling.rs

@@ -13,6 +13,7 @@ mod tests;
 // XXX clean up these modules
 
 pub mod keys;
+pub mod shielded_data;
 pub mod tree;
 
 pub use address::Address;
@@ -20,4 +21,5 @@ pub use commitment::{CommitmentRandomness, NoteCommitment, ValueCommitment};
 pub use keys::Diversifier;
 pub use note::{EncryptedNote, Note, Nullifier, WrappedNoteKey};
 pub use output::Output;
+pub use shielded_data::{AnchorVariant, PerSpendAnchor, SharedAnchor, ShieldedData};
 pub use spend::Spend;

zebra-chain/src/sapling/arbitrary.rs

@@ -3,9 +3,9 @@ use proptest::{arbitrary::any, array, collection::vec, prelude::*};
 
 use crate::primitives::Groth16Proof;
 
-use super::{keys, note, tree, NoteCommitment, Output, Spend, ValueCommitment};
+use super::{keys, note, tree, NoteCommitment, Output, PerSpendAnchor, Spend, ValueCommitment};
 
-impl Arbitrary for Spend {
+impl Arbitrary for Spend<PerSpendAnchor> {
     type Parameters = ();
 
     fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
@@ -16,18 +16,20 @@ impl Arbitrary for Spend {
             any::<Groth16Proof>(),
             vec(any::<u8>(), 64),
         )
-            .prop_map(|(anchor, nullifier, rpk_bytes, proof, sig_bytes)| Self {
+            .prop_map(
-                anchor,
+                |(per_spend_anchor, nullifier, rpk_bytes, proof, sig_bytes)| Self {
-                cv: ValueCommitment(AffinePoint::identity()),
+                    per_spend_anchor,
-                nullifier,
+                    cv: ValueCommitment(AffinePoint::identity()),
-                rk: redjubjub::VerificationKeyBytes::from(rpk_bytes),
+                    nullifier,
-                zkproof: proof,
+                    rk: redjubjub::VerificationKeyBytes::from(rpk_bytes),
-                spend_auth_sig: redjubjub::Signature::from({
+                    zkproof: proof,
-                    let mut b = [0u8; 64];
+                    spend_auth_sig: redjubjub::Signature::from({
-                    b.copy_from_slice(sig_bytes.as_slice());
+                        let mut b = [0u8; 64];
-                    b
+                        b.copy_from_slice(sig_bytes.as_slice());
-                }),
+                        b
-            })
+                    }),
+                },
+            )
             .boxed()
     }
 

zebra-chain/src/sapling/shielded_data.rs

@@ -1,12 +1,60 @@
+//! Sapling shielded data for `V4` and `V5` `Transaction`s.
+//!
+//! Zebra uses a generic shielded data type for `V4` and `V5` transactions.
+//! The `value_balance` change is handled using the default zero value.
+//! The anchor change is handled using the `AnchorVariant` type trait.
+
 use futures::future::Either;
 
 use crate::{
     amount::Amount,
     primitives::redjubjub::{Binding, Signature},
-    sapling::{Nullifier, Output, Spend, ValueCommitment},
+    sapling::{tree, Nullifier, Output, Spend, ValueCommitment},
     serialization::serde_helpers,
 };
 
+use serde::{de::DeserializeOwned, Serialize};
+use std::{
+    cmp::{Eq, PartialEq},
+    fmt::Debug,
+};
+
+/// Per-Spend Sapling anchors, used in Transaction V4 and the
+/// `spends_per_anchor` method.
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+pub struct PerSpendAnchor {}
+
+/// Shared Sapling anchors, used in Transaction V5.
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+pub struct SharedAnchor {}
+
+impl AnchorVariant for PerSpendAnchor {
+    type Shared = ();
+    type PerSpend = tree::Root;
+}
+
+impl AnchorVariant for SharedAnchor {
+    type Shared = tree::Root;
+    type PerSpend = ();
+}
+
+/// A type trait to handle structural differences between V4 and V5 Sapling
+/// Transaction anchors.
+///
+/// In Transaction V4, anchors are per-Spend. In Transaction V5, there is a
+/// single transaction anchor for all Spends in a transaction.
+pub trait AnchorVariant {
+    /// The type of the shared anchor.
+    ///
+    /// `()` means "not present in this transaction version".
+    type Shared: Clone + Debug + DeserializeOwned + Serialize + Eq + PartialEq;
+
+    /// The type of the per-spend anchor.
+    ///
+    /// `()` means "not present in this transaction version".
+    type PerSpend: Clone + Debug + DeserializeOwned + Serialize + Eq + PartialEq;
+}
+
 /// A bundle of [`Spend`] and [`Output`] descriptions and signature data.
 ///
 /// Spend and Output descriptions are optional, but Zcash transactions must
@@ -15,8 +63,29 @@ use crate::{
 /// description with the required signature data, so that an
 /// `Option<ShieldedData>` correctly models the presence or absence of any
 /// shielded data.
+///
+/// # Differences between Transaction Versions
+///
+/// The Sapling `value_balance` field is optional in `Transaction::V5`, but
+/// required in `Transaction::V4`. In both cases, if there is no `ShieldedData`,
+/// then the field value must be zero. Therefore, only need to store
+/// `value_balance` when there is some Sapling `ShieldedData`.
+///
+/// In `Transaction::V4`, each `Spend` has its own anchor. In `Transaction::V5`,
+/// there is a single `shared_anchor` for the entire transaction. This
+/// structural difference is modeled using the `AnchorVariant` type trait.
+/// A type of `()` means "not present in this transaction version".
 #[derive(Clone, Debug, Serialize, Deserialize)]
-pub struct ShieldedData {
+pub struct ShieldedData<AnchorV>
+where
+    AnchorV: AnchorVariant + Clone,
+{
+    /// The net value of Sapling spend transfers minus output transfers.
+    pub value_balance: Amount,
+    /// The shared anchor for all `Spend`s in this transaction.
+    ///
+    /// A type of `()` means "not present in this transaction version".
+    pub shared_anchor: AnchorV::Shared,
     /// Either a spend or output description.
     ///
     /// Storing this separately ensures that it is impossible to construct
@@ -27,12 +96,12 @@ pub struct ShieldedData {
     /// methods provide iterators over all of the [`Spend`]s and
     /// [`Output`]s.
     #[serde(with = "serde_helpers::Either")]
-    pub first: Either<Spend, Output>,
+    pub first: Either<Spend<AnchorV>, Output>,
     /// The rest of the [`Spend`]s for this transaction.
     ///
     /// Note that the [`ShieldedData::spends`] method provides an iterator
     /// over all spend descriptions.
-    pub rest_spends: Vec<Spend>,
+    pub rest_spends: Vec<Spend<AnchorV>>,
     /// The rest of the [`Output`]s for this transaction.
     ///
     /// Note that the [`ShieldedData::outputs`] method provides an iterator
@@ -42,9 +111,37 @@ pub struct ShieldedData {
     pub binding_sig: Signature<Binding>,
 }
 
-impl ShieldedData {
+impl<AnchorV> ShieldedData<AnchorV>
+where
+    AnchorV: AnchorVariant + Clone,
+    Spend<PerSpendAnchor>: From<(Spend<AnchorV>, AnchorV::Shared)>,
+{
     /// Iterate over the [`Spend`]s for this transaction.
-    pub fn spends(&self) -> impl Iterator<Item = &Spend> {
+    ///
+    /// Returns `Spend<PerSpendAnchor>` regardless of the underlying transaction
+    /// version, to allow generic verification over V4 and V5 transactions.
+    ///
+    /// # Correctness
+    ///
+    /// Do not use this function for serialization.
+    pub fn spends_per_anchor(&self) -> impl Iterator<Item = Spend<PerSpendAnchor>> + '_ {
+        self.spends()
+            .cloned()
+            .map(move |spend| Spend::<PerSpendAnchor>::from((spend, self.shared_anchor.clone())))
+    }
+}
+
+impl<AnchorV> ShieldedData<AnchorV>
+where
+    AnchorV: AnchorVariant + Clone,
+{
+    /// Iterate over the [`Spend`]s for this transaction, returning them as
+    /// their generic type.
+    ///
+    /// # Correctness
+    ///
+    /// Use this function for serialization.
+    pub fn spends(&self) -> impl Iterator<Item = &Spend<AnchorV>> {
         match self.first {
             Either::Left(ref spend) => Some(spend),
             Either::Right(_) => None,
@@ -96,13 +193,11 @@ impl ShieldedData {
     /// descriptions of the transaction, and the balancing value.
     ///
     /// https://zips.z.cash/protocol/protocol.pdf#saplingbalance
-    pub fn binding_verification_key(
+    pub fn binding_verification_key(&self) -> redjubjub::VerificationKeyBytes<Binding> {
-        &self,
-        value_balance: Amount,
-    ) -> redjubjub::VerificationKeyBytes<Binding> {
         let cv_old: ValueCommitment = self.spends().map(|spend| spend.cv).sum();
         let cv_new: ValueCommitment = self.outputs().map(|output| output.cv).sum();
-        let cv_balance: ValueCommitment = ValueCommitment::new(jubjub::Fr::zero(), value_balance);
+        let cv_balance: ValueCommitment =
+            ValueCommitment::new(jubjub::Fr::zero(), self.value_balance);
 
         let key_bytes: [u8; 32] = (cv_old - cv_new - cv_balance).into();
 
@@ -114,8 +209,14 @@ impl ShieldedData {
 // of a ShieldedData with at least one spend and at least one output, depending
 // on which goes in the `first` slot.  This is annoying but a smallish price to
 // pay for structural validity.
+//
+// A `ShieldedData<PerSpendAnchor>` can never be equal to a
+// `ShieldedData<SharedAnchor>`, even if they have the same effects.
 
-impl std::cmp::PartialEq for ShieldedData {
+impl<AnchorV> std::cmp::PartialEq for ShieldedData<AnchorV>
+where
+    AnchorV: AnchorVariant + Clone + PartialEq,
+{
     fn eq(&self, other: &Self) -> bool {
         // First check that the lengths match, so we know it is safe to use zip,
         // which truncates to the shorter of the two iterators.
@@ -126,11 +227,14 @@ impl std::cmp::PartialEq for ShieldedData {
             return false;
         }
 
-        // Now check that the binding_sig, spends, outputs match.
+        // Now check that all the fields match
-        self.binding_sig == other.binding_sig
+        self.value_balance == other.value_balance
+            && self.shared_anchor == other.shared_anchor
+            && self.binding_sig == other.binding_sig
             && self.spends().zip(other.spends()).all(|(a, b)| a == b)
             && self.outputs().zip(other.outputs()).all(|(a, b)| a == b)
     }
 }
 
-impl std::cmp::Eq for ShieldedData {}
+impl<AnchorV> std::cmp::Eq for ShieldedData<AnchorV> where AnchorV: AnchorVariant + Clone + PartialEq
+{}

zebra-chain/src/sapling/spend.rs

@@ -1,3 +1,8 @@
+//! Sapling spends for `V4` and `V5` `Transaction`s.
+//!
+//! Zebra uses a generic spend type for `V4` and `V5` transactions.
+//! The anchor change is handled using the `AnchorVariant` type trait.
+
 use std::io;
 
 use crate::{
@@ -10,17 +15,26 @@ use crate::{
     },
 };
 
-use super::{commitment, note, tree};
+use super::{commitment, note, tree, AnchorVariant, PerSpendAnchor, SharedAnchor};
 
 /// A _Spend Description_, as described in [protocol specification §7.3][ps].
 ///
+/// # Differences between Transaction Versions
+///
+/// In `Transaction::V4`, each `Spend` has its own anchor. In `Transaction::V5`,
+/// there is a single `shared_anchor` for the entire transaction. This
+/// structural difference is modeled using the `AnchorVariant` type trait.
+/// A type of `()` means "not present in this transaction version".
+///
 /// [ps]: https://zips.z.cash/protocol/protocol.pdf#spendencoding
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
-pub struct Spend {
+pub struct Spend<AnchorV: AnchorVariant> {
     /// A value commitment to the value of the input note.
     pub cv: commitment::ValueCommitment,
     /// A root of the Sapling note commitment tree at some block height in the past.
-    pub anchor: tree::Root,
+    ///
+    /// A type of `()` means "not present in this transaction version".
+    pub per_spend_anchor: AnchorV::PerSpend,
     /// The nullifier of the input note.
     pub nullifier: note::Nullifier,
     /// The randomized public key for `spend_auth_sig`.
@@ -31,7 +45,29 @@ pub struct Spend {
     pub spend_auth_sig: redjubjub::Signature<SpendAuth>,
 }
 
-impl Spend {
+impl From<(Spend<SharedAnchor>, tree::Root)> for Spend<PerSpendAnchor> {
+    /// Convert a `Spend<SharedAnchor>` and its shared anchor, into a
+    /// `Spend<PerSpendAnchor>`.
+    fn from(shared_spend: (Spend<SharedAnchor>, tree::Root)) -> Self {
+        Spend::<PerSpendAnchor> {
+            per_spend_anchor: shared_spend.1,
+            cv: shared_spend.0.cv,
+            nullifier: shared_spend.0.nullifier,
+            rk: shared_spend.0.rk,
+            zkproof: shared_spend.0.zkproof,
+            spend_auth_sig: shared_spend.0.spend_auth_sig,
+        }
+    }
+}
+
+impl From<(Spend<PerSpendAnchor>, ())> for Spend<PerSpendAnchor> {
+    /// Take the `Spend<PerSpendAnchor>` from a spend + anchor tuple.
+    fn from(per_spend: (Spend<PerSpendAnchor>, ())) -> Self {
+        per_spend.0
+    }
+}
+
+impl Spend<PerSpendAnchor> {
     /// Encodes the primary inputs for the proof statement as 7 Bls12_381 base
     /// field elements, to match bellman::groth16::verify_proof.
     ///
@@ -49,7 +85,8 @@ impl Spend {
         inputs.push(cv_affine.get_u());
         inputs.push(cv_affine.get_v());
 
-        inputs.push(jubjub::Fq::from_bytes(&self.anchor.into()).unwrap());
+        // TODO: V4 only
+        inputs.push(jubjub::Fq::from_bytes(&self.per_spend_anchor.into()).unwrap());
 
         let nullifier_limbs: [jubjub::Fq; 2] = self.nullifier.into();
 
@@ -60,10 +97,11 @@ impl Spend {
     }
 }
 
-impl ZcashSerialize for Spend {
+impl ZcashSerialize for Spend<PerSpendAnchor> {
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
         self.cv.zcash_serialize(&mut writer)?;
-        writer.write_all(&self.anchor.0[..])?;
+        // TODO: V4 only
+        writer.write_all(&self.per_spend_anchor.0[..])?;
         writer.write_32_bytes(&self.nullifier.into())?;
         writer.write_all(&<[u8; 32]>::from(self.rk)[..])?;
         self.zkproof.zcash_serialize(&mut writer)?;
@@ -72,12 +110,13 @@ impl ZcashSerialize for Spend {
     }
 }
 
-impl ZcashDeserialize for Spend {
+impl ZcashDeserialize for Spend<PerSpendAnchor> {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
         use crate::sapling::{commitment::ValueCommitment, note::Nullifier};
         Ok(Spend {
             cv: ValueCommitment::zcash_deserialize(&mut reader)?,
-            anchor: tree::Root(reader.read_32_bytes()?),
+            // TODO: V4 only
+            per_spend_anchor: tree::Root(reader.read_32_bytes()?),
             nullifier: Nullifier::from(reader.read_32_bytes()?),
             rk: reader.read_32_bytes()?.into(),
             zkproof: Groth16Proof::zcash_deserialize(&mut reader)?,

zebra-chain/src/sapling/tests.rs

@@ -1 +1 @@
-
+mod prop;

zebra-chain/src/sapling/tests/prop.rs

@@ -0,0 +1,68 @@
+use proptest::prelude::*;
+
+use super::super::super::transaction::*;
+use super::super::shielded_data::*;
+
+use crate::{
+    block,
+    serialization::{ZcashDeserializeInto, ZcashSerialize},
+};
+
+proptest! {
+    #[test]
+    fn shielded_data_roundtrip(shielded in any::<ShieldedData<PerSpendAnchor>>()) {
+        zebra_test::init();
+
+        // shielded data doesn't serialize by itself, so we have to stick it in
+        // a transaction
+        let tx = Transaction::V4 {
+            inputs: Vec::new(),
+            outputs: Vec::new(),
+            lock_time: LockTime::min_lock_time(),
+            expiry_height: block::Height(0),
+            joinsplit_data: None,
+            sapling_shielded_data: Some(shielded),
+        };
+
+        let data = tx.zcash_serialize_to_vec().expect("tx should serialize");
+        let tx_parsed = data.zcash_deserialize_into().expect("randomized tx should deserialize");
+
+        prop_assert_eq![tx, tx_parsed];
+    }
+
+    /// Check that ShieldedData serialization is equal if `shielded1 == shielded2`
+    #[test]
+    fn shielded_data_serialize_eq(shielded1 in any::<ShieldedData<PerSpendAnchor>>(), shielded2 in any::<ShieldedData<PerSpendAnchor>>()) {
+        zebra_test::init();
+
+        let shielded_eq = shielded1 == shielded2;
+
+        // shielded data doesn't serialize by itself, so we have to stick it in
+        // a transaction
+        let tx1 = Transaction::V4 {
+            inputs: Vec::new(),
+            outputs: Vec::new(),
+            lock_time: LockTime::min_lock_time(),
+            expiry_height: block::Height(0),
+            joinsplit_data: None,
+            sapling_shielded_data: Some(shielded1),
+        };
+        let tx2 = Transaction::V4 {
+            inputs: Vec::new(),
+            outputs: Vec::new(),
+            lock_time: LockTime::min_lock_time(),
+            expiry_height: block::Height(0),
+            joinsplit_data: None,
+            sapling_shielded_data: Some(shielded2),
+        };
+
+        let data1 = tx1.zcash_serialize_to_vec().expect("tx1 should serialize");
+        let data2 = tx2.zcash_serialize_to_vec().expect("tx2 should serialize");
+
+        if shielded_eq {
+            prop_assert_eq![data1, data2];
+        } else {
+            prop_assert_ne![data1, data2];
+        }
+    }
+}

zebra-chain/src/transaction.rs

@@ -7,7 +7,6 @@ mod joinsplit;
 mod lock_time;
 mod memo;
 mod serialize;
-mod shielded_data;
 mod sighash;
 
 #[cfg(any(test, feature = "proptest-impl"))]
@@ -19,11 +18,9 @@ pub use hash::Hash;
 pub use joinsplit::JoinSplitData;
 pub use lock_time::LockTime;
 pub use memo::Memo;
-pub use shielded_data::ShieldedData;
 pub use sighash::HashType;
 
 use crate::{
-    amount::Amount,
     block,
     parameters::NetworkUpgrade,
     primitives::{Bctv14Proof, Groth16Proof},
@@ -92,16 +89,12 @@ pub enum Transaction {
         lock_time: LockTime,
         /// The latest block height that this transaction can be added to the chain.
         expiry_height: block::Height,
-        /// The net value of Sapling spend transfers minus output transfers.
-        value_balance: Amount,
         /// The JoinSplit data for this transaction, if any.
         joinsplit_data: Option<JoinSplitData<Groth16Proof>>,
-        /// The shielded data for this transaction, if any.
+        /// The sapling shielded data for this transaction, if any.
-        shielded_data: Option<ShieldedData>,
+        sapling_shielded_data: Option<sapling::ShieldedData<sapling::PerSpendAnchor>>,
     },
     /// A `version = 5` transaction, which supports `Sapling` and `Orchard`.
-    // TODO: does this transaction type support `Sprout`?
-    // Check for ZIP-225 updates after the decision on 2021-03-05.
     V5 {
         /// The earliest time or block height that this transaction can be added to the
         /// chain.
@@ -224,9 +217,9 @@ impl Transaction {
         match self {
             // JoinSplits with Groth Proofs
             Transaction::V4 {
-                shielded_data: Some(shielded_data),
+                sapling_shielded_data: Some(sapling_shielded_data),
                 ..
-            } => Box::new(shielded_data.nullifiers()),
+            } => Box::new(sapling_shielded_data.nullifiers()),
             Transaction::V5 { .. } => {
                 unimplemented!("v5 transaction format as specified in ZIP-225")
             }
@@ -235,7 +228,7 @@ impl Transaction {
             | Transaction::V2 { .. }
             | Transaction::V3 { .. }
             | Transaction::V4 {
-                shielded_data: None,
+                sapling_shielded_data: None,
                 ..
             } => Box::new(std::iter::empty()),
         }

zebra-chain/src/transaction/arbitrary.rs

@@ -14,7 +14,7 @@ use crate::{
     sapling, sprout, transparent,
 };
 
-use super::{JoinSplitData, LockTime, Memo, ShieldedData, Transaction};
+use super::{JoinSplitData, LockTime, Memo, Transaction};
 
 impl Transaction {
     /// Generate a proptest strategy for V1 Transactions
@@ -79,8 +79,7 @@ impl Transaction {
             vec(any::<transparent::Output>(), 0..10),
             any::<LockTime>(),
             any::<block::Height>(),
-            any::<Amount>(),
+            option::of(any::<sapling::ShieldedData<sapling::PerSpendAnchor>>()),
-            option::of(any::<ShieldedData>()),
             option::of(any::<JoinSplitData<Groth16Proof>>()),
         )
             .prop_map(
@@ -89,21 +88,20 @@ impl Transaction {
                     outputs,
                     lock_time,
                     expiry_height,
-                    value_balance,
+                    sapling_shielded_data,
-                    shielded_data,
                     joinsplit_data,
                 )| Transaction::V4 {
                     inputs,
                     outputs,
                     lock_time,
                     expiry_height,
-                    value_balance,
+                    sapling_shielded_data,
-                    shielded_data,
                     joinsplit_data,
                 },
             )
             .boxed()
     }
+
     /// Generate a proptest strategy for V5 Transactions
     pub fn v5_strategy(ledger_state: LedgerState) -> BoxedStrategy<Self> {
         (
@@ -205,29 +203,34 @@ impl<P: ZkSnarkProof + Arbitrary + 'static> Arbitrary for JoinSplitData<P> {
     type Strategy = BoxedStrategy<Self>;
 }
 
-impl Arbitrary for ShieldedData {
+impl Arbitrary for sapling::ShieldedData<sapling::PerSpendAnchor> {
     type Parameters = ();
 
     fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
         (
+            any::<Amount>(),
             prop_oneof![
-                any::<sapling::Spend>().prop_map(Either::Left),
+                any::<sapling::Spend<sapling::PerSpendAnchor>>().prop_map(Either::Left),
                 any::<sapling::Output>().prop_map(Either::Right)
             ],
-            vec(any::<sapling::Spend>(), 0..10),
+            vec(any::<sapling::Spend<sapling::PerSpendAnchor>>(), 0..10),
             vec(any::<sapling::Output>(), 0..10),
             vec(any::<u8>(), 64),
         )
-            .prop_map(|(first, rest_spends, rest_outputs, sig_bytes)| Self {
+            .prop_map(
-                first,
+                |(value_balance, first, rest_spends, rest_outputs, sig_bytes)| Self {
-                rest_spends,
+                    value_balance,
-                rest_outputs,
+                    shared_anchor: (),
-                binding_sig: redjubjub::Signature::from({
+                    first,
-                    let mut b = [0u8; 64];
+                    rest_spends,
-                    b.copy_from_slice(sig_bytes.as_slice());
+                    rest_outputs,
-                    b
+                    binding_sig: redjubjub::Signature::from({
-                }),
+                        let mut b = [0u8; 64];
-            })
+                        b.copy_from_slice(sig_bytes.as_slice());
+                        b
+                    }),
+                },
+            )
             .boxed()
     }
 

zebra-chain/src/transaction/serialize.rs

@@ -131,8 +131,7 @@ impl ZcashSerialize for Transaction {
                 outputs,
                 lock_time,
                 expiry_height,
-                value_balance,
+                sapling_shielded_data,
-                shielded_data,
                 joinsplit_data,
             } => {
                 // Write version 4 and set the fOverwintered bit.
@@ -142,7 +141,6 @@ impl ZcashSerialize for Transaction {
                 outputs.zcash_serialize(&mut writer)?;
                 lock_time.zcash_serialize(&mut writer)?;
                 writer.write_u32::<LittleEndian>(expiry_height.0)?;
-                value_balance.zcash_serialize(&mut writer)?;
 
                 // The previous match arms serialize in one go, because the
                 // internal structure happens to nicely line up with the
@@ -152,13 +150,16 @@ impl ZcashSerialize for Transaction {
                 // instead we have to interleave serialization of the
                 // ShieldedData and the JoinSplitData.
 
-                match shielded_data {
+                match sapling_shielded_data {
                     None => {
+                        // Signal no value balance.
+                        writer.write_i64::<LittleEndian>(0)?;
                         // Signal no shielded spends and no shielded outputs.
                         writer.write_compactsize(0)?;
                         writer.write_compactsize(0)?;
                     }
                     Some(shielded_data) => {
+                        shielded_data.value_balance.zcash_serialize(&mut writer)?;
                         writer.write_compactsize(shielded_data.spends().count() as u64)?;
                         for spend in shielded_data.spends() {
                             spend.zcash_serialize(&mut writer)?;
@@ -175,7 +176,7 @@ impl ZcashSerialize for Transaction {
                     Some(jsd) => jsd.zcash_serialize(&mut writer)?,
                 }
 
-                match shielded_data {
+                match sapling_shielded_data {
                     Some(sd) => writer.write_all(&<[u8; 64]>::from(sd.binding_sig)[..])?,
                     None => {}
                 }
@@ -194,6 +195,7 @@ impl ZcashSerialize for Transaction {
                 writer.write_u32::<LittleEndian>(expiry_height.0)?;
                 inputs.zcash_serialize(&mut writer)?;
                 outputs.zcash_serialize(&mut writer)?;
+
                 // write the rest
                 writer.write_all(rest)?;
             }
@@ -266,22 +268,31 @@ impl ZcashDeserialize for Transaction {
                 let outputs = Vec::zcash_deserialize(&mut reader)?;
                 let lock_time = LockTime::zcash_deserialize(&mut reader)?;
                 let expiry_height = block::Height(reader.read_u32::<LittleEndian>()?);
+
                 let value_balance = (&mut reader).zcash_deserialize_into()?;
                 let mut shielded_spends = Vec::zcash_deserialize(&mut reader)?;
                 let mut shielded_outputs = Vec::zcash_deserialize(&mut reader)?;
+
                 let joinsplit_data = OptV4Jsd::zcash_deserialize(&mut reader)?;
 
                 use futures::future::Either::*;
-                let shielded_data = if !shielded_spends.is_empty() {
+                // Arbitraily use a spend for `first`, if both are present
-                    Some(ShieldedData {
+                let sapling_shielded_data = if !shielded_spends.is_empty() {
+                    Some(sapling::ShieldedData {
+                        value_balance,
+                        shared_anchor: (),
                         first: Left(shielded_spends.remove(0)),
                         rest_spends: shielded_spends,
                         rest_outputs: shielded_outputs,
                         binding_sig: reader.read_64_bytes()?.into(),
                     })
                 } else if !shielded_outputs.is_empty() {
-                    Some(ShieldedData {
+                    Some(sapling::ShieldedData {
+                        value_balance,
+                        shared_anchor: (),
                         first: Right(shielded_outputs.remove(0)),
+                        // the spends are actually empty here, but we use the
+                        // vec for consistency and readability
                         rest_spends: shielded_spends,
                         rest_outputs: shielded_outputs,
                         binding_sig: reader.read_64_bytes()?.into(),
@@ -295,8 +306,7 @@ impl ZcashDeserialize for Transaction {
                     outputs,
                     lock_time,
                     expiry_height,
-                    value_balance,
+                    sapling_shielded_data,
-                    shielded_data,
                     joinsplit_data,
                 })
             }
@@ -309,6 +319,7 @@ impl ZcashDeserialize for Transaction {
                 let expiry_height = block::Height(reader.read_u32::<LittleEndian>()?);
                 let inputs = Vec::zcash_deserialize(&mut reader)?;
                 let outputs = Vec::zcash_deserialize(&mut reader)?;
+
                 let mut rest = Vec::new();
                 reader.read_to_end(&mut rest)?;
 

zebra-chain/src/transaction/sighash.rs

@@ -419,11 +419,11 @@ impl<'a> SigHasher<'a> {
 
         let shielded_data = match self.trans {
             V4 {
-                shielded_data: Some(shielded_data),
+                sapling_shielded_data: Some(shielded_data),
                 ..
             } => shielded_data,
             V4 {
-                shielded_data: None,
+                sapling_shielded_data: None,
                 ..
             } => return writer.write_all(&[0; 32]),
             V5 { .. } => unimplemented!("v5 transaction hash as specified in ZIP-225 and ZIP-244"),
@@ -439,10 +439,12 @@ impl<'a> SigHasher<'a> {
             .personal(ZCASH_SHIELDED_SPENDS_HASH_PERSONALIZATION)
             .to_state();
 
+        // TODO: make a generic wrapper in `spends.rs` that does this serialization
         for spend in shielded_data.spends() {
             // This is the canonical transaction serialization, minus the `spendAuthSig`.
             spend.cv.zcash_serialize(&mut hash)?;
-            hash.write_all(&spend.anchor.0[..])?;
+            // TODO: ZIP-243 Sapling to Canopy only
+            hash.write_all(&spend.per_spend_anchor.0[..])?;
             hash.write_32_bytes(&spend.nullifier.into())?;
             hash.write_all(&<[u8; 32]>::from(spend.rk)[..])?;
             spend.zkproof.zcash_serialize(&mut hash)?;
@@ -456,11 +458,11 @@ impl<'a> SigHasher<'a> {
 
         let shielded_data = match self.trans {
             V4 {
-                shielded_data: Some(shielded_data),
+                sapling_shielded_data: Some(shielded_data),
                 ..
             } => shielded_data,
             V4 {
-                shielded_data: None,
+                sapling_shielded_data: None,
                 ..
             } => return writer.write_all(&[0; 32]),
             V5 { .. } => unimplemented!("v5 transaction hash as specified in ZIP-225 and ZIP-244"),
@@ -484,10 +486,18 @@ impl<'a> SigHasher<'a> {
     }
 
     fn hash_value_balance<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
+        use crate::amount::Amount;
+        use std::convert::TryFrom;
         use Transaction::*;
 
         let value_balance = match self.trans {
-            V4 { value_balance, .. } => value_balance,
+            V4 {
+                sapling_shielded_data,
+                ..
+            } => match sapling_shielded_data {
+                Some(s) => s.value_balance,
+                None => Amount::try_from(0).unwrap(),
+            },
             V5 { .. } => unimplemented!("v5 transaction hash as specified in ZIP-225 and ZIP-244"),
             V1 { .. } | V2 { .. } | V3 { .. } => unreachable!(ZIP243_EXPLANATION),
         };

zebra-consensus/src/primitives/groth16.rs

@@ -20,7 +20,7 @@ use tokio::sync::broadcast::{channel, error::RecvError, Sender};
 use tower::{util::ServiceFn, Service};
 use tower_batch::{Batch, BatchControl};
 use tower_fallback::Fallback;
-use zebra_chain::sapling::{Output, Spend};
+use zebra_chain::sapling::{Output, PerSpendAnchor, Spend};
 
 mod hash_reader;
 mod params;
@@ -101,8 +101,8 @@ pub type Item = batch::Item<Bls12>;
 
 pub struct ItemWrapper(Item);
 
-impl From<&Spend> for ItemWrapper {
+impl From<&Spend<PerSpendAnchor>> for ItemWrapper {
-    fn from(spend: &Spend) -> Self {
+    fn from(spend: &Spend<PerSpendAnchor>) -> Self {
         Self(Item::from((
             bellman::groth16::Proof::read(&spend.zkproof.0[..]).unwrap(),
             spend.primary_inputs(),

zebra-consensus/src/primitives/groth16/tests.rs

@@ -30,15 +30,18 @@ where
             | Transaction::V2 { .. }
             | Transaction::V3 { .. }
             | Transaction::V5 { .. } => (),
-            Transaction::V4 { shielded_data, .. } => {
+            Transaction::V4 {
-                if let Some(shielded_data) = shielded_data {
+                sapling_shielded_data,
-                    for spend in shielded_data.spends() {
+                ..
+            } => {
+                if let Some(shielded_data) = sapling_shielded_data {
+                    for spend in shielded_data.spends_per_anchor() {
                         tracing::trace!(?spend);
 
                         let spend_rsp = spend_verifier
                             .ready_and()
                             .await?
-                            .call(groth16::ItemWrapper::from(spend).into());
+                            .call(groth16::ItemWrapper::from(&spend).into());
 
                         async_checks.push(spend_rsp);
                     }
@@ -110,8 +113,11 @@ where
             | Transaction::V2 { .. }
             | Transaction::V3 { .. }
             | Transaction::V5 { .. } => (),
-            Transaction::V4 { shielded_data, .. } => {
+            Transaction::V4 {
-                if let Some(shielded_data) = shielded_data {
+                sapling_shielded_data,
+                ..
+            } => {
+                if let Some(shielded_data) = sapling_shielded_data {
                     for output in shielded_data.outputs() {
                         // This changes the primary inputs to the proof
                         // verification, causing it to fail for this proof.

zebra-consensus/src/transaction.rs

@@ -147,9 +147,8 @@ where
                     // outputs,
                     // lock_time,
                     // expiry_height,
-                    value_balance,
                     joinsplit_data,
-                    shielded_data,
+                    sapling_shielded_data,
                     ..
                 } => {
                     // A set of asynchronous checks which must all succeed.
@@ -213,16 +212,16 @@ where
                         async_checks.push(rsp.boxed());
                     }
 
-                    if let Some(shielded_data) = shielded_data {
+                    if let Some(shielded_data) = sapling_shielded_data {
-                        check::shielded_balances_match(&shielded_data, *value_balance)?;
+                        check::shielded_balances_match(&shielded_data)?;
 
-                        for spend in shielded_data.spends() {
+                        for spend in shielded_data.spends_per_anchor() {
                             // Consensus rule: cv and rk MUST NOT be of small
                             // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk
                             // MUST NOT be 𝒪_J.
                             //
                             // https://zips.z.cash/protocol/protocol.pdf#spenddesc
-                            check::spend_cv_rk_not_small_order(spend)?;
+                            check::spend_cv_rk_not_small_order(&spend)?;
 
                             // Consensus rule: The proof π_ZKSpend MUST be valid
                             // given a primary input formed from the other
@@ -236,7 +235,7 @@ where
                             let spend_rsp = spend_verifier
                                 .ready_and()
                                 .await?
-                                .call(primitives::groth16::ItemWrapper::from(spend).into());
+                                .call(primitives::groth16::ItemWrapper::from(&spend).into());
 
                             async_checks.push(spend_rsp.boxed());
 
@@ -282,7 +281,7 @@ where
                             async_checks.push(output_rsp.boxed());
                         }
 
-                        let bvk = shielded_data.binding_verification_key(*value_balance);
+                        let bvk = shielded_data.binding_verification_key();
 
                         // TODO: enable async verification and remove this block - #1939
                         {

zebra-consensus/src/transaction/check.rs

@@ -3,9 +3,8 @@
 //! Code in this file can freely assume that no pre-V4 transactions are present.
 
 use zebra_chain::{
-    amount::Amount,
+    sapling::{AnchorVariant, Output, PerSpendAnchor, ShieldedData, Spend},
-    sapling::{Output, Spend},
+    transaction::Transaction,
-    transaction::{ShieldedData, Transaction},
 };
 
 use crate::error::TransactionError;
@@ -27,7 +26,7 @@ pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError>
             inputs,
             outputs,
             joinsplit_data,
-            shielded_data,
+            sapling_shielded_data,
             ..
         } => {
             let tx_in_count = inputs.len();
@@ -36,11 +35,11 @@ pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError>
                 .as_ref()
                 .map(|d| d.joinsplits().count())
                 .unwrap_or(0);
-            let n_shielded_spend = shielded_data
+            let n_shielded_spend = sapling_shielded_data
                 .as_ref()
                 .map(|d| d.spends().count())
                 .unwrap_or(0);
-            let n_shielded_output = shielded_data
+            let n_shielded_output = sapling_shielded_data
                 .as_ref()
                 .map(|d| d.outputs().count())
                 .unwrap_or(0);
@@ -65,12 +64,14 @@ pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError>
 /// Check that if there are no Spends or Outputs, that valueBalance is also 0.
 ///
 /// https://zips.z.cash/protocol/protocol.pdf#consensusfrombitcoin
-pub fn shielded_balances_match(
+pub fn shielded_balances_match<AnchorV>(
-    shielded_data: &ShieldedData,
+    shielded_data: &ShieldedData<AnchorV>,
-    value_balance: Amount,
+) -> Result<(), TransactionError>
-) -> Result<(), TransactionError> {
+where
+    AnchorV: AnchorVariant + Clone,
+{
     if (shielded_data.spends().count() + shielded_data.outputs().count() != 0)
-        || i64::from(value_balance) == 0
+        || i64::from(shielded_data.value_balance) == 0
     {
         Ok(())
     } else {
@@ -93,9 +94,11 @@ pub fn coinbase_tx_no_joinsplit_or_spend(tx: &Transaction) -> Result<(), Transac
             // The ShieldedData contains both Spends and Outputs, and Outputs
             // are allowed post-Heartwood, so we have to count Spends.
             Transaction::V4 {
-                shielded_data: Some(shielded_data),
+                sapling_shielded_data: Some(sapling_shielded_data),
                 ..
-            } if shielded_data.spends().count() > 0 => Err(TransactionError::CoinbaseHasSpend),
+            } if sapling_shielded_data.spends().count() > 0 => {
+                Err(TransactionError::CoinbaseHasSpend)
+            }
 
             Transaction::V4 { .. } => Ok(()),
 
@@ -116,7 +119,7 @@ pub fn coinbase_tx_no_joinsplit_or_spend(tx: &Transaction) -> Result<(), Transac
 /// i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk MUST NOT be 𝒪_J.
 ///
 /// https://zips.z.cash/protocol/protocol.pdf#spenddesc
-pub fn spend_cv_rk_not_small_order(spend: &Spend) -> Result<(), TransactionError> {
+pub fn spend_cv_rk_not_small_order(spend: &Spend<PerSpendAnchor>) -> Result<(), TransactionError> {
     if bool::from(spend.cv.0.is_small_order())
         || bool::from(
             jubjub::AffinePoint::from_bytes(spend.rk.into())

zebra-state/src/service/non_finalized_state/chain.rs

@@ -166,13 +166,13 @@ impl UpdateWith<PreparedBlock> for Chain {
             .enumerate()
         {
             use transaction::Transaction::*;
-            let (inputs, shielded_data, joinsplit_data) = match transaction.deref() {
+            let (inputs, joinsplit_data, sapling_shielded_data) = match transaction.deref() {
                 V4 {
                     inputs,
-                    shielded_data,
                     joinsplit_data,
+                    sapling_shielded_data,
                     ..
-                } => (inputs, shielded_data, joinsplit_data),
+                } => (inputs, joinsplit_data, sapling_shielded_data),
                 V5 { .. } => unimplemented!("v5 transaction format as specified in ZIP-225"),
                 V1 { .. } | V2 { .. } | V3 { .. } => unreachable!(
                     "older transaction versions only exist in finalized blocks pre sapling",
@@ -195,7 +195,7 @@ impl UpdateWith<PreparedBlock> for Chain {
             // add sprout anchor and nullifiers
             self.update_chain_state_with(joinsplit_data);
             // add sapling anchor and nullifier
-            self.update_chain_state_with(shielded_data);
+            self.update_chain_state_with(sapling_shielded_data);
         }
     }
 
@@ -226,13 +226,13 @@ impl UpdateWith<PreparedBlock> for Chain {
             block.transactions.iter().zip(transaction_hashes.iter())
         {
             use transaction::Transaction::*;
-            let (inputs, shielded_data, joinsplit_data) = match transaction.deref() {
+            let (inputs, joinsplit_data, sapling_shielded_data) = match transaction.deref() {
                 V4 {
                     inputs,
-                    shielded_data,
                     joinsplit_data,
+                    sapling_shielded_data,
                     ..
-                } => (inputs, shielded_data, joinsplit_data),
+                } => (inputs, joinsplit_data, sapling_shielded_data),
                 V5 { .. } => unimplemented!("v5 transaction format as specified in ZIP-225"),
                 V1 { .. } | V2 { .. } | V3 { .. } => unreachable!(
                     "older transaction versions only exist in finalized blocks pre sapling",
@@ -252,7 +252,7 @@ impl UpdateWith<PreparedBlock> for Chain {
             // remove sprout anchor and nullifiers
             self.revert_chain_state_with(joinsplit_data);
             // remove sapling anchor and nullfier
-            self.revert_chain_state_with(shielded_data);
+            self.revert_chain_state_with(sapling_shielded_data);
         }
     }
 }
@@ -336,18 +336,21 @@ impl UpdateWith<Option<transaction::JoinSplitData<Groth16Proof>>> for Chain {
     }
 }
 
-impl UpdateWith<Option<transaction::ShieldedData>> for Chain {
+impl<AnchorV> UpdateWith<Option<sapling::ShieldedData<AnchorV>>> for Chain
-    fn update_chain_state_with(&mut self, shielded_data: &Option<transaction::ShieldedData>) {
+where
+    AnchorV: sapling::AnchorVariant + Clone,
+{
+    fn update_chain_state_with(&mut self, shielded_data: &Option<sapling::ShieldedData<AnchorV>>) {
         if let Some(shielded_data) = shielded_data {
-            for sapling::Spend { nullifier, .. } in shielded_data.spends() {
+            for nullifier in shielded_data.nullifiers() {
                 self.sapling_nullifiers.insert(*nullifier);
             }
         }
     }
 
-    fn revert_chain_state_with(&mut self, shielded_data: &Option<transaction::ShieldedData>) {
+    fn revert_chain_state_with(&mut self, shielded_data: &Option<sapling::ShieldedData<AnchorV>>) {
         if let Some(shielded_data) = shielded_data {
-            for sapling::Spend { nullifier, .. } in shielded_data.spends() {
+            for nullifier in shielded_data.nullifiers() {
                 assert!(
                     self.sapling_nullifiers.remove(nullifier),
                     "nullifier must be present if block was"