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

simplify post compile phase: shifting wire id is not needed 🙈 #280

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Mar 9, 2022

🙈 --> we used to do a weird dance in a post-compile phase, where we would loop through all constraints, logs, debuginfo, hints, and offset the ids of the wires. This was unnecessary complicated; new version just count the number of public and secret variables before calling Define, so, the r1cs builder and plonk builder both know the offset to add to new internal wires... voila.

  • feat: remove post-compile offset id in R1CS builder
  • feat: remove offset shifts in plonk compile

@gbotrel gbotrel added consolidate strengthen an existing feature cleanup labels Mar 9, 2022
@gbotrel gbotrel added this to the v0.7.0 milestone Mar 9, 2022
@gbotrel gbotrel requested a review from ThomasPiellard March 9, 2022 21:41
@gbotrel gbotrel changed the title simplify post compile phase: shifting wire id is not needed simplify post compile phase: shifting wire id is not needed 🙈 Mar 9, 2022
Copy link
Collaborator

@ThomasPiellard ThomasPiellard left a comment

Choose a reason for hiding this comment

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

Indeed it's simpler this way :) Is the Visibility field in Term still needed at all at this point? It was used when the public and secret variables were not a slice of []string, but a sort of wrapper around a variable...

@gbotrel gbotrel merged commit e791d2e into develop Mar 10, 2022
@gbotrel gbotrel deleted the simplify-r1cs-compile branch March 10, 2022 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup consolidate strengthen an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants