feat(confidential): Noir operator_transfer circuit#731
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 OperatorTransfer constraint set from DESIGN.github.md Section 7.8 plus the dual-channel auditor block from Section 8.4. Wrapper binding is inherited indirectly through O3 (allowance-randomness Poseidon pinned to dvk_i, sigma_a), so the circuit does not constrain vk -- the operator never sees the owner's sk. Public-input ordering matches the parent epic's frozen 24-field layout.
Drop the defensive on-curve check on PVK_recipient. Per design doc §10.8, PVK is path (1)/(2) -- proof-constrained at registration (R3) and trusted on read -- not path (3). The transfer circuit follows this doctrine; the operator-transfer circuit now matches. VK regenerated.
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:
WalkthroughThis PR implements the OperatorTransfer Noir circuit enabling operator-initiated confidential transfers from a delegated allowance. The circuit validates operator key ownership, allowance commitment opening, transfer amount bounds, recipient ECDH-derived encryption, new allowance state, and dual-channel auditor visibility via Poseidon2 sponges. Verification key is extracted and published. ChangesOperator Transfer Circuit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces a substantial, self-contained cryptographic circuit with dense constraint logic spanning 311 lines, comprehensive test coverage with 1447 test lines, fixture generation, and integration into the build pipeline. Review requires understanding multiple cryptographic primitives (ECDH, commitments, sponge squeezes, allowance binding) and verifying constraint correctness across happy path and 26 distinct failure modes. Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #731 +/- ##
=======================================
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:
|
The regeneration command in the file's own comment strips everything but the table rows; re-prepend the header block so the regen instructions survive future updates.
![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/constraints.baseline`:
- Line 5: The baseline file has a stale opcode count: update the entry for
circuit_operator_transfer | main in constraints.baseline from 131 to the current
build value 129 to match the `nargo info` output; locate the line containing
"circuit_operator_transfer | main" and replace the opcode count field
(previously 131) with 129 so the baseline matches the current ACIR opcode count.
🪄 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: 00e0e9aa-9819-4da0-bb0c-204e515f448f
📒 Files selected for processing (7)
packages/tokens/src/confidential/circuits/Nargo.tomlpackages/tokens/src/confidential/circuits/constraints.baselinepackages/tokens/src/confidential/circuits/operator_transfer/Nargo.tomlpackages/tokens/src/confidential/circuits/operator_transfer/src/main.nrpackages/tokens/src/confidential/circuits/operator_transfer/src/tests.nrpackages/tokens/src/confidential/circuits/scripts/extract_vks.shpackages/tokens/src/confidential/circuits/vks/operator_transfer.vk.json
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
Implements the confidential operator-transfer circuit (closing issue #708, design doc §7.8):
C_aviadvk_i, allowance randomness anchored toPoseidon2(δ_allow_r, dvk_i, σ_a)(the wrapper-binding hinge -- see §7.8 "Wrapper binding"), 127-bit range validity onv_a,v_tx, andv_a - v_tx.R_e, anti-poisoning binding ofr_txviaPoseidon2(δ_tx_blind, S.x, σ_a)(Section 5.4), transfer commitment, encrypted amount,r_e ≠ 0.r_a' = Poseidon2(δ_allow_r, dvk_i, σ_a'), new allowance commitmentC_a' = (v_a - v_tx)·G + r_a'·H, encrypted allowance scalar.r_e, per-channelSpongeSqueeze_2keyed byσ_a, recipient-auditor(ṽ_aud,r, r̃_aud,r)and owner-auditor(ṽ_aud,s, ã_aud,s)-- the visibility model points balance- and allowance-checkpoint ciphertexts at the funds' owner, not the operator (§8.4).24 public inputs in design-doc canonical order. Auditor keys (the only proof-less entry, Section 10.8 path 3) are validated on-curve + non-identity in-circuit before consumption by O_a1 / O_a5.
PVK_recipientis path (2) -- proof-constrained at registration (R3) and trusted on read -- so no re-check, matching the transfer circuit's doctrine.Wrapper binding (the subtle one): OperatorTransfer does not constrain
vkdirectly; binding is inherited indirectly via the allowance commitment chain. SetOperator derivesdvk_ifrom the wrapper-specificvk(S2, S5), which determinesr_a(S6) and thusC_a(S7). O3 verifies the operator's claimeddvk_iagainst the on-chainC_aviaσ_a. A proof generated against one wrapper'sC_acannot verify against another's. Exercised byrejects_wrong_dvk_against_wrapper.Stacked on #728 (transfer) -- diff against main will include those changes until the parent lands.
Closes #708.
Test plan
nargo test --package circuit_operator_transfer(29 tests pass: 2 happy paths with differentv_tx,full_allowance_transferboundary, wrapper-binding regression, wrongsk_op/ wrong allowance opening / wrong σ_a, under-funded transfer + range overflow onv_aandv_tx, anti-poisoning onC_tx, every tampered ciphertext,r_e = 0, off-curve + non-identity rejection of both auditor keys, wrong recipient PVK)nargo testworkspace-wide (no regressions inlib,register,withdraw,transfer)./scripts/extract_vks.shreproduces the committedvks/operator_transfer.vk.jsonbyte-for-byte with the pinnedbb 0.87.0; existingregister/withdrawVKs re-extract byte-identicallySummary by CodeRabbit
New Features
Tests
Chores