[danielpeng1]

buildSafeRecoverKeyParams turns derived Safe child keys into the params a coin's recover() expects. It threw for eddsaMpc wallets (SOL, ADA, etc.) and for XLM, so those couldn't be recovered. This adds both.

eddsaMpc (slot ④)

• Add an eddsaMpc branch. Signed mode re-encrypts the child shares under a throwaway passphrase; unsigned mode returns only the child common keychain. Both always set isTss: true and broadcast: false.
• Share one MpcSafeRecoverKeyParams<Slot> type and builder between ecdsaMpc and eddsaMpc. EcdsaMpcSafeRecoverKeyParams keeps its shape.
• Remove the runtime "not supported yet" fall-through, so a slot added without a branch is now a compile error.

XLM (slot ②, ed25519Multisig)

• Register xlm and txlm in the ed25519Multisig encoders. rootAddress is not derived (as with HBAR), so the caller still supplies it.

Tests

eddsaMpc

• sdk-core safe/recovery.ts: the MPC signed and unsigned tests now run for both ecdsaMpc and eddsaMpc.
• solSafeRecovery.ts and adaSafeRecovery.ts: derive, build, then run the real recover() with real MPS signing. Signed signatures verify under the derived key, unsigned sweeps spend from the derived address, and shares from the wrong account are rejected.

XLM

• sdk-core safe/recovery.ts: XLM/TXLM golden vectors (signed and unsigned).
• xlmSafeRecovery.ts: signed recovery through the lazy backup signer with both signatures verified, unsigned recovery with no encrypt or decrypt, and RecoveryKeyMismatchError when the derived keys aren't account signers.

Ticket: WCN-3914

[linear-code]

WCN-3914

[github-actions]

⚠️ Compatibility checks are failing on Node.js 27-nightly (experimental, non-blocking). This does not block merge, but it signals an incompatibility to fix before this release becomes supported.

View run

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[danielpeng1]

@claude

[github-actions]

Review of #10090 (eddsaMpc and XLM Safe recovery key params)

I found no blocking correctness or security problems. I read the diff and checked that the encodeEd25519StrKey* helpers exist and are exported from @bitgo/sdk-lib-safes. I did not run the tests.

Critical

None.

Internal information leaks (fix before merge)

• modules/sdk-core/test/unit/bitgo/safe/recovery.ts has a new comment: "WRW passes these requests over IPC, so the real untyped input is parsed JSON…". It names an internal consumer and its transport. Suggested rewrite: "Untyped callers (e.g. parsed JSON) can pass any slot or mode string; this typechecks without a cast because the input is any."
• modules/sdk-core/src/bitgo/safe/recovery.ts, in the existing JSDoc for the MPC params, says "WRW's coin-agnostic engine spreads these params into coin.recover()". The PR only reworded this comment, but it still names an internal system. Suggested rewrite: "Callers typically spread these params into coin.recover(), and ICP is the one coin whose recover() would otherwise submit the sweep to the network."
• Same file, comment on the lazy signer: "(WCN-2895's extended-key signer, invoked by the coin)". This is an internal ticket ID. Suggested rewrite: "the extended-key signer, invoked by the coin".
• xlmSafeRecovery.ts: the comment "Golden vectors generated with stellar-sdk StrKey over the same bytes" describes how the test data was made, not what the code does. Suggested rewrite: "Expected values are the Stellar StrKey encodings of edKeys."

Suggestions

1. In buildSafeRecoverKeyParams, const slot: string = params.slot; deliberately widens the type. The compile-time guarantee in the comment actually comes from the later params.keys.user.pub accesses, which stop typechecking if a new slot variant appears. Consider making that explicit, for example with an assertNever-style check after the secp256k1Multisig guard. The fail-closed runtime check stays, and the guarantee no longer depends on incidental typing.
2. The new unknown-mode guard interpolates ${mode} into the error. That is fine for 'signed' | 'unsigned', but untyped callers could pass an arbitrary object. Consider String(mode), or omit the value from the message.
3. The strict-hex check on keys.user.seed applies to every slot-2 coin, not just XLM. Please confirm no existing ALGO or HBAR caller passes uppercase or 0x-prefixed seeds. Uppercase is accepted by the /i flag. A 0x prefix would now throw, which is the safer behavior.
4. XLM does not derive rootAddress, so the caller must supply it. The doc update covers this. A check inside recover() already gives RecoveryKeyMismatchError, and the new test exercises it.

Design question

• Why is MpcSafeRecoverKeyParams left unexported while its two aliases are exported? That seems fine, but please confirm the generated .d.ts output looks sensible.

Good practices

• ecdsaMpc and eddsaMpc now share one generic builder and one type, so they cannot drift apart, and the public EcdsaMpcSafeRecoverKeyParams shape is unchanged.
• isTss: true and broadcast: false are always set on MPC params, so recovery never auto-broadcasts.
• A fresh 32-byte random ephemeral passphrase is used for the re-encryption.
• The strict-hex validation on seeds avoids silent Buffer.from(…, 'hex') truncation, which would have encoded the wrong private key.
• The tests are thorough:
  • The SOL and ADA tests run real MPS signing end to end.
  • The XLM tests check signatures under the derived keys.
  • Negative tests cover wrong shares, a wrong passphrase, an unknown slot or mode, a malformed seed, and a missing registry entry.
  • The XLM and TXLM golden vectors include a check that rootAddress is preserved.

🤖 Generated with Claude Code