Skip to content

Commit b7d2cf4

Browse files
committed
fix: address validation for SMC wallets
TICKET: WP-7507
1 parent b72aca0 commit b7d2cf4

7 files changed

Lines changed: 421 additions & 5 deletions

File tree

.iyarc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,10 @@
55
# - Lerna only uses tar for PACKING
66
GHSA-8qq5-rm4j-mr97
77

8+
# Excluded because:
9+
# - Affects tar <= 7.5.3, fixed in 7.5.4+
10+
# - Unicode path collision issue on macOS filesystems
11+
# - Only affects archive extraction, not packing
12+
# - Transitive dependency through lerna and yeoman-generator
13+
GHSA-r6q2-hw4h-h46w
14+

modules/sdk-coin-sol/test/unit/sol.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3478,6 +3478,22 @@ describe('SOL:', function () {
34783478
message: `invalid address: ${invalidAddress}`,
34793479
});
34803480
});
3481+
3482+
it('should verify address with derivation prefix for SMC wallets', async function () {
3483+
// Address derived with derivation prefix m/999999/148242355/239336845/1
3484+
const address = 'CC715Q92C8vuEFT5xujE55Do6BkWRdpDvr7Vh8iUNUBw';
3485+
const commonKeychain =
3486+
'8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd';
3487+
const index = '1';
3488+
const testSeed = 'smc-test-seed-123';
3489+
const derivationPrefix = require('@bitgo/sdk-lib-mpc').getDerivationPath(testSeed);
3490+
const keychains = [{ id: '1', type: 'tss' as const, commonKeychain }];
3491+
3492+
// This test verifies that derivationPrefix is accepted as a parameter and correctly validates addresses
3493+
// derived with the SMC wallet derivation prefix
3494+
const result = await basecoin.isWalletAddress({ keychains, address, index, derivationPrefix });
3495+
result.should.equal(true);
3496+
});
34813497
});
34823498

34833499
describe('getAddressFromPublicKey', () => {

modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ export interface VerifyAddressOptions {
154154
error?: string;
155155
coinSpecific?: AddressCoinSpecific;
156156
impliedForwarderVersion?: number;
157+
/**
158+
* Optional derivation prefix for SMC wallets.
159+
* Used when verifying TSS addresses for Self-Managed Custodial wallets.
160+
*/
161+
derivationPrefix?: string;
157162
}
158163

159164
/**
@@ -173,8 +178,15 @@ export interface TssVerifyAddressOptions {
173178
/**
174179
* Derivation index for the address.
175180
* Used to derive child addresses from the root keychain via HD derivation path: m/{index}
181+
* For SMC (Self-Managed Custodial) wallets, the path includes a prefix: m/{derivationPrefix}/{index}
176182
*/
177183
index: number | string;
184+
/**
185+
* Optional derivation prefix for SMC wallets.
186+
* For SMC wallets, addresses are derived using m/{derivationPrefix}/{index} instead of m/{index}.
187+
* This prefix comes from derivedFromParentWithSeed on the user keychain.
188+
*/
189+
derivationPrefix?: string;
178190
}
179191

180192
export function isTssVerifyAddressOptions<T extends VerifyAddressOptions | TssVerifyAddressOptions>(

modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { Ecdsa } from '../../../account-lib/mpc';
22
import { TssVerifyAddressOptions } from '../../baseCoin/iBaseCoin';
33
import { InvalidAddressError } from '../../errors';
4-
import { EDDSAMethods } from '../../tss';
54

65
/**
76
* Extracts and validates the commonKeychain from keychains array.
@@ -65,15 +64,24 @@ export async function verifyMPCWalletAddress(
6564
isValidAddress: (address: string) => boolean,
6665
getAddressFromPublicKey: (publicKey: string) => string
6766
): Promise<boolean> {
68-
const { keychains, address, index } = params;
67+
const { keychains, address, index, derivationPrefix } = params;
6968

7069
if (!isValidAddress(address)) {
7170
throw new InvalidAddressError(`invalid address: ${address}`);
7271
}
7372

74-
const MPC = params.keyCurve === 'secp256k1' ? new Ecdsa() : await EDDSAMethods.getInitializedMpcInstance();
73+
// Lazy import EDDSAMethods to avoid circular dependency: utils/tss -> tss -> utils
74+
const MPC =
75+
params.keyCurve === 'secp256k1'
76+
? new Ecdsa()
77+
: await (await import('../../tss')).EDDSAMethods.getInitializedMpcInstance();
7578
const commonKeychain = extractCommonKeychain(keychains);
76-
const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, 'm/' + index);
79+
80+
// For SMC/custodial wallets with prefix, use derivation path {derivationPrefix}/{index}
81+
// derivationPrefix already includes 'm/' (e.g., "m/999999/12345/67890")
82+
// For wallets without prefix, use derivation path m/{index}
83+
const derivationPath = derivationPrefix ? `${derivationPrefix}/${index}` : `m/${index}`;
84+
const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, derivationPath);
7785

7886
// secp256k1 expects 33 bytes; ed25519 expects 32 bytes
7987
const publicKeySize = params.keyCurve === 'secp256k1' ? 33 : 32;

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
import { SubmitTransactionResponse } from '../inscriptionBuilder';
3333
import { drawKeycard } from '../internal';
3434
import * as internal from '../internal/internal';
35-
import { decryptKeychainPrivateKey, Keychain, KeychainWithEncryptedPrv } from '../keychain';
35+
import { decryptKeychainPrivateKey, Keychain, KeychainWithEncryptedPrv, KeyIndices } from '../keychain';
3636
import { getLightningAuthKey } from '../lightning/lightningWalletUtil';
3737
import { IPendingApproval, PendingApproval, PendingApprovals } from '../pendingApproval';
3838
import { GoStakingWallet, StakingWallet } from '../staking';
@@ -53,6 +53,7 @@ import { postWithCodec } from '../utils/postWithCodec';
5353
import { EcdsaMPCv2Utils, EcdsaUtils } from '../utils/tss/ecdsa';
5454
import EddsaUtils from '../utils/tss/eddsa';
5555
import { getTxRequestApiVersion, validateTxRequestApiVersion } from '../utils/txRequest';
56+
import { getDerivationPath } from '@bitgo/sdk-lib-mpc';
5657
import { buildParamKeys, BuildParams } from './BuildParams';
5758
import {
5859
AccelerateTransactionOptions,
@@ -1408,6 +1409,18 @@ export class Wallet implements IWallet {
14081409
}
14091410

14101411
verificationData.impliedForwarderVersion = forwarderVersion ?? verificationData.coinSpecific?.forwarderVersion;
1412+
1413+
// For SMC (Self-Managed Custodial) wallets, extract derivation prefix from User keychain
1414+
// Custodial wallets don't need this as their commonKeychain already accounts for the prefix
1415+
if (this.multisigType() === 'tss' && this.type() === 'cold' && this._wallet.keys.length > KeyIndices.USER) {
1416+
// Find the user keychain by index
1417+
const userKeychain = keychains[KeyIndices.USER] as any;
1418+
if (userKeychain?.derivedFromParentWithSeed) {
1419+
const derivationPrefix = getDerivationPath(userKeychain.derivedFromParentWithSeed.toString());
1420+
verificationData.derivationPrefix = derivationPrefix;
1421+
}
1422+
}
1423+
14111424
// This condition was added in first place because in celo, when verifyAddress method was called on addresses which were having pendingChainInitialization as true, it used to throw some error
14121425
// In case of forwarder version 1 eth addresses, addresses need to be verified even if the pendingChainInitialization flag is true
14131426
if (
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import * as assert from 'assert';
2+
import 'should';
3+
import { getDerivationPath } from '@bitgo/sdk-lib-mpc';
4+
5+
function getAddressVerificationModule() {
6+
return require('../../../../../src/bitgo/utils/tss/addressVerification');
7+
}
8+
9+
const getExtractCommonKeychain = () => getAddressVerificationModule().extractCommonKeychain;
10+
11+
describe('TSS Address Verification - Derivation Path with Prefix', function () {
12+
const commonKeychain =
13+
'8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd';
14+
15+
describe('extractCommonKeychain', function () {
16+
it('should extract commonKeychain from keychains array', function () {
17+
const extractCommonKeychain = getExtractCommonKeychain();
18+
const keychains = [{ commonKeychain }, { commonKeychain }, { commonKeychain }];
19+
20+
const result = extractCommonKeychain(keychains);
21+
result.should.equal(commonKeychain);
22+
});
23+
24+
it('should throw error if keychains are missing', function () {
25+
const extractCommonKeychain = getExtractCommonKeychain();
26+
assert.throws(() => extractCommonKeychain([]), /missing required param keychains/);
27+
assert.throws(() => extractCommonKeychain(undefined as any), /missing required param keychains/);
28+
});
29+
30+
it('should throw error if commonKeychain is missing', function () {
31+
const extractCommonKeychain = getExtractCommonKeychain();
32+
const keychains = [{ id: '1' }];
33+
assert.throws(() => extractCommonKeychain(keychains as any), /missing required param commonKeychain/);
34+
});
35+
36+
it('should throw error if keychains have mismatched commonKeychains', function () {
37+
const extractCommonKeychain = getExtractCommonKeychain();
38+
const keychains = [{ commonKeychain }, { commonKeychain: 'different-keychain' }];
39+
assert.throws(() => extractCommonKeychain(keychains), /all keychains must have the same commonKeychain/);
40+
});
41+
});
42+
43+
describe('Derivation Path Format Validation', function () {
44+
it('should produce correct derivation path format', function () {
45+
const testSeeds = ['seed1', 'seed-2', '12345', 'test_seed_123'];
46+
47+
testSeeds.forEach((seed) => {
48+
const path = getDerivationPath(seed);
49+
50+
// Expected format: "m/999999/{part1}/{part2}"
51+
path.should.match(/^m\/999999\/\d+\/\d+$/);
52+
53+
path.should.startWith('m/');
54+
55+
// Should have exactly 3 parts
56+
const parts = path.split('/');
57+
parts.length.should.equal(4);
58+
parts[0].should.equal('m');
59+
parts[1].should.equal('999999');
60+
});
61+
});
62+
});
63+
});

0 commit comments

Comments
 (0)