Orchard: Fix trait imports and Arbitrary for Action

zebra-chain/src/orchard/action.rs

@@ -1,6 +1,6 @@
 use std::io;
 
-use halo2::pasta::pallas;
+use halo2::{arithmetic::FieldExt, pasta::pallas};
 
 use crate::{
     primitives::redpallas::{self, SpendAuth},
@@ -27,7 +27,7 @@ pub struct Action {
     /// A value commitment to net value of the input note minus the output note
     pub cv: commitment::ValueCommitment,
     /// The nullifier of the input note being spent.
-    pub nullifer: note::Nullifier,
+    pub nullifier: note::Nullifier,
     /// The randomized validating key for spendAuthSig,
     pub rk: redpallas::VerificationKeyBytes<SpendAuth>,
     /// The 𝑥-coordinate of the note commitment for the output note.

zebra-chain/src/orchard/arbitrary.rs

@@ -1,56 +1,30 @@
-use jubjub::AffinePoint;
-use proptest::{arbitrary::any, array, collection::vec, prelude::*};
+use group::prime::PrimeCurveAffine;
+use halo2::pasta::pallas;
+use proptest::{arbitrary::any, array, prelude::*};
 
-use crate::primitives::Groth16Proof;
+use super::{keys, note, Action, NoteCommitment, ValueCommitment};
 
-use super::{keys, note, tree, NoteCommitment, Output, Spend, ValueCommitment};
-
-impl Arbitrary for Spend {
+impl Arbitrary for Action {
     type Parameters = ();
 
     fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
         (
-            any::<tree::Root>(),
             any::<note::Nullifier>(),
             array::uniform32(any::<u8>()),
-            any::<Groth16Proof>(),
-            vec(any::<u8>(), 64),
-        )
-            .prop_map(|(anchor, nullifier, rpk_bytes, proof, sig_bytes)| Self {
-                anchor,
-                cv: ValueCommitment(AffinePoint::identity()),
-                nullifier,
-                rk: redjubjub::VerificationKeyBytes::from(rpk_bytes),
-                zkproof: proof,
-                spend_auth_sig: redjubjub::Signature::from({
-                    let mut b = [0u8; 64];
-                    b.copy_from_slice(sig_bytes.as_slice());
-                    b
-                }),
-            })
-            .boxed()
-    }
-
-    type Strategy = BoxedStrategy<Self>;
-}
-
-impl Arbitrary for Output {
-    type Parameters = ();
-
-    fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
-        (
             any::<note::EncryptedNote>(),
             any::<note::WrappedNoteKey>(),
-            any::<Groth16Proof>(),
         )
-            .prop_map(|(enc_ciphertext, out_ciphertext, zkproof)| Self {
-                cv: ValueCommitment(AffinePoint::identity()),
-                cm_u: NoteCommitment(AffinePoint::identity()).extract_u(),
-                ephemeral_key: keys::EphemeralPublicKey(AffinePoint::identity()),
-                enc_ciphertext,
-                out_ciphertext,
-                zkproof,
-            })
+            .prop_map(
+                |(nullifier, rpk_bytes, enc_ciphertext, out_ciphertext)| Self {
+                    cv: ValueCommitment(pallas::Affine::identity()),
+                    nullifier,
+                    rk: crate::primitives::redpallas::VerificationKeyBytes::from(rpk_bytes),
+                    cm_x: NoteCommitment(pallas::Affine::identity()).extract_x(),
+                    ephemeral_key: keys::EphemeralPublicKey(pallas::Affine::identity()),
+                    enc_ciphertext,
+                    out_ciphertext,
+                },
+            )
             .boxed()
     }
 

zebra-chain/src/orchard/commitment.rs

@@ -6,7 +6,12 @@
 use std::{convert::TryFrom, fmt, io};
 
 use bitvec::prelude::*;
-use halo2::pasta::pallas;
+use group::{prime::PrimeCurveAffine, GroupEncoding};
+use halo2::{
+    arithmetic::{CurveAffine, FieldExt},
+    pasta::pallas,
+};
+
 use rand_core::{CryptoRng, RngCore};
 
 use crate::{
@@ -44,9 +49,13 @@ pub struct NoteCommitment(#[serde(with = "serde_helpers::Affine")] pub pallas::A
 
 impl fmt::Debug for NoteCommitment {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        // This will panic if the public key is the identity, which is bad news
+        // bears.
+        let (x, y) = self.0.get_xy().unwrap();
+
         f.debug_struct("NoteCommitment")
-            .field("x", &hex::encode(self.0.get_x().to_bytes()))
-            .field("y", &hex::encode(self.0.get_y().to_bytes()))
+            .field("x", &hex::encode(x.to_bytes()))
+            .field("y", &hex::encode(y.to_bytes()))
             .finish()
     }
 }
@@ -136,7 +145,7 @@ impl NoteCommitment {
 ///
 /// https://zips.z.cash/protocol/protocol.pdf#concretehomomorphiccommit
 #[derive(Clone, Copy, Deserialize, PartialEq, Serialize)]
-pub struct ValueCommitment(#[serde(with = "serde_helpers::AffinePoint")] pub pallas::Affine);
+pub struct ValueCommitment(#[serde(with = "serde_helpers::Affine")] pub pallas::Affine);
 
 impl<'a> std::ops::Add<&'a ValueCommitment> for ValueCommitment {
     type Output = Self;
@@ -163,9 +172,13 @@ impl std::ops::AddAssign<ValueCommitment> for ValueCommitment {
 
 impl fmt::Debug for ValueCommitment {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        // This will panic if the public key is the identity, which is bad news
+        // bears.
+        let (x, y) = self.0.get_xy().unwrap();
+
         f.debug_struct("ValueCommitment")
-            .field("x", &hex::encode(self.0.get_x().to_bytes()))
-            .field("y", &hex::encode(self.0.get_y().to_bytes()))
+            .field("x", &hex::encode(x.to_bytes()))
+            .field("y", &hex::encode(y.to_bytes()))
             .finish()
     }
 }
@@ -228,7 +241,7 @@ impl TryFrom<[u8; 32]> for ValueCommitment {
     type Error = &'static str;
 
     fn try_from(bytes: [u8; 32]) -> Result<Self, Self::Error> {
-        let possible_point = pallas::Affine::from_bytes(bytes);
+        let possible_point = pallas::Affine::from_bytes(&bytes);
 
         if possible_point.is_some().into() {
             Ok(Self(possible_point.unwrap()))
@@ -271,12 +284,12 @@ impl ValueCommitment {
     /// https://zips.z.cash/protocol/protocol.pdf#concretehomomorphiccommit
     #[allow(non_snake_case)]
     pub fn new(rcv: pallas::Scalar, value: Amount) -> Self {
-        let v = pallas::Scalar::from(value);
+        let v = pallas::Scalar::from_bytes(value.to_bytes());
 
         // TODO: These generator points can be generated once somewhere else to
         // avoid having to recompute them on every new commitment.
-        let V = pallas_group_hash(*b"z.cash:Orchard-cv", b"v");
-        let R = pallas_group_hash(*b"z.cash:Orchard-cv", b"r");
+        let V = pallas_group_hash(b"z.cash:Orchard-cv", b"v");
+        let R = pallas_group_hash(b"z.cash:Orchard-cv", b"r");
 
         Self::from(V * v + R * rcv)
     }

zebra-chain/src/orchard/keys.rs

@@ -18,7 +18,7 @@ use std::{
 use aes::Aes256;
 use bech32::{self, FromBase32, ToBase32, Variant};
 use fpe::ff1::{BinaryNumeralString, FF1};
-use group::{prime::PrimeCurveAffine, GroupEncoding};
+use group::GroupEncoding;
 use halo2::{
     arithmetic::{CurveAffine, FieldExt},
     pasta::pallas,

zebra-chain/src/orchard/sinsemilla.rs

@@ -2,7 +2,10 @@
 
 use bitvec::prelude::*;
 
-use halo2::pasta::pallas;
+use halo2::{
+    arithmetic::{CurveAffine, CurveExt},
+    pasta::pallas,
+};
 
 /// [Hash Extractor for Pallas][concreteextractorpallas]
 ///
@@ -27,7 +30,9 @@ pub fn extract_p(point: pallas::Point) -> pallas::Base {
 /// https://zips.z.cash/protocol/protocol.pdf#concretegrouphashpallasandvesta
 #[allow(non_snake_case)]
 pub fn pallas_group_hash(D: &[u8], M: &[u8]) -> pallas::Point {
-    pallas::Point::hash_to_curve(D)(M)
+    let domain_separator = std::str::from_utf8(D).unwrap();
+
+    pallas::Point::hash_to_curve(domain_separator)(M)
 }
 
 /// Q(D) := GroupHash^P(︀“z.cash:SinsemillaQ”, D)

zebra-chain/src/orchard/tree.rs

@@ -11,11 +11,11 @@
 //! A root of a note commitment tree is associated with each treestate.
 
 #![allow(clippy::unit_arg)]
-#![allow(dead_code)]
 
 use std::{collections::VecDeque, fmt};
 
 use bitvec::prelude::*;
+use group::GroupEncoding;
 use lazy_static::lazy_static;
 #[cfg(any(test, feature = "proptest-impl"))]
 use proptest_derive::Arbitrary;
@@ -42,7 +42,7 @@ fn merkle_crh_orchard(layer: u8, left: [u8; 32], right: [u8; 32]) -> [u8; 32] {
     s.extend_from_slice(&left.bits::<Lsb0>()[0..255]);
     s.extend_from_slice(&right.bits::<Lsb0>()[0..255]);
 
-    sinsemilla_hash_to_point(*b"Zcash_PH", &s).to_bytes()
+    sinsemilla_hash_to_point(b"Zcash_PH", &s).to_bytes()
 }
 
 lazy_static! {

zebra-chain/src/primitives/redpallas.rs

@@ -1,7 +1,6 @@
-// Extracted from redjubjub for now.
-
-#![deny(missing_docs)]
+// XXX: Extracted from redjubjub for now.
 
+use group::GroupEncoding;
 use halo2::pasta::pallas;
 
 // pub mod batch;

zebra-chain/src/primitives/redpallas/signing_key.rs

@@ -3,9 +3,10 @@ use std::{
     marker::PhantomData,
 };
 
+use halo2::arithmetic::FieldExt;
 use halo2::pasta::pallas;
 
-use super::{SigType, VerificationKey};
+use super::{Error, SigType, VerificationKey};
 
 /// A RedPallas signing key.
 #[derive(Copy, Clone, Debug)]
@@ -17,3 +18,25 @@ pub struct SigningKey<T: SigType> {
     sk: pallas::Scalar,
     pk: VerificationKey<T>,
 }
+
+impl<'a, T: SigType> From<&'a SigningKey<T>> for VerificationKey<T> {
+    fn from(sk: &'a SigningKey<T>) -> VerificationKey<T> {
+        sk.pk.clone()
+    }
+}
+
+impl<T: SigType> TryFrom<[u8; 32]> for SigningKey<T> {
+    type Error = Error;
+
+    fn try_from(bytes: [u8; 32]) -> Result<Self, Self::Error> {
+        let maybe_sk = pallas::Scalar::from_bytes(&bytes);
+
+        if maybe_sk.is_some().into() {
+            let sk = maybe_sk.unwrap();
+            let pk = VerificationKey::from(&sk);
+            Ok(SigningKey { sk, pk })
+        } else {
+            Err(Error::MalformedSigningKey)
+        }
+    }
+}

zebra-chain/src/primitives/redpallas/verification_key.rs

@@ -4,9 +4,10 @@ use std::{
     marker::PhantomData,
 };
 
+use group::{cofactor::CofactorGroup, GroupEncoding};
 use halo2::pasta::pallas;
 
-use super::SigType;
+use super::{Error, SigType, SigningKey};
 
 /// A refinement type for `[u8; 32]` indicating that the bytes represent
 /// an encoding of a RedPallas verification key.
@@ -65,3 +66,57 @@ pub struct VerificationKey<T: SigType> {
     pub(crate) point: pallas::Point,
     pub(crate) bytes: VerificationKeyBytes<T>,
 }
+
+impl<T: SigType> From<VerificationKey<T>> for VerificationKeyBytes<T> {
+    fn from(pk: VerificationKey<T>) -> VerificationKeyBytes<T> {
+        pk.bytes
+    }
+}
+
+impl<T: SigType> From<VerificationKey<T>> for [u8; 32] {
+    fn from(pk: VerificationKey<T>) -> [u8; 32] {
+        pk.bytes.bytes
+    }
+}
+
+impl<T: SigType> TryFrom<VerificationKeyBytes<T>> for VerificationKey<T> {
+    type Error = Error;
+
+    fn try_from(bytes: VerificationKeyBytes<T>) -> Result<Self, Self::Error> {
+        // This checks that the encoding is canonical...
+        let maybe_point = pallas::Affine::from_bytes(&bytes.bytes);
+
+        if maybe_point.is_some().into() {
+            let point: pallas::Point = maybe_point.unwrap().into();
+
+            // This checks that the verification key is not of small order.
+            if <bool>::from(point.is_small_order()) == false {
+                Ok(VerificationKey { point, bytes })
+            } else {
+                Err(Error::MalformedVerificationKey)
+            }
+        } else {
+            Err(Error::MalformedVerificationKey)
+        }
+    }
+}
+
+impl<T: SigType> TryFrom<[u8; 32]> for VerificationKey<T> {
+    type Error = Error;
+
+    fn try_from(bytes: [u8; 32]) -> Result<Self, Self::Error> {
+        use std::convert::TryInto;
+        VerificationKeyBytes::from(bytes).try_into()
+    }
+}
+
+impl<T: SigType> VerificationKey<T> {
+    pub(crate) fn from(s: &pallas::Scalar) -> VerificationKey<T> {
+        let point = &T::basepoint() * s;
+        let bytes = VerificationKeyBytes {
+            bytes: pallas::Affine::from(&point).to_bytes(),
+            _marker: PhantomData,
+        };
+        VerificationKey { bytes, point }
+    }
+}

zebra-chain/src/sapling/keys.rs

@@ -15,7 +15,7 @@ mod test_vectors;
 mod tests;
 
 use std::{
-    convert::{From, Into, TryFrom},
+    convert::{From, Into, TryFrom, TryInto},
     fmt,
     io::{self, Write},
     str::FromStr,
@@ -73,13 +73,13 @@ fn prf_ock(ovk: [u8; 32], cv: [u8; 32], cm_u: [u8; 32], ephemeral_key: [u8; 32])
         .hash_length(32)
         .personal(b"Zcash_Derive_ock")
         .to_state()
-        .update(ovk)
-        .update(cv)
-        .update(cm_u)
-        .update(ephemeral_key)
+        .update(&ovk)
+        .update(&cv)
+        .update(&cm_u)
+        .update(&ephemeral_key)
         .finalize();
 
-    *hash.as_array()
+    *hash.as_bytes().try_into().expect("32 byte array")
 }
 
 /// Invokes Blake2s-256 as _CRH^ivk_, to derive the IncomingViewingKey

zebra-chain/src/serialization/serde_helpers.rs

@@ -1,4 +1,5 @@
-use halo2::pasta::pallas;
+use group::GroupEncoding;
+use halo2::{arithmetic::FieldExt, pasta::pallas};
 use serde_big_array::big_array;
 
 big_array! {