V3 encryption by PaulLaux · Pull Request #38 · QED-it/orchard

PaulLaux · 2022-12-20T16:32:57Z

Added support for note_encryption_V3 and encryption generalization (QED-it/librustzcash#18)

not for review: note_encryption.rs, note_encryptionv2v3.rs and src/test_vectors/note_encryption.rs. These files represent two possible approaches for backward compatibility and will be finalized down the road. (the files were excluded from the build).

* Added .circleci/config.yml

Implements the issuer keys as IssuerAuthorizingKey -> isk IssuerVerifyingKey -> ik Test vectors generated with zcash_test_vectors repo

* Added NoteType to Notes * Added NoteType to value commitment derivation

* Circleci project setup (#1) * Added .circleci/config.yml * Added NoteType to Notes * reformated file * updated `derive` for NoteType * added note_type to value commit derivation * rustfmt * updated ci config * updated ci config * updated ci config * updated derive for note_type * added test for arb note_type * added test for `native` note type * zsa-note-encryption: introduce AssetType and encode and decode it in note plaintexts * zsa-note-encryption: extend the size of compact notes to include asset_type * fixed clippy warrnings * rustfmt * zsa-note-encryption: document parsing requirement * zsa-note-encryption: revert support of ZSA compact action * zsa_value: add NoteType method is_native * zsa-note-encryption: remove dependency on changes in the other crate * zsa-note-encryption: extract memo of ZSA notes * zsa-note-encryption: tests (zcash_test_vectors 77c73492) * zsa-note-encryption: simplify roundtrip test * zsa-note-encryption: more test vectors (zcash_test_vectors c10da464) * Circleci project setup (#1) * Added .circleci/config.yml * issuer keys implementation (#5) Implements the issuer keys as IssuerAuthorizingKey -> isk IssuerVerifyingKey -> ik Test vectors generated with zcash_test_vectors repo * Added NoteType to Notes (#2) * Added NoteType to Notes * Added NoteType to value commitment derivation * zsa-note-encryption: use both native and ZSA in proptests * zsa-note-encryption: test vector commit 51398c93 * zsa-note-encryption: fix after merge Co-authored-by: Paul <3682187+PaulLaux@users.noreply.github.com> Co-authored-by: Paul <lauxpaul@protonmail.com> Co-authored-by: Aurélien Nicolas <info@nau.re> Co-authored-by: Daniel Benarroch <danielbenarroch92@gmail.com>

+ Updated test bsk_consistent_with_bvk to verify mixed note types. + Added NoteType support to the builder and the bundle. + added split_flag to SpentInfo and as input to the Circuit (currently commented out) + added conditional cv_sum calculation (currently commented out) + added padding to actions

- added IssueBundle and IssueAction - added a builder for IssueBundle - added verify_issue_bundle() for consensus verification. - unit tests.

added tests in `tests/zsa.rs`

* disabled split notes and proof check for zsa transfer

* fixes and suggestions * changed "issuer" to "issuance" as per zcash#356 (comment) * terminology fixes * updated naming

* rename 2 note_type -> asset as per zcash#356 (comment) * added a dedicated type for "IssuanceAuth" * disabled codecov github action due to bad behavior. * extracted "is_asset_desc_of_valid_size()" into asset_id.rs

* improved `verify_issue_bundle()`

…l require modification.

…ates

…o V2 and V3

src/note_encryption.rs

vivek-arte

Looks generally alright, made a few comments. I would prefer to pass through src/note_encryption_v3.rs once more with a diff from the original note_encryption.rs because it is currently difficult to see the changes specific to V3. Will update once I do that.

src/bundle/commitments.rs

vivek-arte · 2023-01-26T12:30:24Z

src/issuance.rs

        pub fn arb_issue_action()(
-            note in arb_zsa_note(),
+            note in arb_note(NoteValue::from_raw(10)),
            asset_descr in string_regex(".{1,512}").unwrap()


This is from a previous merge, but is it not that asset_descr needs to "match" the AssetId value?

right, fixed it.

vivek-arte · 2023-01-26T14:53:18Z

src/note_encryption_v3.rs

+    }
+
+    fn enc_ciphertext_compact(&self) -> CompactNoteCiphertextBytes {
+        self.enc_ciphertext.clone()


Do we need to clone?

yes, since we return a value not a ref.

- Updated to support encryption_v3 as given in QED-it/orchard#38 and QED-it/librustzcash#18. - This PR breaks compatibility with OrchardDomainV2

alexeykoren · 2023-01-25T14:02:50Z

src/action.rs

            let encrypted_note = TransmittedNoteCiphertext {
                epk_bytes: [0u8; 32],
-                enc_ciphertext: [0u8; 580],
+                enc_ciphertext: [0u8; 612],


Hardcoded constants make me sad, but that's consistent with current style so probably this is more of a general comment rather than a suggestion

alexeykoren · 2023-01-25T14:02:54Z

src/builder.rs

-            .expect("The spend info is valid");
-        split_spend.split_flag = true;
-        split_spend
+        SpendInfo::new(self.fvk.clone(), self.note, self.merkle_path.clone(), true).unwrap()


Do we prefer unwrap over expect with comment?

only when the expect() comment is trivial.

alexeykoren · 2023-01-25T14:03:01Z

src/bundle/commitments.rs

        ch.update(&action.cmx().to_bytes());
        ch.update(&action.encrypted_note().epk_bytes);
-        ch.update(&action.encrypted_note().enc_ciphertext[..52]);
+        ch.update(&action.encrypted_note().enc_ciphertext[..84]); // TODO: make sure it is backward compatible with [..52]


I don't think it is if I understand correctly. Shall we maybe select proper hasher here based on value of EncCipherText enum? This way we can hash both V2 and V3

yes, for sure. Our final aim should be to replace all hardcoded numbers with the relevant constants. But right now, I'm focusing on creating a simple delta to highlight the changes.

PaulLaux and others added 30 commits November 24, 2022 12:38

Circleci project setup (#1)

9dd7ff7

* Added .circleci/config.yml

issuer keys implementation (#5)

29c4f09

Implements the issuer keys as IssuerAuthorizingKey -> isk IssuerVerifyingKey -> ik Test vectors generated with zcash_test_vectors repo

Added NoteType to Notes (#2)

86abc31

* Added NoteType to Notes * Added NoteType to value commitment derivation

Issuance (#12)

3ce74b2

- added IssueBundle and IssueAction - added a builder for IssueBundle - added verify_issue_bundle() for consensus verification. - unit tests.

E2E tests for issuance (#20)

5312b03

added tests in `tests/zsa.rs`

disabled split notes (#22)

c48d354

* disabled split notes and proof check for zsa transfer

Review fixes (#23)

1a1459a

* fixes and suggestions * changed "issuer" to "issuance" as per zcash#356 (comment) * terminology fixes * updated naming

Review fixes2 (#24)

7b6b6ac

* rename 2 note_type -> asset as per zcash#356 (comment) * added a dedicated type for "IssuanceAuth" * disabled codecov github action due to bad behavior. * extracted "is_asset_desc_of_valid_size()" into asset_id.rs

verify_issue_bundle() cleanup (#25)

ee069b4

* improved `verify_issue_bundle()`

updating Cargo.toml to point to appropriate librustzcash patch

fc5df36

orchard crate updated to build with updated librustzcash - tests stil…

80b2689

…l require modification.

edits to test sections

32b90f8

changes based on reviewed changes in zcash_note_encryption

1f34308

adding zcash_note_encryption to orchard crate for better parallel upd…

ea8bcb2

…ates

using ? operator in place of match

5666246

removing duplicate AEAD_TAG_SIZE definition

577e920

fixing clippy errors

2398891

refactoring names of NotePlaintext, EncNoteCiphertext, and versions t…

569df61

…o V2 and V3

refactoring to rename ZSA-specific constants as '.._ZSA'

3eb1614

fixing tests to work with V3 notes, plus rust-fmt

f6da6a5

updating to use &CompactNote instead of &[u8]

b2ba44b

updating Rust to 1.61.0 to pass Clippy tests

1cf2a2c

further Rustfmt fixes

f47c098

fixing Clippy error

64407d2

updating the bundle commitments to include asset ID personalization

914f4e9

fixing Rustfmt errors

9f30147

updated names for constants

1ce300a

updated naming

4721bdd

vivek-arte reviewed Jan 2, 2023

View reviewed changes

src/note_encryption.rs Outdated Show resolved Hide resolved

PaulLaux added 16 commits January 3, 2023 13:27

added encoding test to note_encryption v2

b1bf0a5

cleanup

fc0c45a

clippy fixes

6a54fea

clippy fixes 2

0940e2b

replaced DynamicUsage

7e7a15c

renamed OrchardDomain to OrchardDomainV2

39e85e6

added note_encryption_v2v3.rs

ffb8d1a

updated note_encryption_v3

24a52f6

adjusted types

c4ab99c

updated domain to V3

d644384

updated bundle commitmennt

55ce92f

removed local zcash_note_encryption and cleanup

baeb1cb

fixed comment

de6e87c

Merge branch 'zsa1' into v3_encryption

c6e048a

updated cargo.toml

343120d

updated dep

49a7775

This was referenced Jan 24, 2023

Testing encryption v3 QED-it/zcash-test-vectors#6

Merged

Zsa1 diff, for reference only #7

Draft

vivek-arte reviewed Jan 26, 2023

View reviewed changes

PaulLaux added 4 commits January 31, 2023 14:15

review fixes1

bd8d553

review fixes2

365d4ef

minor fix

35da288

updeted dep

8df5638

PaulLaux added a commit to QED-it/zcash-test-vectors that referenced this pull request Jan 31, 2023

Testing encryption v3 (#6)

f2566cb

- Updated to support encryption_v3 as given in QED-it/orchard#38 and QED-it/librustzcash#18. - This PR breaks compatibility with OrchardDomainV2

updeted dep

79b0744

PaulLaux merged commit cec48d7 into zsa1 Jan 31, 2023

alexeykoren reviewed Jan 31, 2023

View reviewed changes

PaulLaux mentioned this pull request Jan 31, 2023

OrchardZSA reference implementation zcash/orchard#372

Closed

PaulLaux mentioned this pull request May 14, 2025

enable webassembly support #164

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V3 encryption#38

V3 encryption#38
PaulLaux merged 72 commits intozsa1from
v3_encryption

PaulLaux commented Dec 20, 2022 •

edited

Loading

Uh oh!

Uh oh!

vivek-arte left a comment

Uh oh!

Uh oh!

vivek-arte Jan 26, 2023 •

edited

Loading

Uh oh!

PaulLaux Jan 31, 2023

Uh oh!

vivek-arte Jan 26, 2023

Uh oh!

PaulLaux Jan 31, 2023

Uh oh!

alexeykoren Jan 25, 2023

Uh oh!

alexeykoren Jan 25, 2023

Uh oh!

PaulLaux Jan 31, 2023

Uh oh!

alexeykoren Jan 25, 2023

Uh oh!

PaulLaux Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

PaulLaux commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vivek-arte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vivek-arte Jan 26, 2023 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PaulLaux commented Dec 20, 2022 •

edited

Loading

vivek-arte Jan 26, 2023 •

edited

Loading