diff --git a/docs/MULTISIG_SDK.md b/docs/MULTISIG_SDK.md index 9449ba12..8a3a056a 100644 --- a/docs/MULTISIG_SDK.md +++ b/docs/MULTISIG_SDK.md @@ -741,6 +741,19 @@ const proposal = await multisig.createSwitchGuardianProposal( ); ``` +When the current GUARDIAN is unreachable, create the switch proposal fully +offline instead — nothing is pushed to the current operator (issue #433; +mirrors the Rust `create_proposal_offline`). The returned `ExportedProposal` +already carries the proposer's signature; share its JSON with cosigners for +`importProposal` / `signProposalOffline`, then execute: + +```typescript +const exported = await multisig.createSwitchGuardianProposalOffline( + newGuardianEndpoint, + newGuardianCommitment +); +``` + ### Signing & Executing Proposals ```typescript @@ -817,6 +830,7 @@ await multisig.executeProposal(signedProposal.id); | `createChangeThresholdProposal(threshold, { nonce }?)` | Create threshold change proposal | | `createUpdateProcedureThresholdProposal(procedure, threshold, { nonce }?)` | Create per-procedure threshold override proposal (`threshold: 0` clears the override) | | `createSwitchGuardianProposal(endpoint, pubkey, { nonce }?)` | Create GUARDIAN switch proposal | +| `createSwitchGuardianProposalOffline(endpoint, pubkey, { nonce }?)` | Create GUARDIAN switch proposal without contacting the current GUARDIAN; returns a signed `ExportedProposal` for side-channel cosigning (issue #433) | | `createCustomProposal(requestBytes, label, { nonce }?)` | Create a producer-built custom proposal (issue #266) | | `signProposal(id)` | Sign a proposal | | `executeProposal(id)` | Execute ready proposal | diff --git a/packages/miden-multisig-client/README.md b/packages/miden-multisig-client/README.md index 29b9c46f..ae6e6e82 100644 --- a/packages/miden-multisig-client/README.md +++ b/packages/miden-multisig-client/README.md @@ -388,6 +388,37 @@ const signedJson = await multisig.signProposalOffline(imported.id); console.log(signedJson); ``` +### Leave an Unreachable Guardian (Offline Switch) + +A switch-Guardian proposal can be created fully offline — nothing is pushed to +the current Guardian, so an account can leave an operator that is down +(mirrors the Rust `create_proposal_offline`). Only switch-Guardian proposals +support this: every other type needs a Guardian acknowledgment at execution. +The new endpoint must be reachable; its `/pubkey` commitment is verified +before anything is signed. + +```typescript +// Proposer: build, sign, and cache locally — the current Guardian is never contacted. +const exported = await multisig.createSwitchGuardianProposalOffline( + 'https://new-guardian.example', + newGuardianPubkey, +); +const json = JSON.stringify(exported); // share with cosigners side-channel + +// Cosigner: import, sign, send the updated JSON back. +const imported = await cosigner.importProposal(json); +const signedJson = await cosigner.signProposalOffline(imported.id); + +// Proposer: import the cosigned proposal and execute. The push to the old +// Guardian is best-effort; registration happens on the new Guardian. +const ready = await multisig.importProposal(signedJson); +await multisig.executeProposal(ready.id); +``` + +For hand-rolled export/import flows, `computeCommitmentFromTxSummary` is +exported: a proposal's `commitment` is its transaction summary's commitment, +recomputed from the serialized summary. + ### Custom Proposal Types GUARDIAN accepts any non-empty `proposalType`, so an integration can propose a diff --git a/packages/miden-multisig-client/src/index.ts b/packages/miden-multisig-client/src/index.ts index 1535fbaf..7e53fbca 100644 --- a/packages/miden-multisig-client/src/index.ts +++ b/packages/miden-multisig-client/src/index.ts @@ -76,6 +76,11 @@ export { type P2idTransactionOptions, type P2ideHeightOptions, } from './transaction.js'; +// Commitment derivation for hand-rolled export/import flows (issue #433): +// the proposal id is the tx summary's commitment, recomputed from the +// serialized summary exactly as import verification does. Returns normalized +// hex, directly comparable to `ExportedProposal.commitment` / `Proposal.id`. +export { computeCommitmentFromTxSummary } from './multisig/helpers.js'; export { GuardianHttpClient, GuardianHttpError } from '@openzeppelin/guardian-client'; export type { GuardianErrorMeta } from '@openzeppelin/guardian-client'; diff --git a/packages/miden-multisig-client/src/multisig.test.ts b/packages/miden-multisig-client/src/multisig.test.ts index 4161ce4e..43e00fe6 100644 --- a/packages/miden-multisig-client/src/multisig.test.ts +++ b/packages/miden-multisig-client/src/multisig.test.ts @@ -2306,6 +2306,208 @@ describe('Multisig', () => { }); }); + describe('createSwitchGuardianProposalOffline (issue #433)', () => { + const NEW_GUARDIAN_ENDPOINT = 'http://new-guardian.com'; + const newGuardianPubkey = '0x' + '9'.repeat(64); + + // Routes fetch by target: the new GUARDIAN answers, everything else — in + // particular the current GUARDIAN at localhost:3000 — is network-dead. + // This is the exact scenario the offline path exists for (0xMiden/wallet#782). + function stubFetchWithDeadCurrentGuardian( + newGuardianResponses: Record = {}, + ): void { + mockFetch.mockImplementation(async (url: string) => { + const parsed = new URL(url); + if (parsed.origin !== NEW_GUARDIAN_ENDPOINT) { + throw new Error(`current GUARDIAN unreachable: ${url}`); + } + if (parsed.pathname === '/pubkey') { + return { ok: true, json: async () => ({ commitment: newGuardianPubkey }) }; + } + return { + ok: true, + json: async () => ({ success: true, message: 'ok', ...newGuardianResponses }), + }; + }); + } + + it('creates, signs, and caches the proposal without contacting the current GUARDIAN', async () => { + const config = { + threshold: 1, + signerCommitments: [mockSigner.commitment], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + const multisig = createTestMultisig(config); + stubFetchWithDeadCurrentGuardian(); + + const exported = await multisig.createSwitchGuardianProposalOffline( + NEW_GUARDIAN_ENDPOINT, + newGuardianPubkey, + { nonce: 7 }, + ); + + expect(exported.accountId).toBe(multisig.accountId); + expect(exported.nonce).toBe(7); + expect(exported.commitment).toBe('0x' + 'c'.repeat(64)); + expect(exported.metadata.proposalType).toBe('switch_guardian'); + if (exported.metadata.proposalType === 'switch_guardian') { + expect(exported.metadata.newGuardianEndpoint).toBe(NEW_GUARDIAN_ENDPOINT); + expect(exported.metadata.newGuardianPubkey).toBe(newGuardianPubkey); + expect(exported.metadata.chainAnchor).toBe(MOCK_CHAIN_ANCHOR_B64); + expect(exported.metadata.saltHex).toBe('0x' + 'd'.repeat(64)); + } + + // The proposer's signature is included, over the exported commitment. + expect(exported.signatures).toHaveLength(1); + expect(exported.signatures[0].commitment).toBe(mockSigner.commitment); + expect(exported.signatures[0].scheme).toBe('falcon'); + expect(mockSigner.signCommitment).toHaveBeenCalledWith(exported.commitment); + + // The only network call is the new endpoint's /pubkey verification. + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(mockFetch).toHaveBeenCalledWith( + `${NEW_GUARDIAN_ENDPOINT}/pubkey?scheme=falcon`, + expect.objectContaining({ method: 'GET' }), + ); + // Pre-build node sync (mirrors the Rust sync_network_only). + expect(mockWebClient.syncState).toHaveBeenCalled(); + + // Cached locally, ready at threshold 1 (proposer already signed). + const cached = multisig.listProposals(); + expect(cached).toHaveLength(1); + expect(cached[0].id).toBe(exported.commitment); + expect(cached[0].status).toBe('ready'); + }); + + it('picks up an on-chain threshold change during the pre-build sync (PR #436 review)', async () => { + const signerB = '0x' + '8'.repeat(64); + const config = { + threshold: 1, + signerCommitments: [mockSigner.commitment, signerB], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + const multisig = createTestMultisig(config); + stubFetchWithDeadCurrentGuardian(); + + // The synced store reports the threshold now at 2; with the stale + // cached config (threshold 1) the proposal would flip to 'ready' on the + // proposer's signature alone and fail only at submission. + mockWebClient.getAccount.mockResolvedValueOnce(mockedAccount('0x' + 'b'.repeat(64), 1)); + mockDetectConfig.mockReturnValueOnce({ + threshold: 2, + numSigners: 2, + signerCommitments: [mockSigner.commitment, signerB], + guardianCommitment: '0x' + 'c'.repeat(64), + vaultBalances: [], + procedureThresholds: new Map(), + }); + + const exported = await multisig.createSwitchGuardianProposalOffline( + NEW_GUARDIAN_ENDPOINT, + newGuardianPubkey, + { nonce: 9 }, + ); + + expect(exported.metadata.requiredSignatures).toBe(2); + expect(exported.signatures).toHaveLength(1); + expect(multisig.listProposals()[0].status).toBe('pending'); + }); + + it('rejects when the new endpoint commitment does not match, before building or signing', async () => { + vi.mocked(buildUpdateGuardianTransactionRequest).mockClear(); + const config = { + threshold: 1, + signerCommitments: [mockSigner.commitment], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + const multisig = createTestMultisig(config); + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ commitment: '0x' + '2'.repeat(64) }), + }); + + await expect( + multisig.createSwitchGuardianProposalOffline(NEW_GUARDIAN_ENDPOINT, newGuardianPubkey), + ).rejects.toThrow('Refusing to use GUARDIAN endpoint'); + expect(buildUpdateGuardianTransactionRequest).not.toHaveBeenCalled(); + expect(mockSigner.signCommitment).not.toHaveBeenCalled(); + expect(multisig.listProposals()).toHaveLength(0); + }); + + it('rejects legacy positional callers (issue #387)', async () => { + const config = { + threshold: 1, + signerCommitments: [mockSigner.commitment], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + const multisig = createTestMultisig(config); + + await expect( + (multisig.createSwitchGuardianProposalOffline as any)( + NEW_GUARDIAN_ENDPOINT, + newGuardianPubkey, + 123, + ), + ).rejects.toThrow('trailing options object'); + }); + + it('supports the full offline trio: create → cosign → execute against a dead current GUARDIAN', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const signerB: Signer = { + ...mockSigner, + commitment: '0x' + '8'.repeat(64), + signCommitment: vi.fn().mockReturnValue('0x' + 'e'.repeat(128)), + }; + const config = { + threshold: 2, + signerCommitments: [mockSigner.commitment, signerB.commitment], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + stubFetchWithDeadCurrentGuardian({ ack_pubkey: '0x' + 'f'.repeat(64) }); + + // Proposer (signer A) creates the proposal fully offline. + const proposerClient = createTestMultisig(config); + const exported = await proposerClient.createSwitchGuardianProposalOffline( + NEW_GUARDIAN_ENDPOINT, + newGuardianPubkey, + { nonce: 1 }, + ); + expect(proposerClient.listProposals()[0].status).toBe('pending'); + + // Cosigner (signer B) imports and signs side-channel. + const cosignerClient = createTestMultisig(config, signerB); + const importedByCosigner = await cosignerClient.importProposal(JSON.stringify(exported)); + expect(importedByCosigner.status).toBe('pending'); + const signedJson = await cosignerClient.signProposalOffline(importedByCosigner.id); + + // Proposer imports the cosigned proposal — now at threshold. + const readyProposal = await proposerClient.importProposal(signedJson); + expect(readyProposal.status).toBe('ready'); + expect(readyProposal.signatures).toHaveLength(2); + + // Execution succeeds with the current GUARDIAN unreachable: the + // canonicalization push is best-effort, and registration goes to the + // new GUARDIAN only. + mockWebClient.getAccount.mockResolvedValueOnce({ + serialize: () => new Uint8Array([1, 2, 3]), + }); + await expect(proposerClient.executeProposal(readyProposal.id)).resolves.toBeUndefined(); + + expect(mockWebClient.executeTransaction).toHaveBeenCalledTimes(1); + expect(mockWebClient.submitProvenTransaction).toHaveBeenCalledTimes(1); + expect(proposerClient.listProposals()[0].status).toBe('finalized'); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('pre-switch GUARDIAN'), + expect.any(Error), + ); + } finally { + warnSpy.mockRestore(); + } + }); + }); + describe('createUpdateProcedureThresholdProposal', () => { it('should create procedure-threshold update proposals', async () => { vi.mocked(executeForSummary).mockResolvedValue({ diff --git a/packages/miden-multisig-client/src/multisig.ts b/packages/miden-multisig-client/src/multisig.ts index 66d441fe..29e8cbf2 100644 --- a/packages/miden-multisig-client/src/multisig.ts +++ b/packages/miden-multisig-client/src/multisig.ts @@ -593,6 +593,27 @@ export class Multisig { } } + /** + * Sync the local store with the Miden node, then reload the cached account + * and multisig config from it (mirrors the Rust `sync_network_only`). + * Without the refresh, a summary would be built against the freshly synced + * store while readiness thresholds and signature validation still read the + * stale cached config. Returns the store-backed account, or `null` when the + * store has no record for this account (the cached config is then kept, as + * in `getStoreAccount`); callers that require the account decide how to + * fail. + */ + private async syncNetworkOnly(): Promise { + const webClient = await this.getRawClient(); + await retryRpcRead(() => webClient.syncState(), this.rpcConfig); + const account = await retryRpcRead( + () => webClient.getAccount(AccountId.fromHex(this._accountId)), + this.rpcConfig, + ); + this.refreshConfigFromAccount(account ?? null); + return account ?? null; + } + private refreshConfigFromAccount(account: Account | null): void { if (!account) { return; @@ -925,6 +946,27 @@ export class Multisig { options: CreateProposalOptions = {}, ): Promise { const proposalNonce = resolveProposalNonce('createSwitchGuardianProposal', options); + const { summaryBase64, metadata } = await this.buildSwitchGuardianSummary( + newGuardianEndpoint, + newGuardianPubkey, + ); + + // SwitchGuardian is a regular delta proposal; push it to GUARDIAN so + // sign/execute (which fetch from GUARDIAN) can find it. To leave an + // unreachable GUARDIAN, use createSwitchGuardianProposalOffline instead. + return this.createProposal(proposalNonce, summaryBase64, metadata); + } + + /** + * Shared build step for both switch-GUARDIAN creation paths: verify the new + * endpoint's `/pubkey` commitment, then execute the update-guardian request + * for its summary and metadata. Kept in one place so the online and offline + * proposals for the same operation can never drift apart. + */ + private async buildSwitchGuardianSummary( + newGuardianEndpoint: string, + newGuardianPubkey: string, + ): Promise<{ summaryBase64: string; metadata: ProposalMetadata }> { const webClient = await this.getRawClient(); await this.verifyGuardianEndpointCommitment(newGuardianEndpoint, newGuardianPubkey); @@ -949,9 +991,68 @@ export class Multisig { description: `Switch GUARDIAN to ${newGuardianEndpoint}`, }; - // SwitchGuardian is a regular delta proposal; push it to GUARDIAN so - // sign/execute (which fetch from GUARDIAN) can find it. - return this.createProposal(proposalNonce, summaryBase64, metadata); + return { summaryBase64, metadata }; + } + + /** + * Create a "switch GUARDIAN" proposal fully offline — nothing is pushed to + * the current GUARDIAN, so an account can leave an unreachable operator + * (issue #433; mirrors the Rust `create_proposal_offline`). + * + * The transaction summary is built and signed locally, the proposal is + * cached for `signProposalOffline` / `executeProposal`, and the returned + * `ExportedProposal` (which already includes the proposer's signature) can + * be `JSON.stringify`-ed and shared with cosigners for `importProposal`. + * + * Only switch-GUARDIAN proposals can be created offline: every other + * proposal type requires a GUARDIAN acknowledgment at execution, so a + * proposal the GUARDIAN never saw could collect signatures but never + * execute. The new endpoint must be reachable — its `/pubkey` commitment + * is verified before anything is built or signed. + * + * Note: executeProposal's best-effort canonicalization push cannot reach a + * proposal the current GUARDIAN never received, so even if that GUARDIAN is + * back up at execution time it keeps serving the account until background + * reconciliation (issue #305) — same outcome as executing while it is down. + * + * @param newGuardianEndpoint - The new GUARDIAN server endpoint URL + * @param newGuardianPubkey - The new GUARDIAN server's public key commitment (hex) + * @param options - Optional settings: `nonce` + */ + async createSwitchGuardianProposalOffline( + newGuardianEndpoint: string, + newGuardianPubkey: string, + options: CreateProposalOptions = {}, + ): Promise { + const proposalNonce = resolveProposalNonce('createSwitchGuardianProposalOffline', options); + + // Sync with the Miden node and refresh the cached account/config before + // building (mirrors the Rust `sync_network_only`): with no GUARDIAN push + // to reject a stale delta at creation, a summary built from stale local + // state — or a readiness threshold read from stale config — would only + // fail at execution, after the whole side-channel cosigning ceremony. + await this.syncNetworkOnly(); + + const { summaryBase64, metadata } = await this.buildSwitchGuardianSummary( + newGuardianEndpoint, + newGuardianPubkey, + ); + + const exported: ExportedProposal = { + accountId: this._accountId, + nonce: proposalNonce, + commitment: computeCommitmentFromTxSummary(summaryBase64), + txSummaryBase64: summaryBase64, + signatures: [], + metadata, + }; + + // Reuse the cosigner-side machinery end to end: importProposal validates + // and caches exactly as it would on a cosigner's client, and + // signProposalOffline adds the proposer's signature and re-exports. + const proposal = await this.importProposal(JSON.stringify(exported)); + const signedJson = await this.signProposalOffline(proposal.id); + return JSON.parse(signedJson) as ExportedProposal; } /** @@ -1421,13 +1522,7 @@ export class Multisig { } try { - const webClient = await this.getRawClient(); - await retryRpcRead(() => webClient.syncState(), this.rpcConfig); - - const updatedAccount = await retryRpcRead( - () => webClient.getAccount(accountId), - this.rpcConfig, - ); + const updatedAccount = await this.syncNetworkOnly(); if (!updatedAccount) { throw new Error( `Updated account ${this._accountId} is missing from local client` diff --git a/packages/miden-multisig-client/src/multisig/helpers.test.ts b/packages/miden-multisig-client/src/multisig/helpers.test.ts index 9e822756..cc4319f0 100644 --- a/packages/miden-multisig-client/src/multisig/helpers.test.ts +++ b/packages/miden-multisig-client/src/multisig/helpers.test.ts @@ -43,6 +43,18 @@ describe('multisig helpers', () => { expect(commitment.startsWith('0x')).toBe(true); }); + + it('normalizes case and padding so results compare directly with proposal ids (issue #433)', async () => { + const { TransactionSummary } = await import('@miden-sdk/miden-sdk'); + vi.mocked(TransactionSummary.deserialize).mockReturnValueOnce({ + toCommitment: () => ({ toHex: () => '0x' + 'C'.repeat(63) }), + } as any); + + const base64 = btoa(String.fromCharCode(...new Uint8Array([1]))); + const commitment = computeCommitmentFromTxSummary(base64); + + expect(commitment).toBe('0x0' + 'c'.repeat(63)); + }); }); describe('accountIdToHex', () => { diff --git a/packages/miden-multisig-client/src/multisig/helpers.ts b/packages/miden-multisig-client/src/multisig/helpers.ts index 0be886d9..5c832f01 100644 --- a/packages/miden-multisig-client/src/multisig/helpers.ts +++ b/packages/miden-multisig-client/src/multisig/helpers.ts @@ -1,11 +1,16 @@ import { Account, TransactionSummary, Word } from '@miden-sdk/miden-sdk'; -import { base64ToUint8Array } from '../utils/encoding.js'; +import { base64ToUint8Array, normalizeHexWord } from '../utils/encoding.js'; +/** + * Recompute a proposal's id from its serialized transaction summary. Returns + * normalized hex (lowercase, 0x-prefixed, zero-padded) so the result compares + * equal to `ExportedProposal.commitment` / `Proposal.id` directly. + */ export function computeCommitmentFromTxSummary(txSummaryBase64: string): string { const bytes = base64ToUint8Array(txSummaryBase64); const summary = TransactionSummary.deserialize(bytes); const commitment = summary.toCommitment(); - return commitment.toHex(); + return normalizeHexWord(commitment.toHex()); } export function accountIdToHex(account: Account): string {