Add APIs to prepare ivk and epk and implement them for Sapling #633

daira · 2022-09-12T21:44:04Z

No description provided.

Cargo.toml

codecov · 2022-09-13T11:36:19Z

Codecov Report

Base: 75.92% // Head: 50.76% // Decreases project coverage by -25.15% ⚠️

Coverage data is based on head (badb956) compared to base (3bc8627).
Patch coverage: 14.28% of modified lines in pull request are covered.

❗ Current head badb956 differs from pull request most recent head 20e869f. Consider uploading reports for the commit 20e869f to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #633       +/-   ##
===========================================
- Coverage   75.92%   50.76%   -25.16%     
===========================================
  Files         100       99        -1     
  Lines       10978     9737     -1241     
===========================================
- Hits         8335     4943     -3392     
- Misses       2643     4794     +2151

Impacted Files	Coverage Δ
components/zcash_note_encryption/src/lib.rs	`22.90% <0.00%> (-67.04%)`	⬇️
zcash_primitives/src/sapling/note_encryption.rs	`61.58% <20.00%> (-38.42%)`	⬇️
components/zcash_note_encryption/src/batch.rs	`21.73% <50.00%> (-78.27%)`	⬇️
...imitives/src/sapling/pedersen_hash/test_vectors.rs	`5.56% <0.00%> (-94.44%)`	⬇️
zcash_primitives/src/transaction/util/sha256d.rs	`9.09% <0.00%> (-80.91%)`	⬇️
zcash_client_backend/src/scan.rs	`25.49% <0.00%> (-59.70%)`	⬇️
...h_primitives/src/transaction/components/orchard.rs	`28.57% <0.00%> (-58.29%)`	⬇️
zcash_proofs/src/hashreader.rs	`43.75% <0.00%> (-56.25%)`	⬇️
... and 94 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Cargo.toml

components/zcash_note_encryption/src/lib.rs

zcash_primitives/src/sapling/note_encryption.rs

str4d · 2022-09-13T18:49:54Z

Force-pushed to remove some unnecessary patch directives and clean up the history.

str4d · 2022-09-13T20:13:51Z

Force-pushed to migrate to using zkcrypto/group#36. Compilation is expected to fail because of an issue in zcash_client_backend that needs addressing, but this PR should work as a dependency for zcashd.

str4d · 2022-09-13T21:44:29Z

Force-pushed with a fix to a test. The compilation issue will be addressed by rebasing this PR on main once #636 has been reviewed and merged.

str4d · 2022-09-13T21:52:26Z

Rebased on #636.

str4d · 2022-09-13T23:05:39Z

Force-pushed to update the benchmarks for the API changes.

Cargo.toml

daira

utACK. (I'm the original PR author so I can't Approve.)

daira

~~Actually please hold up on merging this; I want to add a benchmark for the multiple-ivk, multiple-epk case.~~
Done.

str4d · 2022-09-14T14:59:30Z

Force-pushed to address @daira's comment.

…puts). Signed-off-by: Daira Hopwood <daira@jacaranda.org>

Co-authored-by: Jack Grigg <jack@electriccoin.co> Signed-off-by: Daira Hopwood <daira@jacaranda.org>

daira · 2022-09-15T02:49:25Z

Benchmark added. It is added as the first commit, so that it's possible to roll back to that one to get a "before" benchmark. zcash/orchard#357 was updated to reference the second commit with the API changes (515b0a4), and then merged. Finally the last commit in this PR was updated to specify the merge commit for zcash/orchard#357 in Cargo.toml.

str4d

ACK 20e869f

str4d · 2022-09-15T11:44:00Z

Cargo.toml

@@ -21,4 +21,5 @@ codegen-units = 1
 [patch.crates-io]
 zcash_encoding = { path = "components/zcash_encoding" }
 zcash_note_encryption = { path = "components/zcash_note_encryption" }
+orchard = { git = "https://github.com/zcash/orchard.git", rev = "33f1c1141e50adb68715f3359bd75378b4756cca" }


I confirmed that this is the merge commit for zcash/orchard#357.

As of #633, `SaplingDomain::IncomingViewingKey` now allocates memory internally, and this memory persists as long as the `BatchRunner` is alive. Now that we have decoupled the measurement of heap usage for batch tasks from their internals, we can add bounds to all of the generic parameters of `Batch` to enable correctly measuring their actual heap usage. We also add `DynamicUsage` impls for a bunch of `zcash_primitives` types that will be used with `BatchRunner` (or its equivalent implementation in `zcashd`) by callers.

As of zcash/librustzcash#633, `SaplingDomain::IncomingViewingKey` now allocates memory internally, and this memory persists as long as the `BatchRunner` is alive. Now that we have decoupled the measurement of heap usage for batch tasks from their internals, we can add bounds to all of the generic parameters of `Batch` to enable correctly measuring their actual heap usage. Ported from zcash/librustzcash@913aa0a.

daira added the I-performance label Sep 12, 2022

daira force-pushed the prepare-epks-and-ivks branch 2 times, most recently from ec986bf to d0df35c Compare September 12, 2022 22:21

daira commented Sep 13, 2022

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

daira marked this pull request as ready for review September 13, 2022 10:30

daira added this to the Release 5.3.0 milestone Sep 13, 2022

nuttycom reviewed Sep 13, 2022

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

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

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

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

str4d force-pushed the prepare-epks-and-ivks branch from badb956 to 6853ce0 Compare September 13, 2022 18:46

str4d force-pushed the prepare-epks-and-ivks branch 2 times, most recently from d2d299f to ce178ab Compare September 13, 2022 20:12

str4d force-pushed the prepare-epks-and-ivks branch from ce178ab to 71cc74d Compare September 13, 2022 21:43

str4d force-pushed the prepare-epks-and-ivks branch 2 times, most recently from 54c817f to 1b49d6d Compare September 13, 2022 21:51

str4d changed the base branch from main to batch-scanner-tag-ivks September 13, 2022 21:52

nuttycom deleted the branch zcash:main September 13, 2022 22:49

nuttycom closed this Sep 13, 2022

str4d reopened this Sep 13, 2022

str4d changed the base branch from batch-scanner-tag-ivks to main September 13, 2022 22:54

str4d force-pushed the prepare-epks-and-ivks branch from 1b49d6d to 1c967a5 Compare September 13, 2022 23:04

daira commented Sep 14, 2022

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

daira mentioned this pull request Sep 14, 2022

Dummy implementation of Domain API change for prepared ivk and epk zcash/orchard#357

Merged

daira commented Sep 14, 2022

View reviewed changes

str4d force-pushed the prepare-epks-and-ivks branch from 1c967a5 to 7790f12 Compare September 14, 2022 14:58

str4d force-pushed the prepare-epks-and-ivks branch from 7790f12 to 2279915 Compare September 14, 2022 14:59

daira and others added 2 commits September 15, 2022 03:20

Change batch decryption benchmark to cover more cases of (nivks, nout…

d77ed9c

…puts). Signed-off-by: Daira Hopwood <daira@jacaranda.org>

zcash_note_encryption: Add API to prepare epk.

515b0a4

Co-authored-by: Jack Grigg <jack@electriccoin.co> Signed-off-by: Daira Hopwood <daira@jacaranda.org>

daira force-pushed the prepare-epks-and-ivks branch from 2279915 to c49d1f9 Compare September 15, 2022 02:25

zcash_primitives: Use prepared epk and ivk in Sapling note decryption

20e869f

Co-authored-by: Jack Grigg <jack@electriccoin.co> Signed-off-by: Daira Hopwood <daira@jacaranda.org>

daira force-pushed the prepare-epks-and-ivks branch from c49d1f9 to 20e869f Compare September 15, 2022 02:43

daira mentioned this pull request Sep 15, 2022

[DO NOT MERGE] Use zcash/librustzcash#633 zcash/zcash#6161

Closed

str4d approved these changes Sep 15, 2022

View reviewed changes

str4d merged commit 8483503 into zcash:main Sep 15, 2022

daira deleted the prepare-epks-and-ivks branch September 15, 2022 14:11

str4d mentioned this pull request Sep 24, 2022

zcash_client_backend: Refactor tracking of BatchRunner heap memory usage #652

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add APIs to prepare ivk and epk and implement them for Sapling #633

Add APIs to prepare ivk and epk and implement them for Sapling #633

daira commented Sep 12, 2022

codecov bot commented Sep 13, 2022 •

edited

Loading

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

daira left a comment

daira left a comment •

edited

Loading

str4d commented Sep 14, 2022

daira commented Sep 15, 2022 •

edited

Loading

str4d left a comment

str4d Sep 15, 2022

Add APIs to prepare ivk and epk and implement them for Sapling #633

Add APIs to prepare ivk and epk and implement them for Sapling #633

Conversation

daira commented Sep 12, 2022

codecov bot commented Sep 13, 2022 • edited Loading

Codecov Report

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

str4d commented Sep 13, 2022

daira left a comment

Choose a reason for hiding this comment

daira left a comment • edited Loading

Choose a reason for hiding this comment

str4d commented Sep 14, 2022

daira commented Sep 15, 2022 • edited Loading

str4d left a comment

Choose a reason for hiding this comment

str4d Sep 15, 2022

Choose a reason for hiding this comment

codecov bot commented Sep 13, 2022 •

edited

Loading

daira left a comment •

edited

Loading

daira commented Sep 15, 2022 •

edited

Loading