Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a TypeScript build CLI for Circom circuits, replaces legacy npm build scripts with TS entrypoints, extends dev-setup to support per-circuit key generation (entropy/beacon/name options), and updates README with expanded build, keygen, test, and container-image instructions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Builder as build-all.ts
participant FS as Filesystem
participant Circom
User->>Builder: yarn build:all [--circuit names] [--list]
alt --list
Builder->>FS: scan src for .circom
FS-->>Builder: return list
Builder->>User: print circuits & exit
else
Builder->>FS: scan src for .circom
FS-->>Builder: circuit list
Builder->>Builder: resolve targets (--circuit/--only)
loop per circuit
Builder->>Circom: circom -r1cs -wasm -sym -c -l <lib> -o <out>
Circom-->>FS: write r1cs/wasm/sym
FS-->>Builder: build complete
Builder->>User: log progress
end
Builder->>User: complete (or exit 1 on error)
end
sequenceDiagram
participant User
participant Dev as dev-setup.ts
participant FS as Filesystem
participant Phase1
participant Tools as CircomTools
User->>Dev: yarn dev-setup [-c circuit1,circuit2] [--beacon/--entropy/--name]
alt --circuit provided
loop per circuit
Dev->>FS: check `<circuit>.r1cs`
alt missing
Dev->>User: error hint & exit
else
Dev->>Phase1: fetch/cache phase1.ptau
Phase1-->>FS: phase1 saved
Dev->>Tools: zkey contribute / apply beacon
Tools-->>FS: zkey created
Dev->>Tools: export vkey
Tools-->>FS: vkey created
Dev->>Tools: generate verifier
Tools-->>FS: verifier created
Dev->>User: log success for circuit
end
end
Dev->>User: exit
else
Dev->>FS: run existing non-circuit flow
Dev->>User: complete
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/circuits/package.json (1)
18-18: Add engines field to packages/circuits/package.json to enforce Node.js v18+ requirement.The workspace package depends on Commander 12.1.0, which requires Node.js v18 or newer. However,
packages/circuits/package.jsondoes not declare this requirement in anenginesfield. Add"engines": {"node":">=18"}to the package.json to prevent incompatible Node.js versions from being used.
🧹 Nitpick comments (3)
packages/circuits/scripts/dev-setup.ts (1)
160-199: Consider refactoring to reduce code duplication.The
generateForCircuithelper (lines 168-192) duplicates much of the logic from the main flow (lines 201-237). While the current implementation is correct, consolidating the shared logic would improve maintainability.Consider extracting the common pattern into a shared helper:
+const getCircuitPaths = (baseName: string, buildDir: string, isLegacy: boolean) => ({ + phase1Path: path.join(buildDir, isLegacy ? "powersOfTau28_hez_final_22.ptau" : "powersOfTau28_hez_final_23.ptau"), + r1csPath: path.join(buildDir, `${baseName}.r1cs`), + zkeyPath: path.join(buildDir, `${baseName}.zkey`), + vkeyPath: path.join(buildDir, `${baseName}.vkey`), + verifierPath: path.join(buildDir, isLegacy ? `Groth16LegacyVerifier_${baseName}.sol` : `Groth16Verifier_${baseName}.sol`) +}); + const generateForCircuit = async (baseName: string) => { - const phase1Path = path.join( - buildDir, - args.legacy - ? "powersOfTau28_hez_final_22.ptau" - : "powersOfTau28_hez_final_23.ptau" - ); + const paths = getCircuitPaths(baseName, buildDir, args.legacy); - await downloadPhase1(phase1Path); + await downloadPhase1(paths.phase1Path); - log("✓ Phase 1:", phase1Path); + log("✓ Phase 1:", paths.phase1Path); - - const r1csPath = path.join(buildDir, `${baseName}.r1cs`); - if (!fs.existsSync(r1csPath)) { - throw new Error(`${r1csPath} does not exist.`); + if (!fs.existsSync(paths.r1csPath)) { + throw new Error(`${paths.r1csPath} does not exist.`); } - const zkeyPath = path.join(buildDir, `${baseName}.zkey`); - const vkeyPath = path.join(buildDir, `${baseName}.vkey`); - const verifierPath = path.join( - buildDir, - args.legacy - ? `Groth16LegacyVerifier_${baseName}.sol` - : `Groth16Verifier_${baseName}.sol` - ); - await generateKeys(phase1Path, r1csPath, zkeyPath, vkeyPath, verifierPath); + await generateKeys(paths.phase1Path, paths.r1csPath, paths.zkeyPath, paths.vkeyPath, paths.verifierPath); log(`✓ Keys generated for ${baseName}`); };Then reuse the same helper in the main flow (lines 201-237).
packages/circuits/scripts/build-all.ts (1)
118-124: Consider clarifying the dual flag support for circuit selection.Lines 119-120 accept both
--circuitand--onlyflags for circuit selection. This dual support may cause confusion. If--onlyis intended as an alias or for backward compatibility, consider documenting this or standardizing on a single flag.If
--onlyis not needed, simplify to:const opts: BuildOptions = { outDir: flags.outDir, libDir: flags.libDir, sequential: Boolean(flags.sequential), only: - (flags.circuit ?? flags.only) - ? String(flags.circuit ?? flags.only) + flags.circuit + ? String(flags.circuit) .split(",") .map((s: string) => s.trim()) .filter(Boolean) : undefined, };Otherwise, document the alias in the option description.
packages/circuits/README.md (1)
32-37: Consider varying sentence structure for improved readability.Static analysis flags the repetitive "a starting position" beginnings in lines 32-36. While the technical content justifies the similarity, varying the structure could enhance readability.
Consider rephrasing for variety:
-1. a starting position of the From field in the email header `from_addr_idx`. -1. a starting position of the Subject field in the email header `subject_idx`. -1. a starting position of the email domain in the email address of the From field `domain_idx`. -1. a starting position of the timestamp in the email header `timestamp_idx`. -1. a starting position of the invitation code in the email header `code_idx`. +1. the starting index of the From field in the email header `from_addr_idx`. +1. the starting index of the Subject field in the email header `subject_idx`. +1. the starting index of the email domain in the From field address `domain_idx`. +1. the starting index of the timestamp in the email header `timestamp_idx`. +1. the starting index of the invitation code in the email header `code_idx`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/circuits/README.md(3 hunks)packages/circuits/package.json(1 hunks)packages/circuits/scripts/build-all.ts(1 hunks)packages/circuits/scripts/dev-setup.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/circuits/scripts/build-all.ts (4)
packages/circuits/tests/email_auth_no_timestamp.test.ts (2)
circuit(20-25)circuit(18-262)packages/circuits/tests/email_auth_reveal.test.ts (1)
circuit(20-25)packages/circuits/tests/email_auth_legacy.test.ts (2)
option(12-21)circuit(10-630)packages/circuits/scripts/gen_input.ts (1)
generate(41-91)
packages/circuits/scripts/dev-setup.ts (2)
packages/circuits/scripts/gen_input.ts (1)
generate(41-91)packages/circuits/helpers/email_auth.ts (1)
genEmailCircuitInput(5-39)
🪛 LanguageTool
packages/circuits/README.md
[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d in the email header subject_idx. 1. a starting position of the email domain i...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~35-~35: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ress of the From field domain_idx. 1. a starting position of the timestamp in t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in the email header timestamp_idx. 1. a starting position of the invitation cod...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~43-~43: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...fier of the email email_nullifier. 4. a timestamp in the email header `timestam...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...amp in the email header timestamp. 5. a masked subject where characters either ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~110-~110: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...dded_header[code_idx:code_idx+64]. 12. Let embedded_codebe an integer parsing...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~115-~115: Ensure spelling is correct
Context: ...hat the email address in the subject is assumbed not to overlap with the invitation code...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
packages/circuits/README.md
84-84: Bare URL used
(MD034, no-bare-urls)
117-117: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (11)
packages/circuits/package.json (1)
6-8: LGTM! Build scripts properly configured.The unified TypeScript-based build workflow is well-structured:
buildandbuild:allprovide consistent aliasing for the full build- Memory allocation (16GB) is appropriate for Circom circuit compilation
build:listcorrectly omits the memory override since listing circuits is lightweightpackages/circuits/scripts/dev-setup.ts (2)
24-28: LGTM! CLI options properly defined.The new
--circuitoption enables flexible per-circuit key generation, aligning well with the build-all.ts pattern. The description clearly indicates that circuit names should be provided without the.circomextension.
161-166: LGTM! Consistent parsing pattern.The comma-separated circuit parsing logic matches the pattern used in
build-all.ts, ensuring consistency across the build tooling.packages/circuits/scripts/build-all.ts (5)
1-12: LGTM! Clean imports and type definition.The imports are appropriate for the build orchestration task, and the
BuildOptionstype clearly defines the script's configuration surface.
14-25: LGTM! Secure command execution.The
run()helper correctly usesshell: falseto prevent shell injection vulnerabilities andstdio: "inherit"to provide real-time feedback during compilation.
27-36: LGTM! Circuit discovery logic is sound.The filtering logic correctly identifies buildable circuits while excluding templates by convention. The comment clearly explains the intent.
38-93: LGTM! Build orchestration is well-structured.The build logic is robust with:
- Clear circuit selection and filtering
- Helpful error messages with available circuit hints
- Correct Circom compilation flags (
--r1cs,--wasm,--sym,--c)- Flexible sequential/parallel execution modes
The implementation assumes
circomis available in PATH, which is reasonable for a circuits package.
129-148: LGTM! Main execution flow is clean and user-friendly.The
--listfunctionality provides a helpful way to discover available circuits, and error handling correctly exits with a non-zero code for CI/CD integration.packages/circuits/README.md (3)
50-70: LGTM! Comprehensive build documentation.The new build examples clearly demonstrate all available options:
- Building all circuits
- Selective circuit builds
- Sequential builds for memory management
- Circuit discovery via
--listThis provides excellent guidance for users.
71-79: LGTM! Clear key generation examples.The examples demonstrate per-circuit key generation with both standard and legacy modes, aligning well with the
dev-setup.tsCLI options.
127-163: LGTM! Comprehensive container build documentation.The step-by-step Docker Buildx instructions are clear and actionable, providing excellent guidance for setting up the CI/CD testing environment.
| ### Run tests | ||
|
|
||
| At `packages/circuits`, make a `build` directory, download the zip file from the following link, and place its unzipped directory under `build`. | ||
| https://drive.google.com/file/d/1TChinAnHr9eV8H_OV9SVReF8Rvu6h1XH/view?usp=sharing |
There was a problem hiding this comment.
Fix bare URL in documentation.
Line 84 contains a bare URL that should be formatted as a proper Markdown link for better readability and to satisfy linting rules.
Apply this diff:
-At `packages/circuits`, make a `build` directory, download the zip file from the following link, and place its unzipped directory under `build`.
-https://drive.google.com/file/d/1TChinAnHr9eV8H_OV9SVReF8Rvu6h1XH/view?usp=sharing
+At `packages/circuits`, make a `build` directory, download the zip file from [this Google Drive link](https://drive.google.com/file/d/1TChinAnHr9eV8H_OV9SVReF8Rvu6h1XH/view?usp=sharing), and place its unzipped directory under `build`.Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
84-84: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In packages/circuits/README.md around line 84, replace the bare URL with a
Markdown-formatted link (e.g., [Design
diagram](https://drive.google.com/file/d/1TChinAnHr9eV8H_OV9SVReF8Rvu6h1XH/view?usp=sharing))
so the link has descriptive anchor text and passes linting; update the line to
use the chosen bracketed text followed by the URL in parentheses.
| 14. Let `account_salt` be `PoseidonHash(from_addr|0..0, account_code, 0)`. | ||
| 15. Let `masked_subject` be a string that removes the invitation code with the prefix `code_str` and one email address from `subject`, if they appear in `subject`. | ||
|
|
||
| Note that the email address in the subject is assumbed not to overlap with the invitation code. |
There was a problem hiding this comment.
Fix spelling error.
Line 115 contains a typo: "assumbed" should be "assumed".
Apply this diff:
-Note that the email address in the subject is assumbed not to overlap with the invitation code.
+Note that the email address in the subject is assumed not to overlap with the invitation code.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Note that the email address in the subject is assumbed not to overlap with the invitation code. | |
| Note that the email address in the subject is assumed not to overlap with the invitation code. |
🧰 Tools
🪛 LanguageTool
[grammar] ~115-~115: Ensure spelling is correct
Context: ...hat the email address in the subject is assumbed not to overlap with the invitation code...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In packages/circuits/README.md around line 115, fix the spelling typo "assumbed"
to "assumed" in the sentence "Note that the email address in the subject is
assumbed not to overlap with the invitation code." — update the word to
"assumed" so the line reads "Note that the email address in the subject is
assumed not to overlap with the invitation code."
| Note that the email address in the subject is assumbed not to overlap with the invitation code. | ||
|
|
||
|
|
||
| #### `email_auth_with_body_parsing_with_qp_encoding.circom` |
There was a problem hiding this comment.
Fix heading level increment.
Line 117 jumps from h2 (##) to h4 (####), which violates heading hierarchy. It should be h3 (###).
Apply this diff:
-#### `email_auth_with_body_parsing_with_qp_encoding.circom`
+### `email_auth_with_body_parsing_with_qp_encoding.circom`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### `email_auth_with_body_parsing_with_qp_encoding.circom` | |
| ### `email_auth_with_body_parsing_with_qp_encoding.circom` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
117-117: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 Prompt for AI Agents
In packages/circuits/README.md around line 117, the heading level jumps from h2
(##) to h4 (####); change the heading to h3 (###) so the document's heading
hierarchy is consistent (replace "####
`email_auth_with_body_parsing_with_qp_encoding.circom`" with "###
`email_auth_with_body_parsing_with_qp_encoding.circom`").
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/circuits/README.md (3)
81-83: Fix bare URL in documentation.The bare URL should be formatted as a proper Markdown link for readability and linting compliance.
-At `packages/circuits`, make a `build` directory, download the zip file from the following link, and place its unzipped directory under `build`. -https://drive.google.com/file/d/1TChinAnHr9eV8H_OV9SVReF8Rvu6h1XH/view?usp=sharing +At `packages/circuits`, make a `build` directory, download the zip file from [this Google Drive link](https://drive.google.com/file/d/1TChinAnHr9eV8H_OV9SVReF8Rvu6h1XH/view?usp=sharing), and place its unzipped directory under `build`.
113-113: Fix spelling error."assumbed" should be "assumed".
-Note that the email address in the subject is assumbed not to overlap with the invitation code. +Note that the email address in the subject is assumed not to overlap with the invitation code.
115-115: Fix heading level increment.The heading level jumps from h2 to h4, violating heading hierarchy. It should be h3.
-#### `email_auth_with_body_parsing_with_qp_encoding.circom` +### `email_auth_with_body_parsing_with_qp_encoding.circom`
🧹 Nitpick comments (1)
packages/circuits/README.md (1)
30-34: Consider varying phrasing in the input list.The items 6–10 all begin with "a starting position of" which creates repetitive structure. While acceptable for documentation, you could improve readability by consolidating or varying the phrasing:
-6. a starting position of the From field in the email header `from_addr_idx`. -7. a starting position of the Subject field in the email header `subject_idx`. -8. a starting position of the email domain in the email address of the From field `domain_idx`. -9. a starting position of the timestamp in the email header `timestamp_idx`. -10. a starting position of the invitation code in the email header `code_idx`. +6. the index of the From field in the email header (`from_addr_idx`). +7. the index of the Subject field in the email header (`subject_idx`). +8. the index of the email domain in the email address (`domain_idx`). +9. the index of the timestamp in the email header (`timestamp_idx`). +10. the index of the invitation code in the email header (`code_idx`).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/circuits/README.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/circuits/README.md
[style] ~33-~33: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ress of the From field domain_idx. 9. a starting position of the timestamp in t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n the email header timestamp_idx. 10. a starting position of the invitation cod...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~41-~41: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...fier of the email email_nullifier. 4. a timestamp in the email header `timestam...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~42-~42: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...amp in the email header timestamp. 5. a masked subject where characters either ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~108-~108: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...dded_header[code_idx:code_idx+64]. 12. Let embedded_codebe an integer parsing...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~113-~113: Ensure spelling is correct
Context: ...hat the email address in the subject is assumbed not to overlap with the invitation code...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
packages/circuits/README.md
82-82: Bare URL used
(MD034, no-bare-urls)
115-115: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: relayer
- GitHub Check: circuits
🔇 Additional comments (4)
packages/circuits/README.md (4)
50-77: Build commands section is clear and comprehensive.The new build instruction examples are well-structured, showing multiple approaches (all circuits, specific circuits, sequential mode) with proper memory flags and helpful comments. The commands are easy to follow and appropriate for the new orchestrator workflow.
69-77: Keygen instructions align well with new per-circuit workflow.The per-circuit key generation examples are clear and cover both modern and legacy modes, providing users with practical guidance for the new orchestration approach.
124-161: Container image build section is well-documented.The new section provides clear, step-by-step instructions for building and pushing the container image with proper multi-platform support. The organization and detail level are appropriate for users managing CI/CD infrastructure.
93-123: Specification section is comprehensive and well-structured.The detailed specification steps accurately document the circuit constraints and output computation. The comparison with
email_auth_with_body_parsing_with_qp_encoding.circomclearly highlights the differences in inputs and outputs. The mathematical/formal specification style is appropriate for this documentation.
2 participants
Summary by CodeRabbit
Documentation
New Features
✏️ Tip: You can customize this high-level summary in your review settings.