fix(multisig-client): block-independent metadata↔tx_summary binding

[WiktorStarczewski]

Draft — depends on @miden-sdk/miden-sdk@0.15.10 (companion PR 0xMiden/web-sdk#298) being published, and on the real-summary integration test below. Do not un-draft until both are done.

What

Replaces the block-height-dependent metadata re-execution in verifyProposalMetadataBinding with a deterministic, exhaustive metadata↔tx_summary binding. Fixes the cross-block liveness bug (a second signer couldn't load/sign a pending proposal) while preserving the documented what-you-see-is-what-you-sign guarantee (docs/MULTISIG_SDK.md, docs/CONCEPTS.md: "a compromised Guardian cannot steal funds").

Why

verifyProposalMetadataBinding rebuilt each proposal from metadata and re-executed it (executeForSummary) to compare the resulting TransactionSummary commitment to the signed one. That re-execution runs against the client's current sync height as the reference block, and the summary commits over the transaction fee, derived from that block's fee_parameters. So the check only matched on the same client at the same height as the proposer — a second signer at a later block hit metadata does not match tx_summary and the account load aborted.

Simply deleting the check would fix liveness but drop a documented security control: a compromised/MITM Guardian could serve a cosigner an evil tx_summary under benign metadata, and the cosigner — reviewing metadata but signing the opaque summary commitment — could be tricked into signing a drain. So the check is made deterministic instead.

How

The new assertMetadataMatchesSummary (src/multisig/summaryBinding.ts) decodes the signed summary and, per type, asserts the transaction's effects are exactly the declared effect + the fee, across every intent-bearing dimension — its output notes, its input notes, and the account-storage slots it changes. It never reads the vault delta (the fee is a block-dependent native-asset vault delta); that's safe because assets can only leave via an output note or the fee, so binding the notes + storage slots exactly makes the vault a consequence. The comparison is therefore deterministic across blocks yet still exact — a mislabel, or a piggybacked undeclared effect in any dimension, is rejected.

Per type: p2id — exactly one output note (recipient rebuilt from metadata + the summary's salt; exactly the declared fungible asset), no input notes, no storage change but the per-tx replay marker. consume_notes — input-note-id set equality, no output notes. signer — no notes; threshold_config = [threshold, count, 0, 0]; the signer_public_keys map delta index-bound to the declared set (catches a duplicated/omitted/injected signer). update_procedure_threshold — no notes; procedure_thresholds[PROC_ROOT] = [threshold, 0, 0, 0]. switch_guardian — now bound (was exempt): guardian pubkey map = newGuardianPubkey, storage confined to the guardian slots (+ signer slots if it also rotates signers). custom — exempt (no metadata recipe; WYSIWYS explicitly does not hold — cosigners must verify the raw tx_summary).

Reading the config-type map deltas needs AccountStorageDelta.maps() / valueDeltas(), added in @miden-sdk/miden-sdk@0.15.10 (0xMiden/web-sdk#298); this PR bumps the dep to ^0.15.10.

Tests

35 binding unit tests including a rejecting test for each fund-theft bypass an earlier field-allowlist draft had (consume+drain-note, p2id piggybacked storage write / signer-map write / extra asset, signer duplicate-with-omit, unexpected slot, proc/guardian + drain note, wrong guardian key). Full package suite green (458), tsc --noEmit clean.

Known gaps

• ⚠️ Non-fungible asset drain in p2id (fund theft) — closure known, must land before merge. The p2id binding checks the recipient and the fungible asset separately, and NoteAssets exposes only fungible assets — so an NFT attached to an otherwise-correct p2id note is not bound and could be drained under a "send N tokens" label. Closure: bind the output note by full outputNote.id() equality (a NoteId commits to all assets incl. NFTs; the consume path already binds by note id and p2id.ts has the note-ID reconstruction), sourcing the fungible asset off the note to preserve block-independence. This also binds the note type as a side effect. It has callback-flag/metadata caveats that the pre-merge integration test must validate — so it's tracked below, not shipped in this draft.
• custom proposals get no WYSIWYS — cosigners must verify the raw tx_summary (unavoidable; no metadata recipe).
• Signer unchanged-index: the storage-map delta carries only changed indices, so an unchanged declared index isn't re-verified (a kept index can only hold the pre-existing legitimate signer; an omitted signer is under-delivery/DoS caught at execution, not theft).

⚠️ Pre-merge checklist (do not un-draft until all done)

• @miden-sdk/miden-sdk@0.15.10 published (web-sdk#298 merged + released).
• Close the p2id NFT gap via outputNote.id() equality (see above).
• Real-summary integration test. Every unit test mocks the TransactionSummary, so the encodings are cross-verified by reading the builders/MASM/inspector but not against a genuine executed summary. Add a test that runs a real executeForSummary → serialize → deserialize → assertMetadataMatchesSummary per type and confirms: (a) private p2id passes (its own-output note must expose assets(), else it would false-reject); (b) the threshold_config / procedure-root / guardian-pubkey word encodings match a real delta; (c) the signer pubkey-map delta carries every changed index; (d) the NFT-gap closure doesn't false-reject an honest p2id (callback-flag / note-metadata handling).

Follow-ups (separate)

• The sibling binding in prepareCustomExecution (custom execute path) has the same block-dependence; give it the equivalent treatment.
• The Rust reference client (crates/miden-multisig-client) still re-executes on list/sign — same bug, wants the equivalent block-independent binding.

Closes #409

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e76e1531-d9d8-49ee-a493-a66281b64838

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}