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

Prepare for circom v2.2.0 upgrade #102

Merged
merged 7 commits into from
Dec 3, 2024
Merged

Prepare for circom v2.2.0 upgrade #102

merged 7 commits into from
Dec 3, 2024

Conversation

Chengxuan
Copy link
Contributor

@Chengxuan Chengxuan commented Nov 5, 2024

first part of #100

Signed-off-by: Chengxuan Xing <[email protected]>
Signed-off-by: Chengxuan Xing <[email protected]>
Signed-off-by: Chengxuan Xing <[email protected]>
Signed-off-by: Chengxuan Xing <[email protected]>
@Chengxuan Chengxuan marked this pull request as draft November 5, 2024 20:52
Signed-off-by: Chengxuan Xing <[email protected]>
@Chengxuan Chengxuan marked this pull request as ready for review November 29, 2024 07:14
@Chengxuan Chengxuan changed the title Update circom to v2.2.0 - part 1 Prepare for circom v2.2.0 upgrade Nov 29, 2024
Signed-off-by: Chengxuan Xing <[email protected]>
Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

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

Looks good - a few comments just around formatting

zkp/circuits/basetokens/anon_base.circom Show resolved Hide resolved
zkp/js/test/nf_anon_nullifier.js Show resolved Hide resolved
zkp/circuits/nf_anon.circom Show resolved Hide resolved
@Chengxuan
Copy link
Contributor Author

@EnriqueL8 Thanks for the comments. for Both the formatting and the reason for switching them to use anonymous component syntax are to prepare for organize those loose attributes into "buses" (a concept similar to structs in golang). The hope is those attributes will naturally form buses and we can do a search and replace for all the occurrence. <-- that's why I kept them in a single line.

Copy link
Contributor

@jimthematrix jimthematrix left a comment

Choose a reason for hiding this comment

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

LGTM. approved with a minor request

@@ -4,7 +4,7 @@ name: release
on:
push:
tags:
- 'v*.*.*'
- "v*.*.*"
Copy link
Contributor

Choose a reason for hiding this comment

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

please include vscode settings to make the formatting choice universally applied with any developer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vscode settings are currently ignored. will remove it from gitignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment addressed in 0ed5a04

Signed-off-by: Chengxuan Xing <[email protected]>
@Chengxuan Chengxuan merged commit 3557311 into main Dec 3, 2024
6 checks passed
@Chengxuan Chengxuan deleted the update-circom branch December 3, 2024 11:45
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.

3 participants