Skip to content

Commit 2730531

Browse files
committed
refactor(sdk-core): centralize derivation prefix computation for SMC wallet address verification
TICKET: WP-7507
1 parent 23a1c91 commit 2730531

5 files changed

Lines changed: 34 additions & 42 deletions

File tree

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3485,13 +3485,12 @@ describe('SOL:', function () {
34853485
const commonKeychain =
34863486
'8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd';
34873487
const index = '1';
3488-
const testSeed = 'smc-test-seed-123';
3489-
const derivationPrefix = require('@bitgo/sdk-lib-mpc').getDerivationPath(testSeed);
3488+
const derivedFromParentWithSeed = 'smc-test-seed-123';
34903489
const keychains = [{ id: '1', type: 'tss' as const, commonKeychain }];
34913490

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 });
3491+
// This test verifies that derivedFromParentWithSeed is accepted as a parameter
3492+
// and the verification function correctly computes the derivation prefix internally
3493+
const result = await basecoin.isWalletAddress({ keychains, address, index, derivedFromParentWithSeed });
34953494
result.should.equal(true);
34963495
});
34973496
});

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,10 @@ export interface VerifyAddressOptions {
155155
coinSpecific?: AddressCoinSpecific;
156156
impliedForwarderVersion?: number;
157157
/**
158-
* Optional derivation prefix for SMC wallets.
159-
* Used when verifying TSS addresses for Self-Managed Custodial wallets.
158+
* Optional seed value from user keychain's derivedFromParentWithSeed field.
159+
* For SMC (Self-Managed Custodial) TSS wallets, this is used to compute the derivation prefix.
160160
*/
161-
derivationPrefix?: string;
161+
derivedFromParentWithSeed?: string;
162162
}
163163

164164
/**
@@ -182,11 +182,11 @@ export interface TssVerifyAddressOptions {
182182
*/
183183
index: number | string;
184184
/**
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.
185+
* Optional seed value from user keychain's derivedFromParentWithSeed field.
186+
* For SMC (Self-Managed Custodial) wallets, this is used to compute the derivation prefix.
187+
* The derivation path becomes {computedPrefix}/{index} instead of m/{index}.
188188
*/
189-
derivationPrefix?: string;
189+
derivedFromParentWithSeed?: string;
190190
}
191191

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

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import { getDerivationPath } from '@bitgo/sdk-lib-mpc';
12
import { Ecdsa } from '../../../account-lib/mpc';
23
import { TssVerifyAddressOptions } from '../../baseCoin/iBaseCoin';
34
import { InvalidAddressError } from '../../errors';
5+
import { EDDSAMethods } from '../../tss';
46

57
/**
68
* Extracts and validates the commonKeychain from keychains array.
@@ -64,23 +66,20 @@ export async function verifyMPCWalletAddress(
6466
isValidAddress: (address: string) => boolean,
6567
getAddressFromPublicKey: (publicKey: string) => string
6668
): Promise<boolean> {
67-
const { keychains, address, index, derivationPrefix } = params;
69+
const { keychains, address, index, derivedFromParentWithSeed } = params;
6870

6971
if (!isValidAddress(address)) {
7072
throw new InvalidAddressError(`invalid address: ${address}`);
7173
}
7274

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();
75+
const MPC = params.keyCurve === 'secp256k1' ? new Ecdsa() : await EDDSAMethods.getInitializedMpcInstance();
7876
const commonKeychain = extractCommonKeychain(keychains);
7977

80-
// For SMC cold TSS 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}`;
78+
// Compute derivation path:
79+
// - For SMC wallets with derivedFromParentWithSeed, compute prefix and use: {prefix}/{index}
80+
// - For other wallets, use simple path: m/{index}
81+
const prefix = derivedFromParentWithSeed ? getDerivationPath(derivedFromParentWithSeed.toString()) : undefined;
82+
const derivationPath = prefix ? `${prefix}/${index}` : `m/${index}`;
8483
const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, derivationPath);
8584

8685
// secp256k1 expects 33 bytes; ed25519 expects 32 bytes

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ 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';
5756
import { buildParamKeys, BuildParams } from './BuildParams';
5857
import {
5958
AccelerateTransactionOptions,
@@ -1410,15 +1409,12 @@ export class Wallet implements IWallet {
14101409

14111410
verificationData.impliedForwarderVersion = forwarderVersion ?? verificationData.coinSpecific?.forwarderVersion;
14121411

1413-
// For SMC (Self-Managed Custodial) wallets, extract derivation prefix from User keychain
1412+
// For SMC (Self-Managed Custodial) wallets, pass derivedFromParentWithSeed from user keychain
1413+
// The verification function will compute the derivation prefix internally
14141414
// Custodial wallets don't need this as their commonKeychain already accounts for the prefix
14151415
if (this.multisigType() === 'tss' && this.type() === 'cold' && this._wallet.keys.length > KeyIndices.USER) {
1416-
// Find the user keychain by index
14171416
const userKeychain = keychains[KeyIndices.USER] as Keychain | undefined;
1418-
if (userKeychain?.derivedFromParentWithSeed) {
1419-
const derivationPrefix = getDerivationPath(userKeychain.derivedFromParentWithSeed.toString());
1420-
verificationData.derivationPrefix = derivationPrefix;
1421-
}
1417+
verificationData.derivedFromParentWithSeed = userKeychain?.derivedFromParentWithSeed;
14221418
}
14231419

14241420
// 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

modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as assert from 'assert';
22
import * as sinon from 'sinon';
33
import 'should';
44
import { Wallet } from '../../../../src/bitgo/wallet/wallet';
5-
import { getDerivationPath } from '@bitgo/sdk-lib-mpc';
65
import { KeyIndices } from '../../../../src/bitgo/keychain';
76

87
describe('Wallet - TSS Address Verification with Derivation Prefix', function () {
@@ -98,11 +97,11 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function ()
9897

9998
await wallet.createAddress({ chain: 0 });
10099

101-
// Verify that custodial wallets don't use derivationPrefix
100+
// Verify that custodial wallets don't use derivedFromParentWithSeed
102101
// (their commonKeychain already accounts for the prefix)
103102
const verificationCall = mockBaseCoin.isWalletAddress.getCall(0);
104103
const verificationData = verificationCall.args[0];
105-
assert.strictEqual(verificationData.derivationPrefix, undefined);
104+
assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined);
106105
});
107106
});
108107

@@ -130,13 +129,12 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function ()
130129

131130
await wallet.createAddress({ chain: 0 });
132131

133-
// Verify that isWalletAddress was called with derivationPrefix from User keychain
132+
// Verify that isWalletAddress was called with derivedFromParentWithSeed from User keychain
134133
const verificationCall = mockBaseCoin.isWalletAddress.getCall(0);
135134
const verificationData = verificationCall.args[0];
136135

137-
verificationData.should.have.property('derivationPrefix');
138-
verificationData.derivationPrefix.should.equal(getDerivationPath('test-seed-user'));
139-
verificationData.derivationPrefix.should.match(/^m\/999999\/\d+\/\d+$/);
136+
verificationData.should.have.property('derivedFromParentWithSeed');
137+
verificationData.derivedFromParentWithSeed.should.equal('test-seed-user');
140138
});
141139

142140
it('should handle missing derivedFromParentWithSeed gracefully for cold wallet', async function () {
@@ -160,10 +158,10 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function ()
160158

161159
await wallet.createAddress({ chain: 0 });
162160

163-
// Should not have derivationPrefix if seed is missing (no prefix used in this case)
161+
// Should not have derivedFromParentWithSeed if seed is missing
164162
const verificationCall = mockBaseCoin.isWalletAddress.getCall(0);
165163
const verificationData = verificationCall.args[0];
166-
assert.strictEqual(verificationData.derivationPrefix, undefined);
164+
assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined);
167165
});
168166
});
169167

@@ -192,10 +190,10 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function ()
192190

193191
await wallet.createAddress({ chain: 0 });
194192

195-
// Verify that derivationPrefix is not set for non-TSS wallets
193+
// Verify that derivedFromParentWithSeed is not set for non-TSS wallets
196194
const verificationCall = mockBaseCoin.isWalletAddress.getCall(0);
197195
const verificationData = verificationCall.args[0];
198-
assert.strictEqual(verificationData.derivationPrefix, undefined);
196+
assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined);
199197
});
200198
});
201199

@@ -236,8 +234,8 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function ()
236234

237235
const verificationCall = mockBaseCoin.isWalletAddress.getCall(0);
238236
const verificationData = verificationCall.args[0];
239-
// Should not have derivationPrefix if USER keychain doesn't exist in keys array
240-
assert.strictEqual(verificationData.derivationPrefix, undefined);
237+
// Should not have derivedFromParentWithSeed if USER keychain doesn't exist in keys array
238+
assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined);
241239
});
242240

243241
it('should handle pendingChainInitialization correctly', async function () {

0 commit comments

Comments
 (0)