From d0c16910d697f9e77bd46d6fa5089aba18f0318b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 11 May 2021 18:22:57 +1200 Subject: [PATCH 1/8] book: Document why ivk != 0 --- book/src/design/commitments.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/book/src/design/commitments.md b/book/src/design/commitments.md index 6eff53d1..06733abf 100644 --- a/book/src/design/commitments.md +++ b/book/src/design/commitments.md @@ -23,6 +23,15 @@ $$\mathsf{cm} = \mathit{Commit}^{\mathsf{cm}}_{\mathsf{rcm}}(\text{rest of note} This is the same split (and rationale) as in Sapling, but using the more PLONK-efficient Sinsemilla instead of Bowe--Hopwood Pedersen hashes. -Note that we also deviate from Sapling by using $\mathit{ShortCommit}$ to deriving $\mathsf{ivk}$ -instead of a full PRF. This removes an unnecessary (large) PRF primitive from the circuit, -at the cost of requiring $\mathsf{rivk}$ to be part of the full viewing key. +Note that for $\mathsf{ivk}$, we also deviate from Sapling in two ways: + +- We use $\mathit{ShortCommit}$ to derive $\mathsf{ivk}$ instead of a full PRF. This removes an + unnecessary (large) PRF primitive from the circuit, at the cost of requiring $\mathsf{rivk}$ to be + part of the full viewing key. +- We define $\mathsf{ivk}$ as an integer in $[1, q_P)$; that is, we exclude $\mathsf{ivk} = 0$. For + Sapling, we relied on BLAKE2s to make $\mathsf{ivk} = 0$ infeasible to produce, but it was still + technically possible. For Orchard, we get this by construction: + - $0$ is not a valid x-coordinate for any Pallas point. + - $\mathit{ShortCommit}$ internally maps points to field elements by replacing the identity (which + has no affine coordinates) with $0$. But Sinsemilla is defined using incomplete addition, and + thus will never produce the identity. From 9a828febd738f22bce6789fc41b5db36c02a1a39 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 11 May 2021 19:08:39 +1200 Subject: [PATCH 2/8] Change `commit_ivk` to return a non-zero Pallas base field element The type system now enforces that `ivk != 0`. --- src/keys.rs | 15 +++++++++----- src/spec.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 8 deletions(-) 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]. From 76a39d29c17f76c734c747449bfb961da7cd303f Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 11 May 2021 20:06:16 +1200 Subject: [PATCH 3/8] Change diversify_hash and ka_orchard to use non-zero types This matches the changes to KA^Orchard in spec version 2021.1.23. --- src/address.rs | 6 ++---- src/keys.rs | 4 ++-- src/spec.rs | 29 ++++++++++++++++++++--------- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/address.rs b/src/address.rs index 69543d12..eb1bea02 100644 --- a/src/address.rs +++ b/src/address.rs @@ -1,8 +1,6 @@ -use pasta_curves::pallas; - use crate::{ keys::{DiversifiedTransmissionKey, Diversifier}, - spec::diversify_hash, + spec::{diversify_hash, NonIdentityPallasPoint}, }; /// A shielded payment address. @@ -30,7 +28,7 @@ impl Address { Address { d, pk_d } } - pub(crate) fn g_d(&self) -> pallas::Point { + pub(crate) fn g_d(&self) -> NonIdentityPallasPoint { diversify_hash(self.d.as_array()) } diff --git a/src/keys.rs b/src/keys.rs index de538318..695ba23f 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, NonZeroPallasBase, NonZeroPallasScalar, + to_base, to_scalar, NonIdentityPallasPoint, NonZeroPallasBase, NonZeroPallasScalar, }, }; @@ -363,7 +363,7 @@ impl From<&FullViewingKey> for OutgoingViewingKey { /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents #[derive(Debug, Clone)] -pub(crate) struct DiversifiedTransmissionKey(pallas::Point); +pub(crate) struct DiversifiedTransmissionKey(NonIdentityPallasPoint); impl DiversifiedTransmissionKey { /// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. diff --git a/src/spec.rs b/src/spec.rs index d6911adb..32be743c 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -17,6 +17,18 @@ use crate::{ const PRF_EXPAND_PERSONALIZATION: &[u8; 16] = b"Zcash_ExpandSeed"; +/// A Pallas point that is guaranteed to not be the identity. +#[derive(Clone, Debug)] +pub(crate) struct NonIdentityPallasPoint(pallas::Point); + +impl Deref for NonIdentityPallasPoint { + type Target = pallas::Point; + + fn deref(&self) -> &pallas::Point { + &self.0 + } +} + /// An integer in [1..q_P]. pub(crate) struct NonZeroPallasBase(pallas::Base); @@ -118,15 +130,11 @@ pub(crate) fn commit_ivk( /// Defined in [Zcash Protocol Spec § 5.4.1.6: DiversifyHash^Sapling and DiversifyHash^Orchard Hash Functions][concretediversifyhash]. /// /// [concretediversifyhash]: https://zips.z.cash/protocol/nu5.pdf#concretediversifyhash -pub(crate) fn diversify_hash(d: &[u8; 11]) -> pallas::Point { +pub(crate) fn diversify_hash(d: &[u8; 11]) -> NonIdentityPallasPoint { let hasher = pallas::Point::hash_to_curve("z.cash:Orchard-gd"); let pk_d = hasher(d); - if pk_d.is_identity().into() { - // If the identity occurs, we replace it with a different fixed point. - hasher(&[]) - } else { - pk_d - } + // If the identity occurs, we replace it with a different fixed point. + NonIdentityPallasPoint(CtOption::new(pk_d, !pk_d.is_identity()).unwrap_or_else(|| hasher(&[]))) } /// $PRF^\mathsf{expand}(sk, t) := BLAKE2b-512("Zcash_ExpandSeed", sk || t)$ @@ -162,8 +170,11 @@ pub(crate) fn prf_nf(nk: pallas::Base, rho: pallas::Base) -> pallas::Base { /// Defined in [Zcash Protocol Spec § 5.4.5.5: Orchard Key Agreement][concreteorchardkeyagreement]. /// /// [concreteorchardkeyagreement]: https://zips.z.cash/protocol/nu5.pdf#concreteorchardkeyagreement -pub(crate) fn ka_orchard(sk: &pallas::Scalar, b: &pallas::Point) -> pallas::Point { - b * sk +pub(crate) fn ka_orchard( + sk: &NonZeroPallasScalar, + b: &NonIdentityPallasPoint, +) -> NonIdentityPallasPoint { + NonIdentityPallasPoint(b.deref() * sk.deref()) } /// Coordinate extractor for Pallas. From d8cc596bbe63fe355a47f25eacf821bb8b375e80 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 11 May 2021 22:07:08 +1200 Subject: [PATCH 4/8] Create separate types for protocol-level and user-level ivk Spec version 2021.1.24 added the diversifier key to the encoding of an incoming viewing key (to make them more usable). As a result, we now have two separate types: - `KeyAgreementPrivateKey`: what was previously `IncomingViewingKey`, corresponding to the `ivk` type in the protocol spec. It is now crate-internal. - `IncomingViewingKey`: the user-facing type that encompasses `dk` and `ivk`. --- src/keys.rs | 90 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/src/keys.rs b/src/keys.rs index 695ba23f..1fce23f8 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -57,7 +57,7 @@ impl SpendingKey { // needed. Also, `from` would panic on ask = 0. let ask = SpendAuthorizingKey::derive_inner(&sk); // If ivk = ⊥, discard this key. - let ivk = IncomingViewingKey::derive_inner(&(&sk).into()); + let ivk = KeyAgreementPrivateKey::derive_inner(&(&sk).into()); CtOption::new(sk, !(ask.ct_is_zero() | ivk.is_none())) } } @@ -218,17 +218,18 @@ impl FullViewingKey { /// Returns the default payment address for this key. pub fn default_address(&self) -> Address { - self.address(DiversifierKey::from(self).default_diversifier()) + IncomingViewingKey::from(self).default_address() } /// Returns the payment address for this key at the given index. pub fn address_at(&self, j: impl Into) -> Address { - self.address(DiversifierKey::from(self).get(j)) + IncomingViewingKey::from(self).address_at(j) } /// Returns the payment address for this key corresponding to the given diversifier. pub fn address(&self, d: Diversifier) -> Address { - IncomingViewingKey::from(self).address(d) + // Shortcut: we don't need to derive DiversifierKey. + KeyAgreementPrivateKey::from(self).address(d) } } @@ -297,6 +298,49 @@ impl Diversifier { } } +/// The private key $\mathsf{ivk}$ used in $KA^{Orchard}$, for decrypting incoming notes. +/// +/// In Sapling this is what was encoded as an incoming viewing key. For Orchard, we store +/// both this and [`DiversifierKey`] inside [`IncomingViewingKey`] for usability (to +/// enable deriving the default address for an incoming viewing key), while this separate +/// type represents $\mathsf{ivk}$. +/// +/// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. +/// +/// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents +/// +/// # Implementation notes +/// +/// We store $\mathsf{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 (e.g. for trial +/// decryption of notes). 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)] +struct KeyAgreementPrivateKey(NonZeroPallasScalar); + +impl From<&FullViewingKey> for KeyAgreementPrivateKey { + fn from(fvk: &FullViewingKey) -> Self { + // KeyAgreementPrivateKey cannot be constructed such that this unwrap would fail. + let ivk = KeyAgreementPrivateKey::derive_inner(fvk).unwrap(); + KeyAgreementPrivateKey(ivk.into()) + } +} + +impl KeyAgreementPrivateKey { + /// Derives ask from sk. Internal use only, does not enforce all constraints. + 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) + } + + /// Returns the payment address for this key corresponding to the given diversifier. + fn address(&self, d: Diversifier) -> Address { + let pk_d = DiversifiedTransmissionKey::derive(self, &d); + Address::from_parts(d, pk_d) + } +} + /// A key that provides the capability to detect and decrypt incoming notes from the block /// chain, without being able to spend the notes or detect when they are spent. /// @@ -306,36 +350,38 @@ impl Diversifier { /// This key is not suitable for use on its own in a wallet, as it cannot maintain /// accurate balance. You should use a [`FullViewingKey`] instead. /// -/// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. +/// Defined in [Zcash Protocol Spec § 5.6.4.3: Orchard Raw Incoming Viewing Keys][orchardinviewingkeyencoding]. /// -/// [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. +/// [orchardinviewingkeyencoding]: https://zips.z.cash/protocol/nu5.pdf#orchardinviewingkeyencoding #[derive(Debug)] -pub struct IncomingViewingKey(NonZeroPallasScalar); +pub struct IncomingViewingKey { + dk: DiversifierKey, + ivk: KeyAgreementPrivateKey, +} impl From<&FullViewingKey> for IncomingViewingKey { fn from(fvk: &FullViewingKey) -> Self { - // IncomingViewingKey cannot be constructed such that this unwrap would fail. - let ivk = IncomingViewingKey::derive_inner(fvk).unwrap(); - IncomingViewingKey(ivk.into()) + IncomingViewingKey { + dk: fvk.into(), + ivk: fvk.into(), + } } } impl IncomingViewingKey { - /// Derives ask from sk. Internal use only, does not enforce all constraints. - 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) + /// Returns the default payment address for this key. + pub fn default_address(&self) -> Address { + self.address(self.dk.default_diversifier()) + } + + /// Returns the payment address for this key at the given index. + pub fn address_at(&self, j: impl Into) -> Address { + self.address(self.dk.get(j)) } /// 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); - Address::from_parts(d, pk_d) + self.ivk.address(d) } } @@ -369,7 +415,7 @@ impl DiversifiedTransmissionKey { /// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents - fn derive(ivk: &IncomingViewingKey, d: &Diversifier) -> Self { + fn derive(ivk: &KeyAgreementPrivateKey, d: &Diversifier) -> Self { let g_d = diversify_hash(&d.as_array()); DiversifiedTransmissionKey(ka_orchard(&ivk.0, &g_d)) } From 736de1156bfbed9e2057f2bc7ce6bab37f545371 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 11 May 2021 18:39:24 +0800 Subject: [PATCH 5/8] Ensure that Notes always have valid commitments Implements the change from spec version 2021.1.23 to sample a new rseed if a note is generated without a valid commitment. --- src/address.rs | 2 +- src/keys.rs | 4 ++-- src/note.rs | 42 ++++++++++++++++++++++++++++++------------ src/note/nullifier.rs | 2 +- src/spec.rs | 2 +- 5 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/address.rs b/src/address.rs index eb1bea02..541bb2b1 100644 --- a/src/address.rs +++ b/src/address.rs @@ -13,7 +13,7 @@ use crate::{ /// let sk = SpendingKey::from_bytes([7; 32]).unwrap(); /// let address = FullViewingKey::from(&sk).default_address(); /// ``` -#[derive(Debug, Clone)] +#[derive(Clone, Copy, Debug)] pub struct Address { d: Diversifier, pk_d: DiversifiedTransmissionKey, diff --git a/src/keys.rs b/src/keys.rs index 1fce23f8..b0afafe4 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -288,7 +288,7 @@ impl DiversifierKey { /// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents -#[derive(Debug, Clone)] +#[derive(Clone, Copy, Debug)] pub struct Diversifier([u8; 11]); impl Diversifier { @@ -408,7 +408,7 @@ impl From<&FullViewingKey> for OutgoingViewingKey { /// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents]. /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents -#[derive(Debug, Clone)] +#[derive(Clone, Copy, Debug)] pub(crate) struct DiversifiedTransmissionKey(NonIdentityPallasPoint); impl DiversifiedTransmissionKey { diff --git a/src/note.rs b/src/note.rs index 84e9e400..87cd288d 100644 --- a/src/note.rs +++ b/src/note.rs @@ -2,6 +2,7 @@ use group::GroupEncoding; use pasta_curves::pallas; use rand::RngCore; +use subtle::CtOption; use crate::{ keys::{FullViewingKey, SpendingKey}, @@ -72,11 +73,16 @@ impl Note { rho: Nullifier, mut rng: impl RngCore, ) -> Self { - Note { - recipient, - value, - rho, - rseed: RandomSeed::random(&mut rng), + loop { + let note = Note { + recipient, + value, + rho, + rseed: RandomSeed::random(&mut rng), + }; + if note.commitment_inner().is_some().into() { + break note; + } } } @@ -93,12 +99,12 @@ impl Note { let fvk: FullViewingKey = (&sk).into(); let recipient = fvk.default_address(); - let note = Note { + let note = Note::new( recipient, - value: NoteValue::zero(), - rho: rho.unwrap_or_else(|| Nullifier::dummy(rng)), - rseed: RandomSeed::random(rng), - }; + NoteValue::zero(), + rho.unwrap_or_else(|| Nullifier::dummy(rng)), + rng, + ); (sk, fvk, note) } @@ -114,9 +120,22 @@ impl Note { /// /// [notes]: https://zips.z.cash/protocol/nu5.pdf#notes pub fn commitment(&self) -> NoteCommitment { + // `Note` will always have a note commitment by construction. + self.commitment_inner().unwrap() + } + + /// Derives the commitment to this note. + /// + /// This is the internal fallible API, used to check at construction time that the + /// note has a commitment. Once you have a [`Note`] object, use `note.commitment()` + /// instead. + /// + /// Defined in [Zcash Protocol Spec § 3.2: Notes][notes]. + /// + /// [notes]: https://zips.z.cash/protocol/nu5.pdf#notes + fn commitment_inner(&self) -> CtOption { 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(), @@ -125,7 +144,6 @@ impl Note { self.rseed.psi(), (&self.rseed).into(), ) - .unwrap() } /// Derives the nullifier for this note. diff --git a/src/note/nullifier.rs b/src/note/nullifier.rs index bc0dd6e3..2ae51c94 100644 --- a/src/note/nullifier.rs +++ b/src/note/nullifier.rs @@ -11,7 +11,7 @@ use crate::{ }; /// A unique nullifier for a note. -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] pub struct Nullifier(pub(crate) pallas::Base); impl Nullifier { diff --git a/src/spec.rs b/src/spec.rs index 32be743c..3567bce6 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -18,7 +18,7 @@ use crate::{ const PRF_EXPAND_PERSONALIZATION: &[u8; 16] = b"Zcash_ExpandSeed"; /// A Pallas point that is guaranteed to not be the identity. -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] pub(crate) struct NonIdentityPallasPoint(pallas::Point); impl Deref for NonIdentityPallasPoint { From 4423b5078864ac8e22ca6a512f229d00b702caad Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 11 May 2021 18:50:01 +0800 Subject: [PATCH 6/8] =?UTF-8?q?Include=20=CF=81=20as=20an=20input=20to=20t?= =?UTF-8?q?he=20derivation=20of=20=CF=88,=20esk,=20and=20rcm?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This brings the implementation in line with spec version 2021.2.0 and the Orchard book. --- src/note.rs | 31 +++++++++++++++++++++++-------- src/note/commitment.rs | 17 ++--------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/note.rs b/src/note.rs index 87cd288d..1d879256 100644 --- a/src/note.rs +++ b/src/note.rs @@ -6,7 +6,7 @@ use subtle::CtOption; use crate::{ keys::{FullViewingKey, SpendingKey}, - spec::{prf_expand, to_base, to_scalar}, + spec::{prf_expand_vec, to_base, to_scalar}, value::NoteValue, Address, }; @@ -31,15 +31,25 @@ impl RandomSeed { /// Defined in [Zcash Protocol Spec § 4.7.3: Sending Notes (Orchard)][orchardsend]. /// /// [orchardsend]: https://zips.z.cash/protocol/nu5.pdf#orchardsend - fn psi(&self) -> pallas::Base { - to_base(prf_expand(&self.0, &[0x09])) + fn psi(&self, rho: &Nullifier) -> pallas::Base { + to_base(prf_expand_vec(&self.0, &[&[0x09], &rho.to_bytes()[..]])) } /// Defined in [Zcash Protocol Spec § 4.7.3: Sending Notes (Orchard)][orchardsend]. /// /// [orchardsend]: https://zips.z.cash/protocol/nu5.pdf#orchardsend - fn esk(&self) -> pallas::Scalar { - to_scalar(prf_expand(&self.0, &[0x04])) + fn esk(&self, rho: &Nullifier) -> pallas::Scalar { + to_scalar(prf_expand_vec(&self.0, &[&[0x04], &rho.to_bytes()[..]])) + } + + /// Defined in [Zcash Protocol Spec § 4.7.3: Sending Notes (Orchard)][orchardsend]. + /// + /// [orchardsend]: https://zips.z.cash/protocol/nu5.pdf#orchardsend + fn rcm(&self, rho: &Nullifier) -> commitment::NoteCommitTrapdoor { + commitment::NoteCommitTrapdoor(to_scalar(prf_expand_vec( + &self.0, + &[&[0x05], &rho.to_bytes()[..]], + ))) } } @@ -141,14 +151,19 @@ impl Note { self.recipient.pk_d().to_bytes(), self.value, self.rho.0, - self.rseed.psi(), - (&self.rseed).into(), + self.rseed.psi(&self.rho), + self.rseed.rcm(&self.rho), ) } /// Derives the nullifier for this note. pub fn nullifier(&self, fvk: &FullViewingKey) -> Nullifier { - Nullifier::derive(fvk.nk(), self.rho.0, self.rseed.psi(), self.commitment()) + Nullifier::derive( + fvk.nk(), + self.rho.0, + self.rseed.psi(&self.rho), + self.commitment(), + ) } } diff --git a/src/note/commitment.rs b/src/note/commitment.rs index 784024c7..824bb200 100644 --- a/src/note/commitment.rs +++ b/src/note/commitment.rs @@ -5,22 +5,9 @@ use ff::PrimeField; use pasta_curves::{arithmetic::FieldExt, pallas}; use subtle::CtOption; -use crate::{ - constants::L_ORCHARD_BASE, - primitives::sinsemilla, - spec::{extract_p, prf_expand, to_scalar}, - value::NoteValue, -}; +use crate::{constants::L_ORCHARD_BASE, primitives::sinsemilla, spec::extract_p, value::NoteValue}; -use super::RandomSeed; - -pub(super) struct NoteCommitTrapdoor(pallas::Scalar); - -impl From<&RandomSeed> for NoteCommitTrapdoor { - fn from(rseed: &RandomSeed) -> Self { - NoteCommitTrapdoor(to_scalar(prf_expand(&rseed.0, &[0x05]))) - } -} +pub(super) struct NoteCommitTrapdoor(pub(super) pallas::Scalar); /// A commitment to a note. #[derive(Debug)] From 9585c67ed205c910cd5c270175ab0b4a805a32a3 Mon Sep 17 00:00:00 2001 From: str4d Date: Fri, 21 May 2021 21:23:08 +0100 Subject: [PATCH 7/8] book: Refine types on Commitments page Co-authored-by: Daira Hopwood --- book/src/design/commitments.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/book/src/design/commitments.md b/book/src/design/commitments.md index 06733abf..adb83b31 100644 --- a/book/src/design/commitments.md +++ b/book/src/design/commitments.md @@ -32,6 +32,6 @@ Note that for $\mathsf{ivk}$, we also deviate from Sapling in two ways: Sapling, we relied on BLAKE2s to make $\mathsf{ivk} = 0$ infeasible to produce, but it was still technically possible. For Orchard, we get this by construction: - $0$ is not a valid x-coordinate for any Pallas point. - - $\mathit{ShortCommit}$ internally maps points to field elements by replacing the identity (which - has no affine coordinates) with $0$. But Sinsemilla is defined using incomplete addition, and + - $\mathsf{SinsemillaShortCommit}$ internally maps points to field elements by replacing the identity (which + has no affine coordinates) with $0$. But $\mathsf{SinsemillaCommit}$ is defined using incomplete addition, and thus will never produce the identity. From 2bbbc3ec94ad7fb0d8e31b19146d6f1c929c4762 Mon Sep 17 00:00:00 2001 From: str4d Date: Fri, 21 May 2021 21:24:08 +0100 Subject: [PATCH 8/8] Update comments Co-authored-by: ying tong --- src/keys.rs | 2 +- src/spec.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/keys.rs b/src/keys.rs index b0afafe4..c939c04b 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -328,7 +328,7 @@ impl From<&FullViewingKey> for KeyAgreementPrivateKey { } impl KeyAgreementPrivateKey { - /// Derives ask from sk. Internal use only, does not enforce all constraints. + /// Derives ivk from fvk. Internal use only, does not enforce all constraints. 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 3567bce6..2881ee28 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -134,6 +134,7 @@ pub(crate) fn diversify_hash(d: &[u8; 11]) -> NonIdentityPallasPoint { let hasher = pallas::Point::hash_to_curve("z.cash:Orchard-gd"); let pk_d = hasher(d); // If the identity occurs, we replace it with a different fixed point. + // TODO: Replace the unwrap_or_else with a cached fixed point. NonIdentityPallasPoint(CtOption::new(pk_d, !pk_d.is_identity()).unwrap_or_else(|| hasher(&[]))) }