diff --git a/src/keys.rs b/src/keys.rs index 58116f0e..de538318 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -16,7 +16,7 @@ use crate::{ primitives::redpallas::{self, SpendAuth}, spec::{ commit_ivk, diversify_hash, extract_p, ka_orchard, prf_expand, prf_expand_vec, prf_nf, - to_base, to_scalar, + to_base, to_scalar, NonZeroPallasBase, NonZeroPallasScalar, }, }; @@ -309,20 +309,25 @@ impl Diversifier { /// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents +// +// We store ivk in memory as a scalar instead of a base, so that we aren't incurring an +// expensive serialize-and-parse step every time we use it (for e.g. deriving addresses). +// When we actually want to serialize ivk, we're guaranteed to get a valid base field +// element encoding, because we always construct ivk from an integer in the correct range. #[derive(Debug)] -pub struct IncomingViewingKey(pallas::Scalar); +pub struct IncomingViewingKey(NonZeroPallasScalar); impl From<&FullViewingKey> for IncomingViewingKey { fn from(fvk: &FullViewingKey) -> Self { - let ivk = IncomingViewingKey::derive_inner(fvk); // IncomingViewingKey cannot be constructed such that this unwrap would fail. - IncomingViewingKey(ivk.unwrap()) + let ivk = IncomingViewingKey::derive_inner(fvk).unwrap(); + IncomingViewingKey(ivk.into()) } } impl IncomingViewingKey { /// Derives ask from sk. Internal use only, does not enforce all constraints. - fn derive_inner(fvk: &FullViewingKey) -> CtOption { + fn derive_inner(fvk: &FullViewingKey) -> CtOption { let ak = extract_p(&pallas::Point::from_bytes(&(&fvk.ak.0).into()).unwrap()); commit_ivk(&ak, &fvk.nk.0, &fvk.rivk.0) } diff --git a/src/spec.rs b/src/spec.rs index 265e39ce..d6911adb 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -1,9 +1,10 @@ //! Helper functions defined in the Zcash Protocol Specification. use std::iter; +use std::ops::Deref; use blake2b_simd::Params; -use ff::PrimeField; +use ff::{Field, PrimeField}; use group::{Curve, Group}; use halo2::arithmetic::{CurveAffine, CurveExt, FieldExt}; use pasta_curves::pallas; @@ -16,6 +17,51 @@ use crate::{ const PRF_EXPAND_PERSONALIZATION: &[u8; 16] = b"Zcash_ExpandSeed"; +/// An integer in [1..q_P]. +pub(crate) struct NonZeroPallasBase(pallas::Base); + +impl NonZeroPallasBase { + /// Constructs a wrapper for a base field element that is guaranteed to be non-zero. + /// + /// # Panics + /// + /// Panics if `s.is_zero()`. + fn guaranteed(s: pallas::Base) -> Self { + assert!(!s.is_zero()); + NonZeroPallasBase(s) + } +} + +/// An integer in [1..r_P]. +#[derive(Debug)] +pub(crate) struct NonZeroPallasScalar(pallas::Scalar); + +impl From for NonZeroPallasScalar { + fn from(s: NonZeroPallasBase) -> Self { + NonZeroPallasScalar::guaranteed(mod_r_p(s.0)) + } +} + +impl NonZeroPallasScalar { + /// Constructs a wrapper for a scalar field element that is guaranteed to be non-zero. + /// + /// # Panics + /// + /// Panics if `s.is_zero()`. + fn guaranteed(s: pallas::Scalar) -> Self { + assert!(!s.is_zero()); + NonZeroPallasScalar(s) + } +} + +impl Deref for NonZeroPallasScalar { + type Target = pallas::Scalar; + + fn deref(&self) -> &pallas::Scalar { + &self.0 + } +} + /// $\mathsf{ToBase}^\mathsf{Orchard}(x) := LEOS2IP_{\ell_\mathsf{PRFexpand}}(x) (mod q_P)$ /// /// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. @@ -49,7 +95,7 @@ pub(crate) fn commit_ivk( ak: &pallas::Base, nk: &pallas::Base, rivk: &pallas::Scalar, -) -> CtOption { +) -> CtOption { // 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"); @@ -60,7 +106,13 @@ pub(crate) fn commit_ivk( .chain(nk.to_le_bits().iter().by_val().take(L_ORCHARD_BASE)), rivk, ) - .map(mod_r_p) + // Commit^ivk.Output is specified as [1..q_P] ∪ {⊥}. We get this from + // sinsemilla::CommitDomain::short_commit by construction: + // - 0 is not a valid x-coordinate for any Pallas point. + // - sinsemilla::CommitDomain::short_commit calls extract_p_bottom, which replaces + // the identity (which has no affine coordinates) with 0. but Sinsemilla is + // defined using incomplete addition, and thus will never produce the identity. + .map(NonZeroPallasBase::guaranteed) } /// Defined in [Zcash Protocol Spec § 5.4.1.6: DiversifyHash^Sapling and DiversifyHash^Orchard Hash Functions][concretediversifyhash].