Impl TryFrom for Commitments, call from impl ZcashDeserialize

zebra-chain/src/commitments/sapling.rs

@@ -7,7 +7,7 @@ mod test_vectors;
 
 pub mod pedersen_hashes;
 
-use std::fmt;
+use std::{convert::TryFrom, fmt, io};
 
 use bitvec::prelude::*;
 use rand_core::{CryptoRng, RngCore};
@@ -15,6 +15,7 @@ use rand_core::{CryptoRng, RngCore};
 use crate::{
     keys::sapling::{find_group_hash, Diversifier, TransmissionKey},
     serde_helpers,
+    serialization::{ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize},
     types::amount::{Amount, NonNegative},
 };
 
@@ -37,12 +38,6 @@ impl fmt::Debug for NoteCommitment {
     }
 }
 
-impl From<[u8; 32]> for NoteCommitment {
-    fn from(bytes: [u8; 32]) -> Self {
-        Self(jubjub::AffinePoint::from_bytes(bytes).unwrap())
-    }
-}
-
 impl From<jubjub::ExtendedPoint> for NoteCommitment {
     fn from(extended_point: jubjub::ExtendedPoint) -> Self {
         Self(jubjub::AffinePoint::from(extended_point))
@@ -57,6 +52,33 @@ impl From<NoteCommitment> for [u8; 32] {
 
 impl Eq for NoteCommitment {}
 
+impl TryFrom<[u8; 32]> for NoteCommitment {
+    type Error = &'static str;
+
+    fn try_from(bytes: [u8; 32]) -> Result<Self, Self::Error> {
+        let possible_point = jubjub::AffinePoint::from_bytes(bytes);
+
+        if possible_point.is_some().into() {
+            Ok(Self(possible_point.unwrap()))
+        } else {
+            Err("Invalid jubjub::AffinePoint value")
+        }
+    }
+}
+
+impl ZcashSerialize for NoteCommitment {
+    fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
+        writer.write_all(&<[u8; 32]>::from(*self)[..])?;
+        Ok(())
+    }
+}
+
+impl ZcashDeserialize for NoteCommitment {
+    fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
+        Self::try_from(reader.read_32_bytes()?).map_err(|e| SerializationError::Parse(e))
+    }
+}
+
 impl NoteCommitment {
     /// Generate a new _NoteCommitment_ and the randomness used to create it.
     ///
@@ -128,16 +150,6 @@ impl fmt::Debug for ValueCommitment {
     }
 }
 
-/// LEBS2OSP256(repr_J(cv))
-///
-/// https://zips.z.cash/protocol/protocol.pdf#spendencoding
-/// https://zips.z.cash/protocol/protocol.pdf#jubjub
-impl From<[u8; 32]> for ValueCommitment {
-    fn from(bytes: [u8; 32]) -> Self {
-        Self(jubjub::AffinePoint::from_bytes(bytes).unwrap())
-    }
-}
-
 impl From<jubjub::ExtendedPoint> for ValueCommitment {
     fn from(extended_point: jubjub::ExtendedPoint) -> Self {
         Self(jubjub::AffinePoint::from(extended_point))
@@ -156,6 +168,37 @@ impl From<ValueCommitment> for [u8; 32] {
     }
 }
 
+/// LEBS2OSP256(repr_J(cv))
+///
+/// https://zips.z.cash/protocol/protocol.pdf#spendencoding
+/// https://zips.z.cash/protocol/protocol.pdf#jubjub
+impl TryFrom<[u8; 32]> for ValueCommitment {
+    type Error = &'static str;
+
+    fn try_from(bytes: [u8; 32]) -> Result<Self, Self::Error> {
+        let possible_point = jubjub::AffinePoint::from_bytes(bytes);
+
+        if possible_point.is_some().into() {
+            Ok(Self(possible_point.unwrap()))
+        } else {
+            Err("Invalid jubjub::AffinePoint value")
+        }
+    }
+}
+
+impl ZcashSerialize for ValueCommitment {
+    fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
+        writer.write_all(&<[u8; 32]>::from(*self)[..])?;
+        Ok(())
+    }
+}
+
+impl ZcashDeserialize for ValueCommitment {
+    fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
+        Self::try_from(reader.read_32_bytes()?).map_err(|e| SerializationError::Parse(e))
+    }
+}
+
 impl ValueCommitment {
     /// Generate a new _ValueCommitment_.
     ///

zebra-chain/src/commitments/sapling/arbitrary.rs

@@ -1,3 +1,5 @@
+use std::convert::TryFrom;
+
 use proptest::{arbitrary::any, array, prelude::*};
 
 use crate::commitments::sapling;
@@ -7,10 +9,7 @@ impl Arbitrary for sapling::NoteCommitment {
 
     fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
         array::uniform32(any::<u8>())
-            .prop_filter("Valid jubjub::AffinePoint", |b| {
-                jubjub::AffinePoint::from_bytes(*b).is_some().unwrap_u8() == 1
-            })
-            .prop_map(Self::from)
+            .prop_filter_map("Valid jubjub::AffinePoint", |b| Self::try_from(b).ok())
             .boxed()
     }
 
@@ -22,10 +21,7 @@ impl Arbitrary for sapling::ValueCommitment {
 
     fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
         array::uniform32(any::<u8>())
-            .prop_filter("Valid jubjub::AffinePoint", |b| {
-                jubjub::AffinePoint::from_bytes(*b).is_some().unwrap_u8() == 1
-            })
-            .prop_map(Self::from)
+            .prop_filter_map("Valid jubjub::AffinePoint", |b| Self::try_from(b).ok())
             .boxed()
     }
 

zebra-chain/src/transaction/serialize.rs

@@ -9,7 +9,7 @@ use std::{
 };
 
 use crate::{
-    notes,
+    commitments, notes,
     proofs::ZkSnarkProof,
     serialization::{
         ReadZcashExt, SerializationError, WriteZcashExt, ZcashDeserialize, ZcashSerialize,
@@ -321,7 +321,7 @@ impl<P: ZkSnarkProof> ZcashDeserialize for Option<JoinSplitData<P>> {
 
 impl ZcashSerialize for Spend {
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
-        writer.write_all(&<[u8; 32]>::from(self.cv)[..])?;
+        self.cv.zcash_serialize(&mut writer)?;
         writer.write_all(&self.anchor.0[..])?;
         self.nullifier.zcash_serialize(&mut writer)?;
         writer.write_all(&<[u8; 32]>::from(self.rk)[..])?;
@@ -335,7 +335,7 @@ impl ZcashDeserialize for Spend {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
         use crate::treestate::note_commitment_tree::SaplingNoteTreeRootHash;
         Ok(Spend {
-            cv: reader.read_32_bytes()?.into(),
+            cv: commitments::sapling::ValueCommitment::zcash_deserialize(&mut reader)?,
             anchor: SaplingNoteTreeRootHash(reader.read_32_bytes()?),
             nullifier: notes::sapling::Nullifier::zcash_deserialize(&mut reader)?,
             rk: reader.read_32_bytes()?.into(),
@@ -347,7 +347,7 @@ impl ZcashDeserialize for Spend {
 
 impl ZcashSerialize for Output {
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
-        writer.write_all(&<[u8; 32]>::from(self.cv)[..])?;
+        self.cv.zcash_serialize(&mut writer)?;
         writer.write_all(&self.cm_u.to_bytes())?;
         writer.write_all(&self.ephemeral_key.to_bytes())?;
         self.enc_ciphertext.zcash_serialize(&mut writer)?;
@@ -360,7 +360,7 @@ impl ZcashSerialize for Output {
 impl ZcashDeserialize for Output {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
         Ok(Output {
-            cv: reader.read_32_bytes()?.into(),
+            cv: commitments::sapling::ValueCommitment::zcash_deserialize(&mut reader)?,
             cm_u: jubjub::Fq::from_bytes(&reader.read_32_bytes()?).unwrap(),
             ephemeral_key: jubjub::AffinePoint::from_bytes(reader.read_32_bytes()?).unwrap(),
             enc_ciphertext: notes::sapling::EncryptedCiphertext::zcash_deserialize(&mut reader)?,