Use incomplete addition in SinsemillaHashToPoint

This requires exposing the ⊥ case throughout the return types. We
prevent it from propagating into the Orchard note and key types by
ensuring that:

- When we generate keys or notes, if we encounter ⊥ we discard and
  re-generate.
- When we construct keys or notes via any other pathway (e.g. parsing
  from bytes), we check for and reject ⊥.

src/keys.rs

@@ -35,7 +35,9 @@ impl SpendingKey {
         let sk = SpendingKey(sk);
         // If ask = 0, discard this key.
         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 +265,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

@@ -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,6 +3,7 @@ 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,
@@ -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,7 @@ 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)
     }
 }

src/primitives/sinsemilla.rs

@@ -1,12 +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::*;
@@ -97,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);
@@ -106,14 +111,17 @@ 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> {
+        let point: CtOption<_> = self.hash_to_point(msg).into();
+        extract_p_bottom(point)
     }
 
     /// Returns the Sinsemilla $Q$ constant for this domain.
@@ -153,8 +161,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].
@@ -164,8 +172,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/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].
@@ -127,6 +127,15 @@ pub(crate) fn extract_p(point: &pallas::Point) -> pallas::Base {
         .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)]
 mod tests {
     use group::Group;