-
Notifications
You must be signed in to change notification settings - Fork 343
Add interface and computation of Sapling note commitment #11
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
Changes from all commits
4ef2e9a
eb3d8aa
44e2a53
943df43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ use pairing::{BitIterator, Field, PrimeField, PrimeFieldRepr, bls12_381::{Bls12, | |
|
|
||
| use sapling_crypto::{circuit::multipack, constants::CRH_IVK_PERSONALIZATION, | ||
| jubjub::{edwards, FixedGenerators, JubjubBls12, JubjubEngine, JubjubParams, | ||
| PrimeOrder, ToUniform, Unknown, fs::FsRepr}, | ||
| PrimeOrder, ToUniform, Unknown, fs::{Fs, FsRepr}}, | ||
| pedersen_hash::{pedersen_hash, Personalization}, redjubjub::{self, Signature}}; | ||
|
|
||
| use sapling_crypto::circuit::sprout::{self, TREE_DEPTH as SPROUT_TREE_DEPTH}; | ||
|
|
@@ -25,7 +25,7 @@ use blake2_rfc::blake2s::Blake2s; | |
|
|
||
| use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; | ||
|
|
||
| use rand::OsRng; | ||
| use rand::{OsRng, Rng}; | ||
| use std::io::BufReader; | ||
|
|
||
| use libc::{c_char, c_uchar, size_t, int64_t, uint32_t, uint64_t}; | ||
|
|
@@ -301,6 +301,90 @@ pub extern "system" fn librustzcash_ivk_to_pkd( | |
| } | ||
| } | ||
|
|
||
| /// Test generation of commitment randomness | ||
| #[test] | ||
| fn test_gen_r() { | ||
| let mut r1 = [0u8; 32]; | ||
| let mut r2 = [0u8; 32]; | ||
|
|
||
| // Verify different r values are generated | ||
| librustzcash_sapling_generate_r(&mut r1); | ||
| librustzcash_sapling_generate_r(&mut r2); | ||
| assert_ne!(r1, r2); | ||
|
|
||
| // Verify r values are valid in the field | ||
| let mut repr = FsRepr::default(); | ||
| repr.read_le(&r1[..]).expect("length is not 32 bytes"); | ||
| let _ = Fs::from_repr(repr).unwrap(); | ||
| repr.read_le(&r2[..]).expect("length is not 32 bytes"); | ||
| let _ = Fs::from_repr(repr).unwrap(); | ||
| } | ||
|
|
||
| /// Return 32 byte randomness, uniform, to be used for a Sapling commitment. | ||
| #[no_mangle] | ||
| pub extern "system" fn librustzcash_sapling_generate_r(result: *mut [c_uchar; 32]) { | ||
| // create random 64 byte buffer | ||
| let mut rng = OsRng::new().expect("should be able to construct RNG"); | ||
| let mut buffer = [0u8; 64]; | ||
| for i in 0..buffer.len() { | ||
| buffer[i] = rng.gen(); | ||
| } | ||
|
|
||
| // reduce to uniform value | ||
| let r = <Bls12 as JubjubEngine>::Fs::to_uniform(&buffer[..]); | ||
| let result = unsafe { &mut *result }; | ||
| r.into_repr() | ||
| .write_le(&mut result[..]) | ||
| .expect("result must be 32 bytes"); | ||
| } | ||
|
|
||
| /// Compute Sapling note commitment. | ||
| #[no_mangle] | ||
| pub extern "system" fn librustzcash_sapling_compute_cm( | ||
| diversifier: *const [c_uchar; 11], | ||
| pk_d: *const [c_uchar; 32], | ||
| value: uint64_t, | ||
| r: *const [c_uchar; 32], | ||
| result: *mut [c_uchar; 32], | ||
| ) -> bool { | ||
| let diversifier = sapling_crypto::primitives::Diversifier(unsafe { *diversifier }); | ||
| let g_d = match diversifier.g_d::<Bls12>(&JUBJUB) { | ||
| Some(g_d) => g_d, | ||
| None => return false, | ||
| }; | ||
|
|
||
| let pk_d = match edwards::Point::<Bls12, Unknown>::read(&(unsafe { &*pk_d })[..], &JUBJUB) { | ||
| Ok(p) => p, | ||
| Err(_) => return false, | ||
| }; | ||
|
|
||
| let pk_d = match pk_d.as_prime_order(&JUBJUB) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spec doesn't require pkd to be prime order in keys obtained from another source (pkd in a note is declared as having type KASapling.Public which is J, not J(r)). I could make it require this outside the circuit, but note that the circuit doesn't check it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've looked closely at the changes that would be needed to the spec, and I'm convinced that the spec should be changed to require pkd (and gd, but that's already implied) to be of prime order outside the circuit. |
||
| Some(pk_d) => pk_d, | ||
| None => return false, | ||
| }; | ||
|
|
||
| // Deserialize randomness | ||
| let r = unsafe { *r }; | ||
| let mut repr = FsRepr::default(); | ||
| repr.read_le(&r[..]).expect("length is not 32 bytes"); | ||
| let r = match Fs::from_repr(repr) { | ||
| Ok(p) => p, | ||
| Err(_) => return false, | ||
| }; | ||
|
|
||
| let note = sapling_crypto::primitives::Note { | ||
| value, | ||
| g_d, | ||
| pk_d, | ||
| r, | ||
| }; | ||
|
|
||
| let result = unsafe { &mut *result }; | ||
| write_le(note.cm(&JUBJUB).into_repr(), &mut result[..]); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR uses both
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opened #16. |
||
|
|
||
| true | ||
| } | ||
|
|
||
| /// XOR two uint64_t values and return the result, used | ||
| /// as a temporary mechanism for introducing Rust into | ||
| /// Zcash. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might as well delete this now. (Does not block.) |
||
|
|
||
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.
This API mixes cryptographic code with an unsafe API. I suggest we have a safe API for the cryptography that is usable and testable from Rust, and then a thin wrapper around it for the C++ side. I also think this would be more thoroughly testable if we had the a Rust function with equivalent (safely typed) parameters that constructs and returns the full
sapling_crypto::primitives::Note(so that you can test that it has constructed the correct note), and then a wrapper around that to return 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.
Good idea for the whole library.
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.
Agreed, but not blocking.