docs(wonderland): add all docs for circom onboarding and best practices#138
docs(wonderland): add all docs for circom onboarding and best practices#138
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
oxlumi
added
documentation
| - Provide a symbolic description of the circuit, and | ||
| - Provide an efficient way to compute the witness from the inputs (`wasm` or `C++`). | ||
|
|
||
| Programmers must explicitly add all constraints. Constraints can be simplified and signals removed at compilation time, but new signals are never introduced. |
There was a problem hiding this comment.
This second sentence I'm not understanding. Maybe it would be helpful to briefly define what constraints, signals, variables, etc. are in circom before this.
|
|
||
| ### Other Changes | ||
|
|
||
| - Signals and components must be defined at level 0 |
|
Note
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'docs(wonderland): add all docs for circom onboarding and best practices' accurately describes the primary change: adding comprehensive Circom documentation including onboarding guides and best practices to the Wonderland handbook. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
docs/circom-onboarding
Tip
Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
sites/wonderland/docs/development/circom/onboarding/circom-language.md (1)
204-212: Consider whether the gnark link belongs in a Circom-specific document.The Resources section includes a link to gnark documentation (a Go-based ZK framework). While it may be useful for broadening perspective, it could confuse readers expecting Circom-only resources. Consider moving it to a more general ZK resources page or adding a brief note explaining why it's listed here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sites/wonderland/docs/development/circom/onboarding/circom-language.md` around lines 204 - 212, The Resources section currently lists "gnark Documentation" which is a Go-based ZK framework and may confuse readers of this Circom-specific guide; either remove that gnark link from the Resources list in the "Resources" section of circom-language.md, or move it to a general ZK resources page and replace it here with a brief parenthetical note such as "(also see general ZK resources for other frameworks like gnark)" so readers know why it was listed; update the Resources bullet list (the "Resources" section and the "gnark Documentation" bullet) accordingly to reflect the chosen change.sites/wonderland/docs/development/circom/onboarding/zk-fundamentals.md (2)
98-107: Consider adding language specifier to code blocks.The R1CS constraint examples would benefit from language identifiers for better rendering.
📝 Suggested improvement
-``` +```text d = a x b (mod p) out = d + c (mod p)By substituting, we can combine them:
-
+text
out = a x b + c (mod p)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sites/wonderland/docs/development/circom/onboarding/zk-fundamentals.md` around lines 98 - 107, Add a language specifier to the fenced code blocks containing the R1CS examples (the blocks showing "d = a x b (mod p)", "out = d + c (mod p)" and "out = a x b + c (mod p)") so they render consistently (e.g., use ```text or ```math). Update both fenced blocks that show the intermediate constraints (d = a x b ... out = d + c ...) and the combined constraint (out = a x b + c ...) to include the same language identifier.
50-52: Consider adding language specifier to code block.The code block displaying the BN254 prime lacks a language identifier. While the content is correct, adding a language hint improves syntax highlighting and accessibility.
📝 Suggested improvement
-``` +```text p = 21888242871839275222246405745257275088548364400416034343698204186575808495617</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@sites/wonderland/docs/development/circom/onboarding/zk-fundamentals.md
around lines 50 - 52, Update the fenced code block that contains the BN254 prime
(the line starting with "p =
21888242871839275222246405745257275088548364400416034343698204186575808495617")
to include a language specifier (for example "text" or "none") after the opening
triple backticks so the block is syntax-highlighted/accessibility-friendly;
simply change the opening fence fromtotext (or another appropriate
language) and keep the content unchanged.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@sites/wonderland/docs/development/circom/coding-style.md:
- Around line 296-311: The "RIGHT" example uses GreaterThan(252) which
contradicts the doc guidance to match the value's actual range; update the Good
template to use a narrower bit-width that reflects x's expected range (e.g.,
change GreaterThan(252) to GreaterThan(128) or another appropriate bit size),
keeping the same structure (template Good, signal input x, component isPositive,
isPositive.in[0] <== x, isPositive.in[1] <== 0, isPositive.out === 1) so the
example aligns with the "match your value's actual range" recommendation.- Around line 16-24: The fenced code block that shows the project tree (starting
with the line "circuits/") in coding-style.md is missing a language identifier
which triggers MD040; update the opening triple-backtick to include a language
such as text (i.e. replacewithtext) so the block becomestext followed by the existing tree and closing, ensuring the project tree block
is explicitly marked as plain text.- Around line 85-90: The range check uses LessThan(128) but sets
comparator.in[1] to 2 ** 128 which is out of the allowed [0, 2^128-1] range;
update the comparison so the bound fits the bit-width by either changing the
bound to 2**128 - 1 or increasing the comparator bit-width to LessThan(129),
e.g. adjust the comparator declaration (LessThan) or the comparator.in[1] value
so comparator, comparator.in[0] (publicValue) and comparator.in[1] are all
within the allowed range.In
@sites/wonderland/docs/development/circom/onboarding/challenges.md:
- Around line 148-155: The component declaration uses an unnecessary explicit
public annotation for an output signal—remove the redundant "{public
[commitment]}" on the main component declaration (i.e., change "component main
{public [commitment]} = Commitment();" to simply "component main =
Commitment();") wherever it appears (references: component main, Commitment(),
output signal commitment), or alternatively add a one-line comment next to that
declaration clarifying that Circom 2.0 makes main outputs public automatically
so the explicit {public [...] } is optional; update all occurrences noted in the
review to keep examples consistent for learners.- Around line 272-305: The hint and challenge wording are misleading because
isAdult is already constrained to check.out via the line isAdult <== check.out
in the VulnerableAgeCheck template; fix by updating the challenge text and/or
circuit so the vulnerability matches the intent: either change the hint to say
the circuit fails to assert isAdult == 1 (so a prover can submit isAdult=0 and
still satisfy constraints), or make the circuit actually omit the constraint
(e.g., replace the assignment line or use a non-constraining assignment) so
check.out and isAdult are not bound; reference VulnerableAgeCheck, isAdult,
check.out, and GreaterEqThan when updating the description or code.In
@sites/wonderland/docs/development/circom/onboarding/getting-started.md:
- Around line 76-79: The README shows a directory name mismatch: after cloning
with "git clone https://github.com/wonderland-learning/onboarding-circom.git"
the directory created is onboarding-circom but the next command uses cd
circom-onboarding; update the second command to use the correct directory name
(cd onboarding-circom) so the two lines are consistent and the onboarding steps
work as intended.
Nitpick comments:
In@sites/wonderland/docs/development/circom/onboarding/circom-language.md:
- Around line 204-212: The Resources section currently lists "gnark
Documentation" which is a Go-based ZK framework and may confuse readers of this
Circom-specific guide; either remove that gnark link from the Resources list in
the "Resources" section of circom-language.md, or move it to a general ZK
resources page and replace it here with a brief parenthetical note such as
"(also see general ZK resources for other frameworks like gnark)" so readers
know why it was listed; update the Resources bullet list (the "Resources"
section and the "gnark Documentation" bullet) accordingly to reflect the chosen
change.In
@sites/wonderland/docs/development/circom/onboarding/zk-fundamentals.md:
- Around line 98-107: Add a language specifier to the fenced code blocks
containing the R1CS examples (the blocks showing "d = a x b (mod p)", "out = d
- c (mod p)" and "out = a x b + c (mod p)") so they render consistently (e.g.,
usetext ormath). Update both fenced blocks that show the intermediate
constraints (d = a x b ... out = d + c ...) and the combined constraint (out = a
x b + c ...) to include the same language identifier.
- Around line 50-52: Update the fenced code block that contains the BN254 prime
(the line starting with "p =
21888242871839275222246405745257275088548364400416034343698204186575808495617")
to include a language specifier (for example "text" or "none") after the opening
triple backticks so the block is syntax-highlighted/accessibility-friendly;
simply change the opening fence fromtotext (or another appropriate
language) and keep the content unchanged.</details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ``` | ||
| circuits/ | ||
| ├── templates/ # Reusable circuit templates | ||
| │ └── helpers/ # Small utility circuits | ||
| ├── main/ # Production instantiations | ||
| ├── test/ # Test instantiations (smaller params) | ||
| ├── circuits.json # Circuit configuration | ||
| └── docs/ # Technical documentation | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
The project structure code block lacks a language specifier, which triggers MD040. Use text or plaintext.
Proposed fix
-```
+```text
circuits/
├── templates/ # Reusable circuit templates📝 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.
| ``` | |
| circuits/ | |
| ├── templates/ # Reusable circuit templates | |
| │ └── helpers/ # Small utility circuits | |
| ├── main/ # Production instantiations | |
| ├── test/ # Test instantiations (smaller params) | |
| ├── circuits.json # Circuit configuration | |
| └── docs/ # Technical documentation | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sites/wonderland/docs/development/circom/coding-style.md` around lines 16 -
24, The fenced code block that shows the project tree (starting with the line
"circuits/") in coding-style.md is missing a language identifier which triggers
MD040; update the opening triple-backtick to include a language such as text
(i.e. replace ``` with ```text) so the block becomes ```text followed by the
existing tree and closing ```, ensuring the project tree block is explicitly
marked as plain text.
| // 2. Validate the value range | ||
| // Prevents overflow attacks on comparison | ||
| component comparator = LessThan(128); | ||
| comparator.in[0] <== publicValue; | ||
| comparator.in[1] <== 2 ** 128; | ||
| comparator.out === 1; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
circomlib LessThan component bit width documentation
💡 Result:
In circomlib, LessThan is parameterized by a bit-width n:
- Definition (source of truth):
template LessThan(n) { assert(n <= 252); ... component n2b = Num2Bits(n+1); n2b.in <== in[0] + (1<<n) - in[1]; out <== 1 - n2b.out[n]; }— i.e., it enforcesn <= 252and internally bit-decomposes an (n+1)-bit value viaNum2Bits(n+1). [1] - What
nmeans in practice:nshould match the maximum bit-length of the inputs you intend to compare (typically you also separately range-constrainin[0]andin[1]to be< 2^n), otherwise the comparator can be misused. [2] - Why the hard cap
n <= 252: circomlib enforces this to avoid issues (notably aliasing-style problems) that arise if the bit-decomposition size exceeds safe limits relative to the field prime. [1][3]
Sources:
[1] circomlib comparators.circom (raw) LessThan(n) implementation
[2] Trail of Bits analysis of circomlib LessThan behavior and the need to constrain inputs
[3] ZKSecurity note explaining the n <= 252 restriction and its motivation
🏁 Script executed:
cd sites/wonderland/docs/development/circom && head -100 coding-style.md | tail -30Repository: defi-wonderland/handbook
Length of output: 1056
Example range check uses incorrect bit-width for the comparison bound.
LessThan(128) expects both inputs to be within the range [0, 2^128 - 1]. Passing 2 ** 128 as in[1] exceeds this valid input range. Either use 2**128 - 1 as the bound, or use LessThan(129) to accommodate values up to 2^128.
Suggested fix
// 2. Validate the value range
// Prevents overflow attacks on comparison
- component comparator = LessThan(128);
+ component comparator = LessThan(129);
comparator.in[0] <== publicValue;
comparator.in[1] <== 2 ** 128;
comparator.out === 1;📝 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.
| // 2. Validate the value range | |
| // Prevents overflow attacks on comparison | |
| component comparator = LessThan(128); | |
| comparator.in[0] <== publicValue; | |
| comparator.in[1] <== 2 ** 128; | |
| comparator.out === 1; | |
| // 2. Validate the value range | |
| // Prevents overflow attacks on comparison | |
| component comparator = LessThan(129); | |
| comparator.in[0] <== publicValue; | |
| comparator.in[1] <== 2 ** 128; | |
| comparator.out === 1; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sites/wonderland/docs/development/circom/coding-style.md` around lines 85 -
90, The range check uses LessThan(128) but sets comparator.in[1] to 2 ** 128
which is out of the allowed [0, 2^128-1] range; update the comparison so the
bound fits the bit-width by either changing the bound to 2**128 - 1 or
increasing the comparator bit-width to LessThan(129), e.g. adjust the comparator
declaration (LessThan) or the comparator.in[1] value so comparator,
comparator.in[0] (publicValue) and comparator.in[1] are all within the allowed
range.
| ```circom | ||
| // WRONG: assert doesn't create constraints | ||
| template Bad() { | ||
| signal input x; | ||
| assert(x > 0); // Prover can ignore this! | ||
| } | ||
|
|
||
| // RIGHT: use actual constraints | ||
| template Good() { | ||
| signal input x; | ||
| component isPositive = GreaterThan(252); | ||
| isPositive.in[0] <== x; | ||
| isPositive.in[1] <== 0; | ||
| isPositive.out === 1; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Anti-pattern fix example uses GreaterThan(252) which contradicts earlier guidance.
The "RIGHT" example uses GreaterThan(252) just to verify x > 0, but the guide itself advises to "match your value's actual range" (Line 319). For a simple positivity check, a narrower bit width (e.g., 128 or whatever matches the expected range of x) would be more consistent with the document's own recommendations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sites/wonderland/docs/development/circom/coding-style.md` around lines 296 -
311, The "RIGHT" example uses GreaterThan(252) which contradicts the doc
guidance to match the value's actual range; update the Good template to use a
narrower bit-width that reflects x's expected range (e.g., change
GreaterThan(252) to GreaterThan(128) or another appropriate bit size), keeping
the same structure (template Good, signal input x, component isPositive,
isPositive.in[0] <== x, isPositive.in[1] <== 0, isPositive.out === 1) so the
example aligns with the "match your value's actual range" recommendation.
| signal input nullifier; | ||
| signal output commitment; | ||
|
|
||
| // TODO: commitment = Poseidon(secret, nullifier) | ||
| } | ||
|
|
||
| component main {public [commitment]} = Commitment(); | ||
| ``` |
There was a problem hiding this comment.
Redundant {public [commitment]} on output signal may confuse learners.
In Circom 2.0, all output signals of the main component are automatically public (as documented in your own circom-language.md, Line 176). Writing component main {public [commitment]} = Commitment() where commitment is an output signal is redundant and may teach learners that outputs need explicit public declarations. The same pattern appears on Line 294 and Line 448.
Consider either removing the redundant {public [...]} or adding a brief note explaining that it's technically unnecessary for output signals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sites/wonderland/docs/development/circom/onboarding/challenges.md` around
lines 148 - 155, The component declaration uses an unnecessary explicit public
annotation for an output signal—remove the redundant "{public [commitment]}" on
the main component declaration (i.e., change "component main {public
[commitment]} = Commitment();" to simply "component main = Commitment();")
wherever it appears (references: component main, Commitment(), output signal
commitment), or alternatively add a one-line comment next to that declaration
clarifying that Circom 2.0 makes main outputs public automatically so the
explicit {public [...] } is optional; update all occurrences noted in the review
to keep examples consistent for learners.
| ### Challenge 3.2: The Unconstrained Comparator | ||
|
|
||
| **Objective**: This age verification circuit is broken. Exploit it. | ||
|
|
||
| ```circom | ||
| // circuits/VulnerableAgeCheck.circom | ||
| pragma circom 2.1.0; | ||
|
|
||
| include "circomlib/circuits/comparators.circom"; | ||
|
|
||
| template VulnerableAgeCheck() { | ||
| signal input age; | ||
| signal output isAdult; | ||
|
|
||
| component check = GreaterEqThan(8); | ||
| check.in[0] <== age; | ||
| check.in[1] <== 18; | ||
|
|
||
| isAdult <== check.out; | ||
| // What's missing? | ||
| } | ||
|
|
||
| component main {public [isAdult]} = VulnerableAgeCheck(); | ||
| ``` | ||
|
|
||
| **Your tasks**: | ||
| 1. Prove you're "an adult" with `age=5` | ||
| 2. Explain why this works | ||
| 3. Fix the circuit | ||
|
|
||
| <details> | ||
| <summary>Hint</summary> | ||
| The comparator computes `isAdult`, but is there any constraint forcing `isAdult` to match the comparison result? | ||
| </details> |
There was a problem hiding this comment.
Challenge 3.2 hint may be misleading — <== already constrains the output.
On Line 290, isAdult <== check.out both assigns and constrains isAdult to equal the comparator's output. The hint on Line 304 asks "is there any constraint forcing isAdult to match the comparison result?" — but <== does exactly that. A prover with age=5 would get isAdult=0, not isAdult=1.
If the intended vulnerability is that the circuit doesn't enforce isAdult === 1 (so a valid proof can be generated with isAdult=0), the hint and framing should clarify that the issue is the missing assertion that the user is actually an adult, not a missing assignment constraint. Otherwise, consider redesigning this challenge (e.g., use <-- instead of <== to create an actual exploitable gap).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sites/wonderland/docs/development/circom/onboarding/challenges.md` around
lines 272 - 305, The hint and challenge wording are misleading because isAdult
is already constrained to check.out via the line isAdult <== check.out in the
VulnerableAgeCheck template; fix by updating the challenge text and/or circuit
so the vulnerability matches the intent: either change the hint to say the
circuit fails to assert isAdult == 1 (so a prover can submit isAdult=0 and still
satisfy constraints), or make the circuit actually omit the constraint (e.g.,
replace the assignment line or use a non-constraining assignment) so check.out
and isAdult are not bound; reference VulnerableAgeCheck, isAdult, check.out, and
GreaterEqThan when updating the description or code.
| ```bash | ||
| git clone https://github.com/wonderland-learning/onboarding-circom.git | ||
| cd circom-onboarding | ||
| ``` |
There was a problem hiding this comment.
Directory name mismatch after git clone.
The repo is onboarding-circom, so git clone creates a directory named onboarding-circom, not circom-onboarding.
Proposed fix
git clone https://github.com/wonderland-learning/onboarding-circom.git
-cd circom-onboarding
+cd onboarding-circom🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sites/wonderland/docs/development/circom/onboarding/getting-started.md`
around lines 76 - 79, The README shows a directory name mismatch: after cloning
with "git clone https://github.com/wonderland-learning/onboarding-circom.git"
the directory created is onboarding-circom but the next command uses cd
circom-onboarding; update the second command to use the correct directory name
(cd onboarding-circom) so the two lines are consistent and the onboarding steps
work as intended.
2 participants
Summary
What's Included
New files in
docs/development/circom/:_category_.jsoncoding-style.mdonboarding/getting-started.mdonboarding/zk-fundamentals.mdonboarding/circom-language.md<==,<--,===)onboarding/challenges.mdonboarding/tornado-cash.mdonboarding/best-practices.mdonboarding/testing-guidelines.mdonboarding/challenge-solutions.mdHighlights
Closes 0XB-496.
Summary by CodeRabbit