feat(confidential): Noir set_operator circuit#730
Merged
Conversation
Replaces the per-plaintext encrypt_auditor_tx / encrypt_auditor_bal
helpers with the per-channel Poseidon2 sponge described in design doc
v0.5 -> v0.6 (Section 2.5, Section 8.1). One absorb of
(delta_channel, S.x, sigma) is followed by N squeezes from the same
permutation; rate=3 lets every current call serve all squeezes without
a second permutation. AUDITOR_TX and AUDITOR_BALANCE domain tags are
dropped in favour of AUDITOR_SENDER (delta_aud_s) and AUDITOR_RECIPIENT
(delta_aud_r) at the same numeric slots (10, 11).
Adds the sponge_squeeze_2 gadget for nargo info accounting and pins
the new sponge outputs in testdata/sponge_squeeze_{1,2}.json. The
constraints baseline gains one row; no existing primitive's output
changed.
Implements the spend-side block of the Withdraw circuit from design doc Section 7.5: owner key ownership (W1), wrapper-bound viewing key (W2), spendable balance opening (W3), range validity on v, a, and v - a (W4), deterministic randomness for the new commitment (W5), the refreshed spendable commitment (W6), and the encrypted balance scalar (W7). The auditor block (W_a1-W_a5, K_aud_s + r_e + R_e + b_tilde_aud_s) lands in a follow-up commit so this one stays focused on balance conservation and key ownership; once that lands the public input signature grows from 10 to the design doc's frozen 15 fields. Tests reuse the lib's pinned (sk, wrap, sigma, v, r) fixtures so any drift in commit / vk_from_sk / encrypt_balance / derive_spend_r is caught by `withdraw_fixtures_match_lib` before the rest of the harness runs. Negative coverage: under-funded withdrawal (W4), v or a out of range, wrong sk (W1), wrong balance opening (W3), wrong wrap (W2/W6/W7 chain), tampered b_tilde (W7), tampered C_spend' (W6). VK extraction script and constraints baseline updated.
Adds the sender-auditor visibility block to the Withdraw circuit
(design doc Section 7.5, Section 8.1): the ephemeral key R_e = r_e * H
(W_a1), the sender-auditor ECDH shared secret S_{a,s} = r_e * K_{aud,s}
(W_a2), the single-squeeze sender-channel sponge mask m_b (W_a3), the
sender-auditor balance checkpoint ciphertext b_tilde_aud_s = (v - a) +
m_b (W_a4), and the r_e != 0 non-collapse constraint (W_a5).
The public input signature grows from 10 to the design-doc-frozen 15
fields, in canonical order (C_spend, Y, wrap, K_aud_s, a, C_spend',
sigma, b_tilde, R_e, b_tilde_aud_s). K_aud_s carries an explicit
on-curve + non-identity check at the entrypoint because the verifier
doesn't check curve membership and an off-curve K_aud_s would break the
soundness of W_a2 (Section 10.8).
`encrypt_auditor_sender_balance` lands in the lib as the canonical
W_a3+W_a4 composition; the same helper covers RevokeOperator V_a3/V_a5
and SetOperator S_a3/S_a5 when those circuits land. Pinned via testdata
and a lib round-trip test that ties it back to sponge_squeeze_1.
Auditor-side negative coverage: r_e = 0 (W_a5), wrong r_e (W_a1),
off-curve K_aud_s, identity K_aud_s, valid-but-wrong K_aud_s, tampered
b_tilde_aud_s (W_a4). Closes the under-funded-withdrawal + tampered-
ciphertext criteria from issue #705.
VK extracted via `bash scripts/extract_vks.sh` after the Withdraw circuit landed in the workspace. Same UltraHonk universal-SRS pipeline as the register VK -- no new trusted-setup contribution required. The committed JSON is the integration contract with the on-chain verifier (#701); CI re-runs the extraction script and diffs against this copy.
Aligns W4 with design doc §7.5 / §2.6 / §3.4: the value domain is the SEP-41 non-negative i128 range [0, 2^127), not [0, 2^128). The circuit now uses Noir stdlib's `Field::assert_max_bit_size::<127>()` directly (the 127-bit decomposition spelled out in §2.6) instead of the lib's u128-cast helper. Doc comments for W4 and the public-input table for `a` are restated against the tighter bound and reference §3.4 for the entrypoint-level `a >= 0` check. Test boundaries retarget to 2^127 (the smallest value W4 must reject), not 2^128. Withdraw VK regenerated; constraint count drops from 130 to 92 ACIR opcodes (stdlib's assert_max_bit_size is cheaper than the u128 round-trip). The lib's `assert_range_128` primitive is now unused by Withdraw but stays in place for backwards compat -- a separate cleanup can remove it once no callers reference it.
The test previously fired W3 (commit(v_huge, R) != stale C_SPEND) before W4 ever ran, so the range check was never the load-bearing assertion. Recompute C_spend from v_huge so W3 passes and the very first W4 line (v.assert_max_bit_size::<127>()) becomes the failing constraint. rejects_a_out_of_range already isolates W4 (V is unchanged, W3 passes, W4 hits a.assert_max_bit_size::<127>() on a_huge = 2^127) so it is left as-is.
The design doc added delta_addr = 1 (§2.7), shifting every other tag up by one. Realign the lib constants, re-pin all Poseidon-derived fixtures and testdata, regenerate VKs, and fix a stale W_a5 label in the withdraw test (now W8).
Implements the SetOperator circuit per design doc §7.7 (issue #707): owner balance split (S1-S4, S9-S11), delegation + allowance escrow with the dvk-handoff ECDH expanded into its three §7.11 sub-constraints (S5-S8, S12, S13), and the owner-auditor block (S_a1-S_a5). 24 public inputs in the design-doc canonical order. Tests cover the happy path, the v_a = v boundary, S4 underflow / range overflow, the wrong op_i case, wrong Y_op (S12(b) via the cipher), every tampered ciphertext including both escrowed_dvk limbs, r_e = 0 (S13), and on-curve / non-identity rejection for K_aud_s.
Pastes the values emitted by `print_fixtures` into the tests.nr globals and adds circuit_set_operator's row to constraints.baseline (128 ACIR opcodes, 72 Brillig). `set_operator_fixtures_match_lib` and `set_operator_auditor_fixtures_match_lib` re-derive the same values from the lib primitives so a future lib drift fails the test immediately.
VK extracted via `bash scripts/extract_vks.sh` after the SetOperator circuit landed in the workspace. Same UltraHonk universal-SRS pipeline as the register / withdraw / transfer VKs -- no new trusted-setup contribution required. The committed JSON is the integration contract with the on-chain verifier (#701); CI re-runs the extraction script and diffs against this copy.
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
brozorec
changed the title
5 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #730 +/- ##
=======================================
Coverage 96.71% 96.71%
=======================================
Files 67 67
Lines 6798 6798
=======================================
Hits 6575 6575
Misses 223 223 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/tokens/src/confidential/circuits/set_operator/src/tests.nr`:
- Line 117: The test contains a typo variable name "yoy" assigned from y_op.y
which breaks the CI fixture-check; rename the variable to the intended name
(e.g., "y" or "y_op_y") in the assignment (replace "let yoy = y_op.y" in the
test) and update all its usages (e.g., the "{yoy}" occurrence in the fixture
printer) to the new identifier so the fixture token check passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e093e8a0-1944-4afd-b8ff-046f54790b44
📒 Files selected for processing (7)
packages/tokens/src/confidential/circuits/Nargo.tomlpackages/tokens/src/confidential/circuits/constraints.baselinepackages/tokens/src/confidential/circuits/scripts/extract_vks.shpackages/tokens/src/confidential/circuits/set_operator/Nargo.tomlpackages/tokens/src/confidential/circuits/set_operator/src/main.nrpackages/tokens/src/confidential/circuits/set_operator/src/tests.nrpackages/tokens/src/confidential/circuits/vks/set_operator.vk.json
`yoy` (Y_op.y in print_fixtures) trips the workflow-pinned typos ruleset, which flags it as a misspelling of `you`. Rename `yox` / `yoy` to `yopx` / `yopy` to match the pattern (Y_op-x, Y_op-y) and avoid the substring match. Test-only change; no circuit semantics affected.
CI runs LC_ALL=C sort to normalize nargo info output; the baseline rows were in insertion order instead of sort order, so check-test-info failed after the set_operator circuit was added.
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
2 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.
Summary
main.vks/set_operator.vk.jsonvia the existingscripts/extract_vks.shpipeline.Closes #707.
Notes for reviewers
feat/confidential-withdraw-circuit(the parent feature branch), notmain. Retarget once the parent merges.Y_opis treated as path-(2) of §10.8 (constrained on-curve at registration R1, trusted from wrapper storage). The wrapper that consumes this VK must loadY_opfrom the operator's storedspending_key, not from caller input — same doctrine asPVK_Bin Transfer.K_aud_sis path-(3) (public-input key) and carries the in-circuitassert_on_curve_non_identitycheck beforeS_a2.r_eis shared between the auditor block (S_a1) and the dvk escrow (S12(b)); S12(a) forcesescrowed_dvk_r_x == R_e.xso the on-chainBytesN<64>escrowed_dvk encoding (R_x ‖ dvk_cipher, §7.11) cannot drift from the auditor-block ephemeral.assert(r_e != 0)runs first inmainto reject the trivial-escrow / identity-collapse case before anyscalar_mulagainstr_e.Test plan
nargo test --package circuit_set_operator— 26 tests pass (happy path +full_allowance_escrowboundary + 18 isolated negative tests covering every constraint S1-S13 + S_a1-S_a5 including bothescrowed_dvklimbs, on-curve / identity rejection forK_aud_s,r_e = 0, and wrongop_i/Y_op/wrap/sk)nargo test(workspace) greenLC_ALL=C nargo info | grep '^|' | LC_ALL=C sortmatches the updatedconstraints.baselinebash scripts/extract_vks.shregeneratesvks/set_operator.vk.jsonbyte-identically🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests