-
Notifications
You must be signed in to change notification settings - Fork 0
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
Constant-time note commitment for ZEC and ZSA #54
Conversation
f3fd3f6
to
2f9de93
Compare
src/note/commitment.rs
Outdated
let type_bits = BitArray::<_, Lsb0>::new(asset.to_bytes()); | ||
let zsa_note_bits = iter::empty() | ||
.chain(g_d_bits.iter().by_vals()) | ||
.chain(pk_d_bits.iter().by_vals()) | ||
.chain(v_bits.iter().by_vals()) | ||
.chain(rho_bits.iter().by_vals().take(L_ORCHARD_BASE)) | ||
.chain(psi_bits.iter().by_vals().take(L_ORCHARD_BASE)) | ||
.chain(type_bits.iter().by_vals()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is no need to compute the mutual part twice. Somthing like this should work:
let common_note_bits = iter::empty()
.chain(g_d_bits.iter().by_vals())
.chain(pk_d_bits.iter().by_vals())
.chain(v_bits.iter().by_vals())
.chain(rho_bits.iter().by_vals().take(L_ORCHARD_BASE))
.chain(psi_bits.iter().by_vals().take(L_ORCHARD_BASE));
let zec_note_bits = common_note_bits.cloned();
let type_bits = BitArray::<_, Lsb0>::new(asset.to_bytes());
let zsa_note_bits =
common_note_bits
.chain(type_bits.iter().by_vals());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some difficulties to clone the Chain.
I could convert the chain into a vector, then clone it and then convert it back into an iterator... (see my last commit)
We could discuss about it Monday morning.
Also blocked by QED-it/zcash-test-vectors#8 (review) |
d84f909
to
e35aca4
Compare
.chain(type_bits.iter().by_vals()); | ||
|
||
let zec_domain = sinsemilla::CommitDomain::new(NOTE_COMMITMENT_PERSONALIZATION); | ||
let zsa_domain = sinsemilla::CommitDomain::new_with_personalization( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should rename it to sinsemilla::CommitDomain::new_with_blind_personalization()
. Either now or remember to do it after ECC review for our PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I rename it, I have also to rename it in qedit/halo2 but the PR in qedit/halo2 has already been merged on zsa1.
I prefer to rename it after ECC review for our PR.
src/note/commitment.rs
Outdated
use rand::{rngs::OsRng, Rng}; | ||
|
||
#[test] | ||
fn test_note_commit() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As previously mentioned, this does not test derive()
. This seems to be a lower-level test, am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is a lower level test. It just tests that splitting the commitment evaluation into hashing and blinding gives the same result than directly evaluating the commitment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed this test
We would like to have a constant-time evaluation of the note commitment for both ZEC and ZSA.
ZEC_note_commitment=Extract_P(SinsemillaHashToPoint(zec_personalization, common_bits) + [rcm]R)
ZSA_note_commitment=Extract_P(SinsemillaHashToPoint(zsa_personalization, common_bits || asset) + [rcm]R)
R is the same constant for ZEC and ZSA note commitments.