zcash / orchard

Implementation of the Zcash Orchard Protocol
https://zcash.github.io/orchard/
Other
54 stars 39 forks source link

Remove redundant checks during note encryption #394

Closed str4d closed 1 year ago

str4d commented 1 year ago

The consistency check between esk and ephemeral_key is checked inside zcash_note_encryption::try_output_recovery_with_ock, and the requirement to check it inside the Domain implementation is being lifted in zcash/librustzcash#848.

Removing the check here improves performance, both because we avoid an extra scalar multiplication from esk.derive_public(), and because we avoid an unnecessary spec::diversify_hash() call which is expensive for Orchard.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.37 :warning:

Comparison is base (3619b86) 83.42% compared to head (90e64cb) 83.06%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #394 +/- ## ========================================== - Coverage 83.42% 83.06% -0.37% ========================================== Files 32 32 Lines 2691 2627 -64 ========================================== - Hits 2245 2182 -63 + Misses 446 445 -1 ``` | [Impacted Files](https://app.codecov.io/gh/zcash/orchard/pull/394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zcash) | Coverage Δ | | |---|---|---| | [src/note\_encryption.rs](https://app.codecov.io/gh/zcash/orchard/pull/394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zcash#diff-c3JjL25vdGVfZW5jcnlwdGlvbi5ycw==) | `84.37% <100.00%> (+0.21%)` | :arrow_up: | ... and [8 files with indirect coverage changes](https://app.codecov.io/gh/zcash/orchard/pull/394/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zcash)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

daira commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.37 ⚠️

Comparison is base (3619b86) 83.42% compared to head (90e64cb) 83.06%.

Additional details and impacted files

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

Looking at the full report, it's actually an increase in coverage — which makes sense, because we previously had a error case that wasn't reachable. I think the reported overall decrease is due to nondeterminism.