From 23a1c91e905364bb9d8d9fd21941cae7d90d7760 Mon Sep 17 00:00:00 2001 From: mrdanish26 Date: Wed, 21 Jan 2026 15:10:06 -0800 Subject: [PATCH 1/2] fix: address validation for SMC wallets TICKET: WP-7507 --- .iyarc | 6 + modules/sdk-coin-sol/test/unit/sol.ts | 16 + .../sdk-core/src/bitgo/baseCoin/iBaseCoin.ts | 12 + .../bitgo/utils/tss/addressVerification.ts | 16 +- modules/sdk-core/src/bitgo/wallet/wallet.ts | 15 +- .../bitgo/utils/tss/addressVerification.ts | 63 ++++ .../wallet/walletTssAddressVerification.ts | 297 ++++++++++++++++++ 7 files changed, 420 insertions(+), 5 deletions(-) create mode 100644 modules/sdk-core/test/unit/bitgo/utils/tss/addressVerification.ts create mode 100644 modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts diff --git a/.iyarc b/.iyarc index 9bed6f4399..fc6747b310 100644 --- a/.iyarc +++ b/.iyarc @@ -4,4 +4,10 @@ # - This CVE affects archive EXTRACTION (unpacking malicious symlinks/hardlinks) # - Lerna only uses tar for PACKING GHSA-8qq5-rm4j-mr97 + +# Excluded because: +# - Transitive dependency through lerna and yeoman-generator, which currently pin tar to a +# < 7.5.4 range; We only use their tar integration for +# archive PACKING, not extraction, GHSA-r6q2-hw4h-h46w + diff --git a/modules/sdk-coin-sol/test/unit/sol.ts b/modules/sdk-coin-sol/test/unit/sol.ts index 9d1d8f64a7..90a4783346 100644 --- a/modules/sdk-coin-sol/test/unit/sol.ts +++ b/modules/sdk-coin-sol/test/unit/sol.ts @@ -3478,6 +3478,22 @@ describe('SOL:', function () { message: `invalid address: ${invalidAddress}`, }); }); + + it('should verify address with derivation prefix for SMC wallets', async function () { + // Address derived with derivation prefix m/999999/148242355/239336845/1 + const address = 'CC715Q92C8vuEFT5xujE55Do6BkWRdpDvr7Vh8iUNUBw'; + const commonKeychain = + '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd'; + const index = '1'; + const testSeed = 'smc-test-seed-123'; + const derivationPrefix = require('@bitgo/sdk-lib-mpc').getDerivationPath(testSeed); + const keychains = [{ id: '1', type: 'tss' as const, commonKeychain }]; + + // This test verifies that derivationPrefix is accepted as a parameter and correctly validates addresses + // derived with the SMC wallet derivation prefix + const result = await basecoin.isWalletAddress({ keychains, address, index, derivationPrefix }); + result.should.equal(true); + }); }); describe('getAddressFromPublicKey', () => { diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index 7f88ebbd04..4fdab6971f 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -154,6 +154,11 @@ export interface VerifyAddressOptions { error?: string; coinSpecific?: AddressCoinSpecific; impliedForwarderVersion?: number; + /** + * Optional derivation prefix for SMC wallets. + * Used when verifying TSS addresses for Self-Managed Custodial wallets. + */ + derivationPrefix?: string; } /** @@ -173,8 +178,15 @@ export interface TssVerifyAddressOptions { /** * Derivation index for the address. * Used to derive child addresses from the root keychain via HD derivation path: m/{index} + * For SMC (Self-Managed Custodial) wallets, the path includes a prefix: m/{derivationPrefix}/{index} */ index: number | string; + /** + * Optional derivation prefix for SMC wallets. + * For SMC wallets, addresses are derived using m/{derivationPrefix}/{index} instead of m/{index}. + * This prefix comes from derivedFromParentWithSeed on the user keychain. + */ + derivationPrefix?: string; } export function isTssVerifyAddressOptions( diff --git a/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts b/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts index 4e5fd92080..3545ca56de 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts @@ -1,7 +1,6 @@ import { Ecdsa } from '../../../account-lib/mpc'; import { TssVerifyAddressOptions } from '../../baseCoin/iBaseCoin'; import { InvalidAddressError } from '../../errors'; -import { EDDSAMethods } from '../../tss'; /** * Extracts and validates the commonKeychain from keychains array. @@ -65,15 +64,24 @@ export async function verifyMPCWalletAddress( isValidAddress: (address: string) => boolean, getAddressFromPublicKey: (publicKey: string) => string ): Promise { - const { keychains, address, index } = params; + const { keychains, address, index, derivationPrefix } = params; if (!isValidAddress(address)) { throw new InvalidAddressError(`invalid address: ${address}`); } - const MPC = params.keyCurve === 'secp256k1' ? new Ecdsa() : await EDDSAMethods.getInitializedMpcInstance(); + // Lazy import EDDSAMethods to avoid circular dependency: utils/tss -> tss -> utils + const MPC = + params.keyCurve === 'secp256k1' + ? new Ecdsa() + : await (await import('../../tss')).EDDSAMethods.getInitializedMpcInstance(); const commonKeychain = extractCommonKeychain(keychains); - const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, 'm/' + index); + + // For SMC cold TSS wallets with prefix, use derivation path {derivationPrefix}/{index} + // derivationPrefix already includes 'm/' (e.g., "m/999999/12345/67890") + // For wallets without prefix, use derivation path m/{index} + const derivationPath = derivationPrefix ? `${derivationPrefix}/${index}` : `m/${index}`; + const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, derivationPath); // secp256k1 expects 33 bytes; ed25519 expects 32 bytes const publicKeySize = params.keyCurve === 'secp256k1' ? 33 : 32; diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 4f6616d06f..17c1626d35 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -32,7 +32,7 @@ import { import { SubmitTransactionResponse } from '../inscriptionBuilder'; import { drawKeycard } from '../internal'; import * as internal from '../internal/internal'; -import { decryptKeychainPrivateKey, Keychain, KeychainWithEncryptedPrv } from '../keychain'; +import { decryptKeychainPrivateKey, Keychain, KeychainWithEncryptedPrv, KeyIndices } from '../keychain'; import { getLightningAuthKey } from '../lightning/lightningWalletUtil'; import { IPendingApproval, PendingApproval, PendingApprovals } from '../pendingApproval'; import { GoStakingWallet, StakingWallet } from '../staking'; @@ -53,6 +53,7 @@ import { postWithCodec } from '../utils/postWithCodec'; import { EcdsaMPCv2Utils, EcdsaUtils } from '../utils/tss/ecdsa'; import EddsaUtils from '../utils/tss/eddsa'; import { getTxRequestApiVersion, validateTxRequestApiVersion } from '../utils/txRequest'; +import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; import { buildParamKeys, BuildParams } from './BuildParams'; import { AccelerateTransactionOptions, @@ -1408,6 +1409,18 @@ export class Wallet implements IWallet { } verificationData.impliedForwarderVersion = forwarderVersion ?? verificationData.coinSpecific?.forwarderVersion; + + // For SMC (Self-Managed Custodial) wallets, extract derivation prefix from User keychain + // Custodial wallets don't need this as their commonKeychain already accounts for the prefix + if (this.multisigType() === 'tss' && this.type() === 'cold' && this._wallet.keys.length > KeyIndices.USER) { + // Find the user keychain by index + const userKeychain = keychains[KeyIndices.USER] as Keychain | undefined; + if (userKeychain?.derivedFromParentWithSeed) { + const derivationPrefix = getDerivationPath(userKeychain.derivedFromParentWithSeed.toString()); + verificationData.derivationPrefix = derivationPrefix; + } + } + // 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 // In case of forwarder version 1 eth addresses, addresses need to be verified even if the pendingChainInitialization flag is true if ( diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/addressVerification.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/addressVerification.ts new file mode 100644 index 0000000000..c9ce76bc84 --- /dev/null +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/addressVerification.ts @@ -0,0 +1,63 @@ +import * as assert from 'assert'; +import 'should'; +import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; + +function getAddressVerificationModule() { + return require('../../../../../src/bitgo/utils/tss/addressVerification'); +} + +const getExtractCommonKeychain = () => getAddressVerificationModule().extractCommonKeychain; + +describe('TSS Address Verification - Derivation Path with Prefix', function () { + const commonKeychain = + '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd'; + + describe('extractCommonKeychain', function () { + it('should extract commonKeychain from keychains array', function () { + const extractCommonKeychain = getExtractCommonKeychain(); + const keychains = [{ commonKeychain }, { commonKeychain }, { commonKeychain }]; + + const result = extractCommonKeychain(keychains); + result.should.equal(commonKeychain); + }); + + it('should throw error if keychains are missing', function () { + const extractCommonKeychain = getExtractCommonKeychain(); + assert.throws(() => extractCommonKeychain([]), /missing required param keychains/); + assert.throws(() => extractCommonKeychain(undefined as any), /missing required param keychains/); + }); + + it('should throw error if commonKeychain is missing', function () { + const extractCommonKeychain = getExtractCommonKeychain(); + const keychains = [{ id: '1' }]; + assert.throws(() => extractCommonKeychain(keychains as any), /missing required param commonKeychain/); + }); + + it('should throw error if keychains have mismatched commonKeychains', function () { + const extractCommonKeychain = getExtractCommonKeychain(); + const keychains = [{ commonKeychain }, { commonKeychain: 'different-keychain' }]; + assert.throws(() => extractCommonKeychain(keychains), /all keychains must have the same commonKeychain/); + }); + }); + + describe('Derivation Path Format Validation', function () { + it('should produce correct derivation path format', function () { + const testSeeds = ['seed1', 'seed-2', '12345', 'test_seed_123']; + + testSeeds.forEach((seed) => { + const path = getDerivationPath(seed); + + // Expected format: "m/999999/{part1}/{part2}" + path.should.match(/^m\/999999\/\d+\/\d+$/); + + path.should.startWith('m/'); + + // Should have exactly 3 parts + const parts = path.split('/'); + parts.length.should.equal(4); + parts[0].should.equal('m'); + parts[1].should.equal('999999'); + }); + }); + }); +}); diff --git a/modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts b/modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts new file mode 100644 index 0000000000..0b0816b84d --- /dev/null +++ b/modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts @@ -0,0 +1,297 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import 'should'; +import { Wallet } from '../../../../src/bitgo/wallet/wallet'; +import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; +import { KeyIndices } from '../../../../src/bitgo/keychain'; + +describe('Wallet - TSS Address Verification with Derivation Prefix', function () { + let wallet: Wallet; + let mockBitGo: any; + let mockBaseCoin: any; + let mockWalletData: any; + let mockKeychains: any[]; + + beforeEach(function () { + mockBitGo = { + post: sinon.stub(), + setRequestTracer: sinon.stub(), + }; + + mockBaseCoin = { + supportsTss: sinon.stub().returns(true), + getFamily: sinon.stub().returns('sol'), + getMPCAlgorithm: sinon.stub().returns('eddsa'), + isValidAddress: sinon.stub().returns(true), + isWalletAddress: sinon.stub(), + keychains: sinon.stub(), + url: sinon.stub().returns('/api/v2/sol/wallet/test-wallet-id/address'), + }; + + // Default keychains setup - all with commonKeychain + mockKeychains = [ + { + id: 'user-key-id', + type: 'tss', + commonKeychain: + '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd', + derivedFromParentWithSeed: 'test-seed-user', + }, + { + id: 'backup-key-id', + type: 'tss', + commonKeychain: + '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd', + }, + { + id: 'bitgo-key-id', + type: 'tss', + commonKeychain: + '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd', + }, + ]; + + mockBaseCoin.keychains.returns({ + get: sinon.stub().callsFake((params: any) => { + const keyIndex = mockWalletData.keys.indexOf(params.id); + return Promise.resolve(mockKeychains[keyIndex]); + }), + }); + + mockWalletData = { + id: 'test-wallet-id', + keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], + multisigType: 'tss', + coinSpecific: { + walletVersion: 3, + }, + }; + + wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); + }); + + afterEach(function () { + sinon.restore(); + }); + + describe('Custodial TSS Wallet - No Derivation Prefix', function () { + beforeEach(function () { + mockWalletData.type = 'custodial'; + wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); + }); + + it('should not extract derivation prefix for custodial wallets', async function () { + const mockAddressResponse = { + id: 'address-id', + address: '6FjshVqwmDH74wfxkZrJaRGEjTeJQL4ViL6X18VXUNAY', + index: 0, + coinSpecific: {}, + }; + + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(mockAddressResponse), + }), + }); + + mockBaseCoin.isWalletAddress.resolves(true); + + await wallet.createAddress({ chain: 0 }); + + // Verify that custodial wallets don't use derivationPrefix + // (their commonKeychain already accounts for the prefix) + const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); + const verificationData = verificationCall.args[0]; + assert.strictEqual(verificationData.derivationPrefix, undefined); + }); + }); + + describe('Cold/SMC TSS Wallet - Derivation Prefix from User Keychain', function () { + beforeEach(function () { + mockWalletData.type = 'cold'; + wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); + }); + + it('should extract derivation prefix from User keychain for cold/SMC wallets', async function () { + const mockAddressResponse = { + id: 'address-id', + address: '6FjshVqwmDH74wfxkZrJaRGEjTeJQL4ViL6X18VXUNAY', + index: 0, + coinSpecific: {}, + }; + + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(mockAddressResponse), + }), + }); + + mockBaseCoin.isWalletAddress.resolves(true); + + await wallet.createAddress({ chain: 0 }); + + // Verify that isWalletAddress was called with derivationPrefix from User keychain + const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); + const verificationData = verificationCall.args[0]; + + verificationData.should.have.property('derivationPrefix'); + verificationData.derivationPrefix.should.equal(getDerivationPath('test-seed-user')); + verificationData.derivationPrefix.should.match(/^m\/999999\/\d+\/\d+$/); + }); + + it('should handle missing derivedFromParentWithSeed gracefully for cold wallet', async function () { + // Remove derivedFromParentWithSeed from User keychain + mockKeychains[KeyIndices.USER].derivedFromParentWithSeed = undefined; + + const mockAddressResponse = { + id: 'address-id', + address: '6FjshVqwmDH74wfxkZrJaRGEjTeJQL4ViL6X18VXUNAY', + index: 0, + coinSpecific: {}, + }; + + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(mockAddressResponse), + }), + }); + + mockBaseCoin.isWalletAddress.resolves(true); + + await wallet.createAddress({ chain: 0 }); + + // Should not have derivationPrefix if seed is missing (no prefix used in this case) + const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); + const verificationData = verificationCall.args[0]; + assert.strictEqual(verificationData.derivationPrefix, undefined); + }); + }); + + describe('Non-TSS Wallet - No Derivation Prefix', function () { + beforeEach(function () { + mockWalletData.multisigType = 'onchain'; + mockWalletData.type = 'cold'; + wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); + }); + + it('should not extract derivation prefix for non-TSS wallets', async function () { + const mockAddressResponse = { + id: 'address-id', + address: '6FjshVqwmDH74wfxkZrJaRGEjTeJQL4ViL6X18VXUNAY', + index: 0, + coinSpecific: {}, + }; + + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(mockAddressResponse), + }), + }); + + mockBaseCoin.isWalletAddress.resolves(true); + + await wallet.createAddress({ chain: 0 }); + + // Verify that derivationPrefix is not set for non-TSS wallets + const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); + const verificationData = verificationCall.args[0]; + assert.strictEqual(verificationData.derivationPrefix, undefined); + }); + }); + + describe('Edge Cases', function () { + it('should handle wallet without USER keychain', async function () { + // Set keys array to only have backup keychain (no USER keychain at index 0) + mockWalletData.keys = ['backup-key-id']; // Only backup keychain, no USER + mockWalletData.type = 'cold'; + + // Update mock to return backup keychain when requested + mockBaseCoin.keychains.returns({ + get: sinon.stub().callsFake((params: any) => { + if (params.id === 'backup-key-id') { + return Promise.resolve(mockKeychains[KeyIndices.BACKUP]); + } + return Promise.resolve(null); + }), + }); + + wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); + + const mockAddressResponse = { + id: 'address-id', + address: '6FjshVqwmDH74wfxkZrJaRGEjTeJQL4ViL6X18VXUNAY', + index: 0, + coinSpecific: {}, + }; + + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(mockAddressResponse), + }), + }); + + mockBaseCoin.isWalletAddress.resolves(true); + + await wallet.createAddress({ chain: 0 }); + + const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); + const verificationData = verificationCall.args[0]; + // Should not have derivationPrefix if USER keychain doesn't exist in keys array + assert.strictEqual(verificationData.derivationPrefix, undefined); + }); + + it('should handle pendingChainInitialization correctly', async function () { + mockWalletData.type = 'cold'; + wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); + + const mockAddressResponse = { + id: 'address-id', + address: '6FjshVqwmDH74wfxkZrJaRGEjTeJQL4ViL6X18VXUNAY', + index: 0, + coinSpecific: { + pendingChainInitialization: true, + }, + }; + + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(mockAddressResponse), + }), + }); + + // Should skip verification when pendingChainInitialization is true + await wallet.createAddress({ chain: 0 }); + + // isWalletAddress should not be called when pendingChainInitialization is true + mockBaseCoin.isWalletAddress.should.not.have.been.called; + }); + + it('should handle forwarderVersion 1 with pendingChainInitialization', async function () { + mockWalletData.type = 'cold'; + wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData); + + const mockAddressResponse = { + id: 'address-id', + address: '6FjshVqwmDH74wfxkZrJaRGEjTeJQL4ViL6X18VXUNAY', + index: 0, + coinSpecific: { + pendingChainInitialization: true, + forwarderVersion: 1, + }, + }; + + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(mockAddressResponse), + }), + }); + + mockBaseCoin.isWalletAddress.resolves(true); + + // Should verify even with pendingChainInitialization when forwarderVersion is 1 + await wallet.createAddress({ chain: 0, forwarderVersion: 1 }); + + mockBaseCoin.isWalletAddress.should.have.been.calledOnce; + }); + }); +}); From 2730531955bbfc9a98b526c9acb91e1641b4b824 Mon Sep 17 00:00:00 2001 From: Marzooqa Naeema Kather Date: Thu, 22 Jan 2026 13:24:55 +0530 Subject: [PATCH 2/2] refactor(sdk-core): centralize derivation prefix computation for SMC wallet address verification TICKET: WP-7507 --- modules/sdk-coin-sol/test/unit/sol.ts | 9 ++++--- .../sdk-core/src/bitgo/baseCoin/iBaseCoin.ts | 14 +++++------ .../bitgo/utils/tss/addressVerification.ts | 19 +++++++-------- modules/sdk-core/src/bitgo/wallet/wallet.ts | 10 +++----- .../wallet/walletTssAddressVerification.ts | 24 +++++++++---------- 5 files changed, 34 insertions(+), 42 deletions(-) diff --git a/modules/sdk-coin-sol/test/unit/sol.ts b/modules/sdk-coin-sol/test/unit/sol.ts index 90a4783346..2c5c79e6e7 100644 --- a/modules/sdk-coin-sol/test/unit/sol.ts +++ b/modules/sdk-coin-sol/test/unit/sol.ts @@ -3485,13 +3485,12 @@ describe('SOL:', function () { const commonKeychain = '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd'; const index = '1'; - const testSeed = 'smc-test-seed-123'; - const derivationPrefix = require('@bitgo/sdk-lib-mpc').getDerivationPath(testSeed); + const derivedFromParentWithSeed = 'smc-test-seed-123'; const keychains = [{ id: '1', type: 'tss' as const, commonKeychain }]; - // This test verifies that derivationPrefix is accepted as a parameter and correctly validates addresses - // derived with the SMC wallet derivation prefix - const result = await basecoin.isWalletAddress({ keychains, address, index, derivationPrefix }); + // This test verifies that derivedFromParentWithSeed is accepted as a parameter + // and the verification function correctly computes the derivation prefix internally + const result = await basecoin.isWalletAddress({ keychains, address, index, derivedFromParentWithSeed }); result.should.equal(true); }); }); diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index 4fdab6971f..cfb424aba3 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -155,10 +155,10 @@ export interface VerifyAddressOptions { coinSpecific?: AddressCoinSpecific; impliedForwarderVersion?: number; /** - * Optional derivation prefix for SMC wallets. - * Used when verifying TSS addresses for Self-Managed Custodial wallets. + * Optional seed value from user keychain's derivedFromParentWithSeed field. + * For SMC (Self-Managed Custodial) TSS wallets, this is used to compute the derivation prefix. */ - derivationPrefix?: string; + derivedFromParentWithSeed?: string; } /** @@ -182,11 +182,11 @@ export interface TssVerifyAddressOptions { */ index: number | string; /** - * Optional derivation prefix for SMC wallets. - * For SMC wallets, addresses are derived using m/{derivationPrefix}/{index} instead of m/{index}. - * This prefix comes from derivedFromParentWithSeed on the user keychain. + * Optional seed value from user keychain's derivedFromParentWithSeed field. + * For SMC (Self-Managed Custodial) wallets, this is used to compute the derivation prefix. + * The derivation path becomes {computedPrefix}/{index} instead of m/{index}. */ - derivationPrefix?: string; + derivedFromParentWithSeed?: string; } export function isTssVerifyAddressOptions( diff --git a/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts b/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts index 3545ca56de..67e41d1ade 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts @@ -1,6 +1,8 @@ +import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; import { Ecdsa } from '../../../account-lib/mpc'; import { TssVerifyAddressOptions } from '../../baseCoin/iBaseCoin'; import { InvalidAddressError } from '../../errors'; +import { EDDSAMethods } from '../../tss'; /** * Extracts and validates the commonKeychain from keychains array. @@ -64,23 +66,20 @@ export async function verifyMPCWalletAddress( isValidAddress: (address: string) => boolean, getAddressFromPublicKey: (publicKey: string) => string ): Promise { - const { keychains, address, index, derivationPrefix } = params; + const { keychains, address, index, derivedFromParentWithSeed } = params; if (!isValidAddress(address)) { throw new InvalidAddressError(`invalid address: ${address}`); } - // Lazy import EDDSAMethods to avoid circular dependency: utils/tss -> tss -> utils - const MPC = - params.keyCurve === 'secp256k1' - ? new Ecdsa() - : await (await import('../../tss')).EDDSAMethods.getInitializedMpcInstance(); + const MPC = params.keyCurve === 'secp256k1' ? new Ecdsa() : await EDDSAMethods.getInitializedMpcInstance(); const commonKeychain = extractCommonKeychain(keychains); - // For SMC cold TSS wallets with prefix, use derivation path {derivationPrefix}/{index} - // derivationPrefix already includes 'm/' (e.g., "m/999999/12345/67890") - // For wallets without prefix, use derivation path m/{index} - const derivationPath = derivationPrefix ? `${derivationPrefix}/${index}` : `m/${index}`; + // Compute derivation path: + // - For SMC wallets with derivedFromParentWithSeed, compute prefix and use: {prefix}/{index} + // - For other wallets, use simple path: m/{index} + const prefix = derivedFromParentWithSeed ? getDerivationPath(derivedFromParentWithSeed.toString()) : undefined; + const derivationPath = prefix ? `${prefix}/${index}` : `m/${index}`; const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, derivationPath); // secp256k1 expects 33 bytes; ed25519 expects 32 bytes diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 17c1626d35..6454d5b4b8 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -53,7 +53,6 @@ import { postWithCodec } from '../utils/postWithCodec'; import { EcdsaMPCv2Utils, EcdsaUtils } from '../utils/tss/ecdsa'; import EddsaUtils from '../utils/tss/eddsa'; import { getTxRequestApiVersion, validateTxRequestApiVersion } from '../utils/txRequest'; -import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; import { buildParamKeys, BuildParams } from './BuildParams'; import { AccelerateTransactionOptions, @@ -1410,15 +1409,12 @@ export class Wallet implements IWallet { verificationData.impliedForwarderVersion = forwarderVersion ?? verificationData.coinSpecific?.forwarderVersion; - // For SMC (Self-Managed Custodial) wallets, extract derivation prefix from User keychain + // For SMC (Self-Managed Custodial) wallets, pass derivedFromParentWithSeed from user keychain + // The verification function will compute the derivation prefix internally // Custodial wallets don't need this as their commonKeychain already accounts for the prefix if (this.multisigType() === 'tss' && this.type() === 'cold' && this._wallet.keys.length > KeyIndices.USER) { - // Find the user keychain by index const userKeychain = keychains[KeyIndices.USER] as Keychain | undefined; - if (userKeychain?.derivedFromParentWithSeed) { - const derivationPrefix = getDerivationPath(userKeychain.derivedFromParentWithSeed.toString()); - verificationData.derivationPrefix = derivationPrefix; - } + verificationData.derivedFromParentWithSeed = userKeychain?.derivedFromParentWithSeed; } // 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 diff --git a/modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts b/modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts index 0b0816b84d..6a3368a9fb 100644 --- a/modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts +++ b/modules/sdk-core/test/unit/bitgo/wallet/walletTssAddressVerification.ts @@ -2,7 +2,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import 'should'; import { Wallet } from '../../../../src/bitgo/wallet/wallet'; -import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; import { KeyIndices } from '../../../../src/bitgo/keychain'; describe('Wallet - TSS Address Verification with Derivation Prefix', function () { @@ -98,11 +97,11 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function () await wallet.createAddress({ chain: 0 }); - // Verify that custodial wallets don't use derivationPrefix + // Verify that custodial wallets don't use derivedFromParentWithSeed // (their commonKeychain already accounts for the prefix) const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); const verificationData = verificationCall.args[0]; - assert.strictEqual(verificationData.derivationPrefix, undefined); + assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined); }); }); @@ -130,13 +129,12 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function () await wallet.createAddress({ chain: 0 }); - // Verify that isWalletAddress was called with derivationPrefix from User keychain + // Verify that isWalletAddress was called with derivedFromParentWithSeed from User keychain const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); const verificationData = verificationCall.args[0]; - verificationData.should.have.property('derivationPrefix'); - verificationData.derivationPrefix.should.equal(getDerivationPath('test-seed-user')); - verificationData.derivationPrefix.should.match(/^m\/999999\/\d+\/\d+$/); + verificationData.should.have.property('derivedFromParentWithSeed'); + verificationData.derivedFromParentWithSeed.should.equal('test-seed-user'); }); it('should handle missing derivedFromParentWithSeed gracefully for cold wallet', async function () { @@ -160,10 +158,10 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function () await wallet.createAddress({ chain: 0 }); - // Should not have derivationPrefix if seed is missing (no prefix used in this case) + // Should not have derivedFromParentWithSeed if seed is missing const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); const verificationData = verificationCall.args[0]; - assert.strictEqual(verificationData.derivationPrefix, undefined); + assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined); }); }); @@ -192,10 +190,10 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function () await wallet.createAddress({ chain: 0 }); - // Verify that derivationPrefix is not set for non-TSS wallets + // Verify that derivedFromParentWithSeed is not set for non-TSS wallets const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); const verificationData = verificationCall.args[0]; - assert.strictEqual(verificationData.derivationPrefix, undefined); + assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined); }); }); @@ -236,8 +234,8 @@ describe('Wallet - TSS Address Verification with Derivation Prefix', function () const verificationCall = mockBaseCoin.isWalletAddress.getCall(0); const verificationData = verificationCall.args[0]; - // Should not have derivationPrefix if USER keychain doesn't exist in keys array - assert.strictEqual(verificationData.derivationPrefix, undefined); + // Should not have derivedFromParentWithSeed if USER keychain doesn't exist in keys array + assert.strictEqual(verificationData.derivedFromParentWithSeed, undefined); }); it('should handle pendingChainInitialization correctly', async function () {