feat(wasm-mps): add support for RedPallas DKG, key derivation, DSG

[johnoliverdriscoll]

Ticket: HSM-361

[linear-code]

HSM-361

[Copilot]

Pull request overview

Adds RedPallas support to the wasm-mps package, extending the WASM-exposed multi-party signing/keygen surface beyond Ed25519 by introducing RedPallas DKG, Orchard key-derivation session driving, and RedPallas DSG signing/verification helpers.

Changes:

• Add RedPallas DKG round APIs plus Orchard derivation-session init/step and FVK→IVK utilities in the Rust WASM module.
• Expand Mocha tests to cover RedPallas flows (DKG, derivation progression, DSG) and add more negative-prefix cases.
• Update build/test configuration and Rust dependencies/profiles to support the new crypto stack and optimize release builds.

Reviewed changes

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

Show a summary per file

File	Description
packages/wasm-mps/test/mps.ts	Reorganizes existing Ed25519 tests and adds extensive RedPallas DKG/derivation/DSG test coverage (including negative cases).
packages/wasm-mps/src/lib.rs	Implements RedPallas DKG/DSG + derivation session plumbing and exposes new wasm-bindgen types/functions.
packages/wasm-mps/package.json	Adjusts Mocha invocation to tweak Node warning behavior.
packages/wasm-mps/Makefile	Changes wasm-opt flags (SIMD + optimization level).
packages/wasm-mps/Cargo.toml	Adds Orchard/zcash/pasta_curves deps and release profile tuning; patches Orchard source.
packages/wasm-mps/Cargo.lock	Locks newly introduced dependency graph for RedPallas/Orchard/zcash components.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Logicwax]

@claude please review

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[johnoliverdriscoll]

@copilot If you're only finding low level corrections at this point, would you feel comfortable merging this PR?

[Copilot]

@copilot If you're only finding low level corrections at this point, would you feel comfortable merging this PR?

Yes, the PR looks ready to merge. All critical issues have been resolved:

• Test logic bugs corrected (proper round state usage, correct arguments)
• Input validation added (no more panics on short buffers)
• Documentation updated to reflect actual behavior

The remaining suggestions in resolved threads are about making error messages more parameter-specific (e.g., "decryption_key must be 32 bytes" vs "Invalid input"). These would be nice quality-of-life improvements but aren't blockers.

[Logicwax]

claude review....

PR audit: ca08435 "feat(wasm-mps): add support for RedPallas DKG, key derivation, DSG"

Files: 6 · Lines: +2632 / -638 · Lenses run: Security, Correctness, Conventions, Tests, API/Compat, Performance

Verdict

BLOCKED-ON-SECURITY — the full Orchard spending key scalar is returned to JavaScript, breaking MPC threshold guarantees. Must be fixed before merge.

---

Security

• [HIGH] lib.rs:1417 — Full Orchard ask spending key scalar returned to JavaScript. MsgDerivation.ask() is a public #[wasm_bindgen] getter returning self.ask.clone() — the full 32-byte Pallas scalar. Any single party's JS runtime receives the complete spending authorization key after derivation, defeating the MPC threshold model. The redpallas_ask_to_ak function (line 1748) confirms this is the private scalar: it multiplies ask by G to produce ak. Fix: compute ak = ask * G inside WASM, expose only ak, nk, and rivk, then zeroize ask before returning.

• [MED] lib.rs:1682 — Per-signature randomizer alpha exposed to JavaScript. RedPallasSignature.alpha() returns the Pallas scalar used to blind the signing key as rk = ak + alpha*G. A caller holding both alpha and rk can recover ak = rk - alpha*G, linking the spend action to the signer's base key and breaking Orchard spend unlinkability. If the spend proof must be constructed in JS, document alpha explicitly as private spend-critical material that must never be logged or persisted; otherwise absorb proof construction into WASM.

• [MED] Cargo.toml:22 — Core derivation protocol (DerivationSession) sourced from an unaudited private third-party git repo (silence-laboratories/garbling, no crates.io presence). The rev is pinned (correct), but the ZK proof verification and commitment-consistency logic in this crate cannot be independently audited from this diff. A formal audit of the pinned commit should be on record before this ships.

---

Correctness

• [MED] lib.rs:1545 — ed25519_dsg_round3_process WASM export has parameter named round2_message instead of round3_message. wasm-bindgen generates TypeScript bindings directly from Rust parameter names; the published .d.ts will expose round2Message: Uint8Array for a round-3 input. Tests pass because they call positionally. Fix: rename round2_message → round3_message in the #[wasm_bindgen] wrapper.

• [MED] lib.rs:159 — IntoSignReady<G> encodes T via bincode and .try_into::<[u8; 32]>() with no static size guarantee. Currently T is pallas::Scalar (exactly 32 bytes under standard bincode config), so it works. If the upstream type ever gets a newtype wrapper or length prefix, this fails with a misleading SerializationError at DSG round 2 rather than a compile-time error. Fix: add a debug_assert_eq!(encoded.len(), 32) with a clear message, or bound T: Into<[u8; 32]> directly.

• [LOW] lib.rs:883 — Sign-correction loop in redpallas_fvk_to_ivks has no iteration cap and no comment explaining termination. The loop is guaranteed to exit in ≤2 iterations because negating a non-zero Pallas scalar flips its encoding's sign bit, but there is no assertion or comment expressing this invariant. Fix: add a debug_assert or cap at 2 iterations with unreachable!() on the third; add a comment citing the pallas sign-bit-flip property.

---

Conventions & style

• [MED] lib.rs:790 / lib.rs:1388 — Zero-byte sentinel values returned for all FVK key fields when done == false. When derivation is incomplete, ask/nk/rivk/internal_ivk/external_ivk are all zero-filled ([0u8; 32], [0u8; 64]). Callers who skip the done check silently receive zero keys with no type-level indication anything is missing. Either use Option<Vec<u8>> (becomes undefined in JS) for the key fields, or add a prominent WHY comment that zeros are intentional sentinels and must never be used before done == true.

• [MED] lib.rs:~2406 (MsgDerivationInit) — drv field named inconsistently with msg on MsgDerivation; both represent the same derivation message pool. The test has to rename it explicitly (message = Buffer.concat(...drv)) which is a clear signal. Rename drv → msg on MsgDerivationInit to match MsgState.msg and MsgDerivation.msg.

• [LOW] lib.rs:821 — Derivation path hardcoded to "m" in redpallas_dsg_round0_process with no WHY comment and a dead map_err arm. The Ed25519 counterpart accepts derivation_path: String. The asymmetry is intentional (Orchard key randomization is handled by DerivationSession, not BIP32), but without a comment callers reading both APIs will expect to pass a path. Fix: add a doc comment — "RedPallas signing always uses the root path; key randomization is handled by the derivation session." Remove the dead "m".parse() error arm or replace with unwrap().

• [LOW] lib.rs:~2067 — WHAT inline comment in redpallas_derivation_process: // Find and consume the first message in the pool addressed to this party. Remove it per project convention (WHY comments only).

---

Tests

• [HIGH] test/mps.ts:536 — RedPallas has no input-validation tests. describe("ed25519") has a full describe("input handling") block with 10+ shouldThrow cases covering bad party size, bad keys, bad seed, empty buffers, and wrong types for every DKG/DSG round function. describe("redpallas") has none. The WASM input-coercion paths (try_into, js_array_to_2_bufs) are present for both curves; any panic-safety gap in RedPallas is invisible. Add a matching describe("input handling") block.

• [MED] test/mps.ts:778 — shares declared as Array<mps.Share> but assigned the return value of redpallas_dkg_round2_process(), which returns MsgDerivationInit (shape: share/pk/drv/state), not Share (shape: share/pk/chaincode). TypeScript doesn't catch this because downstream code reads only .share. Fix: change the declaration to Array<mps.MsgDerivationInit>.

• [MED] lib.rs:1748 / test/mps.ts (absent) — redpallas_ask_to_ak is exported via #[wasm_bindgen] but never called in mps.ts. Add a smoke test: derive ak from a known ask, verify output is 32 bytes and non-zero, verify a 31-byte input is rejected.

• [MED] test/mps.ts:949 — redpallas_verify tested only with a valid signature. Negative paths (wrong message, corrupted signature, different key) are untested. verify() should return false in those cases, not throw; add assertions for that.

• [MED] lib.rs (absent) — No test_redpallas_dkg Rust unit test. There are test_ed25519_dkg, test_ed25519_dsg, and test_redpallas_dsg, but DKG and redpallas_derivation_process have no Rust-level tests. Add test_redpallas_dkg and test_redpallas_derivation.

• [LOW] test/mps.ts:637,664,709 — Each party receives a distinct crypto.randomBytes(32) as derivation_seed, while the Rust unit test uses a single shared seed across all parties. If derivation_seed is a shared commitment rather than per-party entropy, the TS tests are exercising an invalid input scenario. Align or add a comment clarifying intent.

---

API / compatibility

• [MED] package.json:4 — npm version not bumped: still 0.0.1 while Cargo.toml is 0.1.0. 10+ new #[wasm_bindgen] exports were added. Bump package.json to 0.1.0 to match.

---

Performance

• [LOW] lib.rs:192 — rem_prefix takes &Vec<u8> instead of &[u8], forcing .to_vec() at all 11 call sites (lines 561, 578, 629, 650, 668, 703, 719, 753, 840, 857, 920). Each creates an unnecessary heap allocation for state blobs that can be several KB. Fix: change signature to data: &[u8] and remove all .to_vec() at call sites.

• [LOW] lib.rs:347,407,448,489 — Outgoing round message is .clone()d into the state struct, then the original is also serialized separately into MsgState.msg. Every round function allocates a redundant cryptographic message clone. Fix: serialize to bytes first (consuming the original), then move into the state struct.

---

Nits

• lib.rs:200 — add_prefix double-allocates via [...].concat(); use Vec::with_capacity + extend_from_slice.
• lib.rs:229 — strip_prefix(...).ok_or(...)? discards the Some result and reads like an accidental drop; replace with an explicit if comparison.
• lib.rs:391 — state.msg.clone() in internal_dkg_round2_process is avoidable; destructure the owned state and move msg directly.
• lib.rs:1733,1747 — redpallas_verify and redpallas_ask_to_ak are the only new #[wasm_bindgen] functions with no doc comments.
• test/mps.ts:16 — shouldThrow returns the error but callers never assert on its type or message; a binding TypeError and a deliberate MpsError::InvalidInput are indistinguishable.
• test/mps.ts:~3800 — derivedKeys.get(2) with no comment explaining party 2 is the HSM; rename or annotate.

---

What looks good

• New dep supply chain is clean: all crates.io deps have pinned checksums in Cargo.lock; both git deps are pinned to specific commit hashes.
• Protocol structure is well-mirrored: the RedPallas DKG/DSG round functions follow the same pattern as Ed25519, making the code navigable.
• Round-trip serialization discipline: the add_prefix/rem_prefix state tagging pattern correctly prevents state from one protocol being fed into another.

---

Open questions for the author

• lib.rs:1417 — Is exposing ask (the full spending key scalar) to JS intentional? It breaks the MPC threat model: any single JS process after derivation holds the full private key. If Orchard spend proof construction genuinely requires ask in JS, this should be explicitly documented as a known trust-boundary decision — the PR description doesn't mention it.
• test/mps.ts:637 — Is derivation_seed meant to be shared across all parties (like a protocol nonce) or is per-party randomness deliberately supported? The Rust and TypeScript tests contradict each other on this point.

[johnoliverdriscoll]

@Logicwax Majority is AI slop, but I have resolved the few actual issues.

[HIGH] lib.rs:1417 — Full Orchard ask spending key scalar returned to JavaScript. MsgDerivation.ask() is a public #[wasm_bindgen] getter returning self.ask.clone() — the full 32-byte Pallas scalar. Any single party's JS runtime receives the complete spending authorization key after derivation, defeating the MPC threshold model. The redpallas_ask_to_ak function (line 1748) confirms this is the private scalar: it multiplies ask by G to produce ak. Fix: compute ak = ask * G inside WASM, expose only ak, nk, and rivk, then zeroize ask before returning.

The ask key is not used for spend authorization. It's one of the protocol authorization keys that platform needs access to. The sk key is still protected by the MPC distribution. I also don't understand why claude thinks there's some security model distinction between what runs in WASM versus what runs in the JS runtime. It's compromisable client code.

[MED] lib.rs:1682 — Per-signature randomizer alpha exposed to JavaScript. RedPallasSignature.alpha() returns the Pallas scalar used to blind the signing key as rk = ak + alphaG. A caller holding both alpha and rk can recover ak = rk - alphaG, linking the spend action to the signer's base key and breaking Orchard spend unlinkability. If the spend proof must be constructed in JS, document alpha explicitly as private spend-critical material that must never be logged or persisted; otherwise absorb proof construction into WASM.

Same as before. alpha is needed by wallet platform for proof generation.

[MED] Cargo.toml:22 — Core derivation protocol (DerivationSession) sourced from an unaudited private third-party git repo (silence-laboratories/garbling, no crates.io presence). The rev is pinned (correct), but the ZK proof verification and commitment-consistency logic in this crate cannot be independently audited from this diff. A formal audit of the pinned commit should be on record before this ships.

Garbling circuit is a public github repo and SL has gone through its release process which includes internal review and audit.

[MED] lib.rs:1545 — ed25519_dsg_round3_process WASM export has parameter named round2_message instead of round3_message. wasm-bindgen generates TypeScript bindings directly from Rust parameter names; the published .d.ts will expose round2Message: Uint8Array for a round-3 input. Tests pass because they call positionally. Fix: rename round2_message → round3_message in the #[wasm_bindgen] wrapper.

Fixed.

[MED] lib.rs:159 — IntoSignReady encodes T via bincode and .try_into::<[u8; 32]>() with no static size guarantee. Currently T is pallas::Scalar (exactly 32 bytes under standard bincode config), so it works. If the upstream type ever gets a newtype wrapper or length prefix, this fails with a misleading SerializationError at DSG round 2 rather than a compile-time error. Fix: add a debug_assert_eq!(encoded.len(), 32) with a clear message, or bound T: Into<[u8; 32]> directly.

This is stupid. This wants us to cause a runtime trap rather than just throw an error as we currently are doing. Will not fix.

[LOW] lib.rs:883 — Sign-correction loop in redpallas_fvk_to_ivks has no iteration cap and no comment explaining termination. The loop is guaranteed to exit in ≤2 iterations because negating a non-zero Pallas scalar flips its encoding's sign bit, but there is no assertion or comment expressing this invariant. Fix: add a debug_assert or cap at 2 iterations with unreachable!() on the third; add a comment citing the pallas sign-bit-flip property.

This function is only used for testing and is not part of the protocol. Will not fix.

[HIGH] test/mps.ts:536 — RedPallas has no input-validation tests. describe("ed25519") has a full describe("input handling") block with 10+ shouldThrow cases covering bad party size, bad keys, bad seed, empty buffers, and wrong types for every DKG/DSG round function. describe("redpallas") has none. The WASM input-coercion paths (try_into, js_array_to_2_bufs) are present for both curves; any panic-safety gap in RedPallas is invisible. Add a matching describe("input handling") block.

I believe the ed25519 tests prove that exact same methods used for RedPallas argument coercion are sane. We are not doing anything new here. Will not fix.

[MED] test/mps.ts:778 — shares declared as Array<mps.Share> but assigned the return value of redpallas_dkg_round2_process(), which returns MsgDerivationInit (shape: share/pk/drv/state), not Share (shape: share/pk/chaincode). TypeScript doesn't catch this because downstream code reads only .share. Fix: change the declaration to Array<mps.MsgDerivationInit>.

Fixed.

[MED] lib.rs:1748 / test/mps.ts (absent) — redpallas_ask_to_ak is exported via #[wasm_bindgen] but never called in mps.ts. Add a smoke test: derive ak from a known ask, verify output is 32 bytes and non-zero, verify a 31-byte input is rejected.

If it's not used in the tests, then it's not need. Will remove redapallas_ask_to_ak.

[MED] test/mps.ts:949 — redpallas_verify tested only with a valid signature. Negative paths (wrong message, corrupted signature, different key) are untested. verify() should return false in those cases, not throw; add assertions for that.

redpallas_verify is a wrapper around the orchard library. We are using it as a helper to verify produced signatures, not testing the orchard library. We assume orchard's own implementation is correct.

[MED] lib.rs (absent) — No test_redpallas_dkg Rust unit test. There are test_ed25519_dkg, test_ed25519_dsg, and test_redpallas_dsg, but DKG and redpallas_derivation_process have no Rust-level tests. Add test_redpallas_dkg and test_redpallas_derivation.

DKG is tested in the DSG test. Derivation is a long running test, I don't see the need to duplicate it in Rust when we already test it in JS.

[LOW] test/mps.ts:637,664,709 — Each party receives a distinct crypto.randomBytes(32) as derivation_seed, while the Rust unit test uses a single shared seed across all parties. If derivation_seed is a shared commitment rather than per-party entropy, the TS tests are exercising an invalid input scenario. Align or add a comment clarifying intent.

derivation_seed is not a shared commitment it is a seed for a PRNG.

[MED] package.json:4 — npm version not bumped: still 0.0.1 while Cargo.toml is 0.1.0. 10+ new #[wasm_bindgen] exports were added. Bump package.json to 0.1.0 to match.

Our CI bumps package version as part of release.

[LOW] lib.rs:192 — rem_prefix takes &Vec instead of &[u8], forcing .to_vec() at all 11 call sites (lines 561, 578, 629, 650, 668, 703, 719, 753, 840, 857, 920). Each creates an unnecessary heap allocation for state blobs that can be several KB. Fix: change signature to data: &[u8] and remove all .to_vec() at call sites.

This is actually a good suggestion. Fixed.

[LOW] lib.rs:347,407,448,489 — Outgoing round message is .clone()d into the state struct, then the original is also serialized separately into MsgState.msg. Every round function allocates a redundant cryptographic message clone. Fix: serialize to bytes first (consuming the original), then move into the state struct.

That's not how serialization works. These have to be done separately as the message property is part of a serialized object.

lib.rs:200 — add_prefix double-allocates via [...].concat(); use Vec::with_capacity + extend_from_slice.

Fixed.

lib.rs:229 — strip_prefix(...).ok_or(...)? discards the Some result and reads like an accidental drop; replace with an explicit if comparison.

Prefer the concise way it is written, but i'll assign the Some result to _ so its discard is clear.

lib.rs:391 — state.msg.clone() in internal_dkg_round2_process is avoidable; destructure the owned state and move msg directly.

Removed clone and it works without destructuring.

lib.rs:1733,1747 — redpallas_verify and redpallas_ask_to_ak are the only new #[wasm_bindgen] functions with no doc comments.

Because they are helper functions for the JS tests and are not used in the actual protocol.

test/mps.ts:16 — shouldThrow returns the error but callers never assert on its type or message; a binding TypeError and a deliberate MpsError::InvalidInput are indistinguishable.

They are distinguishable, we just aren't testing that deeply. It's fine for now. Will not fix.

test/mps.ts:~3800 — derivedKeys.get(2) with no comment explaining party 2 is the HSM; rename or annotate.

As if code cannot be self explanatory. But whatever, comment added.

[johnoliverdriscoll]

[MED] lib.rs:790 / lib.rs:1388 — Zero-byte sentinel values returned for all FVK key fields when done == false. When derivation is incomplete, ask/nk/rivk/internal_ivk/external_ivk are all zero-filled ([0u8; 32], [0u8; 64]). Callers who skip the done check silently receive zero keys with no type-level indication anything is missing. Either use Option<Vec> (becomes undefined in JS) for the key fields, or add a prominent WHY comment that zeros are intentional sentinels and must never be used before done == true.

Made these Options.

[MED] lib.rs:~2406 (MsgDerivationInit) — drv field named inconsistently with msg on MsgDerivation; both represent the same derivation message pool. The test has to rename it explicitly (message = Buffer.concat(...drv)) which is a clear signal. Rename drv → msg on MsgDerivationInit to match MsgState.msg and MsgDerivation.msg.

Originally named drv to match the Rust to C++ bindings in firmware, but they can be renamed to msg in this case.

[LOW] lib.rs:821 — Derivation path hardcoded to "m" in redpallas_dsg_round0_process with no WHY comment and a dead map_err arm. The Ed25519 counterpart accepts derivation_path: String. The asymmetry is intentional (Orchard key randomization is handled by DerivationSession, not BIP32), but without a comment callers reading both APIs will expect to pass a path. Fix: add a doc comment — "RedPallas signing always uses the root path; key randomization is handled by the derivation session." Remove the dead "m".parse() error arm or replace with unwrap().

Orchard does not support derivation as it is a privacy coin. Will add comment.

[LOW] lib.rs:~2067 — WHAT inline comment in redpallas_derivation_process: // Find and consume the first message in the pool addressed to this party. Remove it per project convention (WHY comments only).

This comment describes what the code is doing. Its purpose is justified. Will not fix.

[Logicwax]

😆 satoshi, is that you?!

[Logicwax]

lgtm