Skip to content

feat(sdk-core): add root-fetch detour in getUserPrv for safe child wallets - #9488

Open
s84krish wants to merge 1 commit into
masterfrom
sibikrishnan/wcn-1200-sdk-multisig-signing-root-fetch-detour-in-getuserprvasync
Open

feat(sdk-core): add root-fetch detour in getUserPrv for safe child wallets#9488
s84krish wants to merge 1 commit into
masterfrom
sibikrishnan/wcn-1200-sdk-multisig-signing-root-fetch-detour-in-getuserprvasync

Conversation

@s84krish

@s84krish s84krish commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Ticket: WCN-1200

Summary

  • Enable safe owner signing for child wallets whose user key has no encryptedPrv: fetch the root, decrypt with the safe passphrase, and hardened-derive the child (m/999999'/') via shared safeDerivation.ts.
  • Add a signing-side accessor (resolveSafeOwnerSigningPrv) with fail-closed multisig pub verification against the registered child pub.
  • Wire call sites (getUserPrv, hot-wallet signTransaction, getUserKeyAndSignTssTransaction, pending-approval TSS recreate) without detouring getPrv / getEncryptedUserKeychain (wallet sharing must not receive root material).
  • Preserve sharee and SMC behavior: sharees decrypt their child encryptedPrv as-is; soft deriveKeyWithSeed remains for non-safe SMC only.

Test plan

  • Unit: safe owner root fetch + hardened derive (not soft deriveKeyWithSeed)
  • Unit: pub mismatch aborts locally
  • Unit: sharee path — no root fetch, no soft derive (incl. explicit prv)
  • Unit: SMC coldDerivationSeed still applied for non-safe wallets
  • Unit: fetchRootKeychainForSafeChild rejects non-user root source
  • Unit: signing guards + pending-approval safe path use getUserPrv
  • Confirm wallet-share / getEncryptedUserKeychain still skips for safe owners (no root exposure)
  • E2E multisig send from a safe child (tracked with WCN-1187 / WCN-1206)

@linear-code

linear-code Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

WCN-1200

@s84krish
s84krish force-pushed the sibikrishnan/wcn-1200-sdk-multisig-signing-root-fetch-detour-in-getuserprvasync branch from ebc1572 to a8653a5 Compare August 13, 2026 15:34
@s84krish
s84krish marked this pull request as ready for review August 13, 2026 16:18
@s84krish
s84krish requested review from a team as code owners August 13, 2026 16:18
@s84krish
s84krish force-pushed the sibikrishnan/wcn-1200-sdk-multisig-signing-root-fetch-detour-in-getuserprvasync branch from a8653a5 to fb3f9e8 Compare August 13, 2026 17:57

const decryptedPrv = await this.wallet.getPrv({ walletPassphrase });
let decryptedPrv: string;
if (this.wallet.safeId()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the wallet does come from a safe - if the spender is someone who the wallet was shared with, they will not require this detour - it is only required if it is the wallet minter.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, removed the large if safeId() block and just detoured to getUserPrv if it is the safe wallet minter

@s84krish
s84krish force-pushed the sibikrishnan/wcn-1200-sdk-multisig-signing-root-fetch-detour-in-getuserprvasync branch 2 times, most recently from 2a91432 to 8850785 Compare August 14, 2026 16:24
pranavjain97
pranavjain97 previously approved these changes Aug 14, 2026
);
}

const rootKeychain = params.rootKeychain ?? (await fetchRootKeychainForSafeChild(keychains, childKeychain));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: doesn't assert rootKeychain.id === childKeychain.parent before use, the pub-match check below catches a mismatch but an explicit assertion would make the invariant self-documenting

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this could be a bit redundant - am okay with adding if we strongly feel it adds value though!


// Soft seed derivation for SMC (and any explicit coldDerivationSeed), including
// after decrypting encryptedPrv when no params.prv was passed.
if (userPrv && params.coldDerivationSeed) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: coldDerivationSeed now applies after decrypting encryptedPrv too, previously only for direct params.prv. no call site currently passes both together, intentional new path or unused scope creep?

@s84krish s84krish Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for calling this out, it was unintentional. No callers as you mentioned but looking to keep the existing paths the same and only add the additional path for the safe children. Closed this gap

@s84krish
s84krish force-pushed the sibikrishnan/wcn-1200-sdk-multisig-signing-root-fetch-detour-in-getuserprvasync branch from 8850785 to 82522a1 Compare August 14, 2026 19:31
@s84krish
s84krish force-pushed the sibikrishnan/wcn-1200-sdk-multisig-signing-root-fetch-detour-in-getuserprvasync branch from 82522a1 to b767ca2 Compare August 14, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants