Skip to content

Circom 0.5 binary formats support#17

Open
jbaylina wants to merge 4 commits into
kobigurk:masterfrom
jbaylina:circom_0_5
Open

Circom 0.5 binary formats support#17
jbaylina wants to merge 4 commits into
kobigurk:masterfrom
jbaylina:circom_0_5

Conversation

@jbaylina
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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?

Comment thread phase2/src/circom_circuit.rs Outdated
q[i] = reader.read_u64::<LittleEndian>()?;
}

if q != [
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants