From 67f7e8fc74527a8a4f974936ce5d0a360ac599dd Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 11:26:19 -0600 Subject: [PATCH] Address comments from code review. --- CHANGELOG.md | 6 ++++++ src/action.rs | 7 ++++++- src/builder.rs | 9 ++++----- src/circuit.rs | 2 +- src/note.rs | 40 +++++++++++++++++++++++----------------- src/note_encryption.rs | 18 +++++++++--------- 6 files changed, 49 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bbd79f04..929952772 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to Rust's notion of ### Added - `orchard::note::Rho` +- `orchard::action::Action::rho` +- `orchard::note_encryption::CompactAction::rho` ### Changed - The following methods have their `Nullifier`-typed argument or return value @@ -17,6 +19,10 @@ and this project adheres to Rust's notion of - `orchard::note::Note::from_parts` - `orchard::note::Note::rho` +### Removed +- `orchard::note_encryption::OrchardDomain::for_nullifier` (use `for_action` + or `for_compact_action` instead). + ## [0.7.1] - 2024-02-29 ### Added - `impl subtle::ConstantTimeEq for orchard::note::Nullifier` diff --git a/src/action.rs b/src/action.rs index 76ec740d2..db8dc853b 100644 --- a/src/action.rs +++ b/src/action.rs @@ -1,7 +1,7 @@ use memuse::DynamicUsage; use crate::{ - note::{ExtractedNoteCommitment, Nullifier, TransmittedNoteCiphertext}, + note::{ExtractedNoteCommitment, Nullifier, Rho, TransmittedNoteCiphertext}, primitives::redpallas::{self, SpendAuth}, value::ValueCommitment, }; @@ -66,6 +66,11 @@ impl Action { &self.encrypted_note } + /// Obtains the [`Rho`] value that was used to construct the new note being created. + pub fn rho(&self) -> Rho { + Rho::from_paired_spend_revealed_nf(self.nf) + } + /// Returns the commitment to the net value created or consumed by this action. pub fn cv_net(&self) -> &ValueCommitment { &self.cv_net diff --git a/src/builder.rs b/src/builder.rs index 7f153a154..ded3cfcfa 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -9,7 +9,6 @@ use nonempty::NonEmpty; use pasta_curves::pallas; use rand::{prelude::SliceRandom, CryptoRng, RngCore}; -use crate::note::Rho; use crate::{ action::Action, address::Address, @@ -19,7 +18,7 @@ use crate::{ FullViewingKey, OutgoingViewingKey, Scope, SpendAuthorizingKey, SpendValidatingKey, SpendingKey, }, - note::{Note, TransmittedNoteCiphertext}, + note::{Note, Rho, TransmittedNoteCiphertext}, note_encryption::OrchardNoteEncryption, primitives::redpallas::{self, Binding, SpendAuth}, tree::{Anchor, MerklePath}, @@ -335,8 +334,8 @@ impl ActionInfo { let v_net = self.value_sum(); let cv_net = ValueCommitment::derive(v_net, self.rcv.clone()); - let nf_revealed = self.spend.note.nullifier(&self.spend.fvk); - let rho = Rho::from_paired_spend_revealed_nf(nf_revealed); + let nf_old = self.spend.note.nullifier(&self.spend.fvk); + let rho = Rho::from_paired_spend_revealed_nf(nf_old); let ak: SpendValidatingKey = self.spend.fvk.clone().into(); let alpha = pallas::Scalar::random(&mut rng); let rk = ak.randomize(&alpha); @@ -355,7 +354,7 @@ impl ActionInfo { ( Action::from_parts( - nf_revealed, + nf_old, rk, cmx, encrypted_note, diff --git a/src/circuit.rs b/src/circuit.rs index 22a05fb7b..d137dbf27 100644 --- a/src/circuit.rs +++ b/src/circuit.rs @@ -406,7 +406,7 @@ impl plonk::Circuit for Circuit { let rho_old = assign_free_advice( layouter.namespace(|| "witness rho_old"), config.advices[0], - self.rho_old.map(|rho| rho.0), + self.rho_old.map(|rho| rho.into_inner()), )?; // Witness cm_old diff --git a/src/note.rs b/src/note.rs index adc2f68de..b6cff87eb 100644 --- a/src/note.rs +++ b/src/note.rs @@ -21,28 +21,22 @@ pub use self::commitment::{ExtractedNoteCommitment, NoteCommitment}; pub(crate) mod nullifier; pub use self::nullifier::Nullifier; -// We know that `pallas::Base` doesn't allocate internally. -memuse::impl_no_dynamic_usage!(Rho); - /// The randomness used to construct a note. -/// -/// The [`Rho`] value for a note should always be constructed from the revealed nullifier of the -/// paired spend in the process of creating an [`Action`]. -/// -/// [`Action`]: crate::action::Action #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] -pub struct Rho(pub(crate) pallas::Base); +pub struct Rho(pallas::Base); -impl Rho { - /// Constructs the [`Rho`] value to be used to construct a new note from the revealed nullifier - /// of the note being spent in the [`Action`] under construction. - /// - /// [`Action`]: crate::action::Action - pub fn from_paired_spend_revealed_nf(nf: Nullifier) -> Self { - Rho(nf.0) - } +// We know that `pallas::Base` doesn't allocate internally. +memuse::impl_no_dynamic_usage!(Rho); +impl Rho { /// Deserialize the rho value from a byte array. + /// + /// This should only be used in cases where the components of a `Note` are being serialized and + /// stored individually. Use [`Action::rho`] or [`CompactAction::rho`] to obtain the [`Rho`] + /// value otherwise. + /// + /// [`Action::rho`] crate::action::Action::rho + /// [`CompactAction::rho`] crate::note_encryption::CompactAction::rho pub fn from_bytes(bytes: &[u8; 32]) -> CtOption { pallas::Base::from_repr(*bytes).map(Rho) } @@ -51,6 +45,18 @@ impl Rho { pub fn to_bytes(self) -> [u8; 32] { self.0.to_repr() } + + /// Constructs the [`Rho`] value to be used to construct a new note from the revealed nullifier + /// of the note being spent in the [`Action`] under construction. + /// + /// [`Action`] crate::action::Action + pub(crate) fn from_paired_spend_revealed_nf(nf: Nullifier) -> Self { + Rho(nf.0) + } + + pub(crate) fn into_inner(self) -> pallas::Base { + self.0 + } } /// The ZIP 212 seed randomness for a note. diff --git a/src/note_encryption.rs b/src/note_encryption.rs index af3fe9160..e1781229b 100644 --- a/src/note_encryption.rs +++ b/src/note_encryption.rs @@ -97,17 +97,12 @@ impl memuse::DynamicUsage for OrchardDomain { impl OrchardDomain { /// Constructs a domain that can be used to trial-decrypt this action's output note. pub fn for_action(act: &Action) -> Self { - Self::for_nullifier(*act.nullifier()) + Self { rho: act.rho() } } - /// Constructs a domain from a nullifier. - /// - /// The provided nullifier must be the nullifier revealed in the action of the note being - /// encrypted or decrypted. - pub fn for_nullifier(nullifier: Nullifier) -> Self { - OrchardDomain { - rho: Rho::from_paired_spend_revealed_nf(nullifier), - } + /// Constructs a domain that can be used to trial-decrypt this action's output note. + pub fn for_compact_action(act: &CompactAction) -> Self { + Self { rho: act.rho() } } } @@ -338,6 +333,11 @@ impl CompactAction { pub fn cmx(&self) -> ExtractedNoteCommitment { self.cmx } + + /// Obtains the [`Rho`] value that was used to construct the new note being created. + pub fn rho(&self) -> Rho { + Rho::from_paired_spend_revealed_nf(self.nullifier) + } } #[cfg(test)]