Merge pull request #63 from zcash/note-commitment-updates

Note commitment updates

Cargo.toml

@@ -31,15 +31,15 @@ subtle = "2.3"
 
 [dependencies.halo2]
 git = "https://github.com/zcash/halo2.git"
-rev = "b079624ea78b4a07d44cb3c725dd734093577062"
+rev = "6acacf1aca12f34fc311aa59056e40adc0e6d8bd"
 
 [dependencies.pasta_curves]
 git = "https://github.com/zcash/pasta_curves.git"
-rev = "0a6b2f6eb5acbe1dabc9e77ed0bcb748cc640196"
+rev = "b55a6960dfafd7f767e2820ddf1adaa499322f98"
 
 [dependencies.reddsa]
 git = "https://github.com/str4d/redjubjub.git"
-rev = "f8ff124a52d86e122e0705e8e9272f2099fe4c46"
+rev = "62a26a02fda5165b68a4b4773aa18ea6eb749dbc"
 
 [dev-dependencies]
 criterion = "0.3"

src/bundle.rs

@@ -4,7 +4,7 @@ use nonempty::NonEmpty;
 
 use crate::{
     circuit::Proof,
-    note::{EncryptedNote, NoteCommitment, Nullifier},
+    note::{EncryptedNote, ExtractedNoteCommitment, Nullifier},
     primitives::redpallas::{self, Binding, SpendAuth},
     tree::Anchor,
     value::{ValueCommitment, ValueSum},
@@ -25,7 +25,7 @@ pub struct Action<T> {
     /// The randomized verification key for the note being spent.
     rk: redpallas::VerificationKey<SpendAuth>,
     /// A commitment to the new note being created.
-    cm_new: NoteCommitment,
+    cm_new: ExtractedNoteCommitment,
     /// The encrypted output note.
     encrypted_note: EncryptedNote,
     /// A commitment to the net value created or consumed by this action.

src/keys.rs

@@ -33,9 +33,14 @@ impl SpendingKey {
     /// Returns `None` if the bytes do not correspond to a valid Orchard spending key.
     pub fn from_bytes(sk: [u8; 32]) -> CtOption<Self> {
         let sk = SpendingKey(sk);
-        // If ask = 0, discard this key.
+        // If ask = 0, discard this key. We call `derive_inner` rather than
+        // `SpendAuthorizingKey::from` here because we only need to know
+        // whether ask = 0; the adjustment to potentially negate ask is not
+        // needed. Also, `from` would panic on ask = 0.
         let ask = SpendAuthorizingKey::derive_inner(&sk);
-        CtOption::new(sk, !ask.ct_is_zero())
+        // If ivk = ⊥, discard this key.
+        let ivk = IncomingViewingKey::derive_inner(&(&sk).into());
+        CtOption::new(sk, !(ask.ct_is_zero() | ivk.is_none()))
     }
 }
 
@@ -263,12 +268,19 @@ pub struct IncomingViewingKey(pallas::Scalar);
 
 impl From<&FullViewingKey> for IncomingViewingKey {
     fn from(fvk: &FullViewingKey) -> Self {
-        let ak = extract_p(&pallas::Point::from_bytes(&(&fvk.ak.0).into()).unwrap());
-        IncomingViewingKey(commit_ivk(&ak, &fvk.nk.0, &fvk.rivk.0))
+        let ivk = IncomingViewingKey::derive_inner(fvk);
+        // IncomingViewingKey cannot be constructed such that this unwrap would fail.
+        IncomingViewingKey(ivk.unwrap())
     }
 }
 
 impl IncomingViewingKey {
+    /// Derives ask from sk. Internal use only, does not enforce all constraints.
+    fn derive_inner(fvk: &FullViewingKey) -> CtOption<pallas::Scalar> {
+        let ak = extract_p(&pallas::Point::from_bytes(&(&fvk.ak.0).into()).unwrap());
+        commit_ivk(&ak, &fvk.nk.0, &fvk.rivk.0)
+    }
+
     /// Returns the payment address for this key corresponding to the given diversifier.
     pub fn address(&self, d: Diversifier) -> Address {
         let pk_d = DiversifiedTransmissionKey::derive(self, &d);

src/note.rs

@@ -9,7 +9,7 @@ use crate::{
 };
 
 mod commitment;
-pub use self::commitment::NoteCommitment;
+pub use self::commitment::{ExtractedNoteCommitment, NoteCommitment};
 
 mod nullifier;
 pub use self::nullifier::Nullifier;
@@ -61,6 +61,7 @@ impl Note {
     pub fn commitment(&self) -> NoteCommitment {
         let g_d = self.recipient.g_d();
 
+        // `Note` will always have a note commitment by construction.
         NoteCommitment::derive(
             g_d.to_bytes(),
             self.recipient.pk_d().to_bytes(),
@@ -69,6 +70,7 @@ impl Note {
             self.rseed.psi(),
             (&self.rseed).into(),
         )
+        .unwrap()
     }
 
     /// Derives the nullifier for this note.

src/note/commitment.rs

@@ -3,11 +3,12 @@ use std::iter;
 use bitvec::{array::BitArray, order::Lsb0};
 use ff::PrimeField;
 use pasta_curves::pallas;
+use subtle::CtOption;
 
 use crate::{
     constants::L_ORCHARD_BASE,
     primitives::sinsemilla,
-    spec::{prf_expand, to_scalar},
+    spec::{extract_p, prf_expand, to_scalar},
     value::NoteValue,
 };
 
@@ -38,10 +39,10 @@ impl NoteCommitment {
         rho: pallas::Base,
         psi: pallas::Base,
         rcm: NoteCommitTrapdoor,
-    ) -> Self {
+    ) -> CtOption<Self> {
         let domain = sinsemilla::CommitDomain::new("z.cash:Orchard-NoteCommit");
-        NoteCommitment(
-            domain.commit(
+        domain
+            .commit(
                 iter::empty()
                     .chain(BitArray::<Lsb0, _>::new(g_d).iter().by_val())
                     .chain(BitArray::<Lsb0, _>::new(pk_d).iter().by_val())
@@ -49,7 +50,17 @@ impl NoteCommitment {
                     .chain(rho.to_le_bits().iter().by_val().take(L_ORCHARD_BASE))
                     .chain(psi.to_le_bits().iter().by_val().take(L_ORCHARD_BASE)),
                 &rcm.0,
-            ),
-        )
+            )
+            .map(NoteCommitment)
+    }
+}
+
+/// The x-coordinate of the commitment to a note.
+#[derive(Debug)]
+pub struct ExtractedNoteCommitment(pub(super) pallas::Base);
+
+impl From<NoteCommitment> for ExtractedNoteCommitment {
+    fn from(cm: NoteCommitment) -> Self {
+        ExtractedNoteCommitment(extract_p(&cm.0))
     }
 }

src/primitives/sinsemilla.rs

@@ -1,10 +1,13 @@
 //! The Sinsemilla hash function and commitment scheme.
 
-use group::Group;
 use halo2::arithmetic::CurveExt;
 use pasta_curves::pallas;
+use subtle::CtOption;
 
-use crate::spec::extract_p;
+use crate::spec::extract_p_bottom;
+
+mod addition;
+use self::addition::IncompletePoint;
 
 mod constants;
 pub use constants::*;
@@ -95,8 +98,12 @@ impl HashDomain {
     /// $\mathsf{SinsemillaHashToPoint}$ from [§ 5.4.1.9][concretesinsemillahash].
     ///
     /// [concretesinsemillahash]: https://zips.z.cash/protocol/nu5.pdf#concretesinsemillahash
+    pub(crate) fn hash_to_point(&self, msg: impl Iterator<Item = bool>) -> CtOption<pallas::Point> {
+        self.hash_to_point_inner(msg).into()
+    }
+
     #[allow(non_snake_case)]
-    pub(crate) fn hash_to_point(&self, msg: impl Iterator<Item = bool>) -> pallas::Point {
+    fn hash_to_point_inner(&self, msg: impl Iterator<Item = bool>) -> IncompletePoint {
         let padded: Vec<_> = Pad::new(msg).collect();
 
         let hasher_S = pallas::Point::hash_to_curve(S_PERSONALIZATION);
@@ -104,14 +111,16 @@ impl HashDomain {
 
         padded
             .chunks(K)
-            .fold(self.Q, |acc, chunk| acc.double() + S(chunk))
+            .fold(IncompletePoint::from(self.Q), |acc, chunk| {
+                (acc + S(chunk)) + acc
+            })
     }
 
     /// $\mathsf{SinsemillaHash}$ from [§ 5.4.1.9][concretesinsemillahash].
     ///
     /// [concretesinsemillahash]: https://zips.z.cash/protocol/nu5.pdf#concretesinsemillahash
-    pub(crate) fn hash(&self, msg: impl Iterator<Item = bool>) -> pallas::Base {
-        extract_p(&self.hash_to_point(msg))
+    pub(crate) fn hash(&self, msg: impl Iterator<Item = bool>) -> CtOption<pallas::Base> {
+        extract_p_bottom(self.hash_to_point(msg))
     }
 
     /// Returns the Sinsemilla $Q$ constant for this domain.
@@ -151,8 +160,8 @@ impl CommitDomain {
         &self,
         msg: impl Iterator<Item = bool>,
         r: &pallas::Scalar,
-    ) -> pallas::Point {
-        self.M.hash_to_point(msg) + self.R * r
+    ) -> CtOption<pallas::Point> {
+        (self.M.hash_to_point_inner(msg) + self.R * r).into()
     }
 
     /// $\mathsf{SinsemillaShortCommit}$ from [§ 5.4.8.4][concretesinsemillacommit].
@@ -162,8 +171,8 @@ impl CommitDomain {
         &self,
         msg: impl Iterator<Item = bool>,
         r: &pallas::Scalar,
-    ) -> pallas::Base {
-        extract_p(&self.commit(msg, r))
+    ) -> CtOption<pallas::Base> {
+        extract_p_bottom(self.commit(msg, r))
     }
 
     /// Returns the Sinsemilla $R$ constant for this domain.

src/primitives/sinsemilla/addition.rs

@@ -0,0 +1,55 @@
+use std::ops::Add;
+
+use group::Group;
+use pasta_curves::pallas;
+use subtle::{ConstantTimeEq, CtOption};
+
+/// P ∪ {⊥}
+///
+/// Simulated incomplete addition built over complete addition.
+#[derive(Clone, Copy, Debug)]
+pub(super) struct IncompletePoint(CtOption<pallas::Point>);
+
+impl From<pallas::Point> for IncompletePoint {
+    fn from(p: pallas::Point) -> Self {
+        IncompletePoint(CtOption::new(p, 1.into()))
+    }
+}
+
+impl From<IncompletePoint> for CtOption<pallas::Point> {
+    fn from(p: IncompletePoint) -> Self {
+        p.0
+    }
+}
+
+impl Add for IncompletePoint {
+    type Output = IncompletePoint;
+
+    #[allow(clippy::suspicious_arithmetic_impl)]
+    fn add(self, rhs: Self) -> Self::Output {
+        // ⊥ ⊹ ⊥ = ⊥
+        // ⊥ ⊹ P = ⊥
+        IncompletePoint(self.0.and_then(|p| {
+            // P ⊹ ⊥ = ⊥
+            rhs.0.and_then(|q| {
+                // 0 ⊹ 0 = ⊥
+                // 0 ⊹ P = ⊥
+                // P ⊹ 0 = ⊥
+                // (x, y) ⊹ (x', y') = ⊥ if x == x'
+                // (x, y) ⊹ (x', y') = (x, y) + (x', y') if x != x'
+                CtOption::new(
+                    p + q,
+                    !(p.is_identity() | q.is_identity() | p.ct_eq(&q) | p.ct_eq(&-q)),
+                )
+            })
+        }))
+    }
+}
+
+impl Add<pallas::Point> for IncompletePoint {
+    type Output = IncompletePoint;
+
+    fn add(self, rhs: pallas::Point) -> Self::Output {
+        self + IncompletePoint(CtOption::new(rhs, 1.into()))
+    }
+}

src/spec.rs

@@ -7,6 +7,7 @@ use ff::PrimeField;
 use group::{Curve, Group};
 use halo2::arithmetic::{CurveAffine, CurveExt, FieldExt};
 use pasta_curves::pallas;
+use subtle::CtOption;
 
 use crate::{
     constants::L_ORCHARD_BASE,
@@ -48,19 +49,18 @@ pub(crate) fn commit_ivk(
     ak: &pallas::Base,
     nk: &pallas::Base,
     rivk: &pallas::Scalar,
-) -> pallas::Scalar {
+) -> CtOption<pallas::Scalar> {
     // We rely on the API contract that to_le_bits() returns at least PrimeField::NUM_BITS
     // bits, which is equal to L_ORCHARD_BASE.
     let domain = sinsemilla::CommitDomain::new(&"z.cash:Orchard-CommitIvk");
-    // TODO: handle the (negligible probability of) failure of SinsemillaShortCommit.
-    mod_r_p(
-        domain.short_commit(
+    domain
+        .short_commit(
             iter::empty()
                 .chain(ak.to_le_bits().iter().by_val().take(L_ORCHARD_BASE))
                 .chain(nk.to_le_bits().iter().by_val().take(L_ORCHARD_BASE)),
             rivk,
-        ),
-    )
+        )
+        .map(mod_r_p)
 }
 
 /// Defined in [Zcash Protocol Spec § 5.4.1.6: DiversifyHash^Sapling and DiversifyHash^Orchard Hash Functions][concretediversifyhash].
@@ -120,11 +120,20 @@ pub(crate) fn ka_orchard(sk: &pallas::Scalar, b: &pallas::Point) -> pallas::Poin
 ///
 /// [concreteextractorpallas]: https://zips.z.cash/protocol/nu5.pdf#concreteextractorpallas
 pub(crate) fn extract_p(point: &pallas::Point) -> pallas::Base {
-    if let Some((x, _)) = point.to_affine().get_xy().into() {
-        x
-    } else {
-        pallas::Base::zero()
-    }
+    point
+        .to_affine()
+        .coordinates()
+        .map(|c| *c.x())
+        .unwrap_or_else(pallas::Base::zero)
+}
+
+/// Coordinate extractor for Pallas.
+///
+/// Defined in [Zcash Protocol Spec § 5.4.9.7: Coordinate Extractor for Pallas][concreteextractorpallas].
+///
+/// [concreteextractorpallas]: https://zips.z.cash/protocol/nu5.pdf#concreteextractorpallas
+pub(crate) fn extract_p_bottom(point: CtOption<pallas::Point>) -> CtOption<pallas::Base> {
+    point.map(|p| extract_p(&p))
 }
 
 #[cfg(test)]