microsoft / Spartan

Spartan: High-speed zkSNARKs without trusted setup
MIT License
672 stars 112 forks source link

Arbitrary number of variables and contraints #34

Closed elefthei closed 3 years ago

elefthei commented 3 years ago

This commit makes adding an arbitrary number of variables and constraints possible. This eliminates the assertions on number of constraints and number of variables imposed on user input.

  1. When creating a new R1CS Instance throught the public interface, it is required # constraints and # of vars be a power of 2. I remove that requirement by padding with dummy constraints and vars until the nearest power of 2.
  2. The sumcheck protocol in src/sumcheck.rs does not work for 1 constraint, even though 1 is a power of 2. I have to pad to a minimum of two constraints.
  3. Added a test in src/lib.rs called test_padded_constraints.
ghost commented 3 years ago

CLA assistant check
All CLA requirements met.

srinathsetty commented 3 years ago

@elefthei Thanks for the PR! I had two quick questions/suggestions:

(1) Should we place the new test in src/lib.rs instead of in src/r1csproof.rs? The test is checking something on the whole protocol, not just code covered by r1csproof.rs.

(2) For the code that adds padding, is it necessary to keep metadata about the number of padded variables? Wouldn't it be simpler if we have a method (or inline code) that takes an instance from the application and simply pads it without maintaining any additional metadata?

elefthei commented 3 years ago

Hey, happy to help!

(1) Sounds good, I committed the change.

(2) I need the metadata when calling prove I believe, as the user provides the vars_assignment unpadded. At that point, I need to know how many dummy variables were added in the R1CS so I can add a trivial assignment to them. Otherwise they would not have values, but maybe that's fine? I could be missing some workaround though in which case let me know and I'll change.

srinathsetty commented 3 years ago

Thanks, @elefthei!

(1) Great, thanks for the change!

(2) I see. I wonder if it makes sense to accept any-sized vars_assignment and then pad it with zeros when using. For example, the Instance object has an object of type R1CSInstance wrapped that internally tracks num_cons, num_vars, etc.

elefthei commented 3 years ago

@srinathsetty that worked great, I made the change.

srinathsetty commented 3 years ago

Thanks, @elefthei! I'll try to merge this shortly.