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

Incorrect Handling of Constraints in Gates With Unused Slots #1240

Open
nicholas-mainardi opened this issue Sep 19, 2023 · 2 comments
Open
Labels
bug Something isn't working

Comments

@nicholas-mainardi
Copy link
Contributor

It seems to me that the CircuitBuilder might incorrectly handles the generators for unused slots of gates that can have multiple instances on the same row. I tried to reproduce the issue with a simple gate that enforces that the input wire is different from 0, which can be found it here. Since the gate requires only 2 wires, for efficiency the gate packs multiple instances on the same row.

As you can see from the last test in the lib.rs file, the error in the verifier is triggered when some of the instances of the gate are unused. I think that the source of the error relates to how the the CircuitBuilder handles the generators of unused slots. In particular, when adding generators during the build of the circuit, the CircuitBuilder removes the generators for unused slots in each gate, I suppose because these generators might not properly work if the input wires are never set with a value by the witness generator (since they are not connected to any other wire). Nonetheless, the verifier will enforce the constraints of the gate also on these slots, but since the generators have not been run, then the constraint verification might fail.
Nonetheless, this issue does not seem to occur on existing Plonky2 gates: my guess is that by not running the generator, the wires of unused slots will all be set to 0, and it seems to me that all the constraints employed in existing Plonky2 gates that employ more than 1 slots (e.g., ArithmeticGate or RandomAccessGate) might be trivially verified when all the wires are set to 0. However, this is not true for a general gate: for instance, in the NeZeroGate I implemented in the reproducing example, the generator sets an auxiliary wire x to the inverse of the input wire in order to satisfy the constraint that input*x=1; this constraint cannot be satisfied if the generator is not run and thus x=0.

@wborgeaud
Copy link
Contributor

That's a good observation. Indeed, this issue hasn't occurred with existing gates because the ones that support multiple operations are satisfied with an all zeros input.

We could fix this by adding a function fn default_values() -> Vec<F> to trait Gate and use generators to set the targets in the unused operations to these default values. So for example in your NeZeroGate it could return (F::ONE, F::ONE).

@nicholas-mainardi
Copy link
Contributor Author

Thanks for confirming the issue, it definitely seems a good way to fix it

@Nashtare Nashtare added the bug Something isn't working label Oct 23, 2023
@Nashtare Nashtare added this to the Cleanups and Misc. milestone Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants