feat(sdk): import proposal-embedded notes as a recovery primitive (#415) by haseebrabbani · Pull Request #424 · OpenZeppelin/guardian

haseebrabbani · 2026-08-20T16:23:14Z

Closes #415 (sub-issue of #357). Productizes the reconstruction path validated by spike #412.

What

Adds a recovery primitive to both SDKs that rebuilds Miden store records from the note bytes embedded in v2 consume_notes proposals (#229), plus a node-fetched inclusion proof — so it works for private notes without the node ever holding the body.

Rust: MultisigClient::import_notes_from_proposals(&[Proposal]) -> Vec<NoteImportOutcome> (new client/recovery.rs)
TS: standalone importNotesFromProposals(midenClient, proposals, { midenRpcEndpoint, rpc? }) (new src/recovery.ts)

Per unique embedded note (duplicates across proposals fold into one outcome): decode → skip notes the store already tracks → one batched inclusion-proof fetch → import individually (upstream import_notes batches are atomic, so one bad note must not sink the rest) → per-note NoteImportOutcome with status ∈ imported | already-present | already-consumed | not-committed | invalid | failed and a retryable flag. No per-note problem aborts the batch; the Rust method returns a plain Vec because nothing can fail the whole call.

Key behaviors (live-tested against testnet)

Not-committed notes are recorded in Expected state with their tag tracked so a later sync picks them up once they commit; reported not-committed/retryable. On the TS side the WASM details file cannot carry a tag, so the tag is registered via addTag before the import — tag-first so a failure never leaves an untagged dead record.
Chain-nullified notes: miden-client stores these as metadata-less consumption-history records; the primitive reports them already-consumed instead of pretending they were recovered.
Metadata-less store records (details-only expected imports, consumed-external history) expose neither a note ID nor a nullifier, so existing-record matching uses details commitments (Rust) / recipient digests (TS — the WASM NoteFilter has no details-commitment variant); an ID-only lookup would re-import them forever.
Retryability is classified, not assumed: transient RPC failures during the proof fetch or the import's internal node calls are retryable: true.
Decoding is deliberately permissive: the strict note.id() == note_ids[i] binding check stays in the verify/execute path; embedded notes are self-validating, so recovery imports whatever real notes the bytes decode to.

Docs

docs/MULTISIG_SDK.md (both language sections + API tables) and both package READMEs, per AGENTS.md §9. Example wiring (demo/smoke-web) is deferred to the #357 recovery-flow orchestration, where the three primitives (#414/#415/#416) compose.

Notes for reviewers

The TS entry point is a standalone function because the issue specifies that signature; a Multisig convenience method delegating to it would be a natural follow-up.
The TS pre-import store scan reads all input notes once (WASM filter surface limitation, commented in code); extracting a shared matching helper is planned with SDK recovery primitive: historical public-note backfill by tag #416.

coderabbitai · 2026-08-20T16:23:23Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a9b126b-2fd2-45b2-992f-eb5312a60dad

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

Use the checkbox below for a quick retry:

🔍 Trigger review

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

❤️ Share

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

codecov-commenter · 2026-08-20T16:47:46Z

Codecov Report

❌ Patch coverage is 45.08475% with 162 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...rates/miden-multisig-client/src/client/recovery.rs	45.08%	162 Missing ⚠️

📢 Thoughts on this report? Let us know!

zeljkoX · 2026-08-21T11:26:47Z

Does it makes sense to create PRs against feature branch? Given few recovery strategies it would be good to have one final review on all included and orchestration between them.

zeljkoX · 2026-08-21T11:54:26Z

+const multisig = await client.load(accountId, signer);
+const proposals = await multisig.syncProposals();
+
+const outcomes = await importNotesFromProposals(midenClient, proposals, {


probably there should be single exposed recovery method in multisig where user can pass recovery strategies he wants to use

agreed, will handle this in final review of base feature branch

zeljkoX · 2026-08-21T11:55:21Z

+
+const proposals = await multisig.syncProposals();
+const outcomes = await importNotesFromProposals(midenClient, proposals, {
+  midenRpcEndpoint: 'https://rpc.testnet.miden.io',


we should avoid needing to pass rpc url, it should be reused from client,

rust uses client method, ts uses non client method

let proposals = client.list_proposals().await?;
let outcomes = client.import_notes_from_proposals(&proposals).await;

Double checking, is this going to be public api? Users probably should not set rpc on method level but on client level and reuse here. Setting rpc url at client level is already supported.

Copilot

Pull request overview

Adds proposal-embedded note recovery to both multisig SDKs.

Changes:

Implements per-note recovery, deduplication, proof fetching, and status reporting.
Exposes the new Rust and TypeScript APIs.
Adds tests and consumer documentation.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`packages/miden-multisig-client/src/recovery.ts`	Implements TypeScript recovery.
`packages/miden-multisig-client/src/recovery.test.ts`	Tests TypeScript recovery paths.
`packages/miden-multisig-client/src/index.ts`	Exports the recovery API.
`packages/miden-multisig-client/README.md`	Documents TypeScript usage.
`crates/miden-multisig-client/src/client/recovery.rs`	Implements Rust recovery and unit tests.
`crates/miden-multisig-client/src/client/mod.rs`	Registers and exports the Rust module.
`crates/miden-multisig-client/src/lib.rs`	Re-exports public recovery types.
`crates/miden-multisig-client/README.md`	Documents Rust usage.
`docs/MULTISIG_SDK.md`	Documents both SDK recovery APIs.

Suppressed comments (2)

packages/miden-multisig-client/src/recovery.ts:145

This public workflow is over 220 lines and mixes decoding, deduplication, store scanning, RPC transport, per-note mutation, and post-import classification. That makes the recovery invariants difficult to audit and conflicts with the multisig guideline to separate orchestration from transformation. Extract focused helpers for candidate collection, existing-record matching, proof fetching, import, and consumed-state classification, leaving this method as orchestration.

export async function importNotesFromProposals(
  midenClient: RawClientSource,
  proposals: ReadonlyArray<Pick<Proposal, 'id' | 'metadata'>>,
  options: ImportNotesFromProposalsOptions,
): Promise<NoteImportOutcome[]> {

crates/miden-multisig-client/src/client/recovery.rs:190

This method combines store lookup, RPC fetching, expected-note construction, imports, and post-import state classification in one long workflow. The multisig coding guideline calls for small, single-purpose helpers and separation of orchestration from transformation; extracting these stages would make Rust/TypeScript parity and failure isolation materially easier to verify.

    pub async fn import_notes_from_proposals(
        &mut self,
        proposals: &[Proposal],
    ) -> Vec<NoteImportOutcome> {

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+  // Skip notes the store already tracks.
+  const pending: DecodedCandidate[] = [];
+  for (const candidate of decoded) {
+    const record = existing.get(candidate.idHex) ?? existing.get(candidate.recipientDigestHex);


+    pub async fn import_notes_from_proposals(
+        &mut self,
+        proposals: &[Proposal],
+    ) -> Vec<NoteImportOutcome> {


+      const consumed = new Set(
+        consumedRecords.map((record) =>
+          normalizeHexWord(record.details().recipient().digest().toHex()),
+        ),
+      );
+      for (const entry of imported) {
+        if (consumed.has(entry.recipientDigestHex)) {


+  /** Human-readable detail for non-imported statuses. */
+  reason?: string;


+    /// Human-readable detail for non-imported statuses.
+    pub reason: Option<String>,


Addresses the Copilot review on PR #424: - TS record matching now keys on recipient digest plus a canonical asset fingerprint (together equivalent to the details commitment) instead of recipient digest alone, both in the pre-import scan and the post-import consumed-state check; two distinct notes can share a recipient while carrying different assets. Regression tests cover both collision paths. (Rust already keys on the actual details commitment.) - Document that an imported outcome can carry a reason when the post-import consumed-state check fails (both SDKs). - Add a Rust test driving the public import_notes_from_proposals method: decoding, dedup, and invalid isolation happen before any network access, and an unreachable node surfaces per-note retryable failures instead of aborting the batch.

Adds import_notes_from_proposals (Rust) / importNotesFromProposals (TS): rebuilds store records from the note bytes embedded in v2 consume_notes proposals plus a node-fetched inclusion proof, so recovery works for private notes without the node holding the body (validated by spike #412). Per unique embedded note: decode, skip notes the store already tracks (matched by details commitment / recipient digest + asset fingerprint so metadata-less records are recognized), fetch proofs in one batch, import individually (upstream batches are atomic), and classify the outcome. Uncommitted notes are recorded as expected with their tag tracked so a later sync picks them up; chain-nullified notes are recorded as consumption history and reported already-consumed. No per-note problem aborts the batch. Includes the fixes from the PR #424 Copilot review.

Review follow-up on PR #424: the standalone helper required passing the Miden RPC endpoint although the loaded client already knows it (neither the MidenClient facade nor the raw WASM client exposes its endpoint, so a standalone function cannot derive it). The new Multisig method reuses the client's endpoint and resolved retry configuration, and syncs pending proposals from GUARDIAN when none are passed. The standalone export stays for callers holding a raw WASM client or proposals from another source; docs now lead with the method. Also drops a stray README section that a shared rerere resolution replayed from another branch's conflict.

zeljkoX

LGTM

Thanks

haseebrabbani requested a review from zeljkoX as a code owner August 20, 2026 16:23

github-actions Bot added the cla: allowlist label Aug 20, 2026

zeljkoX assigned haseebrabbani Aug 21, 2026

zeljkoX added this to OZ Development for Miden Aug 21, 2026

zeljkoX moved this to Review in OZ Development for Miden Aug 21, 2026

zeljkoX added this to the Guardian #02 - M2 milestone Aug 21, 2026

zeljkoX reviewed Aug 21, 2026

View reviewed changes

zeljkoX requested a balanced review from Copilot August 21, 2026 11:57

Copilot started reviewing on behalf of zeljkoX August 21, 2026 11:57 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

haseebrabbani changed the base branch from main to 357-recovery-primitives August 21, 2026 12:15

haseebrabbani force-pushed the 415-proposal-note-import branch from 0af9534 to 5aeb37f Compare August 21, 2026 14:22

haseebrabbani mentioned this pull request Aug 24, 2026

feat(sdk): historical public-note backfill by tag recovery primitive (#416) #429

Merged

haseebrabbani force-pushed the 415-proposal-note-import branch from 5aeb37f to 0c79960 Compare August 25, 2026 20:52

zeljkoX approved these changes Aug 27, 2026

View reviewed changes

haseebrabbani merged commit 5d9f69e into 357-recovery-primitives Aug 27, 2026
7 checks passed

haseebrabbani deleted the 415-proposal-note-import branch August 27, 2026 13:04

github-project-automation Bot moved this from Review to Done in OZ Development for Miden Aug 27, 2026

github-actions Bot locked and limited conversation to collaborators Aug 27, 2026

Navigation Menu

Uh oh!

feat(sdk): import proposal-embedded notes as a recovery primitive (#415) - #424

feat(sdk): import proposal-embedded notes as a recovery primitive (#415)#424
haseebrabbani merged 2 commits into
357-recovery-primitivesfrom
415-proposal-note-import

haseebrabbani commented Aug 20, 2026

Uh oh!

coderabbitai Bot commented Aug 20, 2026 •

edited

Loading

Review skipped

Uh oh!

codecov-commenter commented Aug 20, 2026

Uh oh!

zeljkoX commented Aug 21, 2026

Uh oh!

zeljkoX Aug 21, 2026

Uh oh!

haseebrabbani Aug 25, 2026

Uh oh!

zeljkoX Aug 21, 2026

Uh oh!

zeljkoX Aug 21, 2026

Uh oh!

zeljkoX Aug 25, 2026

Uh oh!

haseebrabbani Aug 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

zeljkoX left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		/** Human-readable detail for non-imported statuses. */
		reason?: string;

		/// Human-readable detail for non-imported statuses.
		pub reason: Option<String>,

Uh oh!

Conversation

haseebrabbani commented Aug 20, 2026

What

Key behaviors (live-tested against testnet)

Docs

Notes for reviewers

Uh oh!

coderabbitai Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

codecov-commenter commented Aug 20, 2026

Codecov Report

Uh oh!

zeljkoX commented Aug 21, 2026

Uh oh!

zeljkoX Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

haseebrabbani Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

zeljkoX Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

zeljkoX Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

zeljkoX Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

haseebrabbani Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

zeljkoX left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Aug 20, 2026 •

edited

Loading