diff --git a/packages/miden-multisig-client/README.md b/packages/miden-multisig-client/README.md index 0b8ebca0..2c33e92f 100644 --- a/packages/miden-multisig-client/README.md +++ b/packages/miden-multisig-client/README.md @@ -200,7 +200,7 @@ console.log('Signatures:', signedProposal.signatures.length); ### Sync Proposals -Fetches proposals from the GUARDIAN server and updates local state: +Fetches proposals from the GUARDIAN server and reconciles local state — a proposal GUARDIAN reported on an earlier sync but no longer reports (executed, canonicalized, or abandoned) is pruned from the cache. Proposals GUARDIAN has never reported to this client — freshly created, or imported and not yet synced — are left untouched: ```typescript const proposals = await multisig.syncProposals(); diff --git a/packages/miden-multisig-client/src/multisig.test.ts b/packages/miden-multisig-client/src/multisig.test.ts index 7522dd56..c6ce8aca 100644 --- a/packages/miden-multisig-client/src/multisig.test.ts +++ b/packages/miden-multisig-client/src/multisig.test.ts @@ -955,6 +955,168 @@ describe('Multisig', () => { expect(proposals[0].status).toBe('pending'); }); + it('should prune proposals GUARDIAN no longer reports (executed/canonicalized)', async () => { + const config = { + threshold: 2, + signerCommitments: ['0x' + 'a'.repeat(64), '0x' + 'b'.repeat(64)], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + + const multisig = createTestMultisig(config); + + const pendingProposal = { + account_id: '0x' + 'a'.repeat(30), + nonce: 1, + prev_commitment: '0x' + 'b'.repeat(64), + delta_payload: { + tx_summary: { data: 'AQID' }, + signatures: [], + metadata: { + proposal_type: 'add_signer', + target_threshold: 1, + signer_commitments: ['0x' + 'a'.repeat(64)], + description: '', + }, + }, + status: { + status: 'pending', + timestamp: '2024-01-01T00:00:00Z', + proposer_id: '0x' + 'c'.repeat(64), + cosigner_sigs: [ + { + signer_id: '0x' + 'a'.repeat(64), + signature: { scheme: 'falcon', signature: '0x' + 'e'.repeat(128) }, + timestamp: '2024-01-01T00:00:00Z', + }, + ], + }, + }; + + // First sync: GUARDIAN reports the pending proposal, priming the cache. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ proposals: [pendingProposal] }), + }); + const first = await multisig.syncProposals(); + expect(first.length).toBe(1); + + // Second sync: another signer executed the proposal, so GUARDIAN pruned it + // and now returns an empty list. The cache must reconcile to empty rather + // than keep returning the stale (still-pending-looking) proposal forever. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ proposals: [] }), + }); + const second = await multisig.syncProposals(); + expect(second).toEqual([]); + expect(multisig.listProposals()).toEqual([]); + }); + + it('should keep proposals GUARDIAN still reports across syncs', async () => { + // Guards against over-pruning: a proposal the server still reports on a + // later sync must survive the reconcile, not be deleted. + const config = { + threshold: 2, + signerCommitments: ['0x' + 'a'.repeat(64), '0x' + 'b'.repeat(64)], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + + const multisig = createTestMultisig(config); + + const pendingProposal = { + account_id: '0x' + 'a'.repeat(30), + nonce: 1, + prev_commitment: '0x' + 'b'.repeat(64), + delta_payload: { + tx_summary: { data: 'AQID' }, + signatures: [], + metadata: { + proposal_type: 'add_signer', + target_threshold: 1, + signer_commitments: ['0x' + 'a'.repeat(64)], + description: '', + }, + }, + status: { + status: 'pending', + timestamp: '2024-01-01T00:00:00Z', + proposer_id: '0x' + 'c'.repeat(64), + cosigner_sigs: [ + { + signer_id: '0x' + 'a'.repeat(64), + signature: { scheme: 'falcon', signature: '0x' + 'e'.repeat(128) }, + timestamp: '2024-01-01T00:00:00Z', + }, + ], + }, + }; + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ proposals: [pendingProposal] }), + }); + const first = await multisig.syncProposals(); + expect(first.length).toBe(1); + + // GUARDIAN still reports the same pending proposal on the next sync. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ proposals: [pendingProposal] }), + }); + const second = await multisig.syncProposals(); + expect(second.length).toBe(1); + expect(multisig.listProposals().length).toBe(1); + }); + + it('should not prune a locally-created proposal GUARDIAN has not reported yet', async () => { + // createProposal pushes to GUARDIAN then caches. If GUARDIAN's + // read-your-writes lags and the immediately-following sync omits the + // just-pushed proposal, it must NOT be evicted: GUARDIAN never reported it + // to this client, so it is not a prune candidate. + const config = { + threshold: 1, + signerCommitments: ['0x' + 'a'.repeat(64)], + guardianCommitment: '0x' + 'c'.repeat(64), + }; + const multisig = createTestMultisig(config); + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + delta: { + account_id: '0x' + 'a'.repeat(30), + nonce: 1, + prev_commitment: '0x' + 'b'.repeat(64), + delta_payload: { tx_summary: { data: 'AQID' }, signatures: [] }, + status: { + status: 'pending', + timestamp: '2024-01-01T00:00:00Z', + proposer_id: '0x' + 'c'.repeat(64), + cosigner_sigs: [], + }, + }, + commitment: '0x' + 'c'.repeat(64), + }), + }); + const created = await multisig.createProposal(1, 'AQID', { + proposalType: 'add_signer', + targetThreshold: 1, + targetSignerCommitments: ['0x' + 'a'.repeat(64)], + description: '', + }); + expect(multisig.listProposals().length).toBe(1); + + // GUARDIAN's next getDeltaProposals lags and returns []. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ proposals: [] }), + }); + const synced = await multisig.syncProposals(); + expect(synced.length).toBe(1); + expect(synced[0].id).toBe(created.id); + expect(multisig.listProposals().length).toBe(1); + }); + it('should return ready status when enough signatures', async () => { const config = { threshold: 1, // Only 1 signature needed diff --git a/packages/miden-multisig-client/src/multisig.ts b/packages/miden-multisig-client/src/multisig.ts index 2cf17b67..10d60e13 100644 --- a/packages/miden-multisig-client/src/multisig.ts +++ b/packages/miden-multisig-client/src/multisig.ts @@ -160,6 +160,8 @@ export class Multisig { private readonly _accountId: string; private readonly midenRpcEndpoint: string; private proposals: Map = new Map(); + /** Ids GUARDIAN returned on the most recent sync; only these are prunable. */ + private lastReportedProposalIds: Set = new Set(); constructor( account: Account, @@ -537,11 +539,39 @@ export class Multisig { /** * Sync proposals from the GUARDIAN server. + * + * The GUARDIAN response is authoritative for the set of pending proposals: the + * server only reports proposals whose status is still pending and drops them + * once they are executed and canonicalized. The local cache is therefore + * reconciled to the response — a proposal GUARDIAN reported on a previous sync + * but no longer reports is pruned — so a stale proposal does not linger locally + * and keep showing as pending forever, e.g. a co-signer's browser after another + * signer executed the transaction. + * + * Only proposals GUARDIAN has actually reported are eligible for pruning. A + * proposal that is in the cache but that GUARDIAN has not (yet) returned in a + * response is left untouched, so a `createProposal` immediately followed by a + * sync survives a GUARDIAN read-your-writes lag that omits the just-pushed + * proposal, and the offline export/import flow — `importProposal` caches a + * proposal this client has not synced yet — survives until GUARDIAN first + * reports it. A proposal is only dropped once GUARDIAN reported it and then + * stopped (executed / abandoned). + * + * Nonce-based staleness hiding — a proposal the account has already advanced + * past, which GUARDIAN may still briefly report as pending before it prunes it + * — is intentionally left to callers' own visible-proposal filter (see the + * examples' `filterVisibleProposals`). The Rust client applies a + * `proposal.nonce <= account.nonce()` filter directly, but it owns a single + * nonce convention end to end; this shared client serves callers that disagree + * on what the proposal `nonce` means (some store the pre-execution account + * nonce, others the next nonce), so it cannot safely apply that comparison here + * and defers it to the caller. This is an intentional TS/Rust surface difference. */ async syncProposals(): Promise { const deltas = await this.guardian.getDeltaProposals(this._accountId); const factory = this.proposalFactory(); + const reportedIds = new Set(); for (const delta of deltas) { const proposalId = normalizeHexWord( computeCommitmentFromTxSummary(delta.deltaPayload.txSummary.data) @@ -556,13 +586,25 @@ export class Multisig { await this.verifyProposalMetadataBinding(proposal); this.proposals.set(proposal.id, proposal); + reportedIds.add(proposal.id); } + for (const id of this.lastReportedProposalIds) { + if (!reportedIds.has(id)) { + this.proposals.delete(id); + } + } + this.lastReportedProposalIds = reportedIds; + return Array.from(this.proposals.values()); } /** - * List all known proposals + * Returns the proposals cached by the most recent {@link syncProposals} call. + * + * This is that sync's reconciled set, not a durable log: proposals GUARDIAN no + * longer reports were pruned, so do not treat the result as an ever-growing + * history of every proposal ever seen. */ listProposals(): Proposal[] { return Array.from(this.proposals.values());