Confidential Token: specify the client SDK layer - #822
Merged
Conversation
Fills the `SDK (to be added)` placeholder in the design doc's register. The client is load-bearing here — commitment openings exist only off-chain, so a client that mis-derives one makes funds permanently unspendable — but three requirements were stranded as obligations on "wallets and SDKs" in documents that do not own the client layer, and the seed-to-`sk` derivation the recovery guarantee depends on was never specified at all. Also records, in §16, six places where the protocol docs are silent, mutually inconsistent, or behind the circuits, each with the assumption this document makes in the meantime.
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:
WalkthroughThe pull request standardizes ECDH shared-secret scalar names across confidential circuits and tests, adds address-to-field parity vectors, and expands or corrects SDK, design, recovery, compliance, indexer, and selective-disclosure documentation. ChangesCircuit and test alignment
Documentation specification updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Removes amplification, second-register restatements, rhetorical framing, and paragraph-closing punchlines throughout, and repairs the three inbound §16 references left stale by that section's renumbering.
…_field vector Distinct domain tags are not sufficient: SpongeSqueeze_2(d, s, sigma)[0] equals poseidon_with_domain(d, [s, sigma]), so a tag used in both modes over the same inputs reproduces the other mode's pad. address_to_field is the only primitive implemented twice, on-chain and per client, and had no conformance vector.
Specifies the sponge construction in §2.5, replaces the n-squeeze phrasing with the two-lane form, and requires each domain tag to be confined to a single sponge mode. Also types the read methods' return values and renames `get_spender` to `get_spender_delegation`.
Replaces the prohibition on the SEP-0005 signing key with two root classes — the BIP-39 seed and a SEP-0053 signature by a signer on the account — keyed to custody shape, with the tradeoff comparison in §5.2. Also trims §4 to defer to DESIGN.md now that the sponge construction is normative there.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Uses RFC 5869's `IKM` throughout §5, pins the exact byte string each root class supplies (BIP-39 seed output, RFC 8032 `R ‖ S` signature, raw 32 bytes), and states that it is passed verbatim rather than hashed or re-encoded.
An ed25519-controlled address now derives sk from a SEP-0053 signature by its signer, and an address with no ed25519 key of its own from raw bytes supplied by whatever custody mechanism controls it. A seed root added no reach a signer root lacks and made an irreversible per-client commitment out of a class the address should determine.
Collapse restated capability, griefing and expiry text into cross-references. Fix stale names and pointers: get_spender_delegation, a_tilde, the SEP-23 version bytes, the checkpoint event list, delta_disc_bind's role, and the scope of the drawn-scalar enumeration.
brozorec
force-pushed
the
worktree-confidential-sdk-spec
branch
from
July 30, 2026 14:50
06257a1 to
9bff6ed
Compare
…st merge Only Merge resets C_receive, so anchoring recovery's receiving side at the latest checkpoint reconstructed a zero balance whenever a spend followed an inbound transfer with no merge between them. The window now runs from the last Merge at or before the checkpoint, and 9.5 carries the resulting security properties and the two-anchor soundness argument instead of restating the procedure.
Routes the disclosure ciphertext's shared secret through the two-coordinate ECDH funnel, which #778 left behind. Corrects the delta_addr in-circuit claim, the address_to_field fixture status, SetSpender/RevokeSpender lane usage, confidential_balance's return type, D-balance liveness, the ComplianceConfig field count, and the compliance constructor's relation to the core interface.
Every total in §10.3 was one low: SetSpender omitted the S12 escrow ECDH, and the other four dropped their output commitment. The "base + 2 for auditor ECDH" phrasing left three bases unstated, so the counts could not be audited against the circuit sources or constraints.baseline.
brozorec
marked this pull request as ready for review
July 31, 2026 09:34
![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: 11
🤖 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_spender/src/tests.nr`:
- Line 975: Update the zero-r_e explanation in the surrounding test comments to
state that only the shared point becomes the identity, while ecdh(0, k_aud_s)
produces the ECDH-domain hash of that identity point rather than literal zero;
describe s_a_s_zero and s_esc_zero as identity-derived scalars.
In `@packages/tokens/src/confidential/circuits/spender_transfer/src/tests.nr`:
- Around line 1025-1026: Update both zero-r_e comments to use scalar names
instead of the obsolete coordinate notation: in
packages/tokens/src/confidential/circuits/spender_transfer/src/tests.nr lines
1025-1026 and packages/tokens/src/confidential/circuits/transfer/src/tests.nr
lines 849-850, replace s_a_{r,s}.x = 0 with s_a_r_zero = 0 and s_a_s_zero = 0.
In `@packages/tokens/src/confidential/docs/DESIGN_cont.md`:
- Around line 512-514: Update the Provenance text to replace the
collision-resistance justification with the required domain-separation or
pseudorandom-independence assumption for distinct Poseidon2 tags. Retain the
normative requirements that all tags are distinct and each is confined to a
single sponge mode, while preserving the existing positional-assignment
explanation.
In `@packages/tokens/src/confidential/docs/DESIGN.md`:
- Around line 371-383: Define self-transfers as applying both roles during
recovery: in packages/tokens/src/confidential/docs/DESIGN.md lines 371-383,
specify the application order so the event’s sender checkpoint initializes
W_spend before its recipient credit updates W_receive; update
packages/tokens/src/confidential/docs/SDK.md lines 377-388 with the same
normative dual-role rule; and classify self-transfers as both sender-side
checkpoints and recipient-side replay events in
packages/tokens/src/confidential/docs/INDEXER.md lines 43-54.
In `@packages/tokens/src/confidential/docs/INDEXER.md`:
- Around line 12-23: The replay window terminology around T_0 currently implies
that the anchor event is replayed, risking double application after resetting
W_receive. Update the T_0 definition and related recovery wording to state that
T_0 is an exclusive reset anchor: initialize from its state and apply only
events strictly after T_0 through the current ledger, while preserving the
registration fallback.
In `@packages/tokens/src/confidential/docs/SDK.md`:
- Around line 25-26: Update the salt-security statement in SDK.md to classify
salt uniqueness as a confidentiality and unlinkability requirement, describing
that reusing σ repeats the deterministic ephemeral scalar and channel masks
rather than enabling circuit-constraint violations. Ensure the classification
guides conformance tests toward the actual failure mode.
In `@packages/tokens/src/confidential/docs/SELECTIVE_DISCLOSURE.md`:
- Around line 487-497: Qualify the recipient-only decryption claim in §13.2:
update the statement around s_disc = ECDH(r_disc, P_R) to acknowledge that the
prover, who samples r_disc, can compute the shared secret, while only the
intended recipient can decrypt after the prover releases the ciphertext.
- Line 348: Resolve the conflicting opening-storage requirements by choosing and
stating one normative rule for whether the wallet may reconstruct the latest
opening from a checkpoint. Update the requirement in section 15.2 and the SDK
requirements to match that rule, while preserving the existing latest-opening
behavior where required.
In `@packages/tokens/src/confidential/README.md`:
- Around line 37-38: Update the documentation tree in the README so OVERVIEW.md
and INDEXER.md use ├── markers, while SDK.md remains the sole final child using
└──.
In `@packages/tokens/src/confidential/test.rs`:
- Around line 868-870: Update the test loop around
token_storage::address_to_field to invoke the internal helper through
e.as_contract using a generated context address, while continuing to pass the
fixed strkey-derived Address as the vector input and preserving the existing
assertion.
- Around line 874-882: Update the expect calls in hex32 so their messages
explain that the committed test vector is guaranteed to contain valid
hexadecimal digits, rather than only stating the expected value. Apply this to
both high- and low-nibble to_digit conversions.
🪄 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: e0bff243-8a11-4044-9be2-ebf7c437356d
📒 Files selected for processing (27)
packages/tokens/src/confidential/README.mdpackages/tokens/src/confidential/circuits/gadgets/sponge_squeeze_2/src/main.nrpackages/tokens/src/confidential/circuits/lib/src/lib.nrpackages/tokens/src/confidential/circuits/lib/src/tests.nrpackages/tokens/src/confidential/circuits/lib/testdata/README.mdpackages/tokens/src/confidential/circuits/lib/testdata/address_to_field.jsonpackages/tokens/src/confidential/circuits/lib/testdata/encrypt_auditor_sender_balance.jsonpackages/tokens/src/confidential/circuits/lib/testdata/sponge_squeeze_2.jsonpackages/tokens/src/confidential/circuits/revoke_spender/src/main.nrpackages/tokens/src/confidential/circuits/revoke_spender/src/tests.nrpackages/tokens/src/confidential/circuits/set_spender/src/main.nrpackages/tokens/src/confidential/circuits/set_spender/src/tests.nrpackages/tokens/src/confidential/circuits/spender_transfer/src/main.nrpackages/tokens/src/confidential/circuits/spender_transfer/src/tests.nrpackages/tokens/src/confidential/circuits/transfer/src/main.nrpackages/tokens/src/confidential/circuits/transfer/src/tests.nrpackages/tokens/src/confidential/circuits/withdraw/src/main.nrpackages/tokens/src/confidential/circuits/withdraw/src/tests.nrpackages/tokens/src/confidential/docs/COMPLIANCE.mdpackages/tokens/src/confidential/docs/DESIGN.mdpackages/tokens/src/confidential/docs/DESIGN_cont.mdpackages/tokens/src/confidential/docs/INDEXER.mdpackages/tokens/src/confidential/docs/OVERVIEW.mdpackages/tokens/src/confidential/docs/SDK.mdpackages/tokens/src/confidential/docs/SELECTIVE_DISCLOSURE.mdpackages/tokens/src/confidential/storage.rspackages/tokens/src/confidential/test.rs
Specify the dual-role application of self-transfers in recovery, cite the PRF assumption for domain-tag independence, reclassify salt uniqueness as a confidentiality requirement, and rewrite the zero-r_e test comments around identity-derived ECDH scalars. Also fixes the README docs tree.
brozorec
changed the title
ozgunozerk
approved these changes
Jul 31, 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
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.
Adds
packages/tokens/src/confidential/docs/SDK.md, filling theSDK (to be added)placeholder inDESIGN.md's Project Documents register, and — in follow-up commits — resolves the protocol questions drafting it surfaced instead of leaving them recorded as open.SDK.md
The client layer is load-bearing: commitment openings
(v, r)exist only off-chain, so a client that loses or mis-derives one makes funds permanently unspendable while the commitment sits visibly on-chain.SDK.mdspecifies that layer normatively and discharges the requirements previously stranded as obligations on "wallets and SDKs" in documents that don't own the client.Its main addition is §5, a derivation for
sk, which no document specified. The root is keyed to what controls the address — a SEP-0053 signature by the account's ed25519 signer, or raw 32 bytes from the controlling custody mechanism for contract addresses — fed through rejection-sampled HKDF-SHA-512 bound to the deployment (addr_f) and the account (acct_f). Bindingacct_fis a privacy MUST: the sameskregistered under two addresses publishes identicalYandPVKunder both, linking otherwise-unlinkable addresses.Protocol decisions folded back into the owning documents
r_eis derived, not sampled.DESIGN.md§7.6 said sample it;SELECTIVE_DISCLOSURE.mdsaid derive it — and a client built to §7.6 alone permanently forecloses sender-side disclosure.DESIGN.mdnow specifiesr_e = Poseidon(δ_eph, vk, σ)protocol-wide, and §9.4 carries the consequence: avkholder can reconstruct the openings of every transfer the account originated.DESIGN.md§2.5 is now normative for the Poseidon2 sponge — construction, padding, the single-permutation two-lane squeeze, and one sponge mode per domain tag. Previously the only authority was the circuit source, and an implementer reaching for a general-purpose Poseidon2 hash fails every proof.Mergerather than the last checkpoint; the previous anchor reconstructed a zero balance whenever a spend followed an inbound transfer with no intervening merge.DESIGN_cont.md§10.3 (every total was one low), typed read-method returns and theget_spender_delegationrename, pinned δ-tag values, and a batch of stale cross-references and contradictions.Code
No longer docs-only, though nothing behavioral changed: stale
_xsuffixes are removed from circuit and contract identifiers, anaddress_to_field.jsonconformance vector is added, and a Rust test pins the contract'saddress_to_fieldagainst it — the one primitive implemented independently on-chain and in every client, previously with no vector holding the two together.Verification
All 13 sponge-derived vectors in
circuits/lib/testdata/were reproduced from the docs' prose alone, using only a raw permutation and no code from the reference implementation. Every cross-document§Nreference and relative link resolves (checked programmatically). Numeric claims were recomputed rather than asserted.No issue number — this completes an existing documented placeholder rather than introducing a feature.
PR Checklist
address_to_fieldparity vectors🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Bug Fixes
Tests