Skip to content

Simplify Orchard Flavor#176

Merged
ConstanceBeguier merged 9 commits intozsa1from
clean_orchard_flavor
Jul 28, 2025
Merged

Simplify Orchard Flavor#176
ConstanceBeguier merged 9 commits intozsa1from
clean_orchard_flavor

Conversation

@ConstanceBeguier
Copy link
Collaborator

@ConstanceBeguier ConstanceBeguier commented Jul 23, 2025

  • Rename OrchardCommonDomain to OrchardPrimitives
  • Rename domain folder to primitives
  • Replace OrchardFlavor with OrchardCircuit or OrchardPrimitives whenever possible

@QED-it QED-it deleted a comment from what-the-diff bot Jul 23, 2025
Copy link
Collaborator

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it more then our previous naming.

To conclude:

  • can we do somthing with the file primitives.rs? remove or make it relevent
  • the file domain.rs exposes functions belong to primitives. can we move it to primitives.rs and remove the file?
  • folder domain should be renamed to Primitives? (we can create a new Domain folder inside the renamed Primitives.)
  • the struct
#[derive(Debug, Clone)]
pub struct OrchardDomain<P: OrchardPrimitives> {
    /// A parameter needed to generate the nullifier.
    pub rho: Rho,
    phantom: core::marker::PhantomData<P>,
}

should be in zcash_note_encryption_domain.rs or domain.rs not in orchard_primitives.rs

  • instead
orchard_primitives::{OrchardDomain, OrchardPrimitives},

we should probable have:

orchard_primitives::{Domain::OrchardDomain, OrchardPrimitives},

consider it, it is inlide with the suggestion above.

@ConstanceBeguier
Copy link
Collaborator Author

ConstanceBeguier commented Jul 28, 2025

I removed primitives.rs file.
Then I moved domain.rs to primitives.rs and I renamed domain folder into primitives.
I moved OrchardDomain struct into the new file primitives/orchard_domain.rs

@ConstanceBeguier ConstanceBeguier merged commit fab774d into zsa1 Jul 28, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants