fix: fixtures for zk circuits [skip-line-limit]#1350
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ctrlc03
changed the title
📝 WalkthroughWalkthroughReplaces fixture-based ZK circuit testing with pre-compiled circuit artifacts, renames/consolidates several circuit and proof enum variants, updates zk-prover circuit implementations and tests, adds test helper utilities for compiled circuits, adjusts prover invocation to use keccak oracle hash, and updates CI to build, upload, verify, and run zk-prover end-to-end tests with Barretenberg. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as GitHub Actions
participant Builder as build_and_test_circuits
participant Artifact as Artifact Storage
participant BB as Barretenberg (bb)
participant Prover as zk_prover_e2e job (tests)
CI->>Builder: start build_and_test_circuits
Builder->>BB: install BB (using BB_VERSION)
Builder->>Builder: build circuits (circuits/bin/.../target)
Builder->>Artifact: upload compiled-circuits artifact
CI->>Prover: start zk_prover_e2e (depends on build)
Prover->>Artifact: download compiled-circuits
Prover->>BB: install BB (using BB_VERSION)
Prover->>BB: verify artifacts (--oracle_hash keccak)
Prover->>Prover: run e2e zk-prover tests
Prover-->>CI: tests result / artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/zk-prover/tests/local_e2e_tests.rs (1)
221-249:⚠️ Potential issue | 🔴 CriticalDkgShareDecryption circuit lacks e2e test coverage.
The
setup_share_decryption_test()function only coversThresholdShareDecryption(using threshold presets and the threshold module). However,DkgShareDecryptionis referenced in production code (zk_cli.rs, events/proof.rs, signed_proof.rs) and has an activeProvableimplementation. Currently, there is no e2e test for the DKG variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/tests/local_e2e_tests.rs` around lines 221 - 249, The test helper setup_share_decryption_test only constructs a ThresholdShareDecryption path (ThresholdShareDecryptionCircuit and ThresholdShareDecryptionCircuitData) but never exercises the Dkg variant; add a parallel e2e test path for Dkg by either extending setup_share_decryption_test or adding setup_dkg_share_decryption_test that mirrors the existing flow: compile the "threshold"/"share_decryption" circuit, call the DKG-specific sample generator (the DkgShareDecryptionCircuitData equivalent) with appropriate preset/committee, construct a ZkProver, and return DkgShareDecryptionCircuit and its sample (or create a separate test that uses DkgShareDecryptionCircuit, DkgShareDecryptionCircuitData, BfvPreset, and ZkProver) so the Dkg share-decryption Provable implementation is covered by the e2e tests.
🧹 Nitpick comments (8)
crates/zk-prover/tests/common/helpers.rs (2)
87-118:env!("CARGO_TARGET_TMPDIR")is only available in integration tests.This is fine since the file lives under
tests/, but worth noting that this helper cannot be used from#[cfg(test)]unit test modules withinsrc/. The doc comment could mention this constraint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/tests/common/helpers.rs` around lines 87 - 118, Update the doc comment for setup_test_prover to note that it relies on env!("CARGO_TARGET_TMPDIR") which is only available in integration tests under tests/, so this helper cannot be used from #[cfg(test)] unit test modules inside src/; mention the constraint and suggest using TempDir::new() or an alternative temp directory strategy for unit tests if callers need to run from src unit tests.
114-115: Missing#[cfg(not(unix))]fallback for non-Unix platforms.The integration test in
crates/tests/tests/integration.rs(line 95) has acompile_error!for non-unix. This helper silently skips the symlink on non-unix, which would cause cryptic test failures.Proposed fix for consistency
#[cfg(unix)] std::os::unix::fs::symlink(bb, &backend.bb_binary).unwrap(); + #[cfg(not(unix))] + compile_error!("Test helpers require unix symlink support");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/tests/common/helpers.rs` around lines 114 - 115, The unix-only symlink call std::os::unix::fs::symlink(bb, &backend.bb_binary).unwrap() lacks a non-Unix fallback and so silently skips on non-Unix platforms; add a #[cfg(not(unix))] branch that performs a file copy instead (e.g., std::fs::copy(bb, &backend.bb_binary).unwrap()) so backend.bb_binary exists on non-Unix systems (optionally preserve permissions if needed), keeping the same symbols bb and backend.bb_binary and the existing unix symlink branch.scripts/build-circuits.ts (1)
126-151:cleanTargetDirsis duplicated verbatim ingenerate-verifiers.ts.This helper is copy-pasted between
build-circuits.tsandgenerate-verifiers.ts(lines 315–337 there). Consider extracting it into a shared utility (e.g., incircuit-constants.tsor a new shared module) to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-circuits.ts` around lines 126 - 151, The cleanTargetDirs function is duplicated verbatim; extract it into a shared helper module and import it from both callers to avoid drift: create a new exported function (e.g., cleanTargetDirs) in a shared file (e.g., circuit-constants or a new utilities module), move the implementation there (preserving behavior around this.circuitsDir and CircuitInfo inputs), update build-circuits.ts and generate-verifiers.ts to call the shared cleanTargetDirs instead of their local copies, and remove the duplicate implementations so both files reuse the single canonical function..github/workflows/ci.yml (2)
36-36:rust_testsnow runs integration tests that may trigger a fullensure_installed()download.The
rust_testsjob doesn't installbbor download compiled circuit artifacts. The integration test incrates/tests/tests/integration.rswill fall through toensure_installed(), downloading bb + circuits on every run. This is functional but could significantly slow the job. Consider either:
- Adding
bband circuit artifacts to this job (likezk_prover_e2e), or- Splitting integration tests into a separate job that depends on
build_and_test_circuits.#!/bin/bash # Check if the integration test is gated or if it always runs ensure_installed without bb rg -n 'ensure_installed\|find_bb\|setup_test_zk_backend' crates/tests/tests/integration.rsAlso applies to: 76-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 36, The rust_tests job is currently running integration tests that call ensure_installed() and thus download bb and circuit artifacts each run; update CI so rust_tests either installs bb and the compiled circuit artifacts (same as zk_prover_e2e) before running tests or move the integration tests into a separate job that depends on build_and_test_circuits; specifically, add steps to the rust_tests job to fetch/install bb and circuit outputs (or reuse artifacts from build_and_test_circuits), or create a new job that runs crates/tests/tests/integration.rs and lists build_and_test_circuits as a needs dependency to avoid repeated ensure_installed() downloads (check symbols ensure_installed(), find_bb, setup_test_zk_backend, build_and_test_circuits, zk_prover_e2e to locate related logic).
579-586: Duplicated Barretenberg install steps — extract to a composite action.The bb install block is copy-pasted in
build_and_test_circuitsandzk_prover_e2e. Consider extracting it into a reusable composite action (e.g.,.github/actions/install-bb) to keep the version and install logic in one place.Also applies to: 634-641
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 579 - 586, Duplicate Barretenberg install steps exist in the build_and_test_circuits and zk_prover_e2e job blocks; extract the repeated shell sequence into a reusable composite action (e.g., .github/actions/install-bb) that accepts BB_VERSION (or reads env.BB_VERSION) and performs the curl, tar, find/move to /usr/local/bin/bb, chmod +x and a bb --version check; then replace the original inline run steps in both jobs with a uses: ./ .github/actions/install-bb (or uses: ./.github/actions/install-bb and pass with: bb_version: ${{ env.BB_VERSION }}), ensuring the composite action's action.yml exposes the input and preserves sudo/permission behavior so the pipeline behavior remains identical.crates/events/src/enclave_event/signed_proof.rs (2)
294-313: Tests don't cover the newT6DecryptedSharesAggregationmapping.The
proof_type_circuit_name_mappingtest verifies a subset of mappings but omitsT6DecryptedSharesAggregation → DecryptedSharesAggregationBn, which is one of the more nuanced changes in this PR.Proposed addition
assert_eq!( ProofType::T5ShareDecryption.circuit_name(), CircuitName::ThresholdShareDecryption ); + assert_eq!( + ProofType::T6DecryptedSharesAggregation.circuit_name(), + CircuitName::DecryptedSharesAggregationBn + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/events/src/enclave_event/signed_proof.rs` around lines 294 - 313, The test proof_type_circuit_name_mapping is missing an assertion for the new mapping; add an assertion in that test to verify ProofType::T6DecryptedSharesAggregation.circuit_name() == CircuitName::DecryptedSharesAggregationBn so the new enum variant is covered (update the test function proof_type_circuit_name_mapping to include this additional assert_eq! using the existing pattern).
58-62:T6DecryptedSharesAggregationhardcodes the Bn variant — noProofTypecovers the Mod variant.
T6DecryptedSharesAggregation.circuit_name()returnsCircuitName::DecryptedSharesAggregationBn. Sincevalid_circuits()in theProvableimpl exposes bothBnandModvariants, there is a gap: if a proof withCircuitName::DecryptedSharesAggregationModever reaches verification, it has no correspondingProofTypefor fault attribution.This gap is intentional by design:
decrypted_shares_aggregation_modis executable only in insecure/test mode, whiledecrypted_shares_aggregation_bnis used in secure mode. SinceModproofs are never expected in production, they don't need aProofType. However, this should be documented or enforced in the code (e.g., removeModfromvalid_circuits()or add a production variant if the scope changes).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/events/src/enclave_event/signed_proof.rs` around lines 58 - 62, The code maps ProofType::T6DecryptedSharesAggregation to CircuitName::DecryptedSharesAggregationBn but valid_circuits() (in the Provable impl) lists both DecryptedSharesAggregationBn and DecryptedSharesAggregationMod, leaving Mod without a corresponding ProofType; fix this by making the policy explicit: either remove CircuitName::DecryptedSharesAggregationMod from valid_circuits() so only the Bn variant is accepted, or add a matching ProofType/arm for the Mod variant (or a clear comment/assert) in circuit_name() and the ProofType enum to document that Mod is test-only; update the Provable impl, ProofType::T6DecryptedSharesAggregation mapping, and add a short comment near circuit_name() and valid_circuits() explaining the intended production/test restriction so the behavior is enforced and clear.crates/zk-prover/tests/local_e2e_tests.rs (1)
417-421: Document a tracking issue for the e_sm commitment check.The skip is well-explained, but the discrepancy between
Bounds::computeusingSEARCH_N (100)and the Noir circuit using committee size(5)sounds like a latent bug or a config that needs reconciliation. Consider linking a tracking issue so this doesn't become permanently skipped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/tests/local_e2e_tests.rs` around lines 417 - 421, Add a tracking issue reference to the skipped e_sm commitment check: create an issue describing the mismatch between Bounds::compute using SEARCH_N (100) and the Noir circuit using the committee size (5), then update the comment around the e_sm skip to include the issue URL/number and a short TODO noting that the discrepancy must be reconciled (reference Bounds::compute, SEARCH_N, and the Noir circuit committee size) so reviewers can find and follow up on the latent bug.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/events/src/enclave_event/proof.rs`:
- Around line 70-73: as_str() for CircuitName returns the same string for
CircuitName::DkgShareDecryption and CircuitName::ThresholdShareDecryption
causing identical Display output; change the returned literal for one (e.g.,
make CircuitName::DkgShareDecryption -> "dkg_share_decryption" or
CircuitName::ThresholdShareDecryption -> "threshold_share_decryption") in the
as_str() match so the Display/printing is unambiguous, and ensure any code that
relies on those strings (the Display impl or callers of as_str()) still compiles
after the rename.
In `@scripts/generate-verifiers.ts`:
- Around line 100-101: The unconditional call to cleanTargetDirs(circuits) is
removing compiled artifacts even when the user passed --no-compile; change the
logic so cleanTargetDirs is only invoked when cleaning is requested or when
compilation will run — e.g., wrap the call in a conditional that checks
this.options.clean === true (or at minimum this.options.compile !== false)
before calling cleanTargetDirs(circuits), so ensureCompiled() later can find
existing artifacts; update any related comments to reflect the new guard.
---
Outside diff comments:
In `@crates/zk-prover/tests/local_e2e_tests.rs`:
- Around line 221-249: The test helper setup_share_decryption_test only
constructs a ThresholdShareDecryption path (ThresholdShareDecryptionCircuit and
ThresholdShareDecryptionCircuitData) but never exercises the Dkg variant; add a
parallel e2e test path for Dkg by either extending setup_share_decryption_test
or adding setup_dkg_share_decryption_test that mirrors the existing flow:
compile the "threshold"/"share_decryption" circuit, call the DKG-specific sample
generator (the DkgShareDecryptionCircuitData equivalent) with appropriate
preset/committee, construct a ZkProver, and return DkgShareDecryptionCircuit and
its sample (or create a separate test that uses DkgShareDecryptionCircuit,
DkgShareDecryptionCircuitData, BfvPreset, and ZkProver) so the Dkg
share-decryption Provable implementation is covered by the e2e tests.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 36: The rust_tests job is currently running integration tests that call
ensure_installed() and thus download bb and circuit artifacts each run; update
CI so rust_tests either installs bb and the compiled circuit artifacts (same as
zk_prover_e2e) before running tests or move the integration tests into a
separate job that depends on build_and_test_circuits; specifically, add steps to
the rust_tests job to fetch/install bb and circuit outputs (or reuse artifacts
from build_and_test_circuits), or create a new job that runs
crates/tests/tests/integration.rs and lists build_and_test_circuits as a needs
dependency to avoid repeated ensure_installed() downloads (check symbols
ensure_installed(), find_bb, setup_test_zk_backend, build_and_test_circuits,
zk_prover_e2e to locate related logic).
- Around line 579-586: Duplicate Barretenberg install steps exist in the
build_and_test_circuits and zk_prover_e2e job blocks; extract the repeated shell
sequence into a reusable composite action (e.g., .github/actions/install-bb)
that accepts BB_VERSION (or reads env.BB_VERSION) and performs the curl, tar,
find/move to /usr/local/bin/bb, chmod +x and a bb --version check; then replace
the original inline run steps in both jobs with a uses: ./
.github/actions/install-bb (or uses: ./.github/actions/install-bb and pass with:
bb_version: ${{ env.BB_VERSION }}), ensuring the composite action's action.yml
exposes the input and preserves sudo/permission behavior so the pipeline
behavior remains identical.
In `@crates/events/src/enclave_event/signed_proof.rs`:
- Around line 294-313: The test proof_type_circuit_name_mapping is missing an
assertion for the new mapping; add an assertion in that test to verify
ProofType::T6DecryptedSharesAggregation.circuit_name() ==
CircuitName::DecryptedSharesAggregationBn so the new enum variant is covered
(update the test function proof_type_circuit_name_mapping to include this
additional assert_eq! using the existing pattern).
- Around line 58-62: The code maps ProofType::T6DecryptedSharesAggregation to
CircuitName::DecryptedSharesAggregationBn but valid_circuits() (in the Provable
impl) lists both DecryptedSharesAggregationBn and DecryptedSharesAggregationMod,
leaving Mod without a corresponding ProofType; fix this by making the policy
explicit: either remove CircuitName::DecryptedSharesAggregationMod from
valid_circuits() so only the Bn variant is accepted, or add a matching
ProofType/arm for the Mod variant (or a clear comment/assert) in circuit_name()
and the ProofType enum to document that Mod is test-only; update the Provable
impl, ProofType::T6DecryptedSharesAggregation mapping, and add a short comment
near circuit_name() and valid_circuits() explaining the intended production/test
restriction so the behavior is enforced and clear.
In `@crates/zk-prover/tests/common/helpers.rs`:
- Around line 87-118: Update the doc comment for setup_test_prover to note that
it relies on env!("CARGO_TARGET_TMPDIR") which is only available in integration
tests under tests/, so this helper cannot be used from #[cfg(test)] unit test
modules inside src/; mention the constraint and suggest using TempDir::new() or
an alternative temp directory strategy for unit tests if callers need to run
from src unit tests.
- Around line 114-115: The unix-only symlink call std::os::unix::fs::symlink(bb,
&backend.bb_binary).unwrap() lacks a non-Unix fallback and so silently skips on
non-Unix platforms; add a #[cfg(not(unix))] branch that performs a file copy
instead (e.g., std::fs::copy(bb, &backend.bb_binary).unwrap()) so
backend.bb_binary exists on non-Unix systems (optionally preserve permissions if
needed), keeping the same symbols bb and backend.bb_binary and the existing unix
symlink branch.
In `@crates/zk-prover/tests/local_e2e_tests.rs`:
- Around line 417-421: Add a tracking issue reference to the skipped e_sm
commitment check: create an issue describing the mismatch between
Bounds::compute using SEARCH_N (100) and the Noir circuit using the committee
size (5), then update the comment around the e_sm skip to include the issue
URL/number and a short TODO noting that the discrepancy must be reconciled
(reference Bounds::compute, SEARCH_N, and the Noir circuit committee size) so
reviewers can find and follow up on the latent bug.
In `@scripts/build-circuits.ts`:
- Around line 126-151: The cleanTargetDirs function is duplicated verbatim;
extract it into a shared helper module and import it from both callers to avoid
drift: create a new exported function (e.g., cleanTargetDirs) in a shared file
(e.g., circuit-constants or a new utilities module), move the implementation
there (preserving behavior around this.circuitsDir and CircuitInfo inputs),
update build-circuits.ts and generate-verifiers.ts to call the shared
cleanTargetDirs instead of their local copies, and remove the duplicate
implementations so both files reuse the single canonical function.
| CircuitName::ShareEncryption => "share_encryption", | ||
| CircuitName::DkgShareDecryption => "share_decryption", | ||
| CircuitName::PkAggregation => "pk_aggregation", | ||
| CircuitName::ThresholdShareDecryption => "threshold_share_decryption", | ||
| CircuitName::DecryptedSharesAggregation => "decrypted_shares_aggregation", | ||
| CircuitName::ThresholdShareDecryption => "share_decryption", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all uses of as_str() on CircuitName
echo "=== Uses of as_str() (full context) ==="
rg -n --type rust 'as_str\(\)' -B 2 -A 2 -g '!target'
echo ""
echo "=== FromStr or from_str implementations ==="
rg -n --type rust 'impl.*FromStr|from_str|parse::<CircuitName' -g '!target'
echo ""
echo "=== String literals matching 'share_decryption' ==="
rg -n --type rust '"share_decryption"' -B 1 -A 1 -g '!target'
echo ""
echo "=== CircuitName enum definition and methods ==="
rg -n --type rust 'enum CircuitName|impl CircuitName' -A 30 -g '!target'Repository: gnosisguild/enclave
Length of output: 42841
🏁 Script executed:
#!/bin/bash
# Check if DkgShareDecryption is actually used anywhere
echo "=== Uses of DkgShareDecryption variant ==="
rg -n 'DkgShareDecryption' --type rust -g '!target'
echo ""
echo "=== Check if there's actual DKG share_decryption circuit ==="
find . -name "*share*decryption*" -type f | grep -v target | head -20Repository: gnosisguild/enclave
Length of output: 2040
as_str() collision between DkgShareDecryption and ThresholdShareDecryption affects logging only; file paths are unambiguous.
Both variants return "share_decryption" from as_str(), causing identical Display output. However, this is a clarity issue, not a functional defect: dir_path() correctly distinguishes them as "dkg/share_decryption" and "threshold/share_decryption". There is no deserialization path that would create ambiguity. Consider prefixing one (e.g., "dkg_share_decryption") to make Display output unambiguous in logs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/events/src/enclave_event/proof.rs` around lines 70 - 73, as_str() for
CircuitName returns the same string for CircuitName::DkgShareDecryption and
CircuitName::ThresholdShareDecryption causing identical Display output; change
the returned literal for one (e.g., make CircuitName::DkgShareDecryption ->
"dkg_share_decryption" or CircuitName::ThresholdShareDecryption ->
"threshold_share_decryption") in the as_str() match so the Display/printing is
unambiguous, and ensure any code that relies on those strings (the Display impl
or callers of as_str()) still compiles after the rename.
| // Clean stale nargo build caches to prevent using outdated artifacts | ||
| this.cleanTargetDirs(circuits) |
There was a problem hiding this comment.
Unconditional cleanTargetDirs conflicts with --no-compile.
cleanTargetDirs runs unconditionally (not gated by this.options.clean), deleting all compiled artifacts. If a user passes --no-compile, ensureCompiled (line 230) will immediately throw because the artifacts it expects were just removed. Either gate the clean behind a flag or skip it when compile is false.
Proposed fix
- // Clean stale nargo build caches to prevent using outdated artifacts
- this.cleanTargetDirs(circuits)
+ // Clean stale nargo build caches to prevent using outdated artifacts
+ if (this.options.compile !== false) {
+ this.cleanTargetDirs(circuits)
+ }📝 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.
| // Clean stale nargo build caches to prevent using outdated artifacts | |
| this.cleanTargetDirs(circuits) | |
| // Clean stale nargo build caches to prevent using outdated artifacts | |
| if (this.options.compile !== false) { | |
| this.cleanTargetDirs(circuits) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/generate-verifiers.ts` around lines 100 - 101, The unconditional call
to cleanTargetDirs(circuits) is removing compiled artifacts even when the user
passed --no-compile; change the logic so cleanTargetDirs is only invoked when
cleaning is requested or when compilation will run — e.g., wrap the call in a
conditional that checks this.options.clean === true (or at minimum
this.options.compile !== false) before calling cleanTargetDirs(circuits), so
ensureCompiled() later can find existing artifacts; update any related comments
to reflect the new guard.
26ab552 to
db0c46f
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/zk-prover/src/prover.rs (1)
179-193:⚠️ Potential issue | 🔴 CriticalChange
--oracle-hash(hyphen) to--oracle_hash(underscore) in theverifycommand at line 184.Line 184 uses
--oracle-hashbut line 88 uses--oracle_hash, and the Barretenberg CLI consistently uses underscores. Fromscripts/README.md, the correct format isbb write_vk --oracle_hash keccak.If the
verifysubcommand rejects the hyphen form, the BB process exits non-zero. Sinceverify_proofreturnsOk(output.status.success())rather than anErr, this silent failure makes every proof appear invalid without any actionable error message—only a warning log.Fix
- "--oracle-hash", + "--oracle_hash", "keccak",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zk-prover/src/prover.rs` around lines 179 - 193, The verify invocation using StdCommand::new(&self.bb_binary) passes the wrong flag "--oracle-hash" to the "verify" subcommand; update the args array in the verify_proof block (the StdCommand::new(...).args([...]) call that includes "verify", "--scheme", "ultra_honk", ...) to use "--oracle_hash" (underscore) instead of "--oracle-hash" so the Barretenberg CLI recognizes the option and the process doesn't silently fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/zk-prover/src/prover.rs`:
- Around line 179-193: The verify invocation using
StdCommand::new(&self.bb_binary) passes the wrong flag "--oracle-hash" to the
"verify" subcommand; update the args array in the verify_proof block (the
StdCommand::new(...).args([...]) call that includes "verify", "--scheme",
"ultra_honk", ...) to use "--oracle_hash" (underscore) instead of
"--oracle-hash" so the Barretenberg CLI recognizes the option and the process
doesn't silently fail.
2 participants
Summary by CodeRabbit
Chores
Tests
Compatibility