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

Circom 0.5 binary formats support #17

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jbaylina
Copy link

Added support for .r1cs file and .wtns file.
This are the default outputs in circom 0.5
Those files are in binary and allow to work wit bigger circuits.

Copy link
Owner

@kobigurk kobigurk left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Owner

@kobigurk kobigurk left a comment

Choose a reason for hiding this comment

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

@jbaylina can you run this through cargo fmt?

q[i] = reader.read_u64::<LittleEndian>()?;
}

if q != [
Copy link
Owner

Choose a reason for hiding this comment

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

What is this constant?

Copy link
Author

Choose a reason for hiding this comment

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

This is the prime field of Fr in the bn254.
This field is in the format because the format accepts any curv.

Copy link
Owner

Choose a reason for hiding this comment

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

Nice, thanks. Can you add a comment about it?




fn circuit_from_r1cs_read_header<E: Engine, R:Read>(circuit : &mut CircomCircuit<E>, reader: &mut R) -> std::io::Result<()> {
Copy link
Owner

Choose a reason for hiding this comment

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

Can we add a link to the format?

Copy link
Author

Choose a reason for hiding this comment

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

Added.

@jbaylina
Copy link
Author

I see that cargo fmt affect many parts of the project. Am I doing some thing wrong? If this affects all the project then it's better to run it a project level?

@kobigurk
Copy link
Owner

I see that cargo fmt affect many parts of the project. Am I doing some thing wrong? If this affects all the project then it's better to run it a project level?

OK, my bad. I'll take care of that later :) Thanks.

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.

None yet

2 participants