Skip to content

Conversation

@erhant
Copy link
Contributor

@erhant erhant commented Apr 3, 2025

Fix Groth16 Circom Adapter

Description

  1. This PR fixes bug: groth16 circom-adapter incorrect public signals #958 which is mainly due to the way public signals were handled in Circom adapter. It appears that we can keep the Circom ordering as-is, without adjusting LRO or witness signals which used to do:
  • Circom: ["1", ..outputs, ...inputs, ...other_signals]
  • Lambda: ["1", ...inputs, ..outputs, ...other_signals]
  1. We also change the way public signals are calculated, before it only cared about public inputs but now it counts for public inputs and public outputs as well.

  2. circom_to_lambda now returns the public signals as well, to not let the user shoot themselves in the foot while computing it as it is different than expected with the addition of constant term at the start. (e.g. if public signals are a, b you will actually get 1, a, b.

  3. We add a CircomR1CS type which helps with several things:

  • we use a well-defined type instead of generic Value with many as_TYPE().unwrap() as TYPEs around the place
  • we add deserialization for the FrElement within the object itself
  • we use BufReader with serde_json::from_reader instead of from_str for performance
  1. Removed some redundant clones and tidied up some parts of the code.

  2. READMEs and tutorial docs are updated and refurbished.

  3. Changed the folder structure to be more inline with Cargo recommendations, mainly that tests folder has the integration tests and one test file per test scenario. Guide docs are updated accordingly.

Type of change

  • New feature
  • Bug fix
  • Optimization

Checklist

  • Linked to Github Issue:
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run

erhant and others added 7 commits April 5, 2025 11:20
* rm all metal features

* fix clippy

* fix attribute

* fix another attribute
* update kzg Readme

* KZG: make sure example is ok

* Add Readme for Merkle Tree

* Add Readme for Circle

* fix circle docs

* fix circle readme

* fix typo, remove unused test and fix markdown style

* changes in circle readme

* fix circle readme

* fix clippy

* fix clippy spaces

---------

Co-authored-by: Nicole <nicole.graus@lambdaclass.com>
Co-authored-by: Joaquin Carletti <joaquin.carletti@lambdaclass.com>
Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com>
@erhant erhant force-pushed the erhant/circom-adapter-fixes branch from b96fa7e to 41a17b4 Compare April 5, 2025 08:22
@MauroToscano MauroToscano merged commit 7fe890e into lambdaclass:main Apr 7, 2025
7 of 8 checks passed
@irfanbozkurt
Copy link
Contributor

🥹

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.

bug: groth16 circom-adapter incorrect public signals

7 participants