Skip to content

fix(mdoc): make mdoc presentations verify as linked - #107

Open
moven0831 wants to merge 4 commits into
mainfrom
fix/mdoc-linked-proof
Open

fix(mdoc): make mdoc presentations verify as linked#107
moven0831 wants to merge 4 commits into
mainfrom
fix/mdoc-linked-proof

Conversation

@moven0831

@moven0831 moven0831 commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Description

Three defects in the mdoc path, each of which on its own made verify_linked fail for every mdoc presentation. Nothing caught them: the CLI benchmark verifies the two proofs separately and never calls verify_linked, and no test compared the two circuits' claim identities.

  • Claim identity hashed at the wrong width (mdoc.circom). mdoc hashed the CBOR-framed identifier at maxIdentifierLen (32) while show.circom and jwt.circom hash the bare name at NAME_ID_LEN (31), so the same attribute produced two different field elements and comm_W_shared could never match. It now hashes over the name, with the header byte pinned to 0x60 + nameLength so a two-byte header cannot pass its own length byte off as the name's first character. Constraints 1759107 to 1758995; witness offsets unchanged.
  • Show inputs carried no claim identities (src/generate-mdoc-inputs.ts). The generator passed seven of eight arguments to generateShowInputs and never set predicateClaimNames, so claimIdentifierHashes came out ["0","0"] and the Show circuit refused the witness at eval-predicates.circom:129, where an active predicate naming a zero identity would wildcard-match a padding slot. Both slots are bound, not just the one the predicate names, because MdocCircuit::shared() commits the mdoc circuit's own identities.
  • Stale witness offsets (ecdsa-spartan2/src/utils.rs). The three MDOC indices sat 8 slots past the compiled signals after d4ad38b and d2e5eba changed the circuit, so MdocCircuit::shared() committed the device key and the signals following it in place of the claim values and the identifier hashes.

hashClaimName and witnessIndices move from tests/common into src so the input generators can reach them; tests/common re-exports both. inputs/mdoc/default.json and inputs/show/mdoc.json are regenerated.

moven0831 and others added 4 commits August 30, 2026 22:26
The three MDOC witness indices were 8 slots past where the compiled
circuit actually puts those signals. Read from build/mdoc/mdoc.sym:
normalizedClaimValues[0] is 2966, claimIdentifierHashes[0] is 2970,
deviceKeyX is 2974.

ae44c09 last calibrated them; d4ad38b and d2e5eba then changed the
circuit without recalibrating. MdocCircuit::shared() was therefore
committing the device key and the signals that follow it in place of the
claim values and the identifier hashes, so comm_W_shared could never
match Show's and verify_linked always returned None.

Nothing caught it. tests/witness_layout.rs self-skips when the .sym is
absent, and build/mdoc/ has never existed in a fresh checkout, so the
mdoc case had never once run. It also never asked for
claimIdentifierHashes, the constant this all turns on, so extend its
symbol list. run_mdoc_prove_pipeline verifies the two proofs separately
and reads expressionResult off Show alone, so the CLI benchmark reported
success throughout.

Compile the circuit before trusting that test:
  cd wallet-unit-poc/circom && yarn compile:mdoc

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgtTGhNXGhNy6G6si1qdXa
generate-mdoc-inputs.ts called generateShowInputs with seven arguments,
omitting the eighth. Its own comment says what that costs: "Default 0
leaves the binding vacuous for flows with no attribute names (JWT); mdoc
callers pass real hashes." It also never set predicateClaimNames or
predicateClaimNameLens.

So claimIdentifierHashes came out ["0","0"] and the Show circuit refused
the witness at eval-predicates.circom:129, where an active predicate must
name a non-zero identity or it would wildcard-match a padding slot.
Witness generation printed "Failed assert in EvalPredicates" and carried
on, so the failure only surfaced later as InvalidSumcheckProof out of
verify. `mdoc benchmark` now reports expressionResult: true.

Both slots get identities, not just the one the predicate names:
MdocCircuit::shared() commits the mdoc circuit's own claimIdentifierHashes,
so Show has to carry the same values or comm_W_shared will not match.

hashClaimName has to run the HashBytesToFieldWithLen circuit — the hash is
over secq256r1 and no JS Poseidon matches — so it and witnessIndices moved
from tests/common into src, where the generators can reach them.
tests/common re-exports both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgtTGhNXGhNy6G6si1qdXa
mdoc.circom hashed the CBOR-framed identifier at maxIdentifierLen (32),
while show.circom derives predicateClaimIdentifiers from the bare name at
NAME_ID_LEN (31) and jwt.circom hashes at that same width. The two
produced different elements for the same attribute:

  H_32(0x6a "birth_date", 11) = 1255558376882134383771...568582
  H_31("birth_date", 10)      = 41364283474712499967556...354074

MdocCircuit::shared() commits the first, ShowCircuit::shared() the second,
so comm_W_shared could never match and every mdoc presentation was
unlinked. The comment at show.circom:37 claiming both use the same hash
was already describing the intended behaviour, not the actual one.

Nothing caught it: the CLI benchmark verifies the two proofs separately
and never calls verify_linked, and the integration test built both
witnesses without ever comparing their identities.

Also constrains the identifier to the single-byte CBOR text-string header.
Stripping the prefix without that would let a two-byte header's length
byte be hashed as the first character of the attribute name. The cost is
that attribute names of 24 bytes or more can no longer be presented, since
canonical CBOR frames those with a two-byte header. The two ISO 18013-5
names that long, family_name_national_character and
given_name_national_character, were provable before this change but never
linked, so no working flow regresses.

Constraints 1759107 -> 1758995; the witness offsets are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgtTGhNXGhNy6G6si1qdXa
The mdoc integration test built both witnesses and derived both sides'
identities from the same helper, so it never noticed that the circuits
disagreed. Assert that the mdoc circuit's claimIdentifierHashes[i] equals
H(name) as Show derives it, for both bound attributes.

Against the pre-fix circuit the four existing tests still pass and this
one fails on H_32(0x6a "birth_date", 11) against H_31("birth_date", 10).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tf6q6hYnaRyQx2rjh5i6F2
@moven0831
moven0831 force-pushed the fix/mdoc-linked-proof branch from 73341da to 45a845f Compare August 30, 2026 14:28
@moven0831
moven0831 requested a review from 0xVikasRushi August 30, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant