Skip to content
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

LessThan circuits and padding #2

Merged
merged 8 commits into from
Mar 18, 2024
Merged

LessThan circuits and padding #2

merged 8 commits into from
Mar 18, 2024

Conversation

Antonio95
Copy link

This PR adds two example circuits to verify a < b, where a is a private witness held by the prover and b is a public constant fixed at set-up. The two circuits rely on a fixed bit length that a is supposed to fit in (when regarded as an unsigned integer, even though in reality it lies in some finite field).

  • The circuit LessThanCircuitSafe performs the aforementioned bit length verification.
  • The circuit LessThanCircuitUnsafe does not. This leads to constraint savings when the circuit is used as a component in a larger circuit which already ensures the bit length meets the required bound.

In the course of writing the examples, we encountered the issue that the existing Spartan machinery did not pad the matrices to power-of-two row and column lengths, as necessitated by the proof system. This caused compilation errors in our examples. Since it seemed like a good idea for this need for padding to be abstracted away from the user, we have implemented padding in the Spartan internals - that is the other significant contribution in this PR. Note that a R1CSShape method .pad() already existed here, but after some experimentation we convinced ourselves that it could not be used to pad the circuit in the cases where it was necessary.

Other minor changes:

  • The aforementioned circuits are accompanied by meaningful tests (in the example code) that try edge cases and also showcase the different execution outcomes of the safe and unsafe variants for some inputs.
  • We have removed the <F> generic on the existing CubicCircuit example, which in turn allowed us to remove the PhantomData therein and didn't require any further code modifications.
  • A section on how to run Rust examples (which the new circuits are) has been added to the README.

Cesar Descalzo Blanco and others added 7 commits March 14, 2024 15:13
Remove unnecessary `PhantomData`
Co-authored-by: Antonio Mejías Gil <anmegi.95@gmail.com>

Progress until 08/03
Co-authored-by: Antonio Mejías Gil <anmegi.95@gmail.com>

constructing range check, wip

working on padding

println nightmare, issue apparently fixed

Remove redundant padding. Fix import.

code cleanup

further cleanup

found better way to fix padding

the 'better way' to fix padding actually broke other tests - reverted

This reverts commit d4727ba.

moved example to examples folder, refactored, tested many cases
Copy link

@mmagician mmagician left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, shouldn't there be an analogous modification for the verifier?

src/lib.rs Show resolved Hide resolved
src/spartan/snark.rs Show resolved Hide resolved
src/spartan/snark.rs Show resolved Hide resolved
@Antonio95
Copy link
Author

Also, shouldn't there be an analogous modification for the verifier?

RIght now, there are two padding-related modifications:

  • One on setup which adds constraint and variable padding: this method affects both the prover and the verifier.
  • One on prove which padds the (1 || witness || instance) vector (cf. above).

The verfier doesn't receive the latter vector, and their circuit is padded during setup. So, as far as I can tell, nothing is needed on the verifier side (cf the verification code here). The tests more or less bear this out, since they broke with either of the aforementioned modifications and they work with the current code.

Copy link

@mmagician mmagician left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, PR away to upstream!

@mmagician mmagician merged commit f991d55 into main Mar 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants