Address comments from code review.

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`

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<T> Action<T> {
         &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

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,

src/circuit.rs

@@ -406,7 +406,7 @@ impl plonk::Circuit<pallas::Base> 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

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<Self> {
         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.

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<T>(act: &Action<T>) -> 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)]