Skip to content

Commit 65a93b6

Browse files
committed
feat: fetch keychain per wallet once
Ticket: CSI-1548
1 parent b32a426 commit 65a93b6

3 files changed

Lines changed: 217 additions & 68 deletions

File tree

modules/bitgo/test/v2/unit/wallets.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3708,6 +3708,99 @@ describe('V2 Wallets:', function () {
37083708
assert.equal(error.message, 'Password shared is incorrect for this wallet');
37093709
}
37103710
});
3711+
3712+
it('should fetch keychain only ONCE when sharing with multiple users', async () => {
3713+
const walletPassphrase = 'bitgo1234';
3714+
const pub = 'Zo1ggzTUKMY5bYnDvT5mtVeZxzf2FaLTbKkmvGUhUQk';
3715+
const prv1 = 'xprv1';
3716+
3717+
// Create multiple users to share with
3718+
const users = [
3719+
{
3720+
userId: 'user1@example.com',
3721+
permissions: ['view', 'spend'],
3722+
pubKey: '02705a6d33a2459feb537e7abe36aaad8c11532cdbffa3a2e4e58868467d51f532',
3723+
path: 'm/999999/1/1',
3724+
},
3725+
{
3726+
userId: 'user2@example.com',
3727+
permissions: ['view', 'spend'],
3728+
pubKey: '03705a6d33a2459feb537e7abe36aaad8c11532cdbffa3a2e4e58868467d51f533',
3729+
path: 'm/999999/1/2',
3730+
},
3731+
{
3732+
userId: 'user3@example.com',
3733+
permissions: ['view', 'spend'],
3734+
pubKey: '02815a6d33a2459feb537e7abe36aaad8c11532cdbffa3a2e4e58868467d51f534',
3735+
path: 'm/999999/1/3',
3736+
},
3737+
];
3738+
3739+
const params: BulkWalletShareOptions = {
3740+
walletPassphrase,
3741+
keyShareOptions: users,
3742+
};
3743+
3744+
const getDecryptedKeychainStub = sinon.stub(wallet, 'getDecryptedKeychainForSharing').resolves({
3745+
prv: prv1,
3746+
pub,
3747+
});
3748+
3749+
const encryptPrvForUserStub = sinon
3750+
.stub(wallet, 'encryptPrvForUser')
3751+
.callsFake((prv, pubKey, userPubKey, path) => {
3752+
return {
3753+
pub: pubKey,
3754+
encryptedPrv: 'dummyEncryptedPrv',
3755+
fromPubKey: 'dummyFromPubKey',
3756+
toPubKey: userPubKey,
3757+
path: path,
3758+
};
3759+
});
3760+
3761+
sinon.stub(wallet, 'createBulkKeyShares').resolves({
3762+
shares: users.map((user) => ({
3763+
id: user.userId,
3764+
coin: walletData.coin,
3765+
wallet: walletData.id,
3766+
fromUser: 'fromUser',
3767+
toUser: user.userId,
3768+
permissions: user.permissions,
3769+
keychain: {
3770+
pub: 'dummyPub',
3771+
encryptedPrv: 'dummyEncryptedPrv',
3772+
fromPubKey: 'dummyFromPubKey',
3773+
toPubKey: user.pubKey,
3774+
path: user.path,
3775+
},
3776+
})),
3777+
});
3778+
3779+
await wallet.createBulkWalletShare(params);
3780+
3781+
// Verify keychain is fetched only ONCE, not once per user
3782+
assert.strictEqual(
3783+
getDecryptedKeychainStub.callCount,
3784+
1,
3785+
'getDecryptedKeychainForSharing should be called exactly once, not once per user'
3786+
);
3787+
3788+
// Verify encryption happens for EACH user (once per user)
3789+
assert.strictEqual(
3790+
encryptPrvForUserStub.callCount,
3791+
users.length,
3792+
`encryptPrvForUser should be called once per user (${users.length} times)`
3793+
);
3794+
3795+
// Verify each call had the correct user's pubKey and path
3796+
users.forEach((user, index) => {
3797+
const call = encryptPrvForUserStub.getCall(index);
3798+
assert.strictEqual(call.args[0], prv1, 'Should use the same decrypted private key for all users');
3799+
assert.strictEqual(call.args[1], pub, 'Should use the same public key for all users');
3800+
assert.strictEqual(call.args[2], user.pubKey, `Should use user${index + 1}'s public key`);
3801+
assert.strictEqual(call.args[3], user.path, `Should use user${index + 1}'s path`);
3802+
});
3803+
});
37113804
});
37123805

37133806
describe('List Wallets:', function () {

modules/sdk-core/src/bitgo/wallet/iWallet.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,11 @@ export interface BulkWalletShareOptions {
645645

646646
export type WalletShareState = 'active' | 'accepted' | 'canceled' | 'rejected' | 'pendingapproval';
647647

648+
export interface DecryptedKeychainData {
649+
prv: string;
650+
pub: string;
651+
}
652+
648653
export interface BulkWalletShareKeychain {
649654
pub: string;
650655
encryptedPrv: string;

modules/sdk-core/src/bitgo/wallet/wallet.ts

Lines changed: 119 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import {
7070
CreatePolicyRuleOptions,
7171
CreateShareOptions,
7272
CrossChainUTXO,
73+
DecryptedKeychainData,
7374
DeployForwardersOptions,
7475
DownloadKeycardOptions,
7576
FanoutUnspentsOptions,
@@ -1685,8 +1686,27 @@ export class Wallet implements IWallet {
16851686
if (params.keyShareOptions.length === 0) {
16861687
throw new Error('No share options provided');
16871688
}
1689+
16881690
const bulkCreateShareOptions: BulkCreateShareOption[] = [];
16891691

1692+
// Check if any share option needs a keychain (has 'spend' permission)
1693+
const anyNeedsKeychain = params.keyShareOptions.some((opt) => opt.permissions && opt.permissions.includes('spend'));
1694+
1695+
// Fetch and decrypt the keychain ONCE for all users
1696+
let decryptedKeychain: DecryptedKeychainData | undefined;
1697+
1698+
if (anyNeedsKeychain) {
1699+
try {
1700+
decryptedKeychain = await this.getDecryptedKeychainForSharing(params.walletPassphrase);
1701+
} catch (e) {
1702+
if (e instanceof MissingEncryptedKeychainError) {
1703+
decryptedKeychain = undefined;
1704+
} else {
1705+
throw e;
1706+
}
1707+
}
1708+
}
1709+
16901710
for (const shareOption of params.keyShareOptions) {
16911711
try {
16921712
common.validateParams(shareOption, ['userId', 'pubKey', 'path'], []);
@@ -1699,36 +1719,22 @@ export class Wallet implements IWallet {
16991719

17001720
const needsKeychain = shareOption.permissions && shareOption.permissions.includes('spend');
17011721

1702-
if (needsKeychain) {
1703-
const sharedKeychain = await this.prepareSharedKeychain(
1704-
params.walletPassphrase,
1722+
if (needsKeychain && decryptedKeychain) {
1723+
const sharedKeychain = this.encryptPrvForUser(
1724+
decryptedKeychain.prv,
1725+
decryptedKeychain.pub,
17051726
shareOption.pubKey,
17061727
shareOption.path
17071728
);
1708-
const keychain = Object.keys(sharedKeychain ?? {}).length === 0 ? undefined : sharedKeychain;
1709-
if (keychain) {
1710-
assert(keychain.pub, 'pub must be defined for sharing');
1711-
assert(keychain.encryptedPrv, 'encryptedPrv must be defined for sharing');
1712-
assert(keychain.fromPubKey, 'fromPubKey must be defined for sharing');
1713-
assert(keychain.toPubKey, 'toPubKey must be defined for sharing');
1714-
assert(keychain.path, 'path must be defined for sharing');
1715-
1716-
const bulkKeychain: BulkWalletShareKeychain = {
1717-
pub: keychain.pub,
1718-
encryptedPrv: keychain.encryptedPrv,
1719-
fromPubKey: keychain.fromPubKey,
1720-
toPubKey: keychain.toPubKey,
1721-
path: keychain.path,
1722-
};
17231729

1724-
bulkCreateShareOptions.push({
1725-
user: shareOption.userId,
1726-
permissions: shareOption.permissions,
1727-
keychain: bulkKeychain,
1728-
});
1729-
}
1730+
bulkCreateShareOptions.push({
1731+
user: shareOption.userId,
1732+
permissions: shareOption.permissions,
1733+
keychain: sharedKeychain,
1734+
});
17301735
}
17311736
}
1737+
17321738
return await this.createBulkKeyShares(bulkCreateShareOptions);
17331739
}
17341740

@@ -1776,62 +1782,107 @@ export class Wallet implements IWallet {
17761782
}
17771783
}
17781784

1779-
async prepareSharedKeychain(
1780-
walletPassphrase: string | undefined,
1781-
pubkey: string,
1782-
path: string
1783-
): Promise<SharedKeyChain> {
1784-
let sharedKeychain: SharedKeyChain = {};
1785+
/**
1786+
* Fetches and decrypts the wallet keychain for sharing.
1787+
* This method fetches the keychain once and decrypts it, returning the decrypted
1788+
* private key and public key info needed for sharing with multiple users.
1789+
*
1790+
* @param walletPassphrase - The passphrase to decrypt the keychain
1791+
* @returns Object containing decrypted prv and pub, or undefined for cold wallets
1792+
*/
1793+
async getDecryptedKeychainForSharing(
1794+
walletPassphrase: string | undefined
1795+
): Promise<DecryptedKeychainData | undefined> {
1796+
const keychain = await this.getEncryptedWalletKeychainForWalletSharing();
17851797

1786-
try {
1787-
const keychain = await this.getEncryptedWalletKeychainForWalletSharing();
1798+
if (!keychain.encryptedPrv) {
1799+
return undefined;
1800+
}
17881801

1789-
// Decrypt the user key with a passphrase
1790-
if (keychain.encryptedPrv) {
1791-
if (!walletPassphrase) {
1792-
throw new Error('Missing walletPassphrase argument');
1793-
}
1802+
if (!walletPassphrase) {
1803+
throw new Error('Missing walletPassphrase argument');
1804+
}
17941805

1795-
const userPrv = decryptKeychainPrivateKey(this.bitgo, keychain, walletPassphrase);
1796-
if (!userPrv) {
1797-
throw new IncorrectPasswordError('Password shared is incorrect for this wallet');
1798-
}
1806+
const prv = decryptKeychainPrivateKey(this.bitgo, keychain, walletPassphrase);
1807+
if (!prv) {
1808+
throw new IncorrectPasswordError('Password shared is incorrect for this wallet');
1809+
}
17991810

1800-
keychain.prv = userPrv;
1801-
const eckey = makeRandomKey();
1802-
const secret = getSharedSecret(eckey, Buffer.from(pubkey, 'hex')).toString('hex');
1803-
const newEncryptedPrv = this.bitgo.encrypt({
1804-
password: secret,
1805-
input: keychain.prv,
1806-
});
1811+
// Only one of pub/commonPub/commonKeychain should be present in the keychain
1812+
let pub = keychain.pub ?? keychain.commonPub;
1813+
if (keychain.commonKeychain) {
1814+
pub =
1815+
this.baseCoin.getMPCAlgorithm() === 'eddsa'
1816+
? EddsaUtils.getPublicKeyFromCommonKeychain(keychain.commonKeychain)
1817+
: EcdsaUtils.getPublicKeyFromCommonKeychain(keychain.commonKeychain);
1818+
}
18071819

1808-
// Only one of pub/commonPub/commonKeychain should be present in the keychain
1809-
let pub = keychain.pub ?? keychain.commonPub;
1810-
if (keychain.commonKeychain) {
1811-
pub =
1812-
this.baseCoin.getMPCAlgorithm() === 'eddsa'
1813-
? EddsaUtils.getPublicKeyFromCommonKeychain(keychain.commonKeychain)
1814-
: EcdsaUtils.getPublicKeyFromCommonKeychain(keychain.commonKeychain);
1815-
}
1820+
if (!pub) {
1821+
throw new Error('Unable to determine public key from keychain');
1822+
}
18161823

1817-
sharedKeychain = {
1818-
pub,
1819-
encryptedPrv: newEncryptedPrv,
1820-
fromPubKey: eckey.publicKey.toString('hex'),
1821-
toPubKey: pubkey,
1822-
path: path,
1823-
};
1824+
return { prv, pub };
1825+
}
1826+
1827+
/**
1828+
* Encrypts a decrypted private key for sharing with a specific user.
1829+
* This is the pure encryption step - no API calls, no decryption.
1830+
*
1831+
* @param decryptedPrv - The already-decrypted private key
1832+
* @param pub - The wallet's public key
1833+
* @param userPubkey - The recipient user's public key
1834+
* @param path - The key path
1835+
* @returns The encrypted keychain for the recipient with all required fields
1836+
*/
1837+
encryptPrvForUser(decryptedPrv: string, pub: string, userPubkey: string, path: string): BulkWalletShareKeychain {
1838+
const eckey = makeRandomKey();
1839+
const secret = getSharedSecret(eckey, Buffer.from(userPubkey, 'hex')).toString('hex');
1840+
const newEncryptedPrv = this.bitgo.encrypt({ password: secret, input: decryptedPrv });
1841+
1842+
const keychain: BulkWalletShareKeychain = {
1843+
pub,
1844+
encryptedPrv: newEncryptedPrv,
1845+
fromPubKey: eckey.publicKey.toString('hex'),
1846+
toPubKey: userPubkey,
1847+
path: path,
1848+
};
1849+
1850+
assert(keychain.pub, 'pub must be defined for sharing');
1851+
assert(keychain.encryptedPrv, 'encryptedPrv must be defined for sharing');
1852+
assert(keychain.fromPubKey, 'fromPubKey must be defined for sharing');
1853+
assert(keychain.toPubKey, 'toPubKey must be defined for sharing');
1854+
assert(keychain.path, 'path must be defined for sharing');
1855+
1856+
return keychain;
1857+
}
1858+
1859+
/**
1860+
* Prepares a keychain for sharing with another user.
1861+
* Fetches the wallet keychain, decrypts it, and encrypts it for the recipient.
1862+
*
1863+
* @param walletPassphrase - The passphrase to decrypt the keychain
1864+
* @param pubkey - The recipient's public key
1865+
* @param path - The key path
1866+
* @returns The encrypted keychain for the recipient
1867+
*/
1868+
async prepareSharedKeychain(
1869+
walletPassphrase: string | undefined,
1870+
pubkey: string,
1871+
path: string
1872+
): Promise<SharedKeyChain> {
1873+
try {
1874+
const decryptedKeychain = await this.getDecryptedKeychainForSharing(walletPassphrase);
1875+
if (!decryptedKeychain) {
1876+
return {};
18241877
}
1878+
return this.encryptPrvForUser(decryptedKeychain.prv, decryptedKeychain.pub, pubkey, path);
18251879
} catch (e) {
18261880
if (e instanceof MissingEncryptedKeychainError) {
1827-
sharedKeychain = {};
18281881
// ignore this error because this looks like a cold wallet
1829-
} else {
1830-
throw e;
1882+
return {};
18311883
}
1884+
throw e;
18321885
}
1833-
1834-
return sharedKeychain;
18351886
}
18361887

18371888
/**

0 commit comments

Comments
 (0)