From 542237723105ef3570d522ccabf1d5889fc9c05e Mon Sep 17 00:00:00 2001 From: Wiktor Starczewski Date: Mon, 17 Aug 2026 14:15:52 +0200 Subject: [PATCH 1/6] fix(multisig-client): block-independent metadata<->tx_summary binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the block-height-dependent re-execution in verifyProposalMetadataBinding with a deterministic per-type decode-and-compare against the signed TransactionSummary (assertMetadataMatchesSummary): - p2id: rebuild the output-note recipient from metadata + the signed salt; require the summary's single output note to carry that recipient digest and a fungible asset of exactly the declared faucet + amount (read off the note, not the vault). - consume_notes: input-note id set equality. - add/remove/change_signer: exact [threshold, count] value slot, and no non-target signer commitment in the public-keys map delta. - update_procedure_threshold: procedure root -> [threshold,0,0,0] map entry. - switch_guardian / custom: exempt (bound elsewhere / opaque). None of these read the vault, so the fee (a block-dependent vault delta) never contaminates the comparison — fixing the cross-block liveness bug while keeping the what-you-see-is-what-you-sign guarantee. Requires @miden-sdk/miden-sdk 0.15.10 (AccountStorageDelta.valueDeltas()/maps()); dep bumped to ^0.15.10. --- packages/miden-multisig-client/package.json | 2 +- .../src/multisig.test.ts | 99 +++--- .../miden-multisig-client/src/multisig.ts | 46 +-- .../src/multisig/summaryBinding.test.ts | 304 ++++++++++++++++++ .../src/multisig/summaryBinding.ts | 241 ++++++++++++++ 5 files changed, 602 insertions(+), 90 deletions(-) create mode 100644 packages/miden-multisig-client/src/multisig/summaryBinding.test.ts create mode 100644 packages/miden-multisig-client/src/multisig/summaryBinding.ts diff --git a/packages/miden-multisig-client/package.json b/packages/miden-multisig-client/package.json index dde64048..16c0edf4 100644 --- a/packages/miden-multisig-client/package.json +++ b/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" diff --git a/packages/miden-multisig-client/src/multisig.test.ts b/packages/miden-multisig-client/src/multisig.test.ts index 7522dd56..449fbd4b 100644 --- a/packages/miden-multisig-client/src/multisig.test.ts +++ b/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, diff --git a/packages/miden-multisig-client/src/multisig.ts b/packages/miden-multisig-client/src/multisig.ts index 2cf17b67..88d860c0 100644 --- a/packages/miden-multisig-client/src/multisig.ts +++ b/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 { 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; } diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts new file mode 100644 index 00000000..75d3f00c --- /dev/null +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts @@ -0,0 +1,304 @@ +import { describe, it, expect } from 'vitest'; +import { + AccountId, + Felt, + FeltArray, + NoteId, + NoteRecipient, + NoteScript, + NoteStorage, + Word, +} from '@miden-sdk/miden-sdk'; + +import { assertMetadataMatchesSummary } from './summaryBinding.js'; +import { deriveP2idSerialNumber } from '../transaction/p2id.js'; +import { getProcedureRoot } from '../procedures.js'; +import { normalizeHexWord } from '../utils/encoding.js'; +import type { ProposalMetadata } from '../types/proposal.js'; + +const PROPOSAL_ID = '0x' + 'c'.repeat(64); +const SALT = Word.fromHex('0x' + 'ab'.repeat(32)); + +// Slot names must match src/account/masm/auth.ts. +const THRESHOLD_CONFIG_SLOT = 'openzeppelin::multisig::threshold_config'; +const SIGNER_PUBLIC_KEYS_SLOT = 'openzeppelin::multisig::signer_public_keys'; +const PROCEDURE_THRESHOLDS_SLOT = 'openzeppelin::multisig::procedure_thresholds'; + +// Two known-valid Miden account ids (arbitrary hex has an invalid version byte). +const ACCOUNT_1 = '0x69817bcc6fb9f99127c2245f6979c5'; +const ACCOUNT_2 = '0x79817bcc6fb9f99127c2245f6979ef'; +const FAUCET = AccountId.fromHex(ACCOUNT_2); + +interface SummaryParts { + salt?: Word; + outputNotes?: unknown[]; + inputNotes?: unknown[]; + valueDeltas?: { slotName: string; valueHex: string }[]; + maps?: { slotName: string; entries: { keyHex: string; valueHex: string }[] }[]; +} + +function mockSummary(parts: SummaryParts): any { + return { + salt: () => parts.salt ?? SALT, + outputNotes: () => ({ notes: () => parts.outputNotes ?? [] }), + inputNotes: () => ({ notes: () => parts.inputNotes ?? [] }), + accountDelta: () => ({ + storage: () => ({ + valueDeltas: () => + (parts.valueDeltas ?? []).map((v) => ({ + slotName: v.slotName, + value: { toHex: () => v.valueHex }, + })), + maps: () => + (parts.maps ?? []).map((m) => ({ + slotName: m.slotName, + entries: () => + m.entries.map((e) => ({ + key: { toHex: () => e.keyHex }, + value: { toHex: () => e.valueHex }, + })), + })), + }), + }), + }; +} + +function wordHex(felts: bigint[]): string { + return normalizeHexWord(Word.newFromFelts(felts.map((f) => new Felt(f))).toHex()); +} + +function p2idRecipientDigest(recipientId: string, salt: Word): string { + const recipient = AccountId.fromHex(recipientId); + const serialNum = deriveP2idSerialNumber(salt); + const noteRecipient = new NoteRecipient( + serialNum, + NoteScript.p2id(), + new NoteStorage(new FeltArray([recipient.suffix(), recipient.prefix()])), + ); + return normalizeHexWord(noteRecipient.digest().toHex()); +} + +function p2idOutputNote(recipientDigestHex: string, faucet: AccountId, amount: bigint): unknown { + return { + recipientDigest: () => ({ toHex: () => recipientDigestHex }), + assets: () => ({ + fungibleAssets: () => [{ faucetId: () => faucet, amount: () => amount }], + }), + }; +} + +function inputNote(idHex: string): unknown { + return { id: () => ({ toString: () => idHex }) }; +} + +function expectReject(metadata: ProposalMetadata, summary: any): void { + expect(() => assertMetadataMatchesSummary(PROPOSAL_ID, metadata, summary)).toThrow( + `Invalid proposal: metadata does not match tx_summary for ${PROPOSAL_ID}`, + ); +} + +function expectPass(metadata: ProposalMetadata, summary: any): void { + expect(() => assertMetadataMatchesSummary(PROPOSAL_ID, metadata, summary)).not.toThrow(); +} + +describe('assertMetadataMatchesSummary', () => { + describe('p2id', () => { + const recipientId = ACCOUNT_1; + const faucetId = FAUCET.toString(); + const metadata: ProposalMetadata = { + proposalType: 'p2id', + recipientId, + faucetId, + amount: '1000', + description: '', + }; + const digest = p2idRecipientDigest(recipientId, SALT); + + it('passes when the output note matches recipient, faucet and amount', () => { + expectPass(metadata, mockSummary({ outputNotes: [p2idOutputNote(digest, FAUCET, 1000n)] })); + }); + + it('rejects a different recipient (mislabel)', () => { + const wrongDigest = p2idRecipientDigest(ACCOUNT_2, SALT); + expectReject(metadata, mockSummary({ outputNotes: [p2idOutputNote(wrongDigest, FAUCET, 1000n)] })); + }); + + it('rejects a different amount', () => { + expectReject(metadata, mockSummary({ outputNotes: [p2idOutputNote(digest, FAUCET, 9999n)] })); + }); + + it('rejects a different faucet', () => { + const otherFaucet = AccountId.fromHex(ACCOUNT_1); + expectReject(metadata, mockSummary({ outputNotes: [p2idOutputNote(digest, otherFaucet, 1000n)] })); + }); + + it('rejects an extra output note (unexpected value leaving)', () => { + const extra = p2idOutputNote(p2idRecipientDigest(ACCOUNT_2, SALT), FAUCET, 1n); + expectReject(metadata, mockSummary({ outputNotes: [p2idOutputNote(digest, FAUCET, 1000n), extra] })); + }); + + it('rejects when no output note is present', () => { + expectReject(metadata, mockSummary({ outputNotes: [] })); + }); + }); + + describe('consume_notes (v1)', () => { + const idA = NoteId.fromHex('0x' + '11'.repeat(32)).toString(); + const idB = NoteId.fromHex('0x' + '22'.repeat(32)).toString(); + const metadata: ProposalMetadata = { + proposalType: 'consume_notes', + noteIds: ['0x' + '11'.repeat(32), '0x' + '22'.repeat(32)], + description: '', + }; + + it('passes on exact set equality', () => { + expectPass(metadata, mockSummary({ inputNotes: [inputNote(idA), inputNote(idB)] })); + }); + + it('rejects an extra consumed note', () => { + const idC = NoteId.fromHex('0x' + '33'.repeat(32)).toString(); + expectReject(metadata, mockSummary({ inputNotes: [inputNote(idA), inputNote(idB), inputNote(idC)] })); + }); + + it('rejects a missing declared note', () => { + expectReject(metadata, mockSummary({ inputNotes: [inputNote(idA)] })); + }); + + it('rejects a substituted note', () => { + const idC = NoteId.fromHex('0x' + '33'.repeat(32)).toString(); + expectReject(metadata, mockSummary({ inputNotes: [inputNote(idA), inputNote(idC)] })); + }); + }); + + describe('add/remove/change_signer', () => { + const signers = ['0x' + '1'.repeat(64), '0x' + '2'.repeat(64)]; + const metadata: ProposalMetadata = { + proposalType: 'add_signer', + targetThreshold: 2, + targetSignerCommitments: signers, + description: '', + }; + const configHex = wordHex([2n, 2n, 0n, 0n]); + + function pubkeyMap(values: string[]) { + return [ + { + slotName: SIGNER_PUBLIC_KEYS_SLOT, + entries: values.map((v, i) => ({ keyHex: wordHex([BigInt(i), 0n, 0n, 0n]), valueHex: normalizeHexWord(v) })), + }, + ]; + } + + it('passes on matching threshold/count and target signer set', () => { + expectPass( + metadata, + mockSummary({ + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: configHex }], + maps: pubkeyMap(signers), + }), + ); + }); + + it('tolerates a zero-value (removal) map entry', () => { + expectPass( + metadata, + mockSummary({ + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: configHex }], + maps: [ + { + slotName: SIGNER_PUBLIC_KEYS_SLOT, + entries: [ + { keyHex: wordHex([0n, 0n, 0n, 0n]), valueHex: normalizeHexWord(signers[0]) }, + { keyHex: wordHex([1n, 0n, 0n, 0n]), valueHex: normalizeHexWord(signers[1]) }, + { keyHex: wordHex([2n, 0n, 0n, 0n]), valueHex: normalizeHexWord('0x' + '00'.repeat(32)) }, + ], + }, + ], + }), + ); + }); + + it('rejects a changed threshold', () => { + expectReject( + metadata, + mockSummary({ + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: wordHex([1n, 2n, 0n, 0n]) }], + maps: pubkeyMap(signers), + }), + ); + }); + + it('rejects a changed signer count', () => { + expectReject( + metadata, + mockSummary({ + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: wordHex([2n, 3n, 0n, 0n]) }], + maps: pubkeyMap(signers), + }), + ); + }); + + it('rejects an unlisted (attacker) signer in the pubkey map', () => { + const attacker = '0x' + '9'.repeat(64); + expectReject( + metadata, + mockSummary({ + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: configHex }], + maps: pubkeyMap([signers[0], attacker]), + }), + ); + }); + + it('rejects when the threshold-config value slot is absent', () => { + expectReject(metadata, mockSummary({ valueDeltas: [], maps: pubkeyMap(signers) })); + }); + }); + + describe('update_procedure_threshold', () => { + const metadata: ProposalMetadata = { + proposalType: 'update_procedure_threshold', + targetProcedure: 'send_asset', + targetThreshold: 3, + description: '', + }; + const procRoot = normalizeHexWord(getProcedureRoot('send_asset')); + const valueHex = wordHex([3n, 0n, 0n, 0n]); + + it('passes when the procedure root maps to the target threshold', () => { + expectPass( + metadata, + mockSummary({ maps: [{ slotName: PROCEDURE_THRESHOLDS_SLOT, entries: [{ keyHex: procRoot, valueHex }] }] }), + ); + }); + + it('rejects a different threshold value', () => { + expectReject( + metadata, + mockSummary({ + maps: [{ slotName: PROCEDURE_THRESHOLDS_SLOT, entries: [{ keyHex: procRoot, valueHex: wordHex([1n, 0n, 0n, 0n]) }] }], + }), + ); + }); + + it('rejects when the declared procedure is not in the delta', () => { + const otherRoot = normalizeHexWord(getProcedureRoot('receive_asset')); + expectReject( + metadata, + mockSummary({ maps: [{ slotName: PROCEDURE_THRESHOLDS_SLOT, entries: [{ keyHex: otherRoot, valueHex }] }] }), + ); + }); + }); + + describe('exempt types', () => { + it('never rejects custom proposals', () => { + expectPass({ proposalType: 'custom', rawProposalType: 'x', description: '' } as ProposalMetadata, mockSummary({})); + }); + + it('never rejects switch_guardian proposals', () => { + expectPass( + { proposalType: 'switch_guardian', newGuardianPubkey: '0x' + '1'.repeat(64), description: '' } as ProposalMetadata, + mockSummary({}), + ); + }); + }); +}); diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.ts new file mode 100644 index 00000000..33476e4e --- /dev/null +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.ts @@ -0,0 +1,241 @@ +import { + AccountId, + Felt, + FeltArray, + Note, + NoteId, + NoteRecipient, + NoteScript, + NoteStorage, + Word, +} from '@miden-sdk/miden-sdk'; +import type { TransactionSummary } from '@miden-sdk/miden-sdk'; + +import { getProcedureRoot } from '../procedures.js'; +import { deriveP2idSerialNumber } from '../transaction/p2id.js'; +import { noteFromBase64, normalizeHexWord } from '../utils/encoding.js'; +import { + isConsumeNotesV2, + type ConsumeNotesProposalMetadata, + type P2IdProposalMetadata, + type ProposalMetadata, + type UpdateProcedureThresholdProposalMetadata, + type UpdateSignersProposalMetadata, +} from '../types/proposal.js'; + +// Multisig auth-component storage slot names (see src/account/masm/auth.ts). +const THRESHOLD_CONFIG_SLOT_NAME = 'openzeppelin::multisig::threshold_config'; +const SIGNER_PUBLIC_KEYS_SLOT_NAME = 'openzeppelin::multisig::signer_public_keys'; +const PROCEDURE_THRESHOLDS_SLOT_NAME = 'openzeppelin::multisig::procedure_thresholds'; + +const ZERO_WORD_HEX = normalizeHexWord(`0x${'00'.repeat(32)}`); + +function reject(proposalId: string): never { + throw new Error(`Invalid proposal: metadata does not match tx_summary for ${proposalId}`); +} + +function wordFromFelts(felts: bigint[]): string { + return normalizeHexWord( + Word.newFromFelts(felts.map((f) => new Felt(f))).toHex(), + ); +} + +/** + * Assert that a proposal's human-readable `metadata` matches the transaction its + * cosigners actually sign (the `TransactionSummary`), WITHOUT re-executing. + * + * The previous implementation rebuilt the transaction from metadata and + * re-executed it (`executeForSummary`) to compare the resulting summary + * commitment. That is block-height dependent: execution charges a fee taken from + * the reference block's fee parameters, and the fee is part of the account delta + * the summary commitment covers. So the check only matched on the same client at + * the same sync height as the proposer, and a second signer at a later block hit + * "metadata does not match tx_summary" and could not load or sign. + * + * This instead decodes the signed summary and compares only the intent-bearing + * components against the metadata. None of those components is the fee (a native + * -asset vault delta that no check below reads), so the comparison is + * deterministic across clients and blocks. It preserves the what-you-see-is-what + * -you-sign guarantee (`docs/MULTISIG_SDK.md`): a mislabeled proposal — one whose + * metadata does not describe what the signed summary does — is rejected before a + * cosigner signs it. + * + * `switch_guardian` is bound separately by `verifyGuardianEndpointCommitment`, + * and `custom` proposals carry no metadata recipe; both are exempt here (the + * id ↔ tx_summary commitment check still applies to them). + */ +export function assertMetadataMatchesSummary( + proposalId: string, + metadata: ProposalMetadata, + summary: TransactionSummary, +): void { + switch (metadata.proposalType) { + case 'custom': + case 'switch_guardian': + return; + case 'p2id': + return assertP2idBinding(proposalId, metadata, summary); + case 'consume_notes': + return assertConsumeNotesBinding(proposalId, metadata, summary); + case 'add_signer': + case 'remove_signer': + case 'change_threshold': + return assertSignerBinding(proposalId, metadata, summary); + case 'update_procedure_threshold': + return assertProcedureThresholdBinding(proposalId, metadata, summary); + } +} + +/** + * p2id: the proposal emits exactly one output note. Rebuild that note's + * recipient from metadata + the SIGNED summary's salt (so a tampered + * `metadata.saltHex` cannot force a match) and require the summary's single + * output note to carry that recipient digest and a fungible asset of exactly the + * declared faucet + amount. The asset is read off the note (not the vault), so + * the fee never contaminates the comparison. + */ +function assertP2idBinding( + proposalId: string, + metadata: P2IdProposalMetadata, + summary: TransactionSummary, +): void { + const recipient = AccountId.fromHex(metadata.recipientId); + const serialNum = deriveP2idSerialNumber(summary.salt()); + const noteRecipient = new NoteRecipient( + serialNum, + NoteScript.p2id(), + new NoteStorage(new FeltArray([recipient.suffix(), recipient.prefix()])), + ); + const expectedRecipientDigest = normalizeHexWord(noteRecipient.digest().toHex()); + + const outputs = summary.outputNotes().notes(); + // Exactly one own output note; extra output notes mean value leaving the + // account that the metadata does not describe. + if (outputs.length !== 1) { + reject(proposalId); + } + const note = outputs[0]; + if (normalizeHexWord(note.recipientDigest().toHex()) !== expectedRecipientDigest) { + reject(proposalId); + } + + const expectedFaucet = AccountId.fromHex(metadata.faucetId).toString(); + const expectedAmount = BigInt(metadata.amount); + const asset = note + .assets() + ?.fungibleAssets() + .find((a) => a.faucetId().toString() === expectedFaucet && a.amount() === expectedAmount); + if (!asset) { + reject(proposalId); + } +} + +/** + * consume_notes: the declared note-id set must equal the summary's input-note-id + * set. Set equality (not membership) so the transaction cannot consume an extra, + * undeclared note. Note ids are the note commitments (hash of details+metadata) + * and are stable whether or not the note is authenticated, so this is + * proof-agnostic. + */ +function assertConsumeNotesBinding( + proposalId: string, + metadata: ConsumeNotesProposalMetadata, + summary: TransactionSummary, +): void { + const declared = new Set(declaredConsumeNoteIds(metadata)); + const actual = new Set( + summary + .inputNotes() + .notes() + .map((n) => n.id().toString()), + ); + if (declared.size !== actual.size) { + reject(proposalId); + } + for (const id of declared) { + if (!actual.has(id)) { + reject(proposalId); + } + } +} + +function declaredConsumeNoteIds(metadata: ConsumeNotesProposalMetadata): string[] { + if (isConsumeNotesV2(metadata) && metadata.notes) { + return metadata.notes.map((b64) => noteFromBase64(b64, Note).id().toString()); + } + // Canonicalize v1 hex ids through NoteId so they compare equal to the + // summary's `NoteId.toString()` regardless of casing / 0x padding. + return metadata.noteIds.map((id) => NoteId.fromHex(id).toString()); +} + +/** + * add/remove/change_signer: bind the threshold + signer count exactly (the + * `[threshold, count, 0, 0]` word written to the threshold-config value slot), + * and forbid any signer commitment appearing in the public-keys map delta that + * is not in the declared target set. Together these reject the dangerous + * mislabels — a changed threshold, a changed signer count, or an unlisted signer + * (e.g. an attacker) being added — while tolerating the delta-shape differences + * between add/remove/threshold-only operations (removals appear as zero-value map + * entries; a threshold-only change leaves the public-keys map untouched). The + * account's on-chain config-hash check is the backstop for the full signer set. + */ +function assertSignerBinding( + proposalId: string, + metadata: UpdateSignersProposalMetadata, + summary: TransactionSummary, +): void { + const storage = summary.accountDelta().storage(); + + const expectedConfig = wordFromFelts([ + BigInt(metadata.targetThreshold), + BigInt(metadata.targetSignerCommitments.length), + 0n, + 0n, + ]); + const configDelta = storage + .valueDeltas() + .find((v) => v.slotName === THRESHOLD_CONFIG_SLOT_NAME); + if (!configDelta || normalizeHexWord(configDelta.value.toHex()) !== expectedConfig) { + reject(proposalId); + } + + const targetCommitments = new Set( + metadata.targetSignerCommitments.map((c) => normalizeHexWord(c)), + ); + const pubkeyMap = storage.maps().find((m) => m.slotName === SIGNER_PUBLIC_KEYS_SLOT_NAME); + if (pubkeyMap) { + for (const entry of pubkeyMap.entries()) { + const value = normalizeHexWord(entry.value.toHex()); + // Zero-value entries are removals; every non-zero (added/kept) public key + // must be one of the declared target signers. + if (value !== ZERO_WORD_HEX && !targetCommitments.has(value)) { + reject(proposalId); + } + } + } +} + +/** + * update_procedure_threshold: the procedure-thresholds map delta must set the + * declared procedure's root to `[threshold, 0, 0, 0]`. + */ +function assertProcedureThresholdBinding( + proposalId: string, + metadata: UpdateProcedureThresholdProposalMetadata, + summary: TransactionSummary, +): void { + const expectedKey = normalizeHexWord(getProcedureRoot(metadata.targetProcedure)); + const expectedValue = wordFromFelts([BigInt(metadata.targetThreshold), 0n, 0n, 0n]); + + const procMap = summary + .accountDelta() + .storage() + .maps() + .find((m) => m.slotName === PROCEDURE_THRESHOLDS_SLOT_NAME); + const entry = procMap + ?.entries() + .find((e) => normalizeHexWord(e.key.toHex()) === expectedKey); + if (!entry || normalizeHexWord(entry.value.toHex()) !== expectedValue) { + reject(proposalId); + } +} From b1eca95afdb4abf9fdb560aa1a03efdcfde485c7 Mon Sep 17 00:00:00 2001 From: Wiktor Starczewski Date: Mon, 17 Aug 2026 14:20:07 +0200 Subject: [PATCH 2/6] fix(multisig-client): fail closed on unhandled proposal type in summary binding --- .../miden-multisig-client/src/multisig/summaryBinding.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.ts index 33476e4e..ced9510e 100644 --- a/packages/miden-multisig-client/src/multisig/summaryBinding.ts +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.ts @@ -83,6 +83,15 @@ export function assertMetadataMatchesSummary( return assertSignerBinding(proposalId, metadata, summary); case 'update_procedure_threshold': return assertProcedureThresholdBinding(proposalId, metadata, summary); + default: { + // Fail closed: a proposal type without a binding recipe must not silently + // pass. The union is exhaustive, so this is also a compile-time guard that + // any new built-in type is given an explicit binding (or an explicit + // exemption above) before it can be signed. + const _exhaustive: never = metadata; + void _exhaustive; + reject(proposalId); + } } } From 8b8b41b2488373e2d684e49fdc089956a4d7d9fd Mon Sep 17 00:00:00 2001 From: Wiktor Starczewski Date: Mon, 17 Aug 2026 14:22:21 +0200 Subject: [PATCH 3/6] fix(multisig-client): p2id binding requires exactly the declared asset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The output-note asset check used find(), which passed as long as the declared asset was present — an attacker could attach extra assets to the note, draining more than the cosigner approved. Require exactly one fungible asset matching the declared faucet+amount. --- .../src/multisig/summaryBinding.test.ts | 15 +++++++++++++++ .../src/multisig/summaryBinding.ts | 14 +++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts index 75d3f00c..7fe48ba3 100644 --- a/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts @@ -140,6 +140,21 @@ describe('assertMetadataMatchesSummary', () => { it('rejects when no output note is present', () => { expectReject(metadata, mockSummary({ outputNotes: [] })); }); + + it('rejects a note carrying an extra asset beyond the declared one', () => { + // Correct recipient + declared asset, but an additional asset would leave + // the account beyond what the metadata describes. + const noteWithExtra = { + recipientDigest: () => ({ toHex: () => digest }), + assets: () => ({ + fungibleAssets: () => [ + { faucetId: () => FAUCET, amount: () => 1000n }, + { faucetId: () => AccountId.fromHex(ACCOUNT_1), amount: () => 500n }, + ], + }), + }; + expectReject(metadata, mockSummary({ outputNotes: [noteWithExtra] })); + }); }); describe('consume_notes (v1)', () => { diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.ts index ced9510e..910b6bd1 100644 --- a/packages/miden-multisig-client/src/multisig/summaryBinding.ts +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.ts @@ -130,11 +130,15 @@ function assertP2idBinding( const expectedFaucet = AccountId.fromHex(metadata.faucetId).toString(); const expectedAmount = BigInt(metadata.amount); - const asset = note - .assets() - ?.fungibleAssets() - .find((a) => a.faucetId().toString() === expectedFaucet && a.amount() === expectedAmount); - if (!asset) { + const assets = note.assets()?.fungibleAssets() ?? []; + // The note must carry EXACTLY the one declared asset. Requiring a single + // matching asset (not merely "contains one") stops an attacker from attaching + // extra assets that would leave the account beyond what the metadata describes. + if ( + assets.length !== 1 || + assets[0].faucetId().toString() !== expectedFaucet || + assets[0].amount() !== expectedAmount + ) { reject(proposalId); } } From 3c0dbb3d97a215f3914d0a28896fc13cf5117acc Mon Sep 17 00:00:00 2001 From: Wiktor Starczewski Date: Mon, 17 Aug 2026 15:11:40 +0200 Subject: [PATCH 4/6] fix(multisig-client): make summary binding exhaustive across all dimensions The per-type binding was a field allowlist that ignored whole summary dimensions, enabling piggyback fund-theft (e.g. a consume_notes summary that also emits a drain output note, or a p2id that also rewrites the signer set). Now each type asserts the transaction touches ONLY the declared dimensions + the fee: output notes exactly the declared set, input notes exactly the declared set, and storage-slot deltas confined to the allowed slots (with index-bound signer checks). switch_guardian is now bound (guardian pubkey + no other effects) instead of exempt. Residual (documented): SDK exposes only fungible note/vault assets (NFT piggyback on a p2id note is invisible); signer check binds changed indices (a real-summary integration test must confirm delta completeness). --- .../src/multisig/summaryBinding.test.ts | 169 +++++++++- .../src/multisig/summaryBinding.ts | 317 +++++++++++++----- 2 files changed, 401 insertions(+), 85 deletions(-) diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts index 7fe48ba3..a3789580 100644 --- a/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts @@ -23,6 +23,7 @@ const SALT = Word.fromHex('0x' + 'ab'.repeat(32)); const THRESHOLD_CONFIG_SLOT = 'openzeppelin::multisig::threshold_config'; const SIGNER_PUBLIC_KEYS_SLOT = 'openzeppelin::multisig::signer_public_keys'; const PROCEDURE_THRESHOLDS_SLOT = 'openzeppelin::multisig::procedure_thresholds'; +const GUARDIAN_PUBLIC_KEY_SLOT = 'openzeppelin::guardian::public_key'; // Two known-valid Miden account ids (arbitrary hex has an invalid version byte). const ACCOUNT_1 = '0x69817bcc6fb9f99127c2245f6979c5'; @@ -40,8 +41,14 @@ interface SummaryParts { function mockSummary(parts: SummaryParts): any { return { salt: () => parts.salt ?? SALT, - outputNotes: () => ({ notes: () => parts.outputNotes ?? [] }), - inputNotes: () => ({ notes: () => parts.inputNotes ?? [] }), + outputNotes: () => ({ + notes: () => parts.outputNotes ?? [], + numNotes: () => (parts.outputNotes ?? []).length, + }), + inputNotes: () => ({ + notes: () => parts.inputNotes ?? [], + numNotes: () => (parts.inputNotes ?? []).length, + }), accountDelta: () => ({ storage: () => ({ valueDeltas: () => @@ -155,6 +162,43 @@ describe('assertMetadataMatchesSummary', () => { }; expectReject(metadata, mockSummary({ outputNotes: [noteWithExtra] })); }); + + it('rejects a piggybacked value-slot write (threshold takeover)', () => { + // Correct p2id note, but the summary ALSO rewrites the threshold config. + expectReject( + metadata, + mockSummary({ + outputNotes: [p2idOutputNote(digest, FAUCET, 1000n)], + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: wordHex([1n, 1n, 0n, 0n]) }], + }), + ); + }); + + it('rejects a piggybacked signer-map write (signer-set takeover)', () => { + // Correct p2id note, but the summary ALSO installs an attacker signer. + expectReject( + metadata, + mockSummary({ + outputNotes: [p2idOutputNote(digest, FAUCET, 1000n)], + maps: [ + { + slotName: SIGNER_PUBLIC_KEYS_SLOT, + entries: [{ keyHex: wordHex([0n, 0n, 0n, 0n]), valueHex: normalizeHexWord('0x' + '9'.repeat(64)) }], + }, + ], + }), + ); + }); + + it('rejects a piggybacked input note', () => { + expectReject( + metadata, + mockSummary({ + outputNotes: [p2idOutputNote(digest, FAUCET, 1000n)], + inputNotes: [inputNote(NoteId.fromHex('0x' + '44'.repeat(32)).toString())], + }), + ); + }); }); describe('consume_notes (v1)', () => { @@ -183,6 +227,25 @@ describe('assertMetadataMatchesSummary', () => { const idC = NoteId.fromHex('0x' + '33'.repeat(32)).toString(); expectReject(metadata, mockSummary({ inputNotes: [inputNote(idA), inputNote(idC)] })); }); + + it('rejects a consume that also emits an output note (drain)', () => { + // Declared input set matches, but the summary ALSO sends value out. + const drain = p2idOutputNote(p2idRecipientDigest(ACCOUNT_2, SALT), FAUCET, 1_000_000n); + expectReject( + metadata, + mockSummary({ inputNotes: [inputNote(idA), inputNote(idB)], outputNotes: [drain] }), + ); + }); + + it('rejects a consume that also writes storage (takeover)', () => { + expectReject( + metadata, + mockSummary({ + inputNotes: [inputNote(idA), inputNote(idB)], + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: wordHex([1n, 1n, 0n, 0n]) }], + }), + ); + }); }); describe('add/remove/change_signer', () => { @@ -267,6 +330,43 @@ describe('assertMetadataMatchesSummary', () => { it('rejects when the threshold-config value slot is absent', () => { expectReject(metadata, mockSummary({ valueDeltas: [], maps: pubkeyMap(signers) })); }); + + it('rejects a duplicated declared signer with an omitted one', () => { + // Declared {A,B}; summary writes index 0->A and index 1->A (B omitted, A + // duplicated). Index 1's value (A) is not the declared signer at index 1 (B). + expectReject( + metadata, + mockSummary({ + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: configHex }], + maps: pubkeyMap([signers[0], signers[0]]), + }), + ); + }); + + it('rejects a signer proposal that also emits an output note (drain)', () => { + const drain = p2idOutputNote(p2idRecipientDigest(ACCOUNT_2, SALT), FAUCET, 1_000_000n); + expectReject( + metadata, + mockSummary({ + outputNotes: [drain], + valueDeltas: [{ slotName: THRESHOLD_CONFIG_SLOT, valueHex: configHex }], + maps: pubkeyMap(signers), + }), + ); + }); + + it('rejects a signer proposal that writes an unexpected extra slot', () => { + expectReject( + metadata, + mockSummary({ + valueDeltas: [ + { slotName: THRESHOLD_CONFIG_SLOT, valueHex: configHex }, + { slotName: PROCEDURE_THRESHOLDS_SLOT, valueHex: wordHex([9n, 0n, 0n, 0n]) }, + ], + maps: pubkeyMap(signers), + }), + ); + }); }); describe('update_procedure_threshold', () => { @@ -302,18 +402,69 @@ describe('assertMetadataMatchesSummary', () => { mockSummary({ maps: [{ slotName: PROCEDURE_THRESHOLDS_SLOT, entries: [{ keyHex: otherRoot, valueHex }] }] }), ); }); + + it('rejects a procedure-threshold proposal that also emits an output note', () => { + const drain = p2idOutputNote(p2idRecipientDigest(ACCOUNT_2, SALT), FAUCET, 1_000_000n); + expectReject( + metadata, + mockSummary({ + outputNotes: [drain], + maps: [{ slotName: PROCEDURE_THRESHOLDS_SLOT, entries: [{ keyHex: procRoot, valueHex }] }], + }), + ); + }); }); - describe('exempt types', () => { - it('never rejects custom proposals', () => { - expectPass({ proposalType: 'custom', rawProposalType: 'x', description: '' } as ProposalMetadata, mockSummary({})); + describe('switch_guardian', () => { + const newGuardianPubkey = '0x' + '7'.repeat(64); + const metadata: ProposalMetadata = { + proposalType: 'switch_guardian', + newGuardianPubkey, + description: '', + }; + const guardianMap = (pubkey: string) => [ + { + slotName: GUARDIAN_PUBLIC_KEY_SLOT, + entries: [{ keyHex: wordHex([0n, 0n, 0n, 0n]), valueHex: normalizeHexWord(pubkey) }], + }, + ]; + + it('passes when the guardian public-key map is set to the declared key', () => { + expectPass(metadata, mockSummary({ maps: guardianMap(newGuardianPubkey) })); }); - it('never rejects switch_guardian proposals', () => { - expectPass( - { proposalType: 'switch_guardian', newGuardianPubkey: '0x' + '1'.repeat(64), description: '' } as ProposalMetadata, - mockSummary({}), + it('rejects a different guardian public key in the summary', () => { + expectReject(metadata, mockSummary({ maps: guardianMap('0x' + '8'.repeat(64)) })); + }); + + it('rejects when no guardian public-key delta is present', () => { + expectReject(metadata, mockSummary({})); + }); + + it('rejects a switch_guardian that also emits a drain output note', () => { + const drain = p2idOutputNote(p2idRecipientDigest(ACCOUNT_2, SALT), FAUCET, 1_000_000n); + expectReject(metadata, mockSummary({ outputNotes: [drain], maps: guardianMap(newGuardianPubkey) })); + }); + + it('rejects a switch_guardian that piggybacks a signer-set write', () => { + expectReject( + metadata, + mockSummary({ + maps: [ + ...guardianMap(newGuardianPubkey), + { + slotName: SIGNER_PUBLIC_KEYS_SLOT, + entries: [{ keyHex: wordHex([0n, 0n, 0n, 0n]), valueHex: normalizeHexWord('0x' + '9'.repeat(64)) }], + }, + ], + }), ); }); }); + + describe('custom', () => { + it('is exempt (no reconstruction recipe; WYSIWYS does not hold)', () => { + expectPass({ proposalType: 'custom', rawProposalType: 'x', description: '' } as ProposalMetadata, mockSummary({})); + }); + }); }); diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.ts index 910b6bd1..302ac920 100644 --- a/packages/miden-multisig-client/src/multisig/summaryBinding.ts +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.ts @@ -19,14 +19,22 @@ import { type ConsumeNotesProposalMetadata, type P2IdProposalMetadata, type ProposalMetadata, + type SwitchGuardianProposalMetadata, type UpdateProcedureThresholdProposalMetadata, type UpdateSignersProposalMetadata, } from '../types/proposal.js'; -// Multisig auth-component storage slot names (see src/account/masm/auth.ts). -const THRESHOLD_CONFIG_SLOT_NAME = 'openzeppelin::multisig::threshold_config'; -const SIGNER_PUBLIC_KEYS_SLOT_NAME = 'openzeppelin::multisig::signer_public_keys'; -const PROCEDURE_THRESHOLDS_SLOT_NAME = 'openzeppelin::multisig::procedure_thresholds'; +// Storage slot names (see src/account/masm/auth.ts). +const THRESHOLD_CONFIG_SLOT = 'openzeppelin::multisig::threshold_config'; +const SIGNER_PUBLIC_KEYS_SLOT = 'openzeppelin::multisig::signer_public_keys'; +const SIGNER_SCHEME_IDS_SLOT = 'openzeppelin::multisig::signer_scheme_ids'; +const PROCEDURE_THRESHOLDS_SLOT = 'openzeppelin::multisig::procedure_thresholds'; +// Every multisig transaction writes this map (a per-tx replay marker), so it is +// always an allowed storage slot. +const EXECUTED_TXS_SLOT = 'openzeppelin::multisig::executed_transactions'; +const GUARDIAN_SELECTOR_SLOT = 'openzeppelin::guardian::selector'; +const GUARDIAN_PUBLIC_KEY_SLOT = 'openzeppelin::guardian::public_key'; +const GUARDIAN_SCHEME_ID_SLOT = 'openzeppelin::guardian::scheme_id'; const ZERO_WORD_HEX = normalizeHexWord(`0x${'00'.repeat(32)}`); @@ -34,10 +42,72 @@ function reject(proposalId: string): never { throw new Error(`Invalid proposal: metadata does not match tx_summary for ${proposalId}`); } -function wordFromFelts(felts: bigint[]): string { - return normalizeHexWord( - Word.newFromFelts(felts.map((f) => new Felt(f))).toHex(), - ); +function wordHexFromFelts(felts: bigint[]): string { + return normalizeHexWord(Word.newFromFelts(felts.map((f) => new Felt(f))).toHex()); +} + +// A shared view of the storage delta, plus the slot-scoping helpers used by every +// per-type check to prove the transaction touches ONLY the slots that type may. +type StorageDelta = ReturnType['storage']>; + +function storageOf(summary: TransactionSummary): StorageDelta { + return summary.accountDelta().storage(); +} + +/** No output notes: nothing leaves the account (beyond the fee, a vault delta). */ +function assertNoOutputNotes(proposalId: string, summary: TransactionSummary): void { + if (summary.outputNotes().numNotes() !== 0) { + reject(proposalId); + } +} + +/** No input notes: the transaction consumes nothing. */ +function assertNoInputNotes(proposalId: string, summary: TransactionSummary): void { + if (summary.inputNotes().numNotes() !== 0) { + reject(proposalId); + } +} + +/** + * Every storage slot the transaction changed (value slots and map slots) must be + * in `allowed`. This is the load-bearing exhaustiveness check: it stops an + * attacker from piggybacking an undeclared storage effect (e.g. rewriting the + * signer set inside a "p2id" proposal). + */ +function assertStorageSlotsWithin( + proposalId: string, + storage: StorageDelta, + allowed: ReadonlySet, +): void { + for (const v of storage.valueDeltas()) { + if (!allowed.has(v.slotName)) { + reject(proposalId); + } + } + for (const m of storage.maps()) { + if (!allowed.has(m.slotName)) { + reject(proposalId); + } + } +} + +function valueDeltaFor(storage: StorageDelta, slotName: string): string | undefined { + const delta = storage.valueDeltas().find((v) => v.slotName === slotName); + return delta ? normalizeHexWord(delta.value.toHex()) : undefined; +} + +function mapEntriesFor( + storage: StorageDelta, + slotName: string, +): Array<{ key: string; value: string }> { + const map = storage.maps().find((m) => m.slotName === slotName); + if (!map) { + return []; + } + return map.entries().map((e) => ({ + key: normalizeHexWord(e.key.toHex()), + value: normalizeHexWord(e.value.toHex()), + })); } /** @@ -52,17 +122,25 @@ function wordFromFelts(felts: bigint[]): string { * the same sync height as the proposer, and a second signer at a later block hit * "metadata does not match tx_summary" and could not load or sign. * - * This instead decodes the signed summary and compares only the intent-bearing - * components against the metadata. None of those components is the fee (a native - * -asset vault delta that no check below reads), so the comparison is - * deterministic across clients and blocks. It preserves the what-you-see-is-what - * -you-sign guarantee (`docs/MULTISIG_SDK.md`): a mislabeled proposal — one whose - * metadata does not describe what the signed summary does — is rejected before a + * This instead decodes the signed summary and, for each type, asserts the + * transaction's effects are EXACTLY the declared effect plus the fee — across + * every intent-bearing dimension of the summary: its output notes, its input + * notes, and the set of account-storage slots it changes. It never reads the + * vault delta (the fee is a native-asset vault delta, block-dependent), which is + * safe because assets can only leave the account through an output note or the + * fee: binding the output/input notes and the storage slots exactly makes the + * vault delta a consequence. So the comparison is deterministic across clients + * and blocks yet still what-you-see-is-what-you-sign — a mislabeled proposal, or + * one that piggybacks an undeclared effect in any dimension, is rejected before a * cosigner signs it. * - * `switch_guardian` is bound separately by `verifyGuardianEndpointCommitment`, - * and `custom` proposals carry no metadata recipe; both are exempt here (the - * id ↔ tx_summary commitment check still applies to them). + * Residual limitations (see the type comments): the SDK exposes only FUNGIBLE + * note/vault assets, so a non-fungible asset attached to an otherwise-correct + * p2id note cannot be detected here; and the signer check binds by storage-map + * index over the CHANGED entries (a real-summary integration test should confirm + * the delta carries every changed index). `custom` proposals carry no metadata + * recipe, so WYSIWYS does not hold for them — a cosigner must verify the raw + * `tx_summary`, never trust a `custom` proposal's `description`. */ export function assertMetadataMatchesSummary( proposalId: string, @@ -71,7 +149,8 @@ export function assertMetadataMatchesSummary( ): void { switch (metadata.proposalType) { case 'custom': - case 'switch_guardian': + // No reconstruction recipe; the id <-> tx_summary commitment check is the + // only guarantee. WYSIWYS does NOT hold for custom proposals. return; case 'p2id': return assertP2idBinding(proposalId, metadata, summary); @@ -83,11 +162,13 @@ export function assertMetadataMatchesSummary( return assertSignerBinding(proposalId, metadata, summary); case 'update_procedure_threshold': return assertProcedureThresholdBinding(proposalId, metadata, summary); + case 'switch_guardian': + return assertSwitchGuardianBinding(proposalId, metadata, summary); default: { // Fail closed: a proposal type without a binding recipe must not silently // pass. The union is exhaustive, so this is also a compile-time guard that - // any new built-in type is given an explicit binding (or an explicit - // exemption above) before it can be signed. + // any new built-in type is given an explicit binding before it can be + // signed. const _exhaustive: never = metadata; void _exhaustive; reject(proposalId); @@ -96,12 +177,15 @@ export function assertMetadataMatchesSummary( } /** - * p2id: the proposal emits exactly one output note. Rebuild that note's - * recipient from metadata + the SIGNED summary's salt (so a tampered - * `metadata.saltHex` cannot force a match) and require the summary's single - * output note to carry that recipient digest and a fungible asset of exactly the - * declared faucet + amount. The asset is read off the note (not the vault), so - * the fee never contaminates the comparison. + * p2id: exactly one output note carrying the declared recipient and exactly the + * declared fungible asset; no input notes; no storage change other than the + * per-tx executed-transactions marker. The recipient is rebuilt from metadata + + * the SIGNED summary's salt (so a tampered `metadata.saltHex` cannot force a + * match); the asset is read off the note (not the vault), so the block-dependent + * fee never contaminates the comparison. + * + * Residual: `NoteAssets` exposes only fungible assets, so a non-fungible asset + * attached to this note is invisible to the SDK and cannot be bound here. */ function assertP2idBinding( proposalId: string, @@ -118,8 +202,6 @@ function assertP2idBinding( const expectedRecipientDigest = normalizeHexWord(noteRecipient.digest().toHex()); const outputs = summary.outputNotes().notes(); - // Exactly one own output note; extra output notes mean value leaving the - // account that the metadata does not describe. if (outputs.length !== 1) { reject(proposalId); } @@ -131,9 +213,6 @@ function assertP2idBinding( const expectedFaucet = AccountId.fromHex(metadata.faucetId).toString(); const expectedAmount = BigInt(metadata.amount); const assets = note.assets()?.fungibleAssets() ?? []; - // The note must carry EXACTLY the one declared asset. Requiring a single - // matching asset (not merely "contains one") stops an attacker from attaching - // extra assets that would leave the account beyond what the metadata describes. if ( assets.length !== 1 || assets[0].faucetId().toString() !== expectedFaucet || @@ -141,14 +220,17 @@ function assertP2idBinding( ) { reject(proposalId); } + + assertNoInputNotes(proposalId, summary); + assertStorageSlotsWithin(proposalId, storageOf(summary), new Set([EXECUTED_TXS_SLOT])); } /** - * consume_notes: the declared note-id set must equal the summary's input-note-id - * set. Set equality (not membership) so the transaction cannot consume an extra, - * undeclared note. Note ids are the note commitments (hash of details+metadata) - * and are stable whether or not the note is authenticated, so this is - * proof-agnostic. + * consume_notes: input-note-id set exactly equals the declared set; NO output + * notes (a consume that also emitted an output note would drain value the + * metadata does not describe); no storage change other than the executed-tx + * marker. Note ids are content commitments, stable across authentication, so the + * comparison is proof-agnostic. */ function assertConsumeNotesBinding( proposalId: string, @@ -170,6 +252,9 @@ function assertConsumeNotesBinding( reject(proposalId); } } + + assertNoOutputNotes(proposalId, summary); + assertStorageSlotsWithin(proposalId, storageOf(summary), new Set([EXECUTED_TXS_SLOT])); } function declaredConsumeNoteIds(metadata: ConsumeNotesProposalMetadata): string[] { @@ -182,73 +267,153 @@ function declaredConsumeNoteIds(metadata: ConsumeNotesProposalMetadata): string[ } /** - * add/remove/change_signer: bind the threshold + signer count exactly (the - * `[threshold, count, 0, 0]` word written to the threshold-config value slot), - * and forbid any signer commitment appearing in the public-keys map delta that - * is not in the declared target set. Together these reject the dangerous - * mislabels — a changed threshold, a changed signer count, or an unlisted signer - * (e.g. an attacker) being added — while tolerating the delta-shape differences - * between add/remove/threshold-only operations (removals appear as zero-value map - * entries; a threshold-only change leaves the public-keys map untouched). The - * account's on-chain config-hash check is the backstop for the full signer set. + * add/remove/change_signer: no notes; the only value slot changed is + * `threshold_config` set to `[threshold, count, 0, 0]`; the only map slots + * changed are the signer public-keys / scheme-ids (and the executed-tx marker); + * and every changed public-keys entry binds by INDEX to the declared signer set. + * + * The MASM writes signer `j` to map key `[j,0,0,0]` (auth.ts + * `update_signers_and_threshold`), and `cleanup_pubkey_mapping` zeroes stale + * higher indices. So a changed entry at index `j` must equal the declared + * commitment at `j` (or be a zero-value removal at an index >= count). This is + * stronger than a subset check: it catches a duplicated signer (same key at two + * indices) or an omitted one. Residual: the storage delta carries only CHANGED + * indices, so an unchanged index is not re-verified here — a real-summary + * integration test should confirm the delta covers every changed index. */ function assertSignerBinding( proposalId: string, metadata: UpdateSignersProposalMetadata, summary: TransactionSummary, ): void { - const storage = summary.accountDelta().storage(); + assertNoOutputNotes(proposalId, summary); + assertNoInputNotes(proposalId, summary); + + const storage = storageOf(summary); + assertStorageSlotsWithin( + proposalId, + storage, + new Set([THRESHOLD_CONFIG_SLOT, SIGNER_PUBLIC_KEYS_SLOT, SIGNER_SCHEME_IDS_SLOT, EXECUTED_TXS_SLOT]), + ); + assertSignerConfigAndKeys(proposalId, storage, metadata.targetThreshold, metadata.targetSignerCommitments); +} - const expectedConfig = wordFromFelts([ - BigInt(metadata.targetThreshold), - BigInt(metadata.targetSignerCommitments.length), +/** + * Shared signer-config check: the `threshold_config` value slot equals + * `[threshold, count, 0, 0]`, and each changed `signer_public_keys` entry binds + * by index to the declared commitment set. Used by the signer bindings and by a + * `switch_guardian` proposal that also rotates the signer set. + */ +function assertSignerConfigAndKeys( + proposalId: string, + storage: StorageDelta, + targetThreshold: number, + targetSignerCommitments: string[], +): void { + const expectedConfig = wordHexFromFelts([ + BigInt(targetThreshold), + BigInt(targetSignerCommitments.length), 0n, 0n, ]); - const configDelta = storage - .valueDeltas() - .find((v) => v.slotName === THRESHOLD_CONFIG_SLOT_NAME); - if (!configDelta || normalizeHexWord(configDelta.value.toHex()) !== expectedConfig) { + if (valueDeltaFor(storage, THRESHOLD_CONFIG_SLOT) !== expectedConfig) { reject(proposalId); } - const targetCommitments = new Set( - metadata.targetSignerCommitments.map((c) => normalizeHexWord(c)), - ); - const pubkeyMap = storage.maps().find((m) => m.slotName === SIGNER_PUBLIC_KEYS_SLOT_NAME); - if (pubkeyMap) { - for (const entry of pubkeyMap.entries()) { - const value = normalizeHexWord(entry.value.toHex()); - // Zero-value entries are removals; every non-zero (added/kept) public key - // must be one of the declared target signers. - if (value !== ZERO_WORD_HEX && !targetCommitments.has(value)) { + // Expected value at each map index key [i,0,0,0]. + const expectedByKey = new Map(); + targetSignerCommitments.forEach((commitment, i) => { + expectedByKey.set(wordHexFromFelts([BigInt(i), 0n, 0n, 0n]), normalizeHexWord(commitment)); + }); + + for (const entry of mapEntriesFor(storage, SIGNER_PUBLIC_KEYS_SLOT)) { + const expected = expectedByKey.get(entry.key); + if (expected !== undefined) { + // A declared index: the written key must be exactly the declared signer. + if (entry.value !== expected) { reject(proposalId); } + } else if (entry.value !== ZERO_WORD_HEX) { + // An index outside the declared set may only be a zero-value removal. + reject(proposalId); } } } /** - * update_procedure_threshold: the procedure-thresholds map delta must set the - * declared procedure's root to `[threshold, 0, 0, 0]`. + * update_procedure_threshold: no notes; no value-slot change; the only map slots + * changed are `procedure_thresholds` (and the executed-tx marker), and it must + * set the declared procedure's root to `[threshold, 0, 0, 0]`. */ function assertProcedureThresholdBinding( proposalId: string, metadata: UpdateProcedureThresholdProposalMetadata, summary: TransactionSummary, ): void { + assertNoOutputNotes(proposalId, summary); + assertNoInputNotes(proposalId, summary); + + const storage = storageOf(summary); + assertStorageSlotsWithin( + proposalId, + storage, + new Set([PROCEDURE_THRESHOLDS_SLOT, EXECUTED_TXS_SLOT]), + ); + const expectedKey = normalizeHexWord(getProcedureRoot(metadata.targetProcedure)); - const expectedValue = wordFromFelts([BigInt(metadata.targetThreshold), 0n, 0n, 0n]); - - const procMap = summary - .accountDelta() - .storage() - .maps() - .find((m) => m.slotName === PROCEDURE_THRESHOLDS_SLOT_NAME); - const entry = procMap - ?.entries() - .find((e) => normalizeHexWord(e.key.toHex()) === expectedKey); - if (!entry || normalizeHexWord(entry.value.toHex()) !== expectedValue) { + const expectedValue = wordHexFromFelts([BigInt(metadata.targetThreshold), 0n, 0n, 0n]); + const entry = mapEntriesFor(storage, PROCEDURE_THRESHOLDS_SLOT).find((e) => e.key === expectedKey); + if (!entry || entry.value !== expectedValue) { reject(proposalId); } } + +/** + * switch_guardian: no notes; the guardian public-key map is set to the declared + * new key (`[0,0,0,0] => newGuardianPubkey`, auth.ts `update_guardian_public_key`); + * and the changed storage slots are confined to the guardian component (plus the + * executed-tx marker) — and, if the proposal also rotates the signer set, the + * multisig signer slots with the same index-bound checks. `verifyGuardianEndpoint + * Commitment` (propose/execute) remains an additional check that the new key is + * live at the declared endpoint. + */ +function assertSwitchGuardianBinding( + proposalId: string, + metadata: SwitchGuardianProposalMetadata, + summary: TransactionSummary, +): void { + assertNoOutputNotes(proposalId, summary); + assertNoInputNotes(proposalId, summary); + + const storage = storageOf(summary); + const allowed = new Set([ + GUARDIAN_SELECTOR_SLOT, + GUARDIAN_PUBLIC_KEY_SLOT, + GUARDIAN_SCHEME_ID_SLOT, + EXECUTED_TXS_SLOT, + ]); + const rotatesSigners = + metadata.targetSignerCommitments !== undefined && metadata.targetThreshold !== undefined; + if (rotatesSigners) { + allowed.add(THRESHOLD_CONFIG_SLOT); + allowed.add(SIGNER_PUBLIC_KEYS_SLOT); + allowed.add(SIGNER_SCHEME_IDS_SLOT); + } + assertStorageSlotsWithin(proposalId, storage, allowed); + + // The guardian public-key map is keyed by the zero word. + const expectedPubkey = normalizeHexWord(metadata.newGuardianPubkey); + const entry = mapEntriesFor(storage, GUARDIAN_PUBLIC_KEY_SLOT).find((e) => e.key === ZERO_WORD_HEX); + if (!entry || entry.value !== expectedPubkey) { + reject(proposalId); + } + + if (rotatesSigners) { + assertSignerConfigAndKeys( + proposalId, + storage, + metadata.targetThreshold as number, + metadata.targetSignerCommitments as string[], + ); + } +} From 3f913746410fcad4be97d8630e0654a109a4b524 Mon Sep 17 00:00:00 2001 From: Wiktor Starczewski Date: Mon, 17 Aug 2026 15:28:10 +0200 Subject: [PATCH 5/6] docs(multisig-client): describe block-independent WYSIWYS binding + residuals --- docs/MULTISIG_SDK.md | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/docs/MULTISIG_SDK.md b/docs/MULTISIG_SDK.md index bc662812..9c555d5a 100644 --- a/docs/MULTISIG_SDK.md +++ b/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) From 061c85f85fd045dd06087f67f9c325dd74fcf180 Mon Sep 17 00:00:00 2001 From: Wiktor Starczewski Date: Mon, 17 Aug 2026 15:37:52 +0200 Subject: [PATCH 6/6] fix(multisig-client): pin guardian selector; clarify NFT gap as pre-merge closure Address final review: bind the guardian selector value in switch_guardian (reject a summary that disables the guardian) rather than relying on the MASM re-enable invariant. Reword the p2id non-fungible-asset residual: it is a fund-theft gap closable via output-note-id equality (to be done + integration-tested before this leaves draft), not an unclosable limit. --- .../src/multisig/summaryBinding.test.ts | 14 +++++++++- .../src/multisig/summaryBinding.ts | 26 +++++++++++++++---- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts index a3789580..08521d0f 100644 --- a/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.test.ts @@ -148,7 +148,7 @@ describe('assertMetadataMatchesSummary', () => { expectReject(metadata, mockSummary({ outputNotes: [] })); }); - it('rejects a note carrying an extra asset beyond the declared one', () => { + it('rejects a note carrying an extra fungible asset beyond the declared one', () => { // Correct recipient + declared asset, but an additional asset would leave // the account beyond what the metadata describes. const noteWithExtra = { @@ -460,6 +460,18 @@ describe('assertMetadataMatchesSummary', () => { }), ); }); + + it('rejects a switch_guardian that disables the guardian (selector off)', () => { + expectReject( + metadata, + mockSummary({ + maps: guardianMap(newGuardianPubkey), + valueDeltas: [ + { slotName: 'openzeppelin::guardian::selector', valueHex: wordHex([0n, 0n, 0n, 0n]) }, + ], + }), + ); + }); }); describe('custom', () => { diff --git a/packages/miden-multisig-client/src/multisig/summaryBinding.ts b/packages/miden-multisig-client/src/multisig/summaryBinding.ts index 302ac920..7717e8ba 100644 --- a/packages/miden-multisig-client/src/multisig/summaryBinding.ts +++ b/packages/miden-multisig-client/src/multisig/summaryBinding.ts @@ -134,9 +134,10 @@ function mapEntriesFor( * one that piggybacks an undeclared effect in any dimension, is rejected before a * cosigner signs it. * - * Residual limitations (see the type comments): the SDK exposes only FUNGIBLE - * note/vault assets, so a non-fungible asset attached to an otherwise-correct - * p2id note cannot be detected here; and the signer check binds by storage-map + * Residual limitations (see the type comments): a non-fungible asset attached to + * an otherwise-correct p2id note is not yet bound (the SDK exposes only fungible + * note assets) — closable via output-note-id equality before merge, see + * `assertP2idBinding`; and the signer check binds by storage-map * index over the CHANGED entries (a real-summary integration test should confirm * the delta carries every changed index). `custom` proposals carry no metadata * recipe, so WYSIWYS does not hold for them — a cosigner must verify the raw @@ -184,8 +185,14 @@ export function assertMetadataMatchesSummary( * match); the asset is read off the note (not the vault), so the block-dependent * fee never contaminates the comparison. * - * Residual: `NoteAssets` exposes only fungible assets, so a non-fungible asset - * attached to this note is invisible to the SDK and cannot be bound here. + * KNOWN GAP (must close before this leaves draft): the recipient and the + * fungible asset are bound separately, so a NON-FUNGIBLE asset attached to the + * note is not bound (`NoteAssets` exposes only fungible assets) — an NFT could + * be drained under a fungible-transfer label. Closure: bind the whole output + * note by `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 keep the callback + * flag and block-independence — validated by the pre-merge integration test. */ function assertP2idBinding( proposalId: string, @@ -408,6 +415,15 @@ function assertSwitchGuardianBinding( reject(proposalId); } + // If the guardian selector value slot changed, it must be ON ([1,0,0,0]). An + // honest guardian switch leaves the guardian enabled (the MASM re-enables it), + // so a summary that disables the guardian is rejected here rather than relying + // on that MASM invariant. + const selector = valueDeltaFor(storage, GUARDIAN_SELECTOR_SLOT); + if (selector !== undefined && selector !== wordHexFromFelts([1n, 0n, 0n, 0n])) { + reject(proposalId); + } + if (rotatesSigners) { assertSignerConfigAndKeys( proposalId,