-
Notifications
You must be signed in to change notification settings - Fork 6
Circuit trait impl for BlindBid circuit #70
Conversation
This is needed because the `Circuit` trait was released on this version.
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.
Just a few small things. I didn't add a comment each time but its worth changing the misspelling of 'eligibility' or 'eligible', it's often written as variations of 'elegible'.
src/proof.rs
Outdated
&mut self, | ||
composer: &mut StandardComposer, | ||
) -> Result<Vec<PublicInput>, Error> { | ||
// Instanciate PI vector. |
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.
Nit. *instantiate
src/proof.rs
Outdated
bid.stealth_address.R().into(), | ||
), | ||
); | ||
let bid_elegibility_ts = |
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.
Nit. *eligibility.
pi.push(PublicInput::BlsScalar( | ||
-branch.root, | ||
composer.circuit_size(), | ||
)); |
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.
Is this supposed to be a double closure?
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.
WDYM by double closure? There's no closure in here. Is just pushing an enum to a vec.
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 format on my screen was squashed and messed up, so it placed the braces in the wrong place.
|
||
// 6. 0 < value <= 2^64 range check | ||
// v < 2^64 | ||
composer.range_gate(bid_value.var, 64usize); |
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.
Isn't the range function naturally taking in a usize? If so, is the input of it here redundant?
src/proof.rs
Outdated
let (ck, _) = pub_params.trim(1 << 16)?; | ||
// Generate & save `ProverKey` with some random values. | ||
let mut prover = Prover::new(b"TestCircuit"); | ||
// Set size & Pi builder |
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.
Nit. *PI
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.
LGTM, lets consider in the future splitting the fn gadget
into two phases
- Prepare the required data
- The circuit itself
Right now we have these two mixed into a single huge function.
Closes #65