fix(sdk-core): throw NeedUserSignupError when shareWallet recipient pubkey is missing#9214
Draft
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
Draft
fix(sdk-core): throw NeedUserSignupError when shareWallet recipient pubkey is missing#9214bitgo-ai-agent-dev[bot] wants to merge 1 commit into
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
Conversation
…issing In shareWallet(), when needsKeychain is true and the recipient has never logged in (so /user/sharingkey returns no pubkey), the undefined pubkey was silently passed into prepareSharedKeychain() → encryptPrvForUser() → Buffer.from(undefined, 'hex'), causing a cryptic error that the UI surfaced as "wallet password incorrect". The bulk sharing path (createBulkWalletShare) already handled this by throwing NeedUserSignupError when pubkey is absent. Apply the same guard to the single-wallet shareWallet() path so callers receive a clear, actionable error indicating the recipient must complete account setup before keys can be shared. Also add two unit tests: - verifies NeedUserSignupError is thrown when pubkey is missing and spend permission is requested - verifies view-only shares (no spend permission) succeed even when pubkey is absent (no keychain needed for view shares) Ticket: POL-41 Session-Id: 963e6992-8423-4bd9-a7b7-06c6352ab017 Task-Id: 80a26fb6-aef8-4b14-868b-d2cb5259805a
Contributor
bitgo-ai-agent-dev
Bot
force-pushed
the
pol-41-fix-share-wallet-missing-pubkey
branch
from
fd2c4df to
9549422
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
shareWallet()(modules/sdk-core/src/bitgo/wallet/wallet.ts), added a guard that throwsNeedUserSignupErrorwhen the recipient'spubkeyis absent from the/user/sharingkeyAPI response and a spend-permission keychain is requiredWhy
johng@coinlist.co) was unable to share keys across ~61 wallets. TheshareWallet()single-wallet path passed anundefinedpubkey into the ECDH encryption step (Buffer.from(undefined, 'hex')), causing a cryptic failure that the UI surfaced as "wallet password incorrect" — a completely misleading messagecreateBulkWalletShare) already handled this correctly by throwingNeedUserSignupErrorwhenpubkeyis absent. This fix bringsshareWallet()into parity, so users receive a clear, actionable error: "the recipient must log in and complete account setup before keys can be shared"/user/sharingkeyendpoint returns only{ userId }(nopubkey/path) for users who have never completed ECDH key setup, regardless of whether they've ever logged inTest plan
yarn unit-test --scope bitgo -- --grep "NeedUserSignupError"shareWalletwith a recipient who has no pubkey now throwsNeedUserSignupErrorinstead of an incorrect-password errorTicket: POL-41