Correctness review

ZK staging correctness review — 10 September 2026

Remediation update: all three findings have code fixes and passing targeted regression tests, committed and pushed to staging in all three repositories. This document preserves the original reviewed-commit findings. See remediation results for commit IDs, implementation details, verification, and remaining deployment prerequisites. No deployment was performed as part of this work.

Outcome

The reviewed stack does not yet meet its intended behavior end to end. I found a high-priority swap-note recovery defect, a relayer compatibility failure, and a conditional production-release validation gap. The circuit and pool checks exercised in this review passed; I did not establish an attacker-controlled minting, double-spend, or withdrawal-amount substitution vulnerability in the current JoinSplit constraints and on-chain settlement path.

This is an engineering review of the checked-out source and available tests, not a claim that every possible witness or deployed configuration is safe. In particular, passing proof-verification tests cannot compensate for losing the secret needed to spend a successfully created note.

Source and scope

Primary scope: the Base flat JoinSplit V2/Groth16 circuit; range, Poseidon, Merkle, and Baby Jubjub helpers; native proving/encoding; deposit constraints and funding binding; verifier router/helper; pool settlement and swap/bridge adapters; SDK witness/statement/calldata construction; WASM key loading and request boundaries; frontend proving, settlement, and recovery; the relayer entry point; and ceremony-key binding, production export, and release validation.

The intended properties were taken from the implementation, BASE_JOINSPLIT_GROTH16.md, JOINSPLIT_AUDIT_HANDOFF.md, docs/AUDIT_SCOPE.md, and the browser/ceremony documentation. Historical reports were context, not evidence that a property still holds. Several scope documents contain stale or conflicting counts and claims.

The review did not include a complete security assessment of every frontend screen, infrastructure service, third-party library, deployed contract, or unrelated NFT/perpetuals/name product. No production secrets were inspected, no transactions were broadcast, and no production source was changed.

Findings

ZK-01 — High / P1: swap output secrets are not persisted before settlement

Primary location: adapterOps.ts:276 (privacy-frontend/src/joinsplit-v2/adapterOps.ts:276).

executeFlatJoinSplitV2Swap generates outputRandom at lines 253–258 and commits its derived outputNpk into the swap. It calls proveAndSend with only the input-token change notes and their delivery records. The new output-token note is reconstructed only after proveAndSend returns, using that local random value at lines 297–307.

The settlement journal saves params.outputs and params.deliveries at proveAndSend.ts:301 (privacy-frontend/src/joinsplit-v2/proveAndSend.ts:301). Neither contains the pending swap output's random value or a recovery record for that output. The generic journal also disallows changing the finalized output set. A successful exact-spend swap can therefore have an empty output/delivery journal even though the pool creates a valuable output-token note.

Trigger and impact: close or crash the page after broadcast, or encounter a receipt-read error after the transaction has settled. The output exists on-chain, but the wallet has not durably saved the blinding factor required to reconstruct and spend it. The error path returns success: false, empty note arrays, and no structured transaction hash or journal id. Ordinary chain rescanning cannot recover the missing random value from outputNpk.

There is an additional rollover trigger. The frontend reads the output tree/token domain before proving, but the pool derives the output domain at settlement, at ShieldedPoolJoinSplit.sol:854 (privacy-protocol/contracts/src/ShieldedPoolJoinSplit.sol:854). An exact-spend swap with no change may consume a retired-tree root after a rollover. The frontend then reconstructs with the old output domain and fails its commitment comparison at adapterOps.ts:203, entering the same nonrecoverable error path. This rollover case was source-traced, not reproduced on a live chain.

Evidence: the isolated swap recovery reproduction (.codex-tmp/zk-review-2026-09-10/swap-recovery-repro.cjs) executes the actual TypeScript operation with mocked chain/prover I/O. It models successful settlement followed by an unavailable receipt. It confirms that the generated random value is absent from the settlement parameters, output/delivery arrays are empty, and the operation reports failure without returning the recovery data. This is a boundary reproduction, not a real funded transaction.

Recommended correction: encrypt and persist a pending swap-output recovery record before broadcast, including the random value, derived note key, intended output token, and binding to the transaction intent. After settlement or restart, complete the note using authenticated receipt/insertion data and the actual tree/token domain. Make output recovery and delivery retryable without regenerating the random value. Test crash/restart, post-settlement RPC failure, and an exact-spend swap across tree rollover.

ZK-02 — Medium / P2: the staging relayer rejects all current Flat V2 settlement selectors

Primary location: transaction-handler.js:212 (stealthos-relayer/src/relayer/transaction-handler.js:212).

BASE_POOL_SELECTORS describes the legacy receipt-based ABI. enforceBaseRelayEnvelope at lines 312–329 rejects selectors outside that set. The current frontend sends the new flat-pool transactions through the pool-relay endpoint at settlement.ts:230 (privacy-frontend/src/joinsplit-v2/settlement.ts:230), and stealthos-relayer/src/server.js:643–683 routes that endpoint into this handler.

Selectors derived from the current SDK ABI are all rejected:

Impact: the intended relayed V2 operations cannot reach settlement on these three staging heads. This fails closed; it is an availability/compatibility problem, not a proof-validation bypass. Self-submission is a separate path and is not affected by this selector gate.

Evidence: the reproduction derives the current selectors and executes the actual envelope gate with synthetic zero-value requests, without sending a request or accessing a network. Rejection occurs before proof decoding or a pool/RPC lookup. The persisted harness is relayer-envelope-repro.cjs (.codex-tmp/zk-review-2026-09-10/relayer-envelope-repro.cjs).

Recommended correction: align the relayer release with the flat-pool ABI and implement a V2-aware validation/fee path. Adding selector strings alone is insufficient: the existing parser and fee checks at lines 250–264 and around line 616 still expect legacy receipts and contract getters. Add cross-repository tests that send each SDK-built transaction through the real relayer validation path.

ZK-03 — Medium / P2: the standalone production marker does not establish phase-2 verification

Primary location: export_joinsplit_verifiers.rs:272 (privacy-protocol/base-decider/src/bin/export_joinsplit_verifiers.rs:272).

The standalone --ceremony exporter imports final.mpcparams, calls bind_ceremony_key, and passes the resulting token to the production renderer. The token checks phase-1 alpha/beta, the gamma convention, and the IC vector against the R1CS. It explicitly omits phase-2 delta_g2 verification: ceremony_key.rs:28 (privacy-protocol/base-decider/src/ceremony_key.rs:28).

The MPC parser checks that a contribution-count word is nonzero but returns without validating the contribution records: snarkjs_proving_key.rs:328 (privacy-protocol/base-decider/src/snarkjs_proving_key.rs:328). That count is not cryptographic evidence that the phase-2 contribution chain was verified. Nevertheless, the renderer can emit isDevelopmentSetup() == false, and the canonical deployment script's setup check relies on this marker at DeployJoinSplitStack.s.sol:116 (privacy-protocol/contracts/script/DeployJoinSplitStack.s.sol:116).

Scope of the finding: this is a release-safety gap when an operator uses standalone export/deployment without enforcing the complete release process. It is not a demonstrated attack against a correctly validated and deployed release. The full validate_release_manifest path does require an external semantic artifact-lineage verifier and threshold release approval. scripts/ceremony/03-finalize.sh also runs snarkjs zkey verify before export and directs the operator to run the full release gate before deployment. Those controls mitigate the gap when actually followed.

The mismatch is that the marker/token and standalone exporter present a stronger guarantee than they independently establish, while the deployment script does not itself enforce the remaining check. A verifier artifact can be labeled production before that remaining phase-2 validation has been demonstrated to the deployment path.

Evidence: confirmed through source tracing across parser → key binding → renderer → deployment guard, and independently reviewed within this session. No malformed-key end-to-end test or live deployment was performed. The full release-validator tests pass, so this finding must not be described as bypassing its signed-policy checks.

Recommended correction: require verified phase-2 lineage, bound to the exact key and R1CS, before issuing the production provenance token; alternatively make the canonical deployment path enforce the complete verified release and exact runtime-bytecode binding. Merely trusting an editable receipt's valid flag would not provide this guarantee. Correct the standalone exporter's documentation to describe exactly what it verifies.

Verification performed

The Rust checks exercised value conservation, 120-bit bounds and field-wrap rejection, ownership/spending-key checks, Merkle membership, ordered/duplicate inputs, dummy slots, token-domain migration, arity binding, action-context substitutions, and ceremony/release validation cases. The Solidity checks included real proof routing, public-input/amount tampering, replay/nullifier rejection, atomic payout behavior, and adapter accounting. These are concrete tested cases, not exhaustive proofs of correctness.

Key commands, from privacy-protocol unless otherwise stated:

cargo test --locked --offline --manifest-path base-decider/Cargo.toml --lib
cargo test --locked --offline --manifest-path base-decider/Cargo.toml --lib joinsplit_engine::tests:: -- --ignored
cargo test --locked --offline --manifest-path base-decider/Cargo.toml --test action_commitment_parity --test poseidon_cross_toolchain
cargo test --locked --offline --manifest-path deposit-circuit/Cargo.toml
cargo test --locked --offline --manifest-path deposit-ceremony/Cargo.toml
forge test --match-path "test/{ShieldedPoolJoinSplit*,BaseJoinSplitVerifier,SwapAdapter,BridgeAdapter,GeneratedVerifierVectors,PoseidonStructures}.t.sol" --summary
npm --prefix packages/sdk test
npm --prefix joinsplit-prover-wasm test
node joinsplit-prover-wasm/bench/web-runtime-smoke.mjs
node joinsplit-prover-wasm/bench/smoke.mjs

Rust and Foundry ran through the installed WSL toolchain. Frontend tests ran from privacy-frontend with CI=true and npm test -- --watchAll=false --runInBand --testPathPattern=src/joinsplit-v2. The standalone reproduction scripts are under the parent workspace's .codex-tmp/zk-review-2026-09-10/ directory.

Limits and remaining evidence

• The three Base fork checks did not run because a Base mainnet RPC configuration was unavailable. No live-chain deployment or real funded swap/withdrawal was attempted.
• The all-action-type exhaustive sweep and the external/at-scale ceremony artifact checks were not run. The R1CS export utilities marked ignored are manual tools, not ordinary failing tests.
• The WASM smoke tests used the existing generated packages; the Rust source was rebuilt for its tests, but the browser WASM was not independently rebuilt in this session. Jest/provider mocks do not establish browser-device or production-network behavior.
• A genuine production ceremony and exact deployment/runtime-hash verification remain separate release evidence. The committed JoinSplit verifier sources inspected here still report development setup.
• This review did not independently cryptanalyze Groth16, BN254, Poseidon, or all upstream arithmetic libraries.

Resolve ZK-01 first because it can strand the output of a successful transaction. Align and integration-test the relayer before enabling relayed V2 flows, and enforce the complete release boundary before relying on production-marked artifacts.