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

Cleanly separate Circom1 and Circom2 traits #60

Merged
merged 5 commits into from
Jul 4, 2024

Conversation

martyall
Copy link
Contributor

@martyall martyall commented Apr 16, 2024

I was working on upgrading the libraries here to use more recent versions of wasmer -- I have tests passing for the most recent versions 4.2.*. In the process I noticed that the separation between CircomBase Circom and Circom2 traits wasn't as clean as it could be, and in fact this was affecting the project I was working on. These should all be non-breaking changes:

  • Rename Circom trait to Circom1 for consistency and to make it easier to explain this work.
  • When creating a WitnessCalculator object, we don't use the SafeMemory object for the Circom2 trait methods and iiuc it won't even work in this case. This field should be optional and take a value of None for the circom2 case.
  • Move ptr based methods from CircomBase to Circom1, as they are not useful in the Circom2 case.
  • move shared functions get_version and get_u32 to CircomBase
  • remove the unused function get_witness_buffer

@martyall
Copy link
Contributor Author

martyall commented Jul 2, 2024

Hey @Pratyush, just wanted to check in on this, is there anything else I can do to help this get merged? I have some other improvements on a fork that I wanted to propose, but they are based on these changes. lmk, thanks :)

@Pratyush Pratyush merged commit 4d99060 into arkworks-rs:master Jul 4, 2024
2 checks 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.

2 participants