Implement Sapling serialization in Transaction V5 (#2020)

* serialize/deserialize spaling shielded data in v5 transaction

* fix serialize/deserialize fields order according to spec

* remove extra clone calls

* more serialize fixes

* clippy: fix empty array

* tidy comments

* Add v4 and v5 transaction tests

Also make sure that serialized bytes match if structs match.

* Test fake v5 blocks made out of pre-NU5 block test vectors

* Add outputs-only tests for v5 shared anchor serialization

* Refactor sapling::ShieldedData V5 serialization into its own impl

* Fix spec name typos

* Simplify sapling shielded data parsing

* Delete redundant V5 transaction wrappers in tests

And split out sapling ShieldedData serialization.

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>

Cargo.lock

@@ -1579,6 +1579,15 @@ version = "2.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "47be2f14c678be2fdcab04ab1171db51b2762ce6f0a8ee87c8dd4a04ed216135"
 
+[[package]]
+name = "itertools"
+version = "0.10.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "37d572918e350e82412fe766d24b15e6682fb2ed2bbe018280caa810397cb319"
+dependencies = [
+ "either",
+]
+
 [[package]]
 name = "itoa"
 version = "0.4.6"
@@ -4044,6 +4053,7 @@ dependencies = [
  "equihash",
  "futures 0.3.14",
  "hex",
+ "itertools",
  "jubjub",
  "lazy_static",
  "primitive-types",

zebra-chain/Cargo.toml

@@ -46,9 +46,13 @@ redjubjub = "0.4"
 
 [dev-dependencies]
 bincode = "1"
+
 color-eyre = "0.5.11"
 spandoc = "0.2"
 tracing = "0.1.25"
+
+itertools = "0.10.0"
+
 proptest = "0.10"
 proptest-derive = "0.3"
 

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

@@ -19,7 +19,12 @@ proptest! {
         let bytes = hash.zcash_serialize_to_vec()?;
         let other_hash: Hash = bytes.zcash_deserialize_into()?;
 
-        prop_assert_eq![hash, other_hash];
+        prop_assert_eq![&hash, &other_hash];
+
+        let bytes2 = other_hash
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![bytes, bytes2, "bytes must be equal if structs are equal"];
     }
 
     #[test]
@@ -38,7 +43,12 @@ proptest! {
         let bytes = header.zcash_serialize_to_vec()?;
         let other_header = bytes.zcash_deserialize_into()?;
 
-        prop_assert_eq![header, other_header];
+        prop_assert_eq![&header, &other_header];
+
+        let bytes2 = other_header
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![bytes, bytes2, "bytes must be equal if structs are equal"];
     }
 
     #[test]
@@ -72,7 +82,6 @@ proptest! {
         zebra_test::init();
 
         let bytes = block.zcash_serialize_to_vec()?;
-        let bytes = &mut bytes.as_slice();
 
         // Check the block commitment
         let commitment = block.commitment(network);
@@ -86,7 +95,12 @@ proptest! {
             // Check deserialization
             let other_block = bytes.zcash_deserialize_into()?;
 
-            prop_assert_eq![block, other_block];
+            prop_assert_eq![&block, &other_block];
+
+            let bytes2 = other_block
+                .zcash_serialize_to_vec()
+                .expect("vec serialization is infallible");
+            prop_assert_eq![bytes, bytes2, "bytes must be equal if structs are equal"];
         } else {
             let serialization_err = bytes.zcash_deserialize_into::<Block>()
                 .expect_err("blocks larger than the maximum size should fail");

zebra-chain/src/block/tests/vectors.rs

@@ -72,14 +72,15 @@ fn deserialize_blockheader() {
 }
 
 #[test]
-fn deserialize_block() {
+fn round_trip_blocks() {
     zebra_test::init();
 
-    // this one has a bad version field
+    // this one has a bad version field, but it is still valid
     zebra_test::vectors::BLOCK_MAINNET_434873_BYTES
         .zcash_deserialize_into::<Block>()
-        .expect("block test vector should deserialize");
+        .expect("bad version block test vector should deserialize");
 
+    // now do a round-trip test on all the block test vectors
     for block_bytes in zebra_test::vectors::BLOCKS.iter() {
         let block = block_bytes
             .zcash_deserialize_into::<Block>()

zebra-chain/src/sapling.rs

@@ -5,23 +5,23 @@ mod address;
 mod arbitrary;
 mod commitment;
 mod note;
-mod output;
-mod spend;
 #[cfg(test)]
 mod tests;
 
 // XXX clean up these modules
 
 pub mod keys;
+pub mod output;
 pub mod shielded_data;
+pub mod spend;
 pub mod tree;
 
 pub use address::Address;
 pub use commitment::{CommitmentRandomness, NoteCommitment, ValueCommitment};
 pub use keys::Diversifier;
 pub use note::{EncryptedNote, Note, Nullifier, WrappedNoteKey};
-pub use output::{Output, OutputInTransactionV4};
+pub use output::{Output, OutputInTransactionV4, OutputPrefixInTransactionV5};
 pub use shielded_data::{
     AnchorVariant, FieldNotPresent, PerSpendAnchor, SharedAnchor, ShieldedData,
 };
-pub use spend::Spend;
+pub use spend::{Spend, SpendPrefixInTransactionV5};

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

@@ -20,7 +20,12 @@ proptest! {
 
         let data = spend.zcash_serialize_to_vec().expect("spend should serialize");
         let spend_parsed = data.zcash_deserialize_into().expect("randomized spend should deserialize");
-        prop_assert_eq![spend, spend_parsed];
+        prop_assert_eq![&spend, &spend_parsed];
+
+        let data2 = spend_parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
     }
 
     /// Serialize and deserialize `Spend<SharedAnchor>`
@@ -34,15 +39,30 @@ proptest! {
 
         let data = prefix.zcash_serialize_to_vec().expect("spend prefix should serialize");
         let parsed = data.zcash_deserialize_into().expect("randomized spend prefix should deserialize");
-        prop_assert_eq![prefix, parsed];
+        prop_assert_eq![&prefix, &parsed];
+
+        let data2 = parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
 
         let data = zkproof.zcash_serialize_to_vec().expect("spend zkproof should serialize");
         let parsed = data.zcash_deserialize_into().expect("randomized spend zkproof should deserialize");
-        prop_assert_eq![zkproof, parsed];
+        prop_assert_eq![&zkproof, &parsed];
+
+        let data2 = parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
 
         let data = spend_auth_sig.zcash_serialize_to_vec().expect("spend auth sig should serialize");
         let parsed = data.zcash_deserialize_into().expect("randomized spend auth sig should deserialize");
-        prop_assert_eq![spend_auth_sig, parsed];
+        prop_assert_eq![&spend_auth_sig, &parsed];
+
+        let data2 = parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
     }
 
     /// Serialize and deserialize `Output`
@@ -57,25 +77,38 @@ proptest! {
         let output_parsed = data.zcash_deserialize_into::<OutputInTransactionV4>().expect("randomized output should deserialize").into_output();
         prop_assert_eq![&output, &output_parsed];
 
+        let data2 = output_parsed
+            .into_v4()
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
+
         // v5 format
         let (prefix, zkproof) = output.into_v5_parts();
 
         let data = prefix.zcash_serialize_to_vec().expect("output prefix should serialize");
         let parsed = data.zcash_deserialize_into().expect("randomized output prefix should deserialize");
-        prop_assert_eq![prefix, parsed];
+        prop_assert_eq![&prefix, &parsed];
+
+        let data2 = parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
 
         let data = zkproof.zcash_serialize_to_vec().expect("output zkproof should serialize");
         let parsed = data.zcash_deserialize_into().expect("randomized output zkproof should deserialize");
-        prop_assert_eq![zkproof, parsed];
+        prop_assert_eq![&zkproof, &parsed];
 
+        let data2 = parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
     }
 }
 
 proptest! {
     /// Serialize and deserialize `PerSpendAnchor` shielded data by including it
     /// in a V4 transaction
-    //
-    // TODO: write a similar test for `ShieldedData<SharedAnchor>` (#1829)
     #[test]
     fn shielded_data_v4_roundtrip(
         shielded_v4 in any::<sapling::ShieldedData<PerSpendAnchor>>(),
@@ -96,15 +129,121 @@ proptest! {
         };
         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];
+        prop_assert_eq![&tx, &tx_parsed];
+
+        let data2 = tx_parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
+    }
+
+    /// Serialize and deserialize `SharedAnchor` shielded data
+    #[test]
+    fn shielded_data_v5_roundtrip(
+        shielded_v5 in any::<sapling::ShieldedData<SharedAnchor>>(),
+    ) {
+        zebra_test::init();
+
+        let data = shielded_v5.zcash_serialize_to_vec().expect("shielded_v5 should serialize");
+        let shielded_v5_parsed = data.zcash_deserialize_into().expect("randomized shielded_v5 should deserialize");
+
+        if let Some(shielded_v5_parsed) = shielded_v5_parsed {
+            prop_assert_eq![&shielded_v5,
+                            &shielded_v5_parsed];
+
+            let data2 = shielded_v5_parsed
+                .zcash_serialize_to_vec()
+                .expect("vec serialization is infallible");
+            prop_assert_eq![data, data2, "data must be equal if structs are equal"];
+        } else {
+            panic!("unexpected parsing error: ShieldedData should be Some(_)");
+        }
+    }
+
+    /// Test v4 with empty spends, but some outputs
+    #[test]
+    fn shielded_data_v4_outputs_only(
+        shielded_v4 in any::<sapling::ShieldedData<PerSpendAnchor>>(),
+    ) {
+        use Either::*;
+
+        zebra_test::init();
+
+        // we need at least one output to delete all the spends
+        prop_assume!(shielded_v4.outputs().count() > 0);
+
+        // TODO: modify the strategy, rather than the shielded data
+        let mut shielded_v4 = shielded_v4;
+        let mut outputs: Vec<_> = shielded_v4.outputs().cloned().collect();
+        shielded_v4.rest_spends = Vec::new();
+        shielded_v4.first = Right(outputs.remove(0));
+        shielded_v4.rest_outputs = outputs;
+
+        // shielded data doesn't serialize by itself, so we have to stick it in
+        // a transaction
+
+        // stick `PerSpendAnchor` shielded data into a v4 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_v4),
+        };
+        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];
+
+        let data2 = tx_parsed
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
+    }
+
+    /// Test the v5 shared anchor serialization condition: empty spends, but some outputs
+    #[test]
+    fn shielded_data_v5_outputs_only(
+        shielded_v5 in any::<sapling::ShieldedData<SharedAnchor>>(),
+    ) {
+        use Either::*;
+
+        zebra_test::init();
+
+        // we need at least one output to delete all the spends
+        prop_assume!(shielded_v5.outputs().count() > 0);
+
+        // TODO: modify the strategy, rather than the shielded data
+        let mut shielded_v5 = shielded_v5;
+        let mut outputs: Vec<_> = shielded_v5.outputs().cloned().collect();
+        shielded_v5.rest_spends = Vec::new();
+        shielded_v5.first = Right(outputs.remove(0));
+        shielded_v5.rest_outputs = outputs;
+        // TODO: delete the shared anchor when there are no spends
+        shielded_v5.shared_anchor = Default::default();
+
+        let data = shielded_v5.zcash_serialize_to_vec().expect("shielded_v5 should serialize");
+        let shielded_v5_parsed = data.zcash_deserialize_into().expect("randomized shielded_v5 should deserialize");
+
+        if let Some(shielded_v5_parsed) = shielded_v5_parsed {
+            prop_assert_eq![&shielded_v5,
+                            &shielded_v5_parsed];
+
+            let data2 = shielded_v5_parsed
+                .zcash_serialize_to_vec()
+                .expect("vec serialization is infallible");
+            prop_assert_eq![data, data2, "data must be equal if structs are equal"];
+        } else {
+            panic!("unexpected parsing error: ShieldedData should be Some(_)");
+        }
     }
 
     /// Check that ShieldedData<PerSpendAnchor> is equal when `first` is swapped
     /// between a spend and an output
-    //
-    // TODO: write a similar test for `ShieldedData<SharedAnchor>` (#1829)
     #[test]
-    fn shielded_data_per_spend_swap_first_eq(shielded1 in any::<sapling::ShieldedData<PerSpendAnchor>>()) {
+    fn shielded_data_per_spend_swap_first_eq(
+        shielded1 in any::<sapling::ShieldedData<PerSpendAnchor>>()
+    ) {
         use Either::*;
 
         zebra_test::init();
@@ -157,12 +296,50 @@ proptest! {
         prop_assert_eq![data1, data2];
     }
 
+    /// Check that ShieldedData<SharedAnchor> is equal when `first` is swapped
+    /// between a spend and an output
+    #[test]
+    fn shielded_data_shared_swap_first_eq(
+        shielded1 in any::<sapling::ShieldedData<SharedAnchor>>()
+    ) {
+        use Either::*;
+
+        zebra_test::init();
+
+        // we need at least one spend and one output to swap them
+        prop_assume!(shielded1.spends().count() > 0 && shielded1.outputs().count() > 0);
+
+        let mut shielded2 = shielded1.clone();
+        let mut spends: Vec<_> = shielded2.spends().cloned().collect();
+        let mut outputs: Vec<_> = shielded2.outputs().cloned().collect();
+        match shielded2.first {
+            Left(_spend) => {
+                shielded2.first = Right(outputs.remove(0));
+                shielded2.rest_outputs = outputs;
+                shielded2.rest_spends = spends;
+            }
+            Right(_output) => {
+                shielded2.first = Left(spends.remove(0));
+                shielded2.rest_spends = spends;
+                shielded2.rest_outputs = outputs;
+            }
+        }
+
+        prop_assert_eq![&shielded1, &shielded2];
+
+        let data1 = shielded1.zcash_serialize_to_vec().expect("shielded1 should serialize");
+        let data2 = shielded2.zcash_serialize_to_vec().expect("shielded2 should serialize");
+
+        prop_assert_eq![data1, data2];
+    }
+
     /// Check that ShieldedData<PerSpendAnchor> serialization is equal if
     /// `shielded1 == shielded2`
-    //
-    // TODO: write a similar test for `ShieldedData<SharedAnchor>` (#1829)
     #[test]
-    fn shielded_data_per_spend_serialize_eq(shielded1 in any::<sapling::ShieldedData<PerSpendAnchor>>(), shielded2 in any::<sapling::ShieldedData<PerSpendAnchor>>()) {
+    fn shielded_data_per_spend_serialize_eq(
+        shielded1 in any::<sapling::ShieldedData<PerSpendAnchor>>(),
+        shielded2 in any::<sapling::ShieldedData<PerSpendAnchor>>()
+    ) {
         zebra_test::init();
 
         let shielded_eq = shielded1 == shielded2;
@@ -202,14 +379,36 @@ proptest! {
         }
     }
 
+    /// Check that ShieldedData<SharedAnchor> serialization is equal if
+    /// `shielded1 == shielded2`
+    #[test]
+    fn shielded_data_shared_serialize_eq(
+        shielded1 in any::<sapling::ShieldedData<SharedAnchor>>(),
+        shielded2 in any::<sapling::ShieldedData<SharedAnchor>>()
+    ) {
+        zebra_test::init();
+
+        let shielded_eq = shielded1 == shielded2;
+
+        let data1 = shielded1.zcash_serialize_to_vec().expect("shielded1 should serialize");
+        let data2 = shielded2.zcash_serialize_to_vec().expect("shielded2 should serialize");
+
+        if shielded_eq {
+            prop_assert_eq![data1, data2];
+        } else {
+            prop_assert_ne![data1, data2];
+        }
+    }
+
     /// Check that ShieldedData<PerSpendAnchor> serialization is equal when we
     /// replace all the known fields.
     ///
     /// This test checks for extra fields that are not in `ShieldedData::eq`.
-    //
-    // TODO: write a similar test for `ShieldedData<SharedAnchor>` (#1829)
     #[test]
-    fn shielded_data_per_spend_field_assign_eq(shielded1 in any::<sapling::ShieldedData<PerSpendAnchor>>(), shielded2 in any::<sapling::ShieldedData<PerSpendAnchor>>()) {
+    fn shielded_data_per_spend_field_assign_eq(
+        shielded1 in any::<sapling::ShieldedData<PerSpendAnchor>>(),
+        shielded2 in any::<sapling::ShieldedData<PerSpendAnchor>>()
+    ) {
         zebra_test::init();
 
         let mut shielded2 = shielded2;
@@ -252,4 +451,37 @@ proptest! {
 
         prop_assert_eq![data1, data2];
     }
+
+    /// Check that ShieldedData<SharedAnchor> serialization is equal when we
+    /// replace all the known fields.
+    ///
+    /// This test checks for extra fields that are not in `ShieldedData::eq`.
+    #[test]
+    fn shielded_data_shared_field_assign_eq(
+        shielded1 in any::<sapling::ShieldedData<SharedAnchor>>(),
+        shielded2 in any::<sapling::ShieldedData<SharedAnchor>>()
+    ) {
+        zebra_test::init();
+
+        let mut shielded2 = shielded2;
+
+        // TODO: modify the strategy, rather than the shielded data
+        //
+        // these fields must match ShieldedData::eq
+        // the spends() and outputs() checks cover first, rest_spends, and rest_outputs
+        shielded2.first = shielded1.first.clone();
+        shielded2.rest_spends = shielded1.rest_spends.clone();
+        shielded2.rest_outputs = shielded1.rest_outputs.clone();
+        // now for the fields that are checked literally
+        shielded2.value_balance = shielded1.value_balance;
+        shielded2.shared_anchor = shielded1.shared_anchor;
+        shielded2.binding_sig = shielded1.binding_sig;
+
+        prop_assert_eq![&shielded1, &shielded2];
+
+        let data1 = shielded1.zcash_serialize_to_vec().expect("shielded1 should serialize");
+        let data2 = shielded2.zcash_serialize_to_vec().expect("shielded2 should serialize");
+
+        prop_assert_eq![data1, data2];
+    }
 }

zebra-chain/src/transaction/arbitrary.rs

@@ -257,7 +257,7 @@ impl Arbitrary for sapling::ShieldedData<sapling::SharedAnchor> {
         )
             .prop_map(
                 |(value_balance, shared_anchor, first, rest_spends, rest_outputs, sig_bytes)| {
-                    Self {
+                    let mut shielded_data = Self {
                         value_balance,
                         shared_anchor,
                         first,
@@ -268,7 +268,12 @@ impl Arbitrary for sapling::ShieldedData<sapling::SharedAnchor> {
                             b.copy_from_slice(sig_bytes.as_slice());
                             b
                         }),
+                    };
+                    if shielded_data.spends().count() == 0 {
+                        // Todo: delete field when there is no spend
+                        shielded_data.shared_anchor = Default::default();
                     }
+                    shielded_data
                 },
             )
             .boxed()

zebra-chain/src/transaction/serialize.rs

@@ -8,16 +8,17 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use crate::{
     block::MAX_BLOCK_BYTES,
     parameters::{OVERWINTER_VERSION_GROUP_ID, SAPLING_VERSION_GROUP_ID, TX_V5_VERSION_GROUP_ID},
-    primitives::ZkSnarkProof,
+    primitives::{Groth16Proof, ZkSnarkProof},
     serialization::{
-        ReadZcashExt, SerializationError, TrustedPreallocate, WriteZcashExt, ZcashDeserialize,
+        zcash_deserialize_external_count, zcash_serialize_external_count, ReadZcashExt,
+        SerializationError, TrustedPreallocate, WriteZcashExt, ZcashDeserialize,
         ZcashDeserializeInto, ZcashSerialize,
     },
     sprout,
 };
 
 use super::*;
-use sapling::Output;
+use sapling::{Output, SharedAnchor, Spend};
 
 impl ZcashDeserialize for jubjub::Fq {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
@@ -32,6 +33,11 @@ impl ZcashDeserialize for jubjub::Fq {
         }
     }
 }
+
+// Transaction V3 and V4 serialize sprout JoinSplitData in a single continuous
+// byte range, so we can implement its serialization and deserialization
+// separately.
+
 impl<P: ZkSnarkProof> ZcashSerialize for JoinSplitData<P> {
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
         writer.write_compactsize(self.joinsplits().count() as u64)?;
@@ -68,6 +74,160 @@ impl<P: ZkSnarkProof> ZcashDeserialize for Option<JoinSplitData<P>> {
     }
 }
 
+// Transaction::V5 serializes sapling ShieldedData in a single continuous byte
+// range, so we can implement its serialization and deserialization separately.
+// (Unlike V4, where it must be serialized as part of the transaction.)
+
+impl ZcashSerialize for Option<sapling::ShieldedData<SharedAnchor>> {
+    fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
+        match self {
+            None => {
+                // nSpendsSapling
+                writer.write_compactsize(0)?;
+                // nOutputsSapling
+                writer.write_compactsize(0)?;
+            }
+            Some(shielded_data) => {
+                shielded_data.zcash_serialize(&mut writer)?;
+            }
+        }
+        Ok(())
+    }
+}
+
+impl ZcashSerialize for sapling::ShieldedData<SharedAnchor> {
+    fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
+        // Collect arrays for Spends
+        // There's no unzip3, so we have to unzip twice.
+        let (spend_prefixes, spend_proofs_sigs): (Vec<_>, Vec<_>) = self
+            .spends()
+            .cloned()
+            .map(sapling::Spend::<SharedAnchor>::into_v5_parts)
+            .map(|(prefix, proof, sig)| (prefix, (proof, sig)))
+            .unzip();
+        let (spend_proofs, spend_sigs) = spend_proofs_sigs.into_iter().unzip();
+
+        // Collect arrays for Outputs
+        let (output_prefixes, output_proofs): (Vec<_>, _) =
+            self.outputs().cloned().map(Output::into_v5_parts).unzip();
+
+        // nSpendsSapling and vSpendsSapling
+        spend_prefixes.zcash_serialize(&mut writer)?;
+        // nOutputsSapling and vOutputsSapling
+        output_prefixes.zcash_serialize(&mut writer)?;
+
+        // valueBalanceSapling
+        self.value_balance.zcash_serialize(&mut writer)?;
+
+        // anchorSapling
+        if !spend_prefixes.is_empty() {
+            writer.write_all(&<[u8; 32]>::from(self.shared_anchor)[..])?;
+        }
+
+        // vSpendProofsSapling
+        zcash_serialize_external_count(&spend_proofs, &mut writer)?;
+        // vSpendAuthSigsSapling
+        zcash_serialize_external_count(&spend_sigs, &mut writer)?;
+
+        // vOutputProofsSapling
+        zcash_serialize_external_count(&output_proofs, &mut writer)?;
+
+        // bindingSigSapling
+        writer.write_all(&<[u8; 64]>::from(self.binding_sig)[..])?;
+
+        Ok(())
+    }
+}
+
+// we can't split ShieldedData out of Option<ShieldedData> deserialization,
+// because the counts are read along with the arrays.
+impl ZcashDeserialize for Option<sapling::ShieldedData<SharedAnchor>> {
+    fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
+        // nSpendsSapling and vSpendsSapling
+        let spend_prefixes: Vec<_> = (&mut reader).zcash_deserialize_into()?;
+
+        // nOutputsSapling and vOutputsSapling
+        let output_prefixes: Vec<_> = (&mut reader).zcash_deserialize_into()?;
+
+        // nSpendsSapling and nOutputsSapling as variables
+        let spends_count = spend_prefixes.len();
+        let outputs_count = output_prefixes.len();
+
+        // All the other fields depend on having spends or outputs
+        if spend_prefixes.is_empty() && output_prefixes.is_empty() {
+            return Ok(None);
+        }
+
+        // valueBalanceSapling
+        let value_balance = (&mut reader).zcash_deserialize_into()?;
+
+        // anchorSapling
+        let mut shared_anchor = None;
+        if spends_count > 0 {
+            shared_anchor = Some(reader.read_32_bytes()?.into());
+        }
+
+        // vSpendProofsSapling
+        let spend_proofs = zcash_deserialize_external_count(spends_count, &mut reader)?;
+        // vSpendAuthSigsSapling
+        let spend_sigs = zcash_deserialize_external_count(spends_count, &mut reader)?;
+
+        // vOutputProofsSapling
+        let output_proofs = zcash_deserialize_external_count(outputs_count, &mut reader)?;
+
+        // bindingSigSapling
+        let binding_sig = reader.read_64_bytes()?.into();
+
+        // Create shielded spends from deserialized parts
+        let mut spends: Vec<_> = spend_prefixes
+            .into_iter()
+            .zip(spend_proofs.into_iter())
+            .zip(spend_sigs.into_iter())
+            .map(|((prefix, proof), sig)| Spend::<SharedAnchor>::from_v5_parts(prefix, proof, sig))
+            .collect();
+
+        // Create shielded outputs from deserialized parts
+        let mut outputs = output_prefixes
+            .into_iter()
+            .zip(output_proofs.into_iter())
+            .map(|(prefix, proof)| Output::from_v5_parts(prefix, proof))
+            .collect();
+
+        // Create shielded data
+        use futures::future::Either::*;
+        // TODO: Use a Spend for first if both are present, because the first
+        //       spend activates the shared anchor.
+        if spends_count > 0 {
+            Ok(Some(sapling::ShieldedData {
+                value_balance,
+                // TODO: cleanup shared anchor parsing
+                shared_anchor: shared_anchor.expect("present when spends_count > 0"),
+                first: Left(spends.remove(0)),
+                rest_spends: spends,
+                rest_outputs: outputs,
+                binding_sig,
+            }))
+        } else {
+            assert!(
+                outputs_count > 0,
+                "parsing returns early when there are no spends and no outputs"
+            );
+
+            Ok(Some(sapling::ShieldedData {
+                value_balance,
+                // TODO: delete shared anchor when there are no spends
+                shared_anchor: shared_anchor.unwrap_or_default(),
+                first: Right(outputs.remove(0)),
+                // the spends are actually empty here, but we use the
+                // vec for consistency and readability
+                rest_spends: spends,
+                rest_outputs: outputs,
+                binding_sig,
+            }))
+        }
+    }
+}
+
 impl ZcashSerialize for Transaction {
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
         // Post-Sapling, transaction size is limited to MAX_BLOCK_BYTES.
@@ -187,8 +347,7 @@ impl ZcashSerialize for Transaction {
                     None => {}
                 }
             }
-            // TODO: serialize sapling shielded data according to the V5 transaction spec
-            #[allow(unused_variables)]
+
             Transaction::V5 {
                 lock_time,
                 expiry_height,
@@ -197,17 +356,26 @@ impl ZcashSerialize for Transaction {
                 sapling_shielded_data,
                 rest,
             } => {
-                // Write version 5 and set the fOverwintered bit.
+                // Transaction V5 spec:
+                // https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus
+
+                // header: Write version 5 and set the fOverwintered bit
                 writer.write_u32::<LittleEndian>(5 | (1 << 31))?;
                 writer.write_u32::<LittleEndian>(TX_V5_VERSION_GROUP_ID)?;
+
+                // transaction validity time and height limits
                 lock_time.zcash_serialize(&mut writer)?;
                 writer.write_u32::<LittleEndian>(expiry_height.0)?;
+
+                // transparent
                 inputs.zcash_serialize(&mut writer)?;
                 outputs.zcash_serialize(&mut writer)?;
 
-                // TODO: serialize sapling shielded data according to the V5 transaction spec
+                // sapling
+                sapling_shielded_data.zcash_serialize(&mut writer)?;
 
-                // write the rest
+                // orchard
+                // TODO: parse orchard into structs
                 writer.write_all(rest)?;
             }
         }
@@ -326,17 +494,25 @@ impl ZcashDeserialize for Transaction {
                 })
             }
             (5, true) => {
+                // header
                 let id = reader.read_u32::<LittleEndian>()?;
                 if id != TX_V5_VERSION_GROUP_ID {
                     return Err(SerializationError::Parse("expected TX_V5_VERSION_GROUP_ID"));
                 }
+
+                // transaction validity time and height limits
                 let lock_time = LockTime::zcash_deserialize(&mut reader)?;
                 let expiry_height = block::Height(reader.read_u32::<LittleEndian>()?);
+
+                // transparent
                 let inputs = Vec::zcash_deserialize(&mut reader)?;
                 let outputs = Vec::zcash_deserialize(&mut reader)?;
 
-                // TODO: deserialize sapling shielded data according to the V5 transaction spec
+                // sapling
+                let sapling_shielded_data = (&mut reader).zcash_deserialize_into()?;
 
+                // orchard
+                // TODO: parse orchard into structs
                 let mut rest = Vec::new();
                 reader.read_to_end(&mut rest)?;
 
@@ -345,8 +521,7 @@ impl ZcashDeserialize for Transaction {
                     expiry_height,
                     inputs,
                     outputs,
-                    // TODO: use deserialized sapling shielded data
-                    sapling_shielded_data: None,
+                    sapling_shielded_data,
                     rest,
                 })
             }

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

@@ -13,7 +13,13 @@ proptest! {
         let data = tx.zcash_serialize_to_vec().expect("tx should serialize");
         let tx2 = data.zcash_deserialize_into().expect("randomized tx should deserialize");
 
-        prop_assert_eq![tx, tx2];
+        prop_assert_eq![&tx, &tx2];
+
+        let data2 = tx2
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+
+        prop_assert_eq![data, data2, "data must be equal if structs are equal"];
     }
 
     #[test]

zebra-chain/src/transaction/tests/vectors.rs

@@ -1,6 +1,14 @@
 use super::super::*;
 
-use crate::serialization::{ZcashDeserialize, ZcashSerialize};
+use crate::{
+    block::Block,
+    sapling::{PerSpendAnchor, SharedAnchor},
+    serialization::{WriteZcashExt, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize},
+};
+
+use block::MAX_BLOCK_BYTES;
+use itertools::Itertools;
+use std::convert::TryInto;
 
 #[test]
 fn librustzcash_tx_deserialize_and_round_trip() {
@@ -85,3 +93,336 @@ fn zip243_deserialize_and_round_trip() {
 
     assert_eq!(&zebra_test::vectors::ZIP243_3[..], &data3[..]);
 }
+
+// Transaction V5 test vectors
+
+/// An empty transaction v5, with no Orchard, Sapling, or Transparent data
+///
+/// empty transaction are invalid, but Zebra only checks this rule in
+/// zebra_consensus::transaction::Verifier
+#[test]
+fn empty_v5_round_trip() {
+    zebra_test::init();
+
+    let tx = Transaction::V5 {
+        lock_time: LockTime::min_lock_time(),
+        expiry_height: block::Height(0),
+        inputs: Vec::new(),
+        outputs: Vec::new(),
+        sapling_shielded_data: None,
+        rest: empty_v5_orchard_data(),
+    };
+
+    let data = tx.zcash_serialize_to_vec().expect("tx should serialize");
+    let tx2 = data
+        .zcash_deserialize_into()
+        .expect("tx should deserialize");
+
+    assert_eq!(tx, tx2);
+
+    let data2 = tx2
+        .zcash_serialize_to_vec()
+        .expect("vec serialization is infallible");
+
+    assert_eq!(data, data2, "data must be equal if structs are equal");
+}
+
+/// An empty transaction v4, with no Sapling, Sprout, or Transparent data
+///
+/// empty transaction are invalid, but Zebra only checks this rule in
+/// zebra_consensus::transaction::Verifier
+#[test]
+fn empty_v4_round_trip() {
+    zebra_test::init();
+
+    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: None,
+    };
+
+    let data = tx.zcash_serialize_to_vec().expect("tx should serialize");
+    let tx2 = data
+        .zcash_deserialize_into()
+        .expect("tx should deserialize");
+
+    assert_eq!(tx, tx2);
+
+    let data2 = tx2
+        .zcash_serialize_to_vec()
+        .expect("vec serialization is infallible");
+
+    assert_eq!(data, data2, "data must be equal if structs are equal");
+}
+
+/// Do a round-trip test on fake v5 transactions created from v4 transactions
+/// in the block test vectors.
+///
+/// Covers Sapling only, Transparent only, and Sapling/Transparent v5
+/// transactions.
+#[test]
+fn fake_v5_round_trip() {
+    zebra_test::init();
+
+    for original_bytes in zebra_test::vectors::BLOCKS.iter() {
+        let original_block = original_bytes
+            .zcash_deserialize_into::<Block>()
+            .expect("block is structurally valid");
+
+        // skip this block if it only contains v5 transactions,
+        // the block round-trip test covers it already
+        if original_block
+            .transactions
+            .iter()
+            .all(|trans| matches!(trans.as_ref(), &Transaction::V5 { .. }))
+        {
+            continue;
+        }
+
+        let mut fake_block = original_block.clone();
+        fake_block.transactions = fake_block
+            .transactions
+            .iter()
+            .map(AsRef::as_ref)
+            .map(transaction_to_fake_v5)
+            .map(Into::into)
+            .collect();
+
+        // test each transaction
+        for (original_tx, fake_tx) in original_block
+            .transactions
+            .iter()
+            .zip(fake_block.transactions.iter())
+        {
+            assert_ne!(
+                &original_tx, &fake_tx,
+                "v1-v4 transactions must change when converted to fake v5"
+            );
+
+            let fake_bytes = fake_tx
+                .zcash_serialize_to_vec()
+                .expect("vec serialization is infallible");
+
+            assert_ne!(
+                &original_bytes[..],
+                fake_bytes,
+                "v1-v4 transaction data must change when converted to fake v5"
+            );
+
+            let fake_tx2 = fake_bytes
+                .zcash_deserialize_into::<Transaction>()
+                .expect("tx is structurally valid");
+
+            assert_eq!(fake_tx.as_ref(), &fake_tx2);
+
+            let fake_bytes2 = fake_tx2
+                .zcash_serialize_to_vec()
+                .expect("vec serialization is infallible");
+
+            assert_eq!(
+                fake_bytes, fake_bytes2,
+                "data must be equal if structs are equal"
+            );
+        }
+
+        // test full blocks
+        assert_ne!(
+            &original_block, &fake_block,
+            "v1-v4 transactions must change when converted to fake v5"
+        );
+
+        let fake_bytes = fake_block
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+
+        assert_ne!(
+            &original_bytes[..],
+            fake_bytes,
+            "v1-v4 transaction data must change when converted to fake v5"
+        );
+
+        // skip fake blocks which exceed the block size limit
+        // because of the changes we made
+        if fake_bytes.len() > MAX_BLOCK_BYTES.try_into().unwrap() {
+            continue;
+        }
+
+        let fake_block2 = match fake_bytes.zcash_deserialize_into::<Block>() {
+            Ok(fake_block2) => fake_block2,
+            Err(err) => {
+                // TODO: work out why transaction parsing succeeds,
+                //       but block parsing doesn't
+                tracing::info!(
+                    ?err,
+                    ?original_block,
+                    ?fake_block,
+                    hex_original_bytes = ?hex::encode(&original_bytes),
+                    hex_fake_bytes = ?hex::encode(&fake_bytes),
+                    original_bytes_len = %original_bytes.len(),
+                    fake_bytes_len = %fake_bytes.len(),
+                    %MAX_BLOCK_BYTES,
+                    "unexpected structurally invalid block during deserialization"
+                );
+
+                continue;
+            }
+        };
+
+        assert_eq!(fake_block, fake_block2);
+
+        let fake_bytes2 = fake_block2
+            .zcash_serialize_to_vec()
+            .expect("vec serialization is infallible");
+
+        assert_eq!(
+            fake_bytes, fake_bytes2,
+            "data must be equal if structs are equal"
+        );
+    }
+}
+
+// Utility functions
+
+/// Return serialized empty Transaction::V5 Orchard data.
+///
+/// TODO: replace with orchard::ShieldedData (#1979)
+fn empty_v5_orchard_data() -> Vec<u8> {
+    let mut buf = Vec::new();
+
+    // nActionsOrchard
+    buf.write_compactsize(0)
+        .expect("serialize to Vec always succeeds");
+
+    // all other orchard fields are only present when `nActionsOrchard > 0`
+    buf
+}
+
+/// Convert `trans` into a fake v5 transaction,
+/// converting sapling shielded data from v4 to v5 if possible.
+fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
+    use Transaction::*;
+
+    match trans {
+        V1 {
+            inputs,
+            outputs,
+            lock_time,
+        } => V5 {
+            inputs: inputs.to_vec(),
+            outputs: outputs.to_vec(),
+            lock_time: *lock_time,
+            expiry_height: block::Height(0),
+            sapling_shielded_data: None,
+            rest: empty_v5_orchard_data(),
+        },
+        V2 {
+            inputs,
+            outputs,
+            lock_time,
+            ..
+        } => V5 {
+            inputs: inputs.to_vec(),
+            outputs: outputs.to_vec(),
+            lock_time: *lock_time,
+            expiry_height: block::Height(0),
+            sapling_shielded_data: None,
+            rest: empty_v5_orchard_data(),
+        },
+        V3 {
+            inputs,
+            outputs,
+            lock_time,
+            expiry_height,
+            ..
+        } => V5 {
+            inputs: inputs.to_vec(),
+            outputs: outputs.to_vec(),
+            lock_time: *lock_time,
+            expiry_height: *expiry_height,
+            sapling_shielded_data: None,
+            rest: empty_v5_orchard_data(),
+        },
+        V4 {
+            inputs,
+            outputs,
+            lock_time,
+            expiry_height,
+            sapling_shielded_data,
+            ..
+        } => V5 {
+            inputs: inputs.to_vec(),
+            outputs: outputs.to_vec(),
+            lock_time: *lock_time,
+            expiry_height: *expiry_height,
+            sapling_shielded_data: sapling_shielded_data
+                .clone()
+                .map(sapling_shielded_v4_to_fake_v5)
+                .flatten(),
+            rest: empty_v5_orchard_data(),
+        },
+        v5 @ V5 { .. } => v5.clone(),
+    }
+}
+
+/// Convert a v4 sapling shielded data into a fake v5 sapling shielded data,
+/// if possible.
+fn sapling_shielded_v4_to_fake_v5(
+    v4_shielded: sapling::ShieldedData<PerSpendAnchor>,
+) -> Option<sapling::ShieldedData<SharedAnchor>> {
+    use futures::future::Either::*;
+    use sapling::ShieldedData;
+
+    let unique_anchors: Vec<_> = v4_shielded
+        .spends()
+        .map(|spend| spend.per_spend_anchor)
+        .unique()
+        .collect();
+
+    let shared_anchor = match unique_anchors.as_slice() {
+        [unique_anchor] => *unique_anchor,
+        // TODO: remove shared anchor when there are no spends
+        [] => Default::default(),
+        // Multiple different anchors, can't convert to v5
+        _ => return None,
+    };
+
+    let first = match v4_shielded.first {
+        Left(spend) => Left(sapling_spend_v4_to_fake_v5(spend)),
+        Right(output) => Right(output),
+    };
+
+    let fake_shielded_v5 = ShieldedData::<SharedAnchor> {
+        value_balance: v4_shielded.value_balance,
+        shared_anchor,
+        first,
+        rest_spends: v4_shielded
+            .rest_spends
+            .iter()
+            .cloned()
+            .map(sapling_spend_v4_to_fake_v5)
+            .collect(),
+        rest_outputs: v4_shielded.rest_outputs,
+        binding_sig: v4_shielded.binding_sig,
+    };
+
+    Some(fake_shielded_v5)
+}
+
+/// Convert a v4 sapling spend into a fake v5 sapling spend.
+fn sapling_spend_v4_to_fake_v5(
+    v4_spend: sapling::Spend<PerSpendAnchor>,
+) -> sapling::Spend<SharedAnchor> {
+    use sapling::Spend;
+
+    Spend::<SharedAnchor> {
+        cv: v4_spend.cv,
+        per_spend_anchor: FieldNotPresent,
+        nullifier: v4_spend.nullifier,
+        rk: v4_spend.rk,
+        zkproof: v4_spend.zkproof,
+        spend_auth_sig: v4_spend.spend_auth_sig,
+    }
+}