Repository navigation
test(zk): derive the Noir pin in the fresh config compile test - #2350
Conversation
The ignored benchmark test compiles with the real nargo but hard-coded 1.0.0-beta.26, so it failed after the rc.3 toolchain upgrade. Read the pin from crates/zk/prover/Cargo.toml as the benchmark runner does.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 49 minutes. View limit details
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Derive and use the pinned Noir version crates/zk/benchmarks/src/circuit_benchmark/tests.rs |
The test derives the Noir version from the prover’s Cargo.toml and uses it for config compilation and artifact loading. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix
Suggested reviewers: neogiacomo
Merge Risk: 
Minimal · up to 0b233
-
No actionable merge-blocking risk remains.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed -
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
What and why
This is a follow-up to the #2349 security review, finding L4.
fresh_config_compilation_records_local_sources_not_hydrated_source_identityis#[ignore]d because it runs the realnargo, but it hard-coded1.0.0-beta.26as the Noir pin. Under the rc.3 toolchain it fails withConfig compiler does not match Rust Noir 1.0.0-beta.26. The test now reads the pin fromcrates/zk/prover/Cargo.tomlwithe3_zk_build::pinned_noir_version, as the benchmark runner does (circuit_benchmark/mod.rs), so future toolchain bumps won't break it.Review findings left unchanged:
releases.yml): already fixed in chore(zk)!: upgrade toolchain to Aztec v6.0.0 (bb 6.0.0, Noir rc.3) #2349 (0b2b25b7f).bb6.rust:test:proofspanics if a VK-substitution proof comes back as anything other thanProofRejected, so a change in bb's wording fails CI.only_an_explicit_verifier_result_is_an_invalid_proofcovers the matcher itself.bbskips the checksum check): intended. An operator-supplied binary is trusted by design, and chore(zk)!: upgrade toolchain to Aztec v6.0.0 (bb 6.0.0, Noir rc.3) #2349 doesn't change that.integration_child_uses_validated_bb_even_when_path_selects_another_binary): its5.2.0comes from synthetic shell stubs that print an arbitrary version, so the test still checks thatE3_CUSTOM_BBis forwarded, independent of the real toolchain.verifyon an adversarial proof): still covered against the realbb6.c0_local_verifier_failures_retry_without_peer_blame(CIzk_prover_e2esuites leg) verifies a proof with substituted public inputs and requiresProofVerificationFailedplus an accusation. That only happens whenverifyreturnsOk(false). I also ranbb6.0.0verifyon a proof with one byte flipped, on altered public inputs, and on a truncated proof. Each exits 1 withProof verification failed, the stringverify_proof_impltreats as an invalid proof.Verification
cargo test --locked -p e3-zk-benchmarks -- --include-ignored: 15 passed. The fixed test failed before this change with nargo 1.0.0-rc.3.cargo test --locked -p e3-zk-prover --lib c0_local_verifier_failures_retry_without_peer_blame: passed withbb6.0.0 and the compiled C0 artifacts present, so it ran rather than skipping.cargo fmt --check: clean.Summary by CodeRabbit