refactor: upgrade package to avoid ci error and add verification for v1

TICKET: WP-7080

Author: danielzhao122

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<boolean>} 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<boolean> {
@@ -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<boolean>} 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<boolean> {
-    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;

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

@@ -238,76 +238,88 @@ 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);
         result.should.equal(true);
       });
 
       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,
         };
 
         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;
+      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`;
 
         const params = {
           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,21 +329,40 @@ 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}`;
 
         const params = {
           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 () {

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/*"

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"