Skip to content

Viewing-key range: demo deriveViewingKey emits keys the pool rejects on 3 residues; SDK assertViewingKey admits k == HALF_ORDER #961

Description

@Blockchain-Oracle

Summary

The reference viewing-key derivation in the demo (demo/src/session.ts, deriveViewingKey) can emit keys the deployed pool contract rejects, and silently remaps one residue to a different key than the derivation implies. Separately, the SDK's assertViewingKey uses an inclusive upper bound and so admits exactly one value the contract rejects.

Both verified at main (5165220).

The contract's rule (source of truth)

A registered key must satisfy 1 <= k < HALF_ORDER, strict:

  • packages/privacy/src/privacy.cairo:259-260ZERO_PRIVATE_KEY for k == 0, then is_canonical_key
  • packages/privacy/src/utils.cairo:273-276is_canonical_key(key) = key.into() < HALF_ORDER
  • utils.cairo:94HALF_ORDER = ORDER / 2 (integer division; ORDER is odd, so ORDER = 2*HALF_ORDER + 1)

Bug 1 — the demo fold (demo/src/session.ts:33-36)

const reduced = folded % order;
const canonical = reduced < MAX_VIEWING_KEY ? reduced : order - reduced;
return canonical === 0n ? 1n : canonical;

Three residues of the Poseidon fold have no legal representative under the pair (k, ORDER-k), and this code handles all three incorrectly:

reduced fold returns contract verdict
0 1 accepted — but it is a different key than the derivation implies (silent remap)
HALF_ORDER ORDER - HALF_ORDER = HALF_ORDER + 1 rejected, PRIVATE_KEY_NOT_CANONICAL
HALF_ORDER + 1 HALF_ORDER rejected, PRIVATE_KEY_NOT_CANONICAL (bound is strict)

The (HALF_ORDER, HALF_ORDER+1) pair straddles the boundary with neither side below it, so the fold cannot rescue those residues — they need an explicit throw-and-rederive, not a remap.

Probability per derivation is ~3·2⁻²⁵¹, so this never fires on random input — but the demo is the published reference for the derivation, integrators copy it verbatim (we did), and a silent wrong-key registration against a WriteOnce, no-rotation slot is a fund-loss bug when it does fire. The zero-residue remap is the sharpest edge: registration succeeds with key 1, and every later derivation of the "same" viewing key still yields 1, but the key is no longer bound to the signature-fold the scheme promises.

Bug 2 — SDK off-by-one (sdk/src/utils/validation.ts:25)

num.assertInRange(viewingKey, 1n, MAX_VIEWING_KEY, "viewingKey");

num.assertInRange in starknet.js is inclusive on both bounds (input >= lower && input <= upper), and sdk/src/interfaces.ts:24 sets MAX_VIEWING_KEY = ec.starkCurve.CURVE.n / 2n — exactly the contract's HALF_ORDER. So the SDK accepts k == HALF_ORDER, which is_canonical_key rejects. A client that validates with the SDK and then submits pays the fee for a guaranteed PRIVATE_KEY_NOT_CANONICAL revert.

Runnable repro

Needs only starknet (tested with 10.5.0):

import { ec, num } from 'starknet'

const ORDER = ec.starkCurve.CURVE.n
const MAX_VIEWING_KEY = ORDER / 2n // == the contract's HALF_ORDER

const contractAccepts = (k: bigint) => k !== 0n && k < MAX_VIEWING_KEY

// demo/src/session.ts:33-36, verbatim:
function demoFold(reduced: bigint): bigint {
  const canonical = reduced < MAX_VIEWING_KEY ? reduced : ORDER - reduced
  return canonical === 0n ? 1n : canonical
}

for (const reduced of [0n, MAX_VIEWING_KEY, MAX_VIEWING_KEY + 1n]) {
  const k = demoFold(reduced)
  console.log(reduced, '->', k, 'contract accepts:', contractAccepts(k))
}

// sdk/src/utils/validation.ts:25 — inclusive upper bound:
num.assertInRange(MAX_VIEWING_KEY, 1n, MAX_VIEWING_KEY, 'viewingKey') // passes
// ...but is_canonical_key(HALF_ORDER) is false on-chain.

Output:

reduced = 0            -> folds to 1            contract accepts: true   (silent remap)
reduced = HALF_ORDER   -> folds to HALF_ORDER+1 contract accepts: false
reduced = HALF_ORDER+1 -> folds to HALF_ORDER   contract accepts: false

Suggested fix

Demo — fold the strict upper half only, then assert instead of remapping; the three bad residues should throw (caller re-derives or reports), never silently coerce:

const reduced = ((folded % order) + order) % order
const k = reduced > MAX_VIEWING_KEY ? order - reduced : reduced
if (k < 1n || k >= MAX_VIEWING_KEY) {
  throw new Error('derived residue has no canonical viewing key — re-derive')
}
return k

SDK — make the upper bound strict so the check matches is_canonical_key:

assert(viewingKey >= 1n && viewingKey < MAX_VIEWING_KEY, () => 'viewingKey out of [1, MAX_VIEWING_KEY)')

(If MAX_VIEWING_KEY is meant to be the largest legal key rather than the exclusive bound, the constant should be ORDER / 2n - 1n and the docs in interfaces.ts:22 updated to match — either way the current pair disagrees with the contract by one.)

Found while integrating the SDK against the deployed mainnet pool.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions