From d707ebd3218e3f72bca3edc8bc70fa1d0a4676fa Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Fri, 30 Nov 2018 01:08:45 +0000
Subject: [PATCH] Use Option<[u8; N]> for JoinSplit pubkey and signature in a
 transaction

---
 zcash_primitives/src/transaction/mod.rs     | 60 ++++++++++++++++-----
 zcash_primitives/src/transaction/sighash.rs |  2 +-
 zcash_primitives/src/transaction/tests.rs   | 40 ++++++++++++++
 3 files changed, 88 insertions(+), 14 deletions(-)

diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs
index 9f12cb3..787c01d 100644
--- a/zcash_primitives/src/transaction/mod.rs
+++ b/zcash_primitives/src/transaction/mod.rs
@@ -43,8 +43,8 @@ pub struct TransactionData {
     pub shielded_spends: Vec<SpendDescription>,
     pub shielded_outputs: Vec<OutputDescription>,
     pub joinsplits: Vec<JSDescription>,
-    pub joinsplit_pubkey: [u8; 32],
-    pub joinsplit_sig: [u8; 64],
+    pub joinsplit_pubkey: Option<[u8; 32]>,
+    pub joinsplit_sig: Option<[u8; 64]>,
     pub binding_sig: Option<Signature>,
 }
 
@@ -62,8 +62,8 @@ impl TransactionData {
             shielded_spends: vec![],
             shielded_outputs: vec![],
             joinsplits: vec![],
-            joinsplit_pubkey: [0u8; 32],
-            joinsplit_sig: [0u8; 64],
+            joinsplit_pubkey: None,
+            joinsplit_sig: None,
             binding_sig: None,
         }
     }
@@ -122,19 +122,22 @@ impl Transaction {
             (Amount(0), vec![], vec![])
         };
 
-        let mut joinsplit_pubkey = [0; 32];
-        let mut joinsplit_sig = [0; 64];
-        let joinsplits = if version >= 2 {
+        let (joinsplits, joinsplit_pubkey, joinsplit_sig) = if version >= 2 {
             let jss = Vector::read(&mut reader, |r| {
                 JSDescription::read(r, overwintered && version >= SAPLING_TX_VERSION)
             })?;
-            if !jss.is_empty() {
+            let (pubkey, sig) = if !jss.is_empty() {
+                let mut joinsplit_pubkey = [0; 32];
+                let mut joinsplit_sig = [0; 64];
                 reader.read_exact(&mut joinsplit_pubkey)?;
                 reader.read_exact(&mut joinsplit_sig)?;
-            }
-            jss
+                (Some(joinsplit_pubkey), Some(joinsplit_sig))
+            } else {
+                (None, None)
+            };
+            (jss, pubkey, sig)
         } else {
-            vec![]
+            (vec![], None, None)
         };
 
         let binding_sig =
@@ -196,8 +199,39 @@ impl Transaction {
         if self.version >= 2 {
             Vector::write(&mut writer, &self.joinsplits, |w, e| e.write(w))?;
             if !self.joinsplits.is_empty() {
-                writer.write_all(&self.joinsplit_pubkey)?;
-                writer.write_all(&self.joinsplit_sig)?;
+                match self.joinsplit_pubkey {
+                    Some(pubkey) => writer.write_all(&pubkey)?,
+                    None => {
+                        return Err(io::Error::new(
+                            io::ErrorKind::InvalidInput,
+                            "Missing JoinSplit pubkey",
+                        ))
+                    }
+                }
+                match self.joinsplit_sig {
+                    Some(sig) => writer.write_all(&sig)?,
+                    None => {
+                        return Err(io::Error::new(
+                            io::ErrorKind::InvalidInput,
+                            "Missing JoinSplit signature",
+                        ))
+                    }
+                }
+            }
+        }
+
+        if self.version < 2 || self.joinsplits.is_empty() {
+            if self.joinsplit_pubkey.is_some() {
+                return Err(io::Error::new(
+                    io::ErrorKind::InvalidInput,
+                    "JoinSplit pubkey should not be present",
+                ));
+            }
+            if self.joinsplit_sig.is_some() {
+                return Err(io::Error::new(
+                    io::ErrorKind::InvalidInput,
+                    "JoinSplit signature should not be present",
+                ));
             }
         }
 
diff --git a/zcash_primitives/src/transaction/sighash.rs b/zcash_primitives/src/transaction/sighash.rs
index b835aa8..3ed8e89 100644
--- a/zcash_primitives/src/transaction/sighash.rs
+++ b/zcash_primitives/src/transaction/sighash.rs
@@ -110,7 +110,7 @@ fn joinsplits_hash(tx: &TransactionData) -> Vec<u8> {
     for js in &tx.joinsplits {
         js.write(&mut data).unwrap();
     }
-    data.extend_from_slice(&tx.joinsplit_pubkey);
+    data.extend_from_slice(&tx.joinsplit_pubkey.unwrap());
     let mut h = Blake2b::with_params(32, &[], &[], ZCASH_JOINSPLITS_HASH_PERSONALIZATION);
     h.update(&data);
     h.finalize().as_ref().to_vec()
diff --git a/zcash_primitives/src/transaction/tests.rs b/zcash_primitives/src/transaction/tests.rs
index 89b12a8..db3a4df 100644
--- a/zcash_primitives/src/transaction/tests.rs
+++ b/zcash_primitives/src/transaction/tests.rs
@@ -156,6 +156,46 @@ fn tx_read_write() {
     assert_eq!(&data[..], &encoded[..]);
 }
 
+#[test]
+fn tx_write_rejects_unexpected_joinsplit_pubkey() {
+    // Succeeds without a JoinSplit pubkey
+    {
+        let tx = TransactionData::new().freeze();
+        let mut encoded = Vec::new();
+        assert!(tx.write(&mut encoded).is_ok());
+    }
+
+    // Fails with an unexpected JoinSplit pubkey
+    {
+        let mut tx = TransactionData::new();
+        tx.joinsplit_pubkey = Some([0; 32]);
+        let tx = tx.freeze();
+
+        let mut encoded = Vec::new();
+        assert!(tx.write(&mut encoded).is_err());
+    }
+}
+
+#[test]
+fn tx_write_rejects_unexpected_joinsplit_sig() {
+    // Succeeds without a JoinSplit signature
+    {
+        let tx = TransactionData::new().freeze();
+        let mut encoded = Vec::new();
+        assert!(tx.write(&mut encoded).is_ok());
+    }
+
+    // Fails with an unexpected JoinSplit signature
+    {
+        let mut tx = TransactionData::new();
+        tx.joinsplit_sig = Some([0; 64]);
+        let tx = tx.freeze();
+
+        let mut encoded = Vec::new();
+        assert!(tx.write(&mut encoded).is_err());
+    }
+}
+
 #[test]
 fn tx_write_rejects_unexpected_binding_sig() {
     // Succeeds without a binding signature