-
Notifications
You must be signed in to change notification settings - Fork 217
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
bulletproofs: add closure to allow increases to prover generator capacity #266
base: develop
Are you sure you want to change the base?
bulletproofs: add closure to allow increases to prover generator capacity #266
Conversation
|
||
// Construct verifier generators | ||
let pc_gens = PedersenGens::default(); | ||
let mut bp_gens = BulletproofGens::new(128, 1); |
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 there be an interface for the verifier to get the needed capacity for a proof (an added field in R1CSProof)?
benches/r1cs.rs
Outdated
@@ -156,7 +156,7 @@ impl KShuffleGadget { | |||
.unzip(); | |||
|
|||
Self::fill_cs(&mut prover, input_vars, output_vars)?; | |||
let proof = prover.prove(&bp_gens)?; | |||
let proof = prover.prove(bp_gens, |capacity, gens| gens.increase_capacity(capacity))?; |
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 instead of sending generators into prove
, and then getting them into closure, instead simply provide the reference from the closure (not sure if borrowck would love this):
let gens = BulletproofGens::new(128, 1);
...
let proof = prover.prove(|capacity| gens.increase_capacity(capacity); &gens )?;
?
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.
And the closure must return Result<&BulletproofGens, R1CSError>
because the prover may decide to return Err if capacity is too high instead of blinding increasing it.
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.
Having the closure provide the reference makes the borrow checker unhappy 😢 says it can't infer the appropriate lifetime?
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.
The prover can provide a closure that doesn't resize the generators (which would result in a failure from insufficient capacity). Do we want to also have the closure return Result<(), R1CSError>
? It feels maybe unnecessary to me since a no-op closure will have the same overall effect
Fixes #265