feat(sdk-core): update wallet share acceptance for multi-user-key #7530
Conversation
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
zahin-mohammad
force-pushed
the
WP-6769/update-wallet-accept-share
branch
from
236afa6 to
f548584
Compare
zahin-mohammad
requested review from
alextse-bg,
kisslove-dewangan,
mohammadalfaiyazbitgo,
mrdanish26 and
pranavjain97
Chent03
reviewed
Nov 17, 2025
mohammadalfaiyazbitgo
previously approved these changes
Nov 17, 2025
… operations Fixed processAcceptShare to correctly use newWalletPassphrase when provided for both keychainOverrideRequired and userMultiKeyRotationRequired cases in bulk wallet share operations. Updated validation to accept either newWalletPassphrase or userLoginPassword for userMultiKeyRotationRequired shares. Added comprehensive tests to verify: - Bulk accept share with userMultiKeyRotationRequired and newWalletPassphrase - Bulk accept share with keychainOverrideRequired and newWalletPassphrase - Error handling when neither password is provided - Correct usage of newWalletPassphrase vs userLoginPassword Fixed existing keychainOverrideRequired tests to use correct API endpoints (ofc instead of tbtc) and made nock matchers more flexible to handle dynamic signatures and payloads. WP-6769 TICKET: WP-6769
zahin-mohammad
force-pushed
the
WP-6769/update-wallet-accept-share
branch
from
ac7d5b5 to
b878014
Compare
mohammadalfaiyazbitgo
approved these changes
Nov 17, 2025
ayush-9
reviewed
Nov 17, 2025
| const keysForWalletShares = walletShares.flatMap((walletShare) => { | ||
| // Handle userMultiKeyRotationRequired case - these shares don't have keychains | ||
| if (walletShare.userMultiKeyRotationRequired) { | ||
| if (!params.userLoginPassword) { |
Contributor
There was a problem hiding this comment.
here params.userLoginPassword is checked but at line 1065 newWalletPassphrase is used which can be params.newWalletPassphrase || params.userLoginPassword;
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds support for accepting wallet shares when
userMultiKeyRotationRequiredis true. When this flag is set, users must provide a password to generate a new keychain, and both the public key and encrypted private key are sent to accept the share.Changes
acceptShare()andprocessAcceptShare(): HandleuserMultiKeyRotationRequiredby requiringuserPassword, generating a new keychain, encrypting the private key, and sending bothpubandencryptedPrvto accept the shareuserMultiKeyRotationRequired?: booleantoWalletShareinterface andpub?: stringtoUpdateShareOptionsinterfacesKey Difference
Unlike
keychainOverrideRequired, whenuserMultiKeyRotationRequiredis true, the wallet is NOT reshared with spenders after acceptance. The user generates their own keychain rather than using a provided one.Ticket: WP-6769