From 5ba7bbc53b536990a85ddc47895172c2fe44e305 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@nutty.land>
Date: Mon, 16 Aug 2021 10:06:38 -0600
Subject: [PATCH] Serialize `Option` values by value, not by reference.

 Co-authored-by: str4d <jack@electriccoin.co>
---
 zcash_primitives/src/merkle_tree.rs             |  8 ++++----
 zcash_primitives/src/merkle_tree/incremental.rs |  8 ++++----
 zcash_primitives/src/serialize.rs               | 17 +++++++++--------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/zcash_primitives/src/merkle_tree.rs b/zcash_primitives/src/merkle_tree.rs
index ebee68769..06ce149e5 100644
--- a/zcash_primitives/src/merkle_tree.rs
+++ b/zcash_primitives/src/merkle_tree.rs
@@ -137,10 +137,10 @@ impl<Node: Hashable> CommitmentTree<Node> {
 
     /// Serializes this tree as an array of bytes.
     pub fn write<W: Write>(&self, mut writer: W) -> io::Result<()> {
-        Optional::write(&mut writer, &self.left, |w, n| n.write(w))?;
-        Optional::write(&mut writer, &self.right, |w, n| n.write(w))?;
+        Optional::write(&mut writer, self.left, |w, n| n.write(w))?;
+        Optional::write(&mut writer, self.right, |w, n| n.write(w))?;
         Vector::write(&mut writer, &self.parents, |w, e| {
-            Optional::write(w, e, |w, n| n.write(w))
+            Optional::write(w, *e, |w, n| n.write(w))
         })
     }
 
@@ -292,7 +292,7 @@ impl<Node: Hashable> IncrementalWitness<Node> {
     pub fn write<W: Write>(&self, mut writer: W) -> io::Result<()> {
         self.tree.write(&mut writer)?;
         Vector::write(&mut writer, &self.filled, |w, n| n.write(w))?;
-        Optional::write(&mut writer, &self.cursor, |w, t| t.write(w))
+        Optional::write(&mut writer, self.cursor.as_ref(), |w, t| t.write(w))
     }
 
     /// Returns the position of the witnessed leaf node in the commitment tree.
diff --git a/zcash_primitives/src/merkle_tree/incremental.rs b/zcash_primitives/src/merkle_tree/incremental.rs
index be2861285..917fa3a53 100644
--- a/zcash_primitives/src/merkle_tree/incremental.rs
+++ b/zcash_primitives/src/merkle_tree/incremental.rs
@@ -65,11 +65,11 @@ pub fn write_nonempty_frontier_v1<H: HashSer, W: Write>(
     match frontier.leaf() {
         Leaf::Left(a) => {
             a.write(&mut writer)?;
-            Optional::write(&mut writer, &None, |w, n: &H| n.write(w))?;
+            Optional::write(&mut writer, None, |w, n: &H| n.write(w))?;
         }
         Leaf::Right(a, b) => {
             a.write(&mut writer)?;
-            Optional::write(&mut writer, &Some(b), |w, n| n.write(w))?;
+            Optional::write(&mut writer, Some(b), |w, n| n.write(w))?;
         }
     }
     Vector::write(&mut writer, &frontier.ommers(), |w, e| e.write(w))?;
@@ -164,8 +164,8 @@ pub fn write_bridge_v1<H: HashSer, W: Write>(
 ) -> io::Result<()> {
     Optional::write(
         &mut writer,
-        &bridge.prior_position().map(<u64>::from),
-        |w, n| w.write_u64::<LittleEndian>(*n),
+        bridge.prior_position().map(<u64>::from),
+        |w, n| w.write_u64::<LittleEndian>(n),
     )?;
     Vector::write(
         &mut writer,
diff --git a/zcash_primitives/src/serialize.rs b/zcash_primitives/src/serialize.rs
index 267c5c1f5..d998762f5 100644
--- a/zcash_primitives/src/serialize.rs
+++ b/zcash_primitives/src/serialize.rs
@@ -122,11 +122,11 @@ pub struct Optional;
 impl Optional {
     pub fn read<R: Read, T, F>(mut reader: R, func: F) -> io::Result<Option<T>>
     where
-        F: Fn(&mut R) -> io::Result<T>,
+        F: Fn(R) -> io::Result<T>,
     {
         match reader.read_u8()? {
             0 => Ok(None),
-            1 => Ok(Some(func(&mut reader)?)),
+            1 => Ok(Some(func(reader)?)),
             _ => Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
                 "non-canonical Option<T>",
@@ -134,15 +134,15 @@ impl Optional {
         }
     }
 
-    pub fn write<W: Write, T, F>(mut writer: W, val: &Option<T>, func: F) -> io::Result<()>
+    pub fn write<W: Write, T, F>(mut writer: W, val: Option<T>, func: F) -> io::Result<()>
     where
-        F: Fn(&mut W, &T) -> io::Result<()>,
+        F: Fn(W, T) -> io::Result<()>,
     {
         match val {
             None => writer.write_u8(0),
             Some(e) => {
                 writer.write_u8(1)?;
-                func(&mut writer, e)
+                func(writer, e)
             }
         }
     }
@@ -226,7 +226,7 @@ mod tests {
         macro_rules! eval {
             ($value:expr, $expected:expr, $write:expr, $read:expr) => {
                 let mut data = vec![];
-                Optional::write(&mut data, &$value, $write).unwrap();
+                Optional::write(&mut data, $value, $write).unwrap();
                 assert_eq!(&data[..], &$expected[..]);
                 match Optional::read(&data[..], $read) {
                     Ok(v) => assert_eq!(v, $value),
@@ -237,7 +237,8 @@ mod tests {
 
         macro_rules! eval_u8 {
             ($value:expr, $expected:expr) => {
-                eval!($value, $expected, |w, e| w.write_u8(*e), |r| r.read_u8())
+                eval!($value, $expected, |w, e| w.write_u8(e), |mut r| r
+                    .read_u8())
             };
         }
 
@@ -246,7 +247,7 @@ mod tests {
                 eval!(
                     $value,
                     $expected,
-                    |w, v| Vector::write(w, v, |w, e| w.write_u8(*e)),
+                    |w, v| Vector::write(w, &v, |w, e| w.write_u8(*e)),
                     |r| Vector::read(r, |r| r.read_u8())
                 )
             };