From f31f558291705b1819ce77e3993a1fe6217f678b Mon Sep 17 00:00:00 2001 From: danielzhao122 Date: Wed, 3 Dec 2025 16:46:19 -0500 Subject: [PATCH 1/3] feat: address verification for icp TICKET: WP-7080 --- modules/sdk-coin-icp/src/icp.ts | 85 +++++++++- modules/sdk-coin-icp/src/lib/iface.ts | 6 + modules/sdk-coin-icp/src/lib/utils.ts | 24 ++- modules/sdk-coin-icp/test/unit/icp.ts | 212 ++++++++++++++++++++++-- modules/sdk-coin-icp/test/unit/utils.ts | 18 +- 5 files changed, 315 insertions(+), 30 deletions(-) diff --git a/modules/sdk-coin-icp/src/icp.ts b/modules/sdk-coin-icp/src/icp.ts index cb22caab29..461ff0683c 100644 --- a/modules/sdk-coin-icp/src/icp.ts +++ b/modules/sdk-coin-icp/src/icp.ts @@ -8,6 +8,7 @@ import { Ecdsa, ECDSAUtils, Environments, + InvalidAddressError, KeyPair, MPCAlgorithm, MultisigType, @@ -17,8 +18,8 @@ import { SignedTransaction, SigningError, SignTransactionOptions, - TssVerifyAddressOptions, VerifyTransactionOptions, + verifyMPCWalletAddress, } from '@bitgo/sdk-core'; import { coins, NetworkType, BaseCoin as StaticsBaseCoin } from '@bitgo/statics'; import { Principal } from '@dfinity/principal'; @@ -41,6 +42,7 @@ import { SigningPayload, IcpTransactionExplanation, TransactionHexParams, + TssVerifyIcpAddressOptions, UnsignedSweepRecoveryTransaction, } from './lib/iface'; import { TransactionBuilderFactory } from './lib/transactionBuilderFactory'; @@ -141,8 +143,81 @@ export class Icp extends BaseCoin { return true; } - async isWalletAddress(params: TssVerifyAddressOptions): Promise { - return this.isValidAddress(params.address); + /** + * Verify that an address belongs to this wallet. + * + * @param {TssVerifyIcpAddressOptions} params - Verification parameters + * @returns {Promise} True if address belongs to wallet + * @throws {InvalidAddressError} If address format is invalid + * @throws {Error} If invalid wallet version or missing parameters + */ + async isWalletAddress(params: TssVerifyIcpAddressOptions): Promise { + const { address, rootAddress, walletVersion } = params; + + if (!this.isValidAddress(address)) { + throw new InvalidAddressError(`invalid address: ${address}`); + } + + if (walletVersion === 1) { + return this.verifyMemoBasedAddress(address, rootAddress); + } + + return this.verifyKeyDerivedAddress(params, address, rootAddress); + } + + /** + * Verifies a memo-based address for wallet version 1. + * + * @param {string} address - The full address to verify (must include memoId) + * @param {string | undefined} rootAddress - The wallet's root address + * @returns {boolean} True if the address is valid + * @throws {Error} If rootAddress is missing or memoId is missing + */ + private verifyMemoBasedAddress(address: string, rootAddress: string | undefined): boolean { + if (!rootAddress) { + throw new Error('rootAddress is required for wallet version 1'); + } + const extractedRootAddress = utils.validateMemoAndReturnRootAddress(address); + if (extractedRootAddress === address) { + throw new Error('memoId is required for wallet version 1 addresses'); + } + + return extractedRootAddress?.toLowerCase() === rootAddress.toLowerCase(); + } + + /** + * Verifies a key-derived address using MPC wallet verification. + * + * @param {TssVerifyIcpAddressOptions} params - Verification parameters + * @param {string} address - The full address to verify + * @param {string | undefined} rootAddress - The wallet's root address + * @returns {Promise} True if the address matches the derived address + * @throws {Error} If keychains are missing or address doesn't match + */ + private async verifyKeyDerivedAddress( + params: TssVerifyIcpAddressOptions, + address: string, + rootAddress: string | undefined + ): Promise { + const { index } = params; + const parsedIndex = typeof index === 'string' ? parseInt(index, 10) : index; + + const isVerifyingRootAddress = rootAddress && address.toLowerCase() === rootAddress.toLowerCase(); + if (isVerifyingRootAddress && parsedIndex !== 0) { + throw new Error(`Root address verification requires index 0, but got index ${index}`); + } + + const result = await verifyMPCWalletAddress( + { ...params, keyCurve: 'secp256k1' }, + this.isValidAddress.bind(this), + (pubKey) => utils.getAddressFromPublicKey(pubKey) + ); + + if (!result) { + throw new InvalidAddressError(`invalid address: ${address}`); + } + + return true; } async parseTransaction(params: ParseTransactionOptions): Promise { @@ -210,7 +285,7 @@ export class Icp extends BaseCoin { return createHash('sha256'); } - private async getAddressFromPublicKey(hexEncodedPublicKey: string) { + private getAddressFromPublicKey(hexEncodedPublicKey: string): string { return utils.getAddressFromPublicKey(hexEncodedPublicKey); } @@ -388,7 +463,7 @@ export class Icp extends BaseCoin { throw new Error('failed to derive public key'); } - const senderAddress = await this.getAddressFromPublicKey(publicKey); + const senderAddress = this.getAddressFromPublicKey(publicKey); const balance = await this.getAccountBalance(publicKey); const feeData = await this.getFeeData(); const actualBalance = balance.minus(feeData); diff --git a/modules/sdk-coin-icp/src/lib/iface.ts b/modules/sdk-coin-icp/src/lib/iface.ts index fc38858523..f1a83621d6 100644 --- a/modules/sdk-coin-icp/src/lib/iface.ts +++ b/modules/sdk-coin-icp/src/lib/iface.ts @@ -1,6 +1,7 @@ import { TransactionExplanation as BaseTransactionExplanation, TransactionType as BitGoTransactionType, + TssVerifyAddressOptions, } from '@bitgo/sdk-core'; export const MAX_INGRESS_TTL = 5 * 60 * 1000_000_000; // 5 minutes in nanoseconds @@ -216,3 +217,8 @@ export interface TransactionHexParams { transactionHex: string; signableHex?: string; } + +export interface TssVerifyIcpAddressOptions extends TssVerifyAddressOptions { + rootAddress?: string; + walletVersion?: number; +} diff --git a/modules/sdk-coin-icp/src/lib/utils.ts b/modules/sdk-coin-icp/src/lib/utils.ts index 43a61d71de..3cb81f0dc8 100644 --- a/modules/sdk-coin-icp/src/lib/utils.ts +++ b/modules/sdk-coin-icp/src/lib/utils.ts @@ -73,8 +73,14 @@ export class Utils implements BaseUtils { return undefined; } const [rootAddress, memoId] = address.split('?memoId='); - if (memoId && this.validateMemo(BigInt(memoId))) { - return rootAddress; + if (memoId) { + try { + if (this.validateMemo(BigInt(memoId))) { + return rootAddress; + } + } catch { + return undefined; + } } return address; } @@ -210,8 +216,14 @@ export class Utils implements BaseUtils { const publicKeyBuffer = Buffer.from(publicKeyHex, 'hex'); const ellipticKey = secp256k1.ProjectivePoint.fromHex(publicKeyBuffer.toString('hex')); const uncompressedPublicKeyHex = ellipticKey.toHex(false); - const derEncodedKey = agent.wrapDER(Buffer.from(uncompressedPublicKeyHex, 'hex'), agent.SECP256K1_OID); - return derEncodedKey; + const uncompressedKeyBuffer = Buffer.from(uncompressedPublicKeyHex, 'hex'); + return agent.wrapDER( + uncompressedKeyBuffer.buffer.slice( + uncompressedKeyBuffer.byteOffset, + uncompressedKeyBuffer.byteOffset + uncompressedKeyBuffer.byteLength + ), + agent.SECP256K1_OID + ); } /** @@ -273,10 +285,10 @@ export class Utils implements BaseUtils { * Retrieves the address associated with a given hex-encoded public key. * * @param {string} hexEncodedPublicKey - The public key in hex-encoded format. - * @returns {Promise} A promise that resolves to the address derived from the provided public key. + * @returns {string} The address derived from the provided public key. * @throws {Error} Throws an error if the provided public key is not in a valid hex-encoded format. */ - async getAddressFromPublicKey(hexEncodedPublicKey: string): Promise { + getAddressFromPublicKey(hexEncodedPublicKey: string): string { if (!this.isValidPublicKey(hexEncodedPublicKey)) { throw new Error('Invalid hex-encoded public key format.'); } diff --git a/modules/sdk-coin-icp/test/unit/icp.ts b/modules/sdk-coin-icp/test/unit/icp.ts index 7dc8ca8a86..f891c8899f 100644 --- a/modules/sdk-coin-icp/test/unit/icp.ts +++ b/modules/sdk-coin-icp/test/unit/icp.ts @@ -87,21 +87,17 @@ describe('Internet computer', function () { accountID.should.deepEqual(validAccountID2); }); - it('should throw an error when invalid public key is provided', async function () { - await basecoin - .getAddressFromPublicKey(invalidPublicKey) - .should.be.rejectedWith(`Invalid hex-encoded public key format.`); + it('should throw an error when invalid public key is provided', function () { + (() => basecoin.getAddressFromPublicKey(invalidPublicKey)).should.throw('Invalid hex-encoded public key format.'); }); - it('should return valid address from a valid hex encoded public key', async function () { - const accountID = await utils.getAddressFromPublicKey(hexEncodedPublicKey); + it('should return valid address from a valid hex encoded public key', function () { + const accountID = utils.getAddressFromPublicKey(hexEncodedPublicKey); accountID.should.deepEqual(validAccountID); }); - it('should throw an error when invalid public key is provided', async function () { - await utils - .getAddressFromPublicKey(invalidPublicKey) - .should.be.rejectedWith(`Invalid hex-encoded public key format.`); + it('should throw an error when invalid public key is provided', function () { + (() => utils.getAddressFromPublicKey(invalidPublicKey)).should.throw('Invalid hex-encoded public key format.'); }); }); @@ -233,4 +229,200 @@ describe('Internet computer', function () { .should.rejectedWith('generated signableHex is not equal to params.signableHex'); }); }); + + describe('Address Verification', () => { + const addressVerificationData = { + commonKeychain: + '036b38ca5e63e9800b5040af498eb6e9a9c77e244ac2858edafa4bd0926a635731c3fabde9007a5771e93621d9fcb1c879660208dc79cc609fe8ddd189f7a955ab', + rootAddress: 'fd3eaed3e2064bd30ab497e22e8ac5a0dcadd81fa5353879dbab64e259ec70c0', + }; + + describe('Wallet VersionKey 1', () => { + it('should verify a valid memo-based address', async function () { + const rootAddress = testData.Accounts.account1.address; + const addressWithMemo = `${rootAddress}?memoId=123`; + + const params = { + address: addressWithMemo, + rootAddress: rootAddress, + walletVersion: 1, + keychains: [], + index: 123, + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(true); + }); + + it('should verify address with memoId=0', async function () { + const rootAddress = testData.Accounts.account1.address; + const addressWithMemo = `${rootAddress}?memoId=0`; + + const params = { + address: addressWithMemo, + rootAddress: rootAddress, + walletVersion: 1, + keychains: [], + index: 0, + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(true); + }); + + it('should fail when base address does not match root address', async function () { + const rootAddress = testData.Accounts.account1.address; + const differentAddress = testData.Accounts.account2.address; + const addressWithMemo = `${differentAddress}?memoId=123`; + + const params = { + address: addressWithMemo, + rootAddress: rootAddress, + walletVersion: 1, + keychains: [], + index: 123, + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(false); + }); + + it('should throw error when rootAddress is missing for wallet version 1', async function () { + const address = `${testData.Accounts.account1.address}?memoId=123`; + + const params = { + address: address, + walletVersion: 1, + keychains: [], + index: 123, + }; + + await basecoin.isWalletAddress(params).should.be.rejectedWith('rootAddress is required for wallet version 1'); + }); + + it('should throw error when memoId is missing for wallet version 1', async function () { + const rootAddress = testData.Accounts.account1.address; + + const params = { + address: rootAddress, + rootAddress: rootAddress, + walletVersion: 1, + keychains: [], + index: 0, + }; + + await basecoin + .isWalletAddress(params) + .should.be.rejectedWith('memoId is required for wallet version 1 addresses'); + }); + + it('should handle large memoId values', async function () { + const rootAddress = testData.Accounts.account1.address; + const largeMemoId = '9007199254740991'; + const addressWithMemo = `${rootAddress}?memoId=${largeMemoId}`; + + const params = { + address: addressWithMemo, + rootAddress: rootAddress, + walletVersion: 1, + keychains: [], + index: Number(largeMemoId), + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(true); + }); + }); + + describe('Wallet VersionKey 2+', () => { + let keychains; + + before(function () { + keychains = [ + { commonKeychain: addressVerificationData.commonKeychain }, + { commonKeychain: addressVerificationData.commonKeychain }, + { commonKeychain: addressVerificationData.commonKeychain }, + ]; + }); + + it('should verify a valid TSS root address (index 0)', async function () { + const params = { + address: addressVerificationData.rootAddress, + rootAddress: addressVerificationData.rootAddress, + keychains: keychains, + index: 0, + walletVersion: 2, + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(true); + }); + + it('should throw error for invalid TSS address', async function () { + const invalidAddress = testData.Accounts.account2.address; + + const params = { + address: invalidAddress, + keychains: keychains, + index: 0, + walletVersion: 2, + }; + + await basecoin.isWalletAddress(params).should.be.rejectedWith(`invalid address: ${invalidAddress}`); + }); + + it('should throw error when keychains is missing', async function () { + const params = { + address: addressVerificationData.rootAddress, + keychains: [], + index: 0, + walletVersion: 2, + }; + + await basecoin.isWalletAddress(params).should.be.rejectedWith('missing required param keychains'); + }); + + it('should throw error when verifying root address with wrong index', async function () { + const params = { + address: addressVerificationData.rootAddress, + rootAddress: addressVerificationData.rootAddress, + keychains: keychains, + index: 1, + walletVersion: 2, + }; + + await basecoin + .isWalletAddress(params) + .should.be.rejectedWith('Root address verification requires index 0, but got index 1'); + }); + + it('should handle string index', async function () { + const params = { + address: addressVerificationData.rootAddress, + rootAddress: addressVerificationData.rootAddress, + keychains: keychains, + index: '0', + walletVersion: 2, + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(true); + }); + }); + + describe('Address validation', () => { + it('should throw error for invalid address format', async function () { + const invalidAddress = 'invalid-address'; + + const params = { + address: invalidAddress, + walletVersion: 2, + keychains: [{ commonKeychain: addressVerificationData.commonKeychain }], + index: 0, + }; + + await basecoin.isWalletAddress(params).should.be.rejectedWith(`invalid address: ${invalidAddress}`); + }); + }); + }); }); diff --git a/modules/sdk-coin-icp/test/unit/utils.ts b/modules/sdk-coin-icp/test/unit/utils.ts index 363b74ecca..4b62ce2950 100644 --- a/modules/sdk-coin-icp/test/unit/utils.ts +++ b/modules/sdk-coin-icp/test/unit/utils.ts @@ -132,23 +132,23 @@ describe('utils', () => { }); describe('getAddressFromPublicKey()', () => { - it('should return the correct address for a valid public key', async () => { - const address1 = await utils.getAddressFromPublicKey(Accounts.account1.publicKey); + it('should return the correct address for a valid public key', () => { + const address1 = utils.getAddressFromPublicKey(Accounts.account1.publicKey); should.equal(address1, Accounts.account1.address); - const address2 = await utils.getAddressFromPublicKey(Accounts.account1.publicKey); + const address2 = utils.getAddressFromPublicKey(Accounts.account1.publicKey); should.equal(address2, Accounts.account1.address); - const address3 = await utils.getAddressFromPublicKey(Accounts.account1.publicKey); + const address3 = utils.getAddressFromPublicKey(Accounts.account1.publicKey); should.equal(address3, Accounts.account1.address); - const address4 = await utils.getAddressFromPublicKey(Accounts.account1.publicKey); + const address4 = utils.getAddressFromPublicKey(Accounts.account1.publicKey); should.equal(address4, Accounts.account1.address); - const address5 = await utils.getAddressFromPublicKey(Accounts.account1.publicKey); + const address5 = utils.getAddressFromPublicKey(Accounts.account1.publicKey); should.equal(address5, Accounts.account1.address); - const address6 = await utils.getAddressFromPublicKey(Accounts.account1.publicKey); + const address6 = utils.getAddressFromPublicKey(Accounts.account1.publicKey); should.equal(address6, Accounts.account1.address); }); - it('should throw an error for an invalid public key', async () => { - await should(utils.getAddressFromPublicKey(Accounts.errorsAccounts.account1.publicKey)).be.rejectedWith( + it('should throw an error for an invalid public key', () => { + (() => utils.getAddressFromPublicKey(Accounts.errorsAccounts.account1.publicKey)).should.throw( 'Invalid hex-encoded public key format.' ); }); From 21f3b21923958153e442dac75cfb59ffd9171a34 Mon Sep 17 00:00:00 2001 From: danielzhao122 Date: Thu, 4 Dec 2025 13:46:20 -0500 Subject: [PATCH 2/3] refactor: upgrade package to avoid ci error and add verification for v1 TICKET: WP-7080 --- modules/sdk-coin-icp/src/icp.ts | 80 +++++++++++---------------- modules/sdk-coin-icp/test/unit/icp.ts | 73 +++++++++++++++++------- package.json | 3 +- yarn.lock | 8 +-- 4 files changed, 90 insertions(+), 74 deletions(-) diff --git a/modules/sdk-coin-icp/src/icp.ts b/modules/sdk-coin-icp/src/icp.ts index 461ff0683c..d4e2d67d18 100644 --- a/modules/sdk-coin-icp/src/icp.ts +++ b/modules/sdk-coin-icp/src/icp.ts @@ -20,6 +20,7 @@ import { SignTransactionOptions, VerifyTransactionOptions, verifyMPCWalletAddress, + UnexpectedAddressError, } from '@bitgo/sdk-core'; import { coins, NetworkType, BaseCoin as StaticsBaseCoin } from '@bitgo/statics'; import { Principal } from '@dfinity/principal'; @@ -146,9 +147,15 @@ export class Icp extends BaseCoin { /** * Verify that an address belongs to this wallet. * + * For wallet version 1 (memo-based): The address format is `rootAddress?memoId=X`. + * We extract the root address and verify it against the commonKeychain at index 0. + * + * For wallet version 2+: The address is derived directly from the commonKeychain + * at the specified index. + * * @param {TssVerifyIcpAddressOptions} params - Verification parameters * @returns {Promise} True if address belongs to wallet - * @throws {InvalidAddressError} If address format is invalid + * @throws {InvalidAddressError} If address format is invalid or doesn't match derived address * @throws {Error} If invalid wallet version or missing parameters */ async isWalletAddress(params: TssVerifyIcpAddressOptions): Promise { @@ -158,63 +165,38 @@ export class Icp extends BaseCoin { throw new InvalidAddressError(`invalid address: ${address}`); } - if (walletVersion === 1) { - return this.verifyMemoBasedAddress(address, rootAddress); - } + let addressToVerify = address; + const parsedIndex = typeof params.index === 'string' ? parseInt(params.index, 10) : params.index; - return this.verifyKeyDerivedAddress(params, address, rootAddress); - } - - /** - * Verifies a memo-based address for wallet version 1. - * - * @param {string} address - The full address to verify (must include memoId) - * @param {string | undefined} rootAddress - The wallet's root address - * @returns {boolean} True if the address is valid - * @throws {Error} If rootAddress is missing or memoId is missing - */ - private verifyMemoBasedAddress(address: string, rootAddress: string | undefined): boolean { - if (!rootAddress) { - throw new Error('rootAddress is required for wallet version 1'); - } - const extractedRootAddress = utils.validateMemoAndReturnRootAddress(address); - if (extractedRootAddress === address) { - throw new Error('memoId is required for wallet version 1 addresses'); - } - - return extractedRootAddress?.toLowerCase() === rootAddress.toLowerCase(); - } - - /** - * Verifies a key-derived address using MPC wallet verification. - * - * @param {TssVerifyIcpAddressOptions} params - Verification parameters - * @param {string} address - The full address to verify - * @param {string | undefined} rootAddress - The wallet's root address - * @returns {Promise} True if the address matches the derived address - * @throws {Error} If keychains are missing or address doesn't match - */ - private async verifyKeyDerivedAddress( - params: TssVerifyIcpAddressOptions, - address: string, - rootAddress: string | undefined - ): Promise { - const { index } = params; - const parsedIndex = typeof index === 'string' ? parseInt(index, 10) : index; - - const isVerifyingRootAddress = rootAddress && address.toLowerCase() === rootAddress.toLowerCase(); - if (isVerifyingRootAddress && parsedIndex !== 0) { - throw new Error(`Root address verification requires index 0, but got index ${index}`); + if (walletVersion === 1) { + if (!rootAddress) { + throw new Error('rootAddress is required for wallet version 1'); + } + const extractedRootAddress = utils.validateMemoAndReturnRootAddress(address); + if (!extractedRootAddress || extractedRootAddress === address) { + throw new Error('memoId is required for wallet version 1 addresses'); + } + if (extractedRootAddress.toLowerCase() !== rootAddress.toLowerCase()) { + throw new UnexpectedAddressError( + `address validation failure: expected ${rootAddress} but got ${extractedRootAddress}` + ); + } + addressToVerify = rootAddress; + } else if (rootAddress && address.toLowerCase() === rootAddress.toLowerCase() && parsedIndex !== 0) { + throw new Error(`Root address verification requires index 0, but got index ${params.index}`); } + const indexToVerify = walletVersion === 1 ? 0 : params.index; const result = await verifyMPCWalletAddress( - { ...params, keyCurve: 'secp256k1' }, + { ...params, address: addressToVerify, index: indexToVerify, keyCurve: 'secp256k1' }, this.isValidAddress.bind(this), (pubKey) => utils.getAddressFromPublicKey(pubKey) ); if (!result) { - throw new InvalidAddressError(`invalid address: ${address}`); + throw new UnexpectedAddressError( + `address validation failure: address ${addressToVerify} is not a wallet address` + ); } return true; diff --git a/modules/sdk-coin-icp/test/unit/icp.ts b/modules/sdk-coin-icp/test/unit/icp.ts index f891c8899f..9ee24005f2 100644 --- a/modules/sdk-coin-icp/test/unit/icp.ts +++ b/modules/sdk-coin-icp/test/unit/icp.ts @@ -238,16 +238,26 @@ describe('Internet computer', function () { }; describe('Wallet VersionKey 1', () => { + let keychains; + + before(function () { + keychains = [ + { commonKeychain: addressVerificationData.commonKeychain }, + { commonKeychain: addressVerificationData.commonKeychain }, + { commonKeychain: addressVerificationData.commonKeychain }, + ]; + }); + it('should verify a valid memo-based address', async function () { - const rootAddress = testData.Accounts.account1.address; + const rootAddress = addressVerificationData.rootAddress; const addressWithMemo = `${rootAddress}?memoId=123`; const params = { address: addressWithMemo, rootAddress: rootAddress, walletVersion: 1, - keychains: [], - index: 123, + keychains: keychains, + index: 0, }; const result = await basecoin.isWalletAddress(params); @@ -255,14 +265,14 @@ describe('Internet computer', function () { }); it('should verify address with memoId=0', async function () { - const rootAddress = testData.Accounts.account1.address; + const rootAddress = addressVerificationData.rootAddress; const addressWithMemo = `${rootAddress}?memoId=0`; const params = { address: addressWithMemo, rootAddress: rootAddress, walletVersion: 1, - keychains: [], + keychains: keychains, index: 0, }; @@ -270,8 +280,8 @@ describe('Internet computer', function () { result.should.equal(true); }); - it('should fail when base address does not match root address', async function () { - const rootAddress = testData.Accounts.account1.address; + it('should fail when extracted root does not match provided rootAddress param', async function () { + const rootAddress = addressVerificationData.rootAddress; const differentAddress = testData.Accounts.account2.address; const addressWithMemo = `${differentAddress}?memoId=123`; @@ -279,35 +289,37 @@ describe('Internet computer', function () { address: addressWithMemo, rootAddress: rootAddress, walletVersion: 1, - keychains: [], - index: 123, + keychains: keychains, + index: 0, }; - const result = await basecoin.isWalletAddress(params); - result.should.equal(false); + // The extracted root (differentAddress) doesn't match provided rootAddress + await basecoin + .isWalletAddress(params) + .should.be.rejectedWith(`address validation failure: expected ${rootAddress} but got ${differentAddress}`); }); it('should throw error when rootAddress is missing for wallet version 1', async function () { - const address = `${testData.Accounts.account1.address}?memoId=123`; + const address = `${addressVerificationData.rootAddress}?memoId=123`; const params = { address: address, walletVersion: 1, - keychains: [], - index: 123, + keychains: keychains, + index: 0, }; await basecoin.isWalletAddress(params).should.be.rejectedWith('rootAddress is required for wallet version 1'); }); it('should throw error when memoId is missing for wallet version 1', async function () { - const rootAddress = testData.Accounts.account1.address; + const rootAddress = addressVerificationData.rootAddress; const params = { address: rootAddress, rootAddress: rootAddress, walletVersion: 1, - keychains: [], + keychains: keychains, index: 0, }; @@ -317,7 +329,7 @@ describe('Internet computer', function () { }); it('should handle large memoId values', async function () { - const rootAddress = testData.Accounts.account1.address; + const rootAddress = addressVerificationData.rootAddress; const largeMemoId = '9007199254740991'; const addressWithMemo = `${rootAddress}?memoId=${largeMemoId}`; @@ -325,13 +337,32 @@ describe('Internet computer', function () { address: addressWithMemo, rootAddress: rootAddress, walletVersion: 1, - keychains: [], - index: Number(largeMemoId), + keychains: keychains, + index: 0, }; const result = await basecoin.isWalletAddress(params); result.should.equal(true); }); + + it('should fail when rootAddress does not match commonKeychain derivation', async function () { + // Use a rootAddress that doesn't match what's derived from commonKeychain + const invalidRootAddress = testData.Accounts.account1.address; + const addressWithMemo = `${invalidRootAddress}?memoId=123`; + + const params = { + address: addressWithMemo, + rootAddress: invalidRootAddress, + walletVersion: 1, + keychains: keychains, + index: 0, + }; + + // rootAddress is cryptographically verified against commonKeychain + await basecoin + .isWalletAddress(params) + .should.be.rejectedWith(`address validation failure: address ${invalidRootAddress} is not a wallet address`); + }); }); describe('Wallet VersionKey 2+', () => { @@ -368,7 +399,9 @@ describe('Internet computer', function () { walletVersion: 2, }; - await basecoin.isWalletAddress(params).should.be.rejectedWith(`invalid address: ${invalidAddress}`); + await basecoin + .isWalletAddress(params) + .should.be.rejectedWith(`address validation failure: address ${invalidAddress} is not a wallet address`); }); it('should throw error when keychains is missing', async function () { diff --git a/package.json b/package.json index 40e7064f6e..399442a54f 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,8 @@ "**/avalanche/store2": "2.14.4", "webpack-dev-server": "5.2.1", "memfs": "4.46.0", - "**/iota-sdk/**/valibot": "1.2.0" + "**/iota-sdk/**/valibot": "1.2.0", + "validator": "13.15.23" }, "workspaces": [ "modules/*" diff --git a/yarn.lock b/yarn.lock index acd859bf1f..87a4eb1375 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20803,10 +20803,10 @@ validate-npm-package-name@^5.0.0: resolved "https://registry.npmjs.org/validate-npm-package-name/-/validate-npm-package-name-5.0.1.tgz" integrity sha512-OljLrQ9SQdOUqTaQxqL5dEfZWrXExyyWsozYlAWFawPVNuD83igl7uJD2RTkNMbniIYgt8l81eCJGIdQF7avLQ== -validator@^13.7.0: - version "13.15.15" - resolved "https://registry.npmjs.org/validator/-/validator-13.15.15.tgz" - integrity sha512-BgWVbCI72aIQy937xbawcs+hrVaN/CZ2UwutgaJ36hGqRrLNM+f5LUT/YPRbo8IV/ASeFzXszezV+y2+rq3l8A== +validator@13.15.23, validator@^13.7.0: + version "13.15.23" + resolved "https://registry.npmjs.org/validator/-/validator-13.15.23.tgz#59a874f84e4594588e3409ab1edbe64e96d0c62d" + integrity sha512-4yoz1kEWqUjzi5zsPbAS/903QXSYp0UOtHsPpp7p9rHAw/W+dkInskAE386Fat3oKRROwO98d9ZB0G4cObgUyw== varuint-bitcoin@^1.0.1, varuint-bitcoin@^1.0.4, varuint-bitcoin@^1.1.2: version "1.1.2" From 4ab78c1df22b036a3cd44a89676bf72558b1d52a Mon Sep 17 00:00:00 2001 From: danielzhao122 Date: Tue, 16 Dec 2025 12:53:03 -0500 Subject: [PATCH 3/3] refactor: remove validation on index = 0 for baseaddress ticket: WP-7080 TICKET: WP-7080 --- modules/sdk-coin-icp/src/icp.ts | 10 ---------- modules/sdk-coin-icp/test/unit/icp.ts | 14 -------------- 2 files changed, 24 deletions(-) diff --git a/modules/sdk-coin-icp/src/icp.ts b/modules/sdk-coin-icp/src/icp.ts index d4e2d67d18..bbf403d0cf 100644 --- a/modules/sdk-coin-icp/src/icp.ts +++ b/modules/sdk-coin-icp/src/icp.ts @@ -147,12 +147,6 @@ export class Icp extends BaseCoin { /** * Verify that an address belongs to this wallet. * - * For wallet version 1 (memo-based): The address format is `rootAddress?memoId=X`. - * We extract the root address and verify it against the commonKeychain at index 0. - * - * For wallet version 2+: The address is derived directly from the commonKeychain - * at the specified index. - * * @param {TssVerifyIcpAddressOptions} params - Verification parameters * @returns {Promise} True if address belongs to wallet * @throws {InvalidAddressError} If address format is invalid or doesn't match derived address @@ -166,8 +160,6 @@ export class Icp extends BaseCoin { } let addressToVerify = address; - const parsedIndex = typeof params.index === 'string' ? parseInt(params.index, 10) : params.index; - if (walletVersion === 1) { if (!rootAddress) { throw new Error('rootAddress is required for wallet version 1'); @@ -182,8 +174,6 @@ export class Icp extends BaseCoin { ); } addressToVerify = rootAddress; - } else if (rootAddress && address.toLowerCase() === rootAddress.toLowerCase() && parsedIndex !== 0) { - throw new Error(`Root address verification requires index 0, but got index ${params.index}`); } const indexToVerify = walletVersion === 1 ? 0 : params.index; diff --git a/modules/sdk-coin-icp/test/unit/icp.ts b/modules/sdk-coin-icp/test/unit/icp.ts index 9ee24005f2..614c1f9985 100644 --- a/modules/sdk-coin-icp/test/unit/icp.ts +++ b/modules/sdk-coin-icp/test/unit/icp.ts @@ -415,20 +415,6 @@ describe('Internet computer', function () { await basecoin.isWalletAddress(params).should.be.rejectedWith('missing required param keychains'); }); - it('should throw error when verifying root address with wrong index', async function () { - const params = { - address: addressVerificationData.rootAddress, - rootAddress: addressVerificationData.rootAddress, - keychains: keychains, - index: 1, - walletVersion: 2, - }; - - await basecoin - .isWalletAddress(params) - .should.be.rejectedWith('Root address verification requires index 0, but got index 1'); - }); - it('should handle string index', async function () { const params = { address: addressVerificationData.rootAddress,