-
Notifications
You must be signed in to change notification settings - Fork 59
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
Refactor R1CS struct to be generic on field #63
Conversation
src/backends/mod.rs
Outdated
pub enum BackendKind { | ||
KimchiVesta(KimchiVesta), | ||
R1CS(R1csBls12_381), | ||
R1csBls12_381(R1CS<R1csBls12381Field>), | ||
R1csBn128(R1CS<R1csBn128Field>), |
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.
should we call it bn254 everywhere? what's the most popular name :D?
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.
snarkjs calls it bn128 :)
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.
maybe better with bn254 as the google results seem to prefer this one
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.
btw we should update the README at some point ^^
constraints: Vec<Constraint>, | ||
witness_vars: Vec<Value<R1csBls12_381>>, | ||
constraints: Vec<Constraint<F>>, | ||
witness_vars: Vec<Value<Self>>, |
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 we need #38 as some point to make Value<F>
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.
maybe at the point when we are implementing the stdlib, so we can review the better way to handle Hint
.
src/cli/cmd_build_and_check.rs
Outdated
); | ||
// verify proof | ||
verifier_index.verify(full_public_inputs, proof)?; | ||
println!("proof verified"); |
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.
maybe keep it unimplemented!()
to not surprise anyone with a different behavior?
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.
That makes sense as this run command is expected to generate circuit and witness files for r1cs, but it currently does inline proving and verifying for kimchi without generating anything. We may revisit this when it is needed to serialize the kimchi circuit and witness.
I updated it to keep unimplemented!
for the kimchi backend for this command.
.public_outputs | ||
.iter() | ||
.map(|var| witness[var.index]) | ||
.collect(); |
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.
Ah yeah! Indeed that is practical
No description provided.