remove the `rest` field of v5 transaction (#2057)

zebra-chain/src/transaction.rs

@@ -108,8 +108,7 @@ pub enum Transaction {
         outputs: Vec<transparent::Output>,
         /// The sapling shielded data for this transaction, if any.
         sapling_shielded_data: Option<sapling::ShieldedData<sapling::SharedAnchor>>,
-        /// The rest of the transaction as bytes
+        // TODO: The orchard data for this transaction, if any.
-        rest: Vec<u8>,
     },
 }
 

zebra-chain/src/transaction/arbitrary.rs

@@ -109,17 +109,15 @@ impl Transaction {
             transparent::Input::vec_strategy(ledger_state, 10),
             vec(any::<transparent::Output>(), 0..10),
             option::of(any::<sapling::ShieldedData<sapling::SharedAnchor>>()),
-            any::<Vec<u8>>(),
         )
             .prop_map(
-                |(lock_time, expiry_height, inputs, outputs, sapling_shielded_data, rest)| {
+                |(lock_time, expiry_height, inputs, outputs, sapling_shielded_data)| {
                     Transaction::V5 {
                         lock_time,
                         expiry_height,
                         inputs,
                         outputs,
                         sapling_shielded_data,
-                        rest,
                     }
                 },
             )

zebra-chain/src/transaction/serialize.rs

@@ -346,7 +346,6 @@ impl ZcashSerialize for Transaction {
                 inputs,
                 outputs,
                 sapling_shielded_data,
-                rest,
             } => {
                 // Transaction V5 spec:
                 // https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus
@@ -367,8 +366,10 @@ impl ZcashSerialize for Transaction {
                 sapling_shielded_data.zcash_serialize(&mut writer)?;
 
                 // orchard
-                // TODO: parse orchard into structs
+                // TODO: Parse orchard into structs
-                writer.write_all(rest)?;
+                //       In the meantime, to keep the transaction valid, we add `0`
+                //       as the nActionsOrchard field
+                writer.write_compactsize(0)?;
             }
         }
         Ok(())
@@ -501,9 +502,14 @@ impl ZcashDeserialize for Transaction {
                 let sapling_shielded_data = (&mut reader).zcash_deserialize_into()?;
 
                 // orchard
-                // TODO: parse orchard into structs
+                // TODO: Parse orchard into structs
-                let mut rest = Vec::new();
+                //       In the meantime, check the orchard section is just `0`
-                reader.read_to_end(&mut rest)?;
+                let empty_orchard_section = reader.read_compactsize()?;
+                if empty_orchard_section != 0 {
+                    return Err(SerializationError::Parse(
+                        "expected orchard section to be just a zero",
+                    ));
+                }
 
                 Ok(Transaction::V5 {
                     lock_time,
@@ -511,7 +517,6 @@ impl ZcashDeserialize for Transaction {
                     inputs,
                     outputs,
                     sapling_shielded_data,
-                    rest,
                 })
             }
             (_, _) => Err(SerializationError::Parse("bad tx header")),

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

@@ -3,7 +3,7 @@ use super::super::*;
 use crate::{
     block::{Block, MAX_BLOCK_BYTES},
     sapling::{PerSpendAnchor, SharedAnchor},
-    serialization::{WriteZcashExt, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize},
+    serialization::{ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize},
 };
 
 use itertools::Itertools;
@@ -110,7 +110,6 @@ fn empty_v5_round_trip() {
         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");
@@ -245,31 +244,13 @@ fn fake_v5_round_trip() {
         );
 
         // 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>() {
+        let fake_block2 = fake_bytes
-            Ok(fake_block2) => fake_block2,
+            .zcash_deserialize_into::<Block>()
-            Err(err) => {
+            .expect("block is structurally valid");
-                // 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);
 
@@ -286,20 +267,6 @@ fn fake_v5_round_trip() {
 
 // 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 {
@@ -316,7 +283,6 @@ fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
             lock_time: *lock_time,
             expiry_height: block::Height(0),
             sapling_shielded_data: None,
-            rest: empty_v5_orchard_data(),
         },
         V2 {
             inputs,
@@ -329,7 +295,6 @@ fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
             lock_time: *lock_time,
             expiry_height: block::Height(0),
             sapling_shielded_data: None,
-            rest: empty_v5_orchard_data(),
         },
         V3 {
             inputs,
@@ -343,7 +308,6 @@ fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
             lock_time: *lock_time,
             expiry_height: *expiry_height,
             sapling_shielded_data: None,
-            rest: empty_v5_orchard_data(),
         },
         V4 {
             inputs,
@@ -361,7 +325,6 @@ fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
                 .clone()
                 .map(sapling_shielded_v4_to_fake_v5)
                 .flatten(),
-            rest: empty_v5_orchard_data(),
         },
         v5 @ V5 { .. } => v5.clone(),
     }