fix: reject duplicate contract address and NFT index keys in CoinMap

modules/statics/src/coins/erc7984Tokens.ts

@@ -24,15 +24,15 @@ export const erc7984Tokens = [
     'eth:ctkn',
     'Confidential Test Token',
     6,
-    '0x0000000000000000000000000000000000000000', // TODO: update with mainnet contract address
+    '0x0000000000000000000000000000000000000001', // TODO: update with mainnet contract address
     UnderlyingAsset['eth:ctkn']
   ),
   erc7984(
     'f47ac10b-58cc-4372-a567-0e02b2c3d480',
     'eth:cusdt',
     'Confidential USDT',
     6,
-    '0x0000000000000000000000000000000000000000', // TODO: update with mainnet contract address
+    '0x0000000000000000000000000000000000000002', // TODO: update with mainnet contract address
     UnderlyingAsset['eth:cusdt']
   ),
 

modules/statics/src/errors.ts

@@ -33,6 +33,20 @@ export class DuplicateCoinIdDefinitionError extends BitGoStaticsError {
   }
 }
 
+export class DuplicateContractAddressDefinitionError extends BitGoStaticsError {
+  public constructor(contractAddressKey: string, existingCoinName: string) {
+    super(`token with contract address '${contractAddressKey}' is already defined as '${existingCoinName}'`);
+    Object.setPrototypeOf(this, DuplicateContractAddressDefinitionError.prototype);
+  }
+}
+
+export class DuplicateNftCollectionIdDefinitionError extends BitGoStaticsError {
+  public constructor(nftCollectionKey: string, existingCoinName: string) {
+    super(`token with NFT collection id '${nftCollectionKey}' is already defined as '${existingCoinName}'`);
+    Object.setPrototypeOf(this, DuplicateNftCollectionIdDefinitionError.prototype);
+  }
+}
+
 export class DisallowedCoinFeatureError extends BitGoStaticsError {
   public constructor(coinName: string, feature: CoinFeature) {
     super(`coin feature '${feature}' is disallowed for coin ${coinName}.`);

modules/statics/src/map.ts

@@ -1,5 +1,11 @@
 import { BaseCoin } from './base';
-import { DuplicateCoinDefinitionError, CoinNotDefinedError, DuplicateCoinIdDefinitionError } from './errors';
+import {
+  DuplicateCoinDefinitionError,
+  CoinNotDefinedError,
+  DuplicateCoinIdDefinitionError,
+  DuplicateContractAddressDefinitionError,
+  DuplicateNftCollectionIdDefinitionError,
+} from './errors';
 import { ContractAddressDefinedToken, NFTCollectionIdDefinedToken } from './account';
 import { EthereumNetwork } from './networks';
 
@@ -8,10 +14,10 @@ export class CoinMap {
   private readonly _coinByIds = new Map<string, Readonly<BaseCoin>>();
   // Holds key equivalences used during an asset name migration
   private readonly _coinByAliases = new Map<string, Readonly<BaseCoin>>();
-  // map of coin by address -> the key is the family:contractAddress
+  // map of coin by address -> the key is the family:networkType:contractAddress
   // the family is the where the coin is e.g l1 chains like eth, bsc etc. or l2 like arbeth, celo etc.
   private readonly _coinByContractAddress = new Map<string, Readonly<BaseCoin>>();
-  // map of coin by NFT collection ID -> the key is the (t)family:nftCollectionID
+  // map of coin by NFT collection ID -> the key is the (t)family:networkType:nftCollectionID
   private readonly _coinByNftCollectionID = new Map<string, Readonly<BaseCoin>>();
   // Lazily initialized cache for chainId to coin name mapping (derived from network definitions)
   private _coinByChainId: Map<number, string> | null = null;
@@ -20,6 +26,14 @@ export class CoinMap {
     // Do not instantiate
   }
 
+  private static contractAddressKey(coin: ContractAddressDefinedToken): string {
+    return `${coin.family}:${coin.network.type}:${coin.contractAddress}`;
+  }
+
+  private static nftCollectionIdKey(coin: NFTCollectionIdDefinedToken): string {
+    return `${coin.prefix}${coin.family}:${coin.network.type}:${coin.nftCollectionId}`;
+  }
+
   static fromCoins(coins: Readonly<BaseCoin>[]): CoinMap {
     const coinMap = new CoinMap();
     coins.forEach((coin) => {
@@ -47,9 +61,19 @@ export class CoinMap {
 
     if (coin.isToken) {
       if (coin instanceof ContractAddressDefinedToken) {
-        this._coinByContractAddress.set(`${coin.family}:${coin.contractAddress}`, coin);
+        const contractAddressKey = CoinMap.contractAddressKey(coin);
+        const existingByContractAddress = this._coinByContractAddress.get(contractAddressKey);
+        if (existingByContractAddress) {
+          throw new DuplicateContractAddressDefinitionError(contractAddressKey, existingByContractAddress.name);
+        }
+        this._coinByContractAddress.set(contractAddressKey, coin);
       } else if (coin instanceof NFTCollectionIdDefinedToken) {
-        this._coinByNftCollectionID.set(`${coin.prefix}${coin.family}:${coin.nftCollectionId}`, coin);
+        const nftCollectionKey = CoinMap.nftCollectionIdKey(coin);
+        const existingByNftCollectionId = this._coinByNftCollectionID.get(nftCollectionKey);
+        if (existingByNftCollectionId) {
+          throw new DuplicateNftCollectionIdDefinitionError(nftCollectionKey, existingByNftCollectionId.name);
+        }
+        this._coinByNftCollectionID.set(nftCollectionKey, coin);
       }
     }
   }
@@ -69,9 +93,9 @@ export class CoinMap {
       }
       if (oldCoin.isToken) {
         if (oldCoin instanceof ContractAddressDefinedToken) {
-          this._coinByContractAddress.delete(`${oldCoin.family}:${oldCoin.contractAddress}`);
+          this._coinByContractAddress.delete(CoinMap.contractAddressKey(oldCoin));
         } else if (oldCoin instanceof NFTCollectionIdDefinedToken) {
-          this._coinByNftCollectionID.delete(`${oldCoin.prefix}${oldCoin.family}:${oldCoin.nftCollectionId}`);
+          this._coinByNftCollectionID.delete(CoinMap.nftCollectionIdKey(oldCoin));
         }
       }
     }

modules/statics/test/unit/coins.ts

@@ -41,7 +41,7 @@ import {
   trimmedDynamicBaseChainConfig,
 } from './resources/amsTokenConfig';
 import { EthLikeErc20Token } from '../../../sdk-coin-evm/src';
-import { ProgramID } from '../../src/account';
+import { ProgramID, taptNFTCollection, terc20 } from '../../src/account';
 import { allCoinsAndTokens } from '../../src/allCoinsAndTokens';
 
 interface DuplicateCoinObject {
@@ -753,6 +753,70 @@ describe('CoinMap', function () {
     (() => CoinMap.fromCoins([btc, btc2])).should.throw(`coin with id '${btc.id}' is already defined`);
   });
 
+  it('should fail to map tokens with duplicated contract address for the same family', () => {
+    const template = coins.get('tusdc');
+    const contractAddress = (template as Erc20Coin).contractAddress;
+    const tokenA = terc20(
+      '11111111-1111-4111-8111-111111111111',
+      'token-a',
+      'Token A',
+      6,
+      contractAddress,
+      template.asset,
+      template.features,
+      template.prefix,
+      template.suffix,
+      template.network
+    );
+    const tokenB = terc20(
+      '22222222-2222-4222-8222-222222222222',
+      'token-b',
+      'Token B',
+      18,
+      contractAddress,
+      template.asset,
+      template.features,
+      template.prefix,
+      template.suffix,
+      template.network
+    );
+    const contractAddressKey = `${tokenA.family}:${tokenA.network.type}:${contractAddress}`;
+    (() => CoinMap.fromCoins([tokenA, tokenB])).should.throw(
+      `token with contract address '${contractAddressKey}' is already defined as 'token-a'`
+    );
+  });
+
+  it('should fail to map tokens with duplicated NFT collection id for the same family', () => {
+    const template = coins.get('tapt:nftcollection1');
+    const nftCollectionId = '0xbbc561fbfa5d105efd8dfb06ae3e7e5be46331165b99d518f094c701e40603b5';
+    const tokenA = taptNFTCollection(
+      '11111111-1111-4111-8111-111111111111',
+      'tapt:nftcollection-a',
+      'NFT Collection A',
+      nftCollectionId,
+      template.asset,
+      template.features,
+      template.prefix,
+      template.suffix,
+      template.network
+    );
+    const tokenB = taptNFTCollection(
+      '22222222-2222-4222-8222-222222222222',
+      'tapt:nftcollection-b',
+      'NFT Collection B',
+      nftCollectionId,
+      template.asset,
+      template.features,
+      template.prefix,
+      template.suffix,
+      template.network
+    );
+    const nftCollectionKey = `${tokenA.prefix}${tokenA.family}:${tokenA.network.type}:${nftCollectionId}`;
+    (() => CoinMap.fromCoins([tokenA, tokenB])).should.throw(
+      `token with NFT collection id '${nftCollectionKey}' is already defined as 'tapt:nftcollection-a'`
+    );
+  });
+
   it('should have iterator', function () {
     [...coins].length.should.be.greaterThan(100);
   });
@@ -783,10 +847,10 @@ describe('CoinMap', function () {
 
   it('should get coin by address', () => {
     const weth = coins.get('weth');
-    const wethByAddress = coins.get(`${weth.family}:${(weth as Erc20Coin).contractAddress}`);
+    const wethByAddress = coins.get(`${weth.family}:${weth.network.type}:${(weth as Erc20Coin).contractAddress}`);
     wethByAddress.should.deepEqual(weth);
     const tweth = coins.get('tweth');
-    const twethByAddress = coins.get(`${tweth.family}:${(tweth as Erc20Coin).contractAddress}`);
+    const twethByAddress = coins.get(`${tweth.family}:${tweth.network.type}:${(tweth as Erc20Coin).contractAddress}`);
     twethByAddress.should.deepEqual(tweth);
   });
 
@@ -795,7 +859,11 @@ describe('CoinMap', function () {
   });
 
   it('should find coin by NFT collection ID', () => {
-    const nftCollectionStatics = coins.get('tapt:0xbbc561fbfa5d105efd8dfb06ae3e7e5be46331165b99d518f094c701e40603b5');
+    const nftCollectionStatics = coins.get(
+      `tapt:${
+        coins.get('tapt:nftcollection1').network.type
+      }:0xbbc561fbfa5d105efd8dfb06ae3e7e5be46331165b99d518f094c701e40603b5`
+    );
     nftCollectionStatics.name.should.eql('tapt:nftcollection1');
   });