Skip to content
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.

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

Add a Rho type, to distinguish from revealed nullifiers of spent notes. #421

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

nuttycom
Copy link
Contributor

No description provided.

@nuttycom nuttycom force-pushed the add_rho_type branch 3 times, most recently from 9fe0e59 to 562896f Compare March 10, 2024 16:33
@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2024

Codecov Report

Attention: Patch coverage is 45.83333% with 26 lines in your changes are missing coverage. Please review.

Project coverage is 79.98%. Comparing base (a6b3407) to head (b2efb4c).

Files Patch % Lines
src/note_encryption.rs 4.00% 24 Missing ⚠️
src/circuit.rs 50.00% 1 Missing ⚠️
src/note.rs 94.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #421      +/-   ##
==========================================
- Coverage   80.49%   79.98%   -0.52%     
==========================================
  Files          30       30              
  Lines        3158     3192      +34     
==========================================
+ Hits         2542     2553      +11     
- Misses        616      639      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nuttycom nuttycom changed the title Add a Rho type, to distinguish from revealed note nullifiers. Add a Rho type, to distinguish from revealed nullifiers of spent notes. Mar 10, 2024
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 562896f.

src/builder.rs Outdated Show resolved Hide resolved
src/builder.rs Outdated Show resolved Hide resolved
src/note.rs Show resolved Hide resolved
@@ -9,7 +9,7 @@ pub(crate) struct TestVector {
pub(crate) rseed: [u8; 32],
pub(crate) memo: [u8; 512],
pub(crate) cv_net: [u8; 32],
pub(crate) rho: [u8; 32],
pub(crate) nf_old: [u8; 32],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're changing this, we need to also change it in zcash-test-vectors which is what generates this.

src/note.rs Outdated Show resolved Hide resolved
src/note.rs Show resolved Hide resolved
src/note.rs Show resolved Hide resolved
src/note.rs Outdated Show resolved Hide resolved
src/note.rs Outdated Show resolved Hide resolved
src/note_encryption.rs Outdated Show resolved Hide resolved
@nuttycom nuttycom force-pushed the add_rho_type branch 5 times, most recently from e0c9064 to 67f7e8f Compare March 10, 2024 19:45
@nuttycom nuttycom requested a review from str4d March 10, 2024 19:45
@nuttycom nuttycom force-pushed the add_rho_type branch 2 times, most recently from a7b1dc7 to 25fe0e4 Compare March 12, 2024 12:57
@nuttycom
Copy link
Contributor Author

force-pushed to rebase to main and address comments from code review.

@nuttycom nuttycom force-pushed the add_rho_type branch 2 times, most recently from 201b012 to b2efb4c Compare March 12, 2024 14:18
@nuttycom
Copy link
Contributor Author

This should probably be re-reviewed in its entirety at this point; by making it possible to create fake CompactAction values directly, I've (afaict) removed all the need to be able to independently evaluate the function Nullifier -> Rho.

This change removes the ability to construct a `Rho` value directly from
the public API, except via deserialization from bytes (which is
necessary in order to be able to serialize a `Note`). Ordinarily, `Rho`
should be obtained either from an already-constructed `Note` or from an
`Action` or `CompactAction`.
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 1a8ded0

@str4d str4d merged commit e74879d into zcash:main Mar 12, 2024
13 checks passed
@nuttycom nuttycom deleted the add_rho_type branch March 13, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants