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

docs/MULTISIG_SDK.md

@@ -403,12 +403,18 @@ SDK's own built-in execution uses. Extending a transaction's advice map with it
 therefore does not collide with the transaction's ordinary inputs; the
 integration extends rather than replaces its advice map.
 
-> **Security:** for first-party types the SDK reconstructs the transaction from
-> metadata and checks it against the signed `tx_summary` commitment. For custom
-> types there is no such reconstruction, so the SDK cannot verify that display
-> metadata (e.g. `description`) matches what the transaction actually does.
-> Cosigners must verify the raw `tx_summary` they are signing — not trust the
-> label or description.
+> **Security:** for first-party types the SDK verifies that the signed
+> `tx_summary` matches the proposal's metadata before a cosigner signs it. It
+> decodes the summary and asserts the transaction's effects — its output notes,
+> its consumed input notes, and the account-storage slots it changes — are
+> *exactly* what the metadata describes and nothing more, so a cosigner is shown
+> what they actually sign. The check is deterministic (it never reads the
+> block-dependent transaction fee, which is why it works across cosigners at
+> different sync heights). Two residual gaps: it cannot see **non-fungible**
+> assets (the SDK exposes only fungible note/vault assets), and for **custom**
+> types there is no metadata recipe at all, so the SDK cannot verify that display
+> metadata (e.g. `description`) matches what the transaction does. Cosigners must
+> verify the raw `tx_summary` for `custom` proposals — not trust the label.
 
 ### Offline Workflow
 
@@ -584,13 +590,15 @@ await multisig.exportNoteToFile(noteId);
 const importedNoteId = await multisig.importNoteFromBytes(noteFileBytes);
 ```
 
-> **Note:** every cosigner device that verifies or signs the consume-notes
-> proposal needs the note in its local store with the on-chain inclusion
-> proof — deliver the note file to each of them (import + sync), not just to
-> the proposer. A cosigner whose store lacks the authenticated note rebuilds
-> the transaction differently (the input-notes commitment distinguishes
-> authenticated from unauthenticated consumption) and rejects the proposal
-> with `metadata does not match tx_summary`. The sender's own device heals
+> **Note:** the device that **executes** the consume-notes proposal needs the
+> note in its local store with the on-chain inclusion proof — deliver the note
+> file to each cosigner (import + sync), any of whom may execute, not just to
+> the proposer. Verifying and signing no longer require the note: the SDK checks
+> the proposal by comparing the signed summary's input-note **ids** to the
+> metadata (note ids are proof-agnostic), so a cosigner can sign without the
+> note. But the device that executes rebuilds the consumption from its own store,
+> so a store lacking the authenticated note produces a transaction that no longer
+> matches the signed summary and fails on-chain. The sender's own device heals
 > itself: it already knows the full note, so a post-commit sync is enough.
 
 #### Consume Notes (Claim Received Funds)

packages/miden-multisig-client/package.json

@@ -36,7 +36,7 @@
     "test:watch": "npm run generate:masm && vitest"
   },
   "dependencies": {
-    "@miden-sdk/miden-sdk": "^0.15.8",
+    "@miden-sdk/miden-sdk": "^0.15.10",
     "@noble/curves": "^1.9.7",
     "@noble/hashes": "^2.0.1",
     "@openzeppelin/guardian-client": "^0.16.2"

packages/miden-multisig-client/src/multisig.test.ts

@@ -7,6 +7,7 @@ import {
   buildUpdateSignersTransactionRequest,
   executeForSummary,
 } from './transaction.js';
+import { assertMetadataMatchesSummary } from './multisig/summaryBinding.js';
 
 const { mockRpcGetAccountDetails, mockAccountDeserialize, mockDetectConfig, mockNoteFileDeserialize } = vi.hoisted(() => ({
   mockRpcGetAccountDetails: vi.fn(),
@@ -139,6 +140,14 @@ vi.mock('./inspector.js', () => ({
   },
 }));
 
+// The metadata<->summary binding is tested exhaustively in
+// ./multisig/summaryBinding.test.ts. Here it is mocked to a no-op so these
+// tests exercise the surrounding flows; individual tests override it to throw
+// when they want to assert that a binding rejection propagates.
+vi.mock('./multisig/summaryBinding.js', () => ({
+  assertMetadataMatchesSummary: vi.fn(),
+}));
+
 // Mock fetch for GUARDIAN client
 const mockFetch = vi.fn();
 vi.stubGlobal('fetch', mockFetch);
@@ -1042,11 +1051,9 @@ describe('Multisig', () => {
         }),
       });
 
-      vi.mocked(executeForSummary).mockResolvedValueOnce({
-        toCommitment: () => ({
-          toHex: () => '0x' + 'f'.repeat(64),
-        }),
-      } as any);
+      vi.mocked(assertMetadataMatchesSummary).mockImplementationOnce((id: string) => {
+        throw new Error(`Invalid proposal: metadata does not match tx_summary for ${id}`);
+      });
 
       await expect(multisig.syncProposals()).rejects.toThrow(
         'Invalid proposal: metadata does not match tx_summary'
@@ -1289,11 +1296,9 @@ describe('Multisig', () => {
         }),
       });
 
-      vi.mocked(executeForSummary).mockResolvedValueOnce({
-        toCommitment: () => ({
-          toHex: () => '0x' + 'f'.repeat(64),
-        }),
-      } as any);
+      vi.mocked(assertMetadataMatchesSummary).mockImplementationOnce((id: string) => {
+        throw new Error(`Invalid proposal: metadata does not match tx_summary for ${id}`);
+      });
 
       await expect(
         multisig.createProposal(1, 'AQID', {
@@ -2024,11 +2029,9 @@ describe('Multisig', () => {
         description: '',
       });
 
-      vi.mocked(executeForSummary).mockResolvedValueOnce({
-        toCommitment: () => ({
-          toHex: () => '0x' + 'f'.repeat(64),
-        }),
-      } as any);
+      vi.mocked(assertMetadataMatchesSummary).mockImplementationOnce((id: string) => {
+        throw new Error(`Invalid proposal: metadata does not match tx_summary for ${id}`);
+      });
 
       await expect(multisig.signProposal('0x' + 'c'.repeat(64))).rejects.toThrow(
         'Invalid proposal: metadata does not match tx_summary'
@@ -2091,11 +2094,9 @@ describe('Multisig', () => {
 
       const multisig = createTestMultisig(config);
 
-      vi.mocked(executeForSummary).mockResolvedValueOnce({
-        toCommitment: () => ({
-          toHex: () => '0x' + 'f'.repeat(64),
-        }),
-      } as any);
+      vi.mocked(assertMetadataMatchesSummary).mockImplementationOnce((id: string) => {
+        throw new Error(`Invalid proposal: metadata does not match tx_summary for ${id}`);
+      });
 
       await expect(
         multisig.importProposal(
@@ -2127,12 +2128,6 @@ describe('Multisig', () => {
 
       const multisig = createTestMultisig(config);
 
-      vi.mocked(executeForSummary).mockResolvedValueOnce({
-        toCommitment: () => ({
-          toHex: () => '0x' + 'c'.repeat(64),
-        }),
-      } as any);
-
       const proposal = await multisig.importProposal(
         JSON.stringify({
           accountId: '0x' + 'a'.repeat(30),
@@ -2156,11 +2151,9 @@ describe('Multisig', () => {
         description: '',
       };
 
-      vi.mocked(executeForSummary).mockResolvedValueOnce({
-        toCommitment: () => ({
-          toHex: () => '0x' + 'f'.repeat(64),
-        }),
-      } as any);
+      vi.mocked(assertMetadataMatchesSummary).mockImplementationOnce((id: string) => {
+        throw new Error(`Invalid proposal: metadata does not match tx_summary for ${id}`);
+      });
 
       await expect(multisig.signProposalOffline(proposal.id)).rejects.toThrow(
         'Invalid proposal: metadata does not match tx_summary'
@@ -2486,17 +2479,13 @@ describe('Multisig', () => {
       const ackSignature = '0x' + '6'.repeat(130);
       const finalRequest = { kind: 'final-change-threshold-request' };
 
-      vi.mocked(buildUpdateSignersTransactionRequest)
-        .mockResolvedValueOnce({
-          request: { kind: 'verify-change-threshold-request' },
-          salt: { toHex: () => '0x' + 'd'.repeat(64) },
-          configHash: { toHex: () => '0x' + 'e'.repeat(64) },
-        } as any)
-        .mockResolvedValueOnce({
-          request: finalRequest,
-          salt: { toHex: () => '0x' + 'd'.repeat(64) },
-          configHash: { toHex: () => '0x' + 'e'.repeat(64) },
-        } as any);
+      // Only one reconstruction now: verifyProposalMetadataBinding no longer
+      // rebuilds the request, so it is built once for execution.
+      vi.mocked(buildUpdateSignersTransactionRequest).mockResolvedValueOnce({
+        request: finalRequest,
+        salt: { toHex: () => '0x' + 'd'.repeat(64) },
+        configHash: { toHex: () => '0x' + 'e'.repeat(64) },
+      } as any);
 
       (multisig as any).proposals.set(cachedProposalId, {
         id: cachedProposalId,
@@ -2723,11 +2712,9 @@ describe('Multisig', () => {
       const multisig = createTestMultisig(config);
       const proposalId = '0x' + 'c'.repeat(64);
 
-      vi.mocked(executeForSummary).mockResolvedValueOnce({
-        toCommitment: () => ({
-          toHex: () => '0x' + 'd'.repeat(64),
-        }),
-      } as any);
+      vi.mocked(assertMetadataMatchesSummary).mockImplementationOnce((id: string) => {
+        throw new Error(`Invalid proposal: metadata does not match tx_summary for ${id}`);
+      });
 
       (multisig as any).proposals.set(proposalId, {
         id: proposalId,
@@ -2881,17 +2868,13 @@ describe('Multisig', () => {
         const proposalId = '0x' + 'c'.repeat(64);
         const finalRequest = { kind: 'fresh-message-word-request' };
 
-        vi.mocked(buildUpdateSignersTransactionRequest)
-          .mockResolvedValueOnce({
-            request: { kind: 'verify-change-threshold-request' },
-            salt: { toHex: () => '0x' + 'd'.repeat(64) },
-            configHash: { toHex: () => '0x' + 'e'.repeat(64) },
-          } as any)
-          .mockResolvedValueOnce({
-            request: finalRequest,
-            salt: { toHex: () => '0x' + 'd'.repeat(64) },
-            configHash: { toHex: () => '0x' + 'e'.repeat(64) },
-          } as any);
+        // Only one reconstruction now: verifyProposalMetadataBinding no longer
+        // rebuilds the request, so it is built once for execution.
+        vi.mocked(buildUpdateSignersTransactionRequest).mockResolvedValueOnce({
+          request: finalRequest,
+          salt: { toHex: () => '0x' + 'd'.repeat(64) },
+          configHash: { toHex: () => '0x' + 'e'.repeat(64) },
+        } as any);
 
         (multisig as any).proposals.set(proposalId, {
           id: proposalId,

packages/miden-multisig-client/src/multisig.ts

@@ -74,6 +74,7 @@ import {
 } from './utils/signature.js';
 import { computeCommitmentFromTxSummary, accountIdToHex } from './multisig/helpers.js';
 import { buildGuardianSignatureFromSigner } from './multisig/signing.js';
+import { assertMetadataMatchesSummary } from './multisig/summaryBinding.js';
 import { AccountInspector } from './inspector.js';
 import { ProposalFactory } from './proposal/factory.js';
 import { ProposalMetadataCodec } from './proposal/metadata.js';
@@ -1834,40 +1835,23 @@ export class Multisig {
     return txSummaryCommitment;
   }
 
+  /**
+   * Verify a proposal's integrity and return the tx_summary commitment cosigners
+   * sign. The id must equal the commitment of the stored tx_summary, and the
+   * human-readable metadata must match what the signed summary actually does.
+   *
+   * The metadata↔summary binding is done by decoding the signed summary and
+   * comparing its intent-bearing components (output/input notes, storage deltas)
+   * against the metadata — NOT by re-executing. Re-execution was block-height
+   * dependent (the fee is derived from the reference block and is part of the
+   * summary's account delta), so it falsely rejected honest proposals whenever a
+   * cosigner synced at a different height than the proposer. See
+   * {@link assertMetadataMatchesSummary}.
+   */
   private async verifyProposalMetadataBinding(proposal: Proposal): Promise<string> {
     const txSummaryCommitment = this.ensureProposalCommitmentMatchesSummary(proposal);
-    if (proposal.metadata.proposalType === 'custom') {
-      // Custom proposals (issue #266) have no per-type reconstruction recipe;
-      // the id ↔ tx_summary commitment match above is the only available
-      // integrity guarantee for an opaque proposal.
-      return txSummaryCommitment;
-    }
-
-    if (proposal.metadata.proposalType === 'switch_guardian') {
-      // Exempt from binding re-execution (mirrors the `custom` exemption above).
-      // The WASM `executeForSummary` leaves the guardian-disabling side effect
-      // applied to the in-session account, so re-execution reconstructs a smaller
-      // delta and falsely rejects with "metadata does not match tx_summary". The
-      // native Rust client does not mutate, so this is an intentional divergence.
-      // The id ↔ tx_summary match above plus `verifyGuardianEndpointCommitment`
-      // at propose/execute time still bind the proposal.
-      return txSummaryCommitment;
-    }
-
     const summary = TransactionSummary.deserialize(base64ToUint8Array(proposal.txSummary));
-    const salt = proposal.metadata.saltHex
-      ? Word.fromHex(normalizeHexWord(proposal.metadata.saltHex))
-      : summary.salt();
-
-    const request = await this.buildTransactionRequestFromMetadata(proposal.metadata, salt);
-    const webClient = await this.getRawClient();
-    const reconstructed = await executeForSummary(webClient, this._accountId, request);
-    const reconstructedCommitment = normalizeHexWord(reconstructed.toCommitment().toHex());
-
-    if (reconstructedCommitment !== txSummaryCommitment) {
-      throw new Error(`Invalid proposal: metadata does not match tx_summary for ${proposal.id}`);
-    }
-
+    assertMetadataMatchesSummary(proposal.id, proposal.metadata, summary);
     return txSummaryCommitment;
   }