Reduce OrchardPrimitives generic parameter usage by ConstanceBeguier · Pull Request #261 · QED-it/orchard

ConstanceBeguier · 2026-07-23T15:43:27Z

This PR has two objectives

reduce the diff between our implementation and upstream
get an architecture closer with upstream (using enums instead of generic parameters to select between the different protocols)

Also

Remove OrchardPrimitives from ZK cirucit and expected_proof_size
Remove OrchardPrimitives from digests function (hash_bundle_txid_data and hash_bundle_auth_data)
Remove OrchardPrimitives from note encryption

ConstanceBeguier · 2026-07-28T12:42:45Z

Rebase on zsa1

…ion test to Vanilla bundle

PaulLaux

Overall very good result. Added comments. Also

Burn is unprotected. Lets add a check to Builder::add_burn() to allow only when self is ZSA.
Consider protecting further: add validate_burn(&burn, bundle_version) returning BurnNotPermitted, called from the three sites that already call validate_action_ciphertext_kind
Same check but in burn_validation.rs (select which set of all options is the most appropriate)
Split the burn logic away from the main logic so it is easier to swallow.

Also,

please run cargo clippy --all-targets --all-features: we have new warnings caused by our new code.

PaulLaux · 2026-08-04T20:35:05Z

                .unwrap();
        }
-        let bundle: Bundle<_, i64, FL> = builder.build(rng).unwrap().unwrap().0;
+        let bundle: Bundle<_, i64> = builder.build(rng).unwrap().unwrap().0;


as discussed cargo bench panics somewhere here

PaulLaux · 2026-08-04T21:00:13Z


        #[test]
-        fn commitment_hashes_the_wire_flag_byte(bundle in BundleArb::<OrchardVanilla>::arb_bundle(3)) {
+        fn commitment_hashes_the_wire_flag_byte(bundle in arb_bundle(3)) {


This will get a ZSA bundle once in 5 draws due to arb_bundle_version() then will build Bundle::from_parts_unchecked( under the assumbption that this is a vanilla bundle. Need to change to arb_bundle_vanilla() to keep semantics unchanged.

PaulLaux · 2026-08-04T21:00:46Z

        #[test]
-        fn ironwood_rejects_v5_commitment_version(bundle in BundleArb::<OrchardVanilla>::arb_bundle(3)) {
-            let bundle_i64 = Bundle::from_parts_unchecked(
+        fn ironwood_rejects_v5_commitment_version(bundle in arb_bundle(3)) {


same like https://github.com/QED-it/orchard/pull/261/changes#r3716144833

PaulLaux · 2026-08-04T21:01:15Z

        /// the same bundle commits to distinct transaction-ID digests under each.
        #[test]
-        fn anchor_placement_follows_tx_version(bundle in BundleArb::<OrchardVanilla>::arb_bundle(3)) {
+        fn anchor_placement_follows_tx_version(bundle in arb_bundle(3)) {


same like https://github.com/QED-it/orchard/pull/261/changes#r3716144833

PaulLaux · 2026-08-04T21:02:21Z


+        #[test]
+        fn try_from_parts_rejects_mismatched_action_ciphertext_kind(
+            bundle in arb_bundle(3)


This bundle can be ZSA and can be Vanilla. Is this what we want?

Fixed

Created arb_bundle_zsa strategy for ZSA bundle

Used this strategy here.

Updated this test. Each actions in a bundle created with this new strategy has a ZSA ciphertext. Thus, the first rows of this test are now unnecessary.

PaulLaux · 2026-08-04T22:04:28Z

-            BundleCommitmentFormat::OrchardV6 | BundleCommitmentFormat::IronwoodV6
+            BundleCommitmentFormat::OrchardV6
+                | BundleCommitmentFormat::IronwoodV6
+                | BundleCommitmentFormat::ZSA


OK, we added ZSA here but it is unused on the ZSA path. Should probably taken into account in hash_bundle_auth_data_zsa(). At least assert.

Added an assert in hash_bundle_auth_data_zsa and a comment in includes_anchor_in_authorizing_digest

PaulLaux · 2026-08-04T22:07:17Z

    /// The arbitrary-bundle strategies generate flags independently of the version; this pairs them
    /// into a combination that a `Bundle` can actually be constructed from.
-    fn flags_for_version(bundle_version: BundleVersion, flags: Flags) -> Flags {
+    pub(crate) fn flags_for_version(bundle_version: BundleVersion, flags: Flags) -> Flags {


Should document the ZSA flag here.

PaulLaux · 2026-08-04T23:31:28Z

+pub(crate) const ENC_CIPHERTEXT_SIZE_VANILLA: usize = NOTE_PLAINTEXT_SIZE_VANILLA + AEAD_TAG_SIZE;
+/// The size of a ZSA encrypted note ciphertext, accounting for the AEAD tag.
+pub(crate) const ENC_CIPHERTEXT_SIZE_ZSA: usize = NOTE_PLAINTEXT_SIZE_ZSA + AEAD_TAG_SIZE;
+


These 2 should be Public.

I updated the visibility to public for those two constants.
I am not sure whether these constants are used in other crates.

PaulLaux · 2026-08-04T23:41:01Z

+    fn encrypted_note_for(
+        note: Note,
+        memo: Vec<u8>,
+        cv_net: &ValueCommitment,
+        cmx: &ExtractedNoteCommitment,
+        mut rng: impl RngCore,
+    ) -> TransmittedNoteCiphertext {
+        // The Orchard, Ironwood and ZSA encryptor aliases share encryption behavior;
+        // `Note::version()` selects the note plaintext lead byte.
+        let encryptor = NoteEncryption::<NoteEncryptionDomain<OrchardVersion>>::new(
+            None,
+            note,
+            memo.try_into().unwrap(),
+        );
+
+        TransmittedNoteCiphertext {
+            epk_bytes: NoteEncryptionDomain::<OrchardVersion>::epk_bytes(encryptor.epk()).0,
+            enc_ciphertext: encryptor.encrypt_note_plaintext(),
+            out_ciphertext: encryptor.encrypt_outgoing_plaintext(cv_net, cmx, &mut rng),
+        }
    }

-    impl<Pr: OrchardPrimitives> ActionArb<Pr> {
-        /// Builds a real, decryptable `TransmittedNoteCiphertext` for `note`,
-        /// mirroring `OutputInfo::build`: the same encryptor yields a non-identity
-        /// ephemeral public key (satisfying the `Action::from_parts` epk invariant)


the diff is strange here. can we reduce so it does not look like our change?

I do not know how to reduce it. It is mainly a diff in indentation because we remove impl<Pr: OrchardPrimitives> ActionArb<Pr>

You could add "?w=1" at the end of the github url. It will hide whitespace changes and make the diff much easier to read.
This URL for the whole PR diff
https://github.com/QED-it/orchard/pull/261/changes?w=1
This URL for the action.rs file diff
https://github.com/QED-it/orchard/pull/261/changes?w=1#diff-f4ccec926f8a15e422ea29ba8516940bab45ed47ee2845930c70531ef4ffa4c9

PaulLaux · 2026-08-04T23:41:55Z

+        )(
+            nf in arb_nullifier(),
+            (rsk, rk) in arb_valid_spendauth_keypair(),
+            note in arb_note(output_value, note_version),


same for strange / unneeded diff

See previous comment
The diff are due to the indentation changes (I removed an impl block which is no longer required)
You could add "?w=1" at the end of the url to hide whitespace changes

…ions

Those digests require only (value_pool, tx_version) to be evaluated.

…tx_version_compatibility

We should update them because we updated ZSA personalizations.

…_bundle_auth_data_zsa

…or_in_authorizing_digest

ConstanceBeguier changed the title ~~Reduce OrchardCircuit generic parameter usage~~ Reduce OrchardPrimitives generic parameter usage Jul 23, 2026

ConstanceBeguier force-pushed the remove_orchard_primitives branch 3 times, most recently from c00574f to 06b2b29 Compare July 24, 2026 15:12

ConstanceBeguier added 5 commits July 28, 2026 14:40

Remove OrchardCircuit generic parameter

0adde63

Remove OrchardPrimtives from expected_proof_size

bd1b7e8

Remove OrchardPrimtives from extract_asset

08994ea

Remove hash_bundle_txid_data from OrchardPrimitives trait

a914771

Remove hash_bundle_auth_data from OrchardPrimitives trait

86e91c7

ConstanceBeguier force-pushed the remove_orchard_primitives branch from 06b2b29 to 86e91c7 Compare July 28, 2026 12:42

ConstanceBeguier changed the base branch from cross_adress_zsa_circuit to zsa1 July 28, 2026 12:42

ConstanceBeguier added 4 commits July 29, 2026 16:08

Remove OrchardPrimtivies from note_encryption

10e03c6

Return result instead of panicking in circuit_version

838b4b7

Add create_rejects_mismatched_proving_key_version test for ZSA circuit

ab34950

rustfmt

ceff107

ConstanceBeguier marked this pull request as ready for review July 29, 2026 15:11

ConstanceBeguier requested a review from PaulLaux July 29, 2026 15:12

ConstanceBeguier added 12 commits July 29, 2026 17:14

Add a check for ZSA circuit

194360c

Update benches to run them on Orchard, Ironwood and ZSA

6b0c1c0

Do not restrict add_bundle_rejects_unsupported_cross_address_restrict…

403dd22

…ion test to Vanilla bundle

Reduce visibility and update comments in bundle/commitment.rs

da45a94

Update comment into circuit/circuit_vanilla.rs

efeb3ab

Add comment into src/actions.rs

248378c

Simplify builder.rs

fa3f5c3

Remove unused permits_vanilla function

58607b2

Update bundle.rs

1058dc5

Update circuit.rs

2a2a177

Update comments in lib.rs

92c461a

Update comment in note.rs

4303e9c

ConstanceBeguier added 7 commits July 30, 2026 17:20

Update note_encryption.rs

51c2739

Update pczt.rs

3bae919

Merge branch 'zsa1' into remove_orchard_primitives

d2ddca1

Remove shielding_bundle_orchard_v3 test

ab21505

Use is_zsa when it is possible in circuit

84b3b0b

Fix includes_anchor_in_authorizing_digest

adda00b

Fix commitment_format function

25987af

PaulLaux reviewed Aug 4, 2026

View reviewed changes

ConstanceBeguier added 15 commits August 5, 2026 15:58

Fix proptests

7136acc

Remove unused imports from previous commit

15ca89a

Fix benches

74ab03b

Update ZSA personalizations to be distinct from Ironwood personalizat…

8de3799

…ions

Updatee hash_bundle_txid/auth_empty signatures

1c27e70

Those digests require only (value_pool, tx_version) to be evaluated.

Restore commitment_format from upstream and add check_bundle_version_…

b7fe247

…tx_version_compatibility

Fix ZSA fixed digests

738ff83

We should update them because we updated ZSA personalizations.

Update visibility of ENC_CIPHERTEXT_SIZE_VANILLA/ZSA to public

c987c4f

Check that sighash_kind is equal to AllEffecting when evaluating hash…

75cf9b6

…_bundle_auth_data_zsa

Fix typo and remove TODO about memo bundle

c2d5281

Update hash_bundle_auth_data_zsa and add description on includes_anch…

548cdf7

…or_in_authorizing_digest

Add comments for flags_for_version

754cf38

Fix clippy warning (clippy version 1.94)

ba3b78e

Add validate_burn function

2e62dfc

Fix typo

a8a04eb

Uh oh!

Conversation

ConstanceBeguier commented Jul 23, 2026 • edited by PaulLaux Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ConstanceBeguier commented Jul 28, 2026

Uh oh!

PaulLaux left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConstanceBeguier Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ConstanceBeguier commented Jul 23, 2026 •

edited by PaulLaux

Loading

PaulLaux left a comment •

edited

Loading

ConstanceBeguier Aug 5, 2026 •

edited

Loading

ConstanceBeguier Aug 5, 2026 •

edited

Loading

ConstanceBeguier Aug 6, 2026 •

edited

Loading