New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Generalize Sapling note encryption to allow reuse with Orchard notes. #358

Merged

str4d merged 24 commits into zcash:master from nuttycom:refactor/component_modules_2

Apr 16, 2021

Contributor

nuttycom commented Mar 22, 2021 •

edited by str4d

Loading

This builds upon #348, #351, and #357.

nuttycom requested review from daira and str4d

March 22, 2021 21:05

r3ld3v added this to the Core Sprint 2021-10 milestone

nuttycom force-pushed the refactor/component_modules_2 branch from b39c005 to c03de6c Compare

March 23, 2021 18:22

codecov bot commented Mar 23, 2021 •

edited

Loading

Codecov Report

Merging #358 (28a4502) into master (1b4aab0) will decrease coverage by 0.37%.
The diff coverage is 71.58%.

@@            Coverage Diff             @@
##           master     #358      +/-   ##
==========================================
- Coverage   64.83%   64.46%   -0.38%     
==========================================
  Files          73       73              
  Lines        7161     7232      +71     
==========================================
+ Hits         4643     4662      +19     
- Misses       2518     2570      +52

Impacted Files	Coverage Δ
zcash_client_sqlite/src/wallet/transact.rs	`99.13% <ø> (-0.02%)`	⬇️
...h_primitives/src/transaction/components/sapling.rs	`51.45% <31.25%> (-3.72%)`	⬇️
zcash_client_backend/src/proto.rs	`50.00% <36.36%> (-14.00%)`	⬇️
zcash_primitives/src/transaction/builder.rs	`56.60% <42.85%> (+0.11%)`	⬆️
zcash_primitives/src/sapling.rs	`67.97% <50.00%> (+1.30%)`	⬆️
zcash_client_backend/src/welding_rig.rs	`91.66% <66.66%> (-0.13%)`	⬇️
components/zcash_note_encryption/src/lib.rs	`69.31% <69.31%> (+69.31%)`	⬆️
zcash_primitives/src/sapling/note_encryption.rs	`75.97% <75.97%> (ø)`
zcash_client_backend/src/data_api/wallet.rs	`86.48% <100.00%> (-0.36%)`	⬇️
zcash_client_sqlite/src/lib.rs	`54.61% <100.00%> (ø)`
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b4aab0...28a4502. Read the comment docs.

nuttycom force-pushed the refactor/component_modules_2 branch 2 times, most recently from e68d57b to 2b1362b Compare

March 25, 2021 00:35

nuttycom marked this pull request as ready for review

March 25, 2021 00:37

nuttycom mentioned this pull request

Empty zcash_note_encryption crate #357

Merged

nuttycom force-pushed the refactor/component_modules_2 branch 2 times, most recently from bd378d9 to 3805a6d Compare

March 27, 2021 14:43

r3ld3v modified the milestones: Core Sprint 2021-10, Core Sprint 2021-12

str4d approved these changes

View reviewed changes

Contributor

str4d left a comment

utACK mod comments.

zcash_primitives/src/sapling/note_encryption.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/Cargo.toml

    
              rand_core = "0.5.1"

              [dev-dependencies]

              zcash_primitives = { version = "0.5", path = "../../zcash_primitives" }

Contributor

str4d Apr 1, 2021

These not-quite-circular dependencies make publishing a pain. I dealt with it for zcash_client_backend <-> zcash_client_sqlite for the 0.5 release by specifying a version range per rust-lang/cargo#4242 but it would be good to formalize this in the repo. Non-blocking, we can sort this out later.

Contributor Author

nuttycom Apr 5, 2021

With the zcash_client_backend issue, I'm planning to address that by making the tests just run against the in-memory backend once that's complete, and move the sqlite tests back to the sqlite crate. Here, I'm not sure what the best approach is; I guess we could move these tests to zcash_primitives and implement a set of tests here that just relies upon the note-encryption functionality without using Sapling.

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

str4d mentioned this pull request

zcash_note_encryption: Use constant-time equality check for EphemeralKeyBytes #370

Closed

nuttycom and others added 8 commits

April 8, 2021 08:19


          Move note_encryption into the sapling module.

5baccdf


          Generalize note encryption and decryption.

e778392

This commit introduces a `Domain` trait which defines the types
and operations that are shared between Sapling and Orchard note
encryption and decryption processes.


          Move generalized note encryption functionality to zcash_note_encrypti…

266285b

…on crate.


          Add try_output_recovery_with_ovk to shared note encryption code.

5b13bb3


          Update zcash_primitives dev dependency for zcash_note_encryption

be225da


          Fix Clippy complaints.

213cd6c


          Fix zcash_note_encryption doctests.

dad8663


          Apply suggestions from code review

879eea8

Co-authored-by: str4d <[email protected]>

nuttycom force-pushed the refactor/component_modules_2 branch from ba9718b to 879eea8 Compare

April 8, 2021 14:20

ebfull requested changes

View reviewed changes

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/Cargo.toml Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

nuttycom and others added 2 commits

April 8, 2021 10:08


          Remove spurious mut references.

a560101


          Update comments describing COMPACT_NOTE_SIZE components.

24e62d3

Co-authored-by: ebfull <[email protected]>

nuttycom requested a review from ebfull

April 8, 2021 16:09


          Add myself to crate contributors.

cfdbafe

daira reviewed

View reviewed changes

Contributor

daira left a comment

Flushing comments.

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

zcash_primitives/src/sapling/note_encryption.rs Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

components/zcash_note_encryption/src/lib.rs Show resolved Hide resolved

daira reviewed

View reviewed changes

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

zcash_client_backend/src/decrypt.rs Outdated Show resolved Hide resolved

r3ld3v removed this from the Core Sprint 2021-12 milestone


          Use ephemeral_key bytes instead of the epk abstract point where speci…

6fc1d1d

…fied.

nuttycom commented

View reviewed changes

components/zcash_note_encryption/src/lib.rs Show resolved Hide resolved


          Update documentation for note encryption traits.

f34e878

nuttycom force-pushed the refactor/component_modules_2 branch from b74d9d1 to f34e878 Compare

April 12, 2021 23:41

nuttycom and others added 3 commits

April 12, 2021 18:40


          Simplify try_sapling_output_recovery.

fae1a15


          Minor comment on epk canonicity.

389e6ca


          Fix formatting.

12cb826

nuttycom requested a review from str4d

April 13, 2021 01:31

str4d added this to the Core Sprint 2021-14 milestone

str4d approved these changes

View reviewed changes

Contributor

str4d left a comment

utACK 12cb826

ebfull approved these changes

View reviewed changes

nuttycom force-pushed the refactor/component_modules_2 branch from 0763d2b to 4c8ffd7 Compare

April 14, 2021 15:48


          Make cmstar check follow the spec more closely.

00d04de

nuttycom force-pushed the refactor/component_modules_2 branch from 4c8ffd7 to 00d04de Compare

April 14, 2021 21:39

daira reviewed

View reviewed changes

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

components/zcash_note_encryption/src/lib.rs Outdated Show resolved Hide resolved


          Fix naming cmstar -> cmstar_bytes and cm -> cmstar

b2b3efd

nuttycom force-pushed the refactor/component_modules_2 branch from 2fee78d to b2b3efd Compare

April 15, 2021 21:24


          Add a note about canonicity of ephemeral public keys.

3434cc8

daira approved these changes

View reviewed changes

Contributor

daira left a comment •

edited

Loading

utACK modulo one remaining comment (now resolved).


          Update comment about which case is covered by example code

dc0f6e7

str4d approved these changes

View reviewed changes

Contributor

str4d left a comment

utACK dc0f6e7


          cargo fmt

28a4502

str4d merged commit 3b02c8b into zcash:master

nuttycom deleted the refactor/component_modules_2 branch

April 16, 2021 22:41

str4d mentioned this pull request

Generalise note encryption logic #349

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet