-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat(rln): add seeded keygen #56
Changes from all commits
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 |
---|---|---|
|
@@ -10,7 +10,8 @@ use ark_relations::r1cs::SynthesisError; | |
use ark_std::{rand::thread_rng, UniformRand}; | ||
use color_eyre::Result; | ||
use num_bigint::BigInt; | ||
use rand::Rng; | ||
use rand::{Rng, SeedableRng}; | ||
use rand_chacha::ChaCha20Rng; | ||
#[cfg(not(target_arch = "wasm32"))] | ||
use std::sync::Mutex; | ||
#[cfg(debug_assertions)] | ||
|
@@ -338,13 +339,31 @@ pub fn compute_tree_root( | |
|
||
// Generates a tupe (identity_secret, id_commitment) where | ||
// identity_secret is random and id_commitment = PoseidonHash(identity_secret) | ||
// RNG is instantiated using thread_rng() | ||
pub fn keygen() -> (Fr, Fr) { | ||
let mut rng = thread_rng(); | ||
let identity_secret = Fr::rand(&mut rng); | ||
let id_commitment = poseidon_hash(&[identity_secret]); | ||
(identity_secret, id_commitment) | ||
} | ||
|
||
// Generates a tupe (identity_secret, id_commitment) where | ||
// identity_secret is random and id_commitment = PoseidonHash(identity_secret) | ||
// RNG is instantiated using 20 rounds of ChaCha seeded with the hash of the input | ||
pub fn seeded_keygen(signal: &[u8]) -> (Fr, Fr) { | ||
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. this function can accept a 1 byte signal, which I assume, is not secure. I'd suggest to fix the signal size (and I am guessing that it should be 32 bytes). 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. of course it won't, although any input would be hashed to become a 32 bytes seed. If we allow users to choose the seed, we're delegating the entropy level and credential security. How does differ passing one byte from passing e.g. an all-0 32 bytes array? They're both unsecure regardless of the input size. 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. PS: can also accept empty signals :) 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. Fair point. |
||
// ChaCha20 requires a seed of exactly 32 bytes. | ||
// We first hash the input seed signal to a 32 bytes array and pass this as seed to ChaCha20 | ||
let mut seed = [0; 32]; | ||
let mut hasher = Keccak::v256(); | ||
hasher.update(signal); | ||
hasher.finalize(&mut seed); | ||
|
||
let mut rng = ChaCha20Rng::from_seed(seed); | ||
let identity_secret = Fr::rand(&mut rng); | ||
let id_commitment = poseidon_hash(&[identity_secret]); | ||
(identity_secret, id_commitment) | ||
} | ||
|
||
// Hashes arbitrary signal to the underlying prime field | ||
pub fn hash_to_field(signal: &[u8]) -> Fr { | ||
// We hash the input signal using Keccak256 | ||
|
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 am worried about the new dependency here. How does it affect the wasm blob size? I intended to add some size checking as part of the CI but it was not as straightforward as I hoped because cargo make is used instead of npm.
Cc @richard-ramos
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 guess this also depends on how much this library delegates to the web crypto subtle API
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.
Right now building this branch, i end up with 530KB which is way better than the 838KB of the version published https://unpkg.com/@waku/[email protected]/bundle/assets/rln_wasm_bg-0185b546.wasm
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.
What changed ???
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.
Probably the poseidon constants that now are generated on the fly while before were hardcoded (~300KB less)
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.
@fryorcraken if ok for you, I'll merge!