feat: on-chain proof verification test for c0 [skip-line-limit]#1319
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds keccak oracle hashing to bb invocations, centralizes zk-prover test helpers and compiled-circuit usage, introduces an on-chain verification integration test (deploys ZKTranscriptLib + DkgPkVerifier to Anvil), updates Solidity artifacts and .gitignore, renames circuit/proof enums, and accepts numeric JSON inputs for circuits. Changes
Sequence DiagramsequenceDiagram
participant Test as On-Chain Test
participant Prover as ZK Prover
participant Anvil as Anvil Node
participant Lib as ZKTranscriptLib
participant Verifier as DkgPkVerifier
Test->>Prover: request proof (PkCircuit, ultra_honk, --oracle_hash keccak)
Prover-->>Test: returns proof bytes + public inputs
Test->>Anvil: start local node / deploy contracts
Anvil-->>Test: RPC ready
Test->>Lib: deploy ZKTranscriptLib
Anvil-->>Lib: lib deployed
Test->>Verifier: link Lib into verifier bytecode & deploy verifier
Anvil-->>Verifier: verifier deployed
Test->>Verifier: call verify(proof, publicInputs)
Verifier->>Lib: generateTranscript(proof, publicInputs)
Lib-->>Verifier: transcript
Verifier-->>Test: returns true
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@crates/zk-prover/tests/common/helpers.rs`:
- Around line 55-57: The code uses env!("CARGO_TARGET_TMPDIR") which is a
compile-time requirement and will fail when the env var is absent; change it to
a runtime lookup using std::env::var("CARGO_TARGET_TMPDIR") and fall back to
std::env::temp_dir() when the var is Err, then pass that Path (or PathBuf) into
TempDir::new_in so TempDir::new_in(target_tmp) no longer depends on a
compile-time env!.
In `@crates/zk-prover/tests/onchain_verification_tests.rs`:
- Around line 219-226: The current conversion from onchain_public_signals to
public_inputs uses chunks(32) which silently right-pads a short final chunk and
corrupts the last field element; change the logic to use chunks_exact(32) (or
check that onchain_public_signals.len() % 32 == 0) and assert or error if there
is a non-empty remainder so you never accept a short final chunk; update the
code building FixedBytes (the map that calls FixedBytes::from on buf) to iterate
only over exact 32-byte chunks (e.g., using chunks_exact and checking
.remainder().is_empty() or explicitly validating the length) and fail fast if
the input length is not a multiple of 32.
In `@scripts/build-circuits.ts`:
- Around line 328-329: The argument parsing branch for '--oracle-hash' currently
assigns args[++i] directly to options.oracleHash which can be undefined and
silently override the default; update the parsing logic in the same parse loop
so you first peek at args[i+1], validate it's defined and not another flag, and
only then consume it (increment i and set options.oracleHash); if it's missing
or invalid, throw a clear error or exit with a message indicating the missing
value for '--oracle-hash' so the default ('keccak') isn't overwritten silently
(refer to options.oracleHash, args and the '--oracle-hash' branch).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/zk-prover/tests/local_e2e_tests.rs (1)
435-439: Consider tracking the e_sm commitment skip as a follow-up issue.The comment explains the mismatch (
SEARCH_N = 100vs. committee size5), but this silently drops test coverage for the smudging-noise commitment. If this is a known limitation, it might be worth creating an issue to reconcile the parameter mismatch and re-enable this check.Would you like me to open a tracking issue for reconciling the
Bounds::compute/ Noir-config committee-size mismatch and re-enabling the e_sm commitment assertion?🤖 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 435 - 439, Add a follow-up tracking issue to record and resolve the skipped e_sm commitment assertion: create an issue that describes the mismatch between Bounds::compute using SEARCH_N (100) for the smudging bound and the Noir circuit's committee size (5), the resulting packing/bit-width discrepancy that causes the e_sm commitment check to be skipped in local_e2e_tests.rs, and a proposed plan (investigate making Bounds::compute use the Noir committee-size, or parameterize both places, and re-enable the e_sm assertion). Reference the test location (e_sm commitment check in crates/zk-prover/tests/local_e2e_tests.rs), the involved symbols (Bounds::compute, SEARCH_N, e_sm), expected behavior, and attach the test snippet or link to the PR so the follow-up can track fixing the parameter mismatch and re-enabling the check.crates/events/src/enclave_event/signed_proof.rs (1)
294-313: Test covers the key renamed mappings but omitsT6DecryptedSharesAggregation.Consider adding an assertion for
T6DecryptedSharesAggregation→DecryptedSharesAggregationBnto lock in the mapping.🤖 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, Add a test assertion to cover the missing mapping for the renamed key: call ProofType::T6DecryptedSharesAggregation.circuit_name() and assert it equals CircuitName::DecryptedSharesAggregationBn so the mapping is locked in alongside the other ProofType assertions in proof_type_circuit_name_mapping().
🤖 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: The CircuitName enum's as_str() returns the same string for
DkgShareDecryption and ThresholdShareDecryption, causing identical output from
the Display impl and possible collisions; update the as_str() match arm for
CircuitName::DkgShareDecryption to return a distinct value (e.g.,
"dkg_share_decryption") so Display (which delegates to as_str()) and any
lookups/deserializations are unambiguous—check the as_str() method and the
Display impl to ensure they now produce distinct strings while leaving
dir_path() behavior unchanged.
---
Nitpick comments:
In `@crates/events/src/enclave_event/signed_proof.rs`:
- Around line 294-313: Add a test assertion to cover the missing mapping for the
renamed key: call ProofType::T6DecryptedSharesAggregation.circuit_name() and
assert it equals CircuitName::DecryptedSharesAggregationBn so the mapping is
locked in alongside the other ProofType assertions in
proof_type_circuit_name_mapping().
In `@crates/zk-prover/tests/local_e2e_tests.rs`:
- Around line 435-439: Add a follow-up tracking issue to record and resolve the
skipped e_sm commitment assertion: create an issue that describes the mismatch
between Bounds::compute using SEARCH_N (100) for the smudging bound and the Noir
circuit's committee size (5), the resulting packing/bit-width discrepancy that
causes the e_sm commitment check to be skipped in local_e2e_tests.rs, and a
proposed plan (investigate making Bounds::compute use the Noir committee-size,
or parameterize both places, and re-enable the e_sm assertion). Reference the
test location (e_sm commitment check in
crates/zk-prover/tests/local_e2e_tests.rs), the involved symbols
(Bounds::compute, SEARCH_N, e_sm), expected behavior, and attach the test
snippet or link to the PR so the follow-up can track fixing the parameter
mismatch and re-enabling the check.
0xjei
reviewed
Feb 19, 2026
0xjei
left a comment
0xjei
left a comment
Contributor
There was a problem hiding this comment.
check the commentsssss good j
hmzakhalid
force-pushed
the
feat/onchain-zk-verification
branch
from
48b95cb to
e3185fd
Compare
0xjei
approved these changes
Feb 19, 2026
cedoor
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For CI to pass for this PR:
We need to merge #1320 and create a new release with the updated circuits.
Then update the circuits JSON files and proving keys (pk) under
crates/zk-prover/tests/fixtures.After that, rerun the CI and it should pass.
Summary by CodeRabbit
New Features
Tests
Chores
Bug Fixes