Merge pull request #8961 from BitGo/revert-8926-CGD-715

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

Author: zahin-mohammad

modules/statics/src/coins/erc7984Tokens.ts

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

modules/statics/src/errors.ts

@@ -33,20 +33,6 @@ 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,11 +1,5 @@
 import { BaseCoin } from './base';
-import {
-  DuplicateCoinDefinitionError,
-  CoinNotDefinedError,
-  DuplicateCoinIdDefinitionError,
-  DuplicateContractAddressDefinitionError,
-  DuplicateNftCollectionIdDefinitionError,
-} from './errors';
+import { DuplicateCoinDefinitionError, CoinNotDefinedError, DuplicateCoinIdDefinitionError } from './errors';
 import { ContractAddressDefinedToken, NFTCollectionIdDefinedToken } from './account';
 import { EthereumNetwork } from './networks';
 
@@ -14,10 +8,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:networkType:contractAddress
+  // map of coin by address -> the key is the family: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:networkType:nftCollectionID
+  // map of coin by NFT collection ID -> the key is the (t)family: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;
@@ -26,14 +20,6 @@ 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) => {
@@ -61,19 +47,9 @@ export class CoinMap {
 
     if (coin.isToken) {
       if (coin instanceof ContractAddressDefinedToken) {
-        const contractAddressKey = CoinMap.contractAddressKey(coin);
-        const existingByContractAddress = this._coinByContractAddress.get(contractAddressKey);
-        if (existingByContractAddress) {
-          throw new DuplicateContractAddressDefinitionError(contractAddressKey, existingByContractAddress.name);
-        }
-        this._coinByContractAddress.set(contractAddressKey, coin);
+        this._coinByContractAddress.set(`${coin.family}:${coin.contractAddress}`, coin);
       } else if (coin instanceof NFTCollectionIdDefinedToken) {
-        const nftCollectionKey = CoinMap.nftCollectionIdKey(coin);
-        const existingByNftCollectionId = this._coinByNftCollectionID.get(nftCollectionKey);
-        if (existingByNftCollectionId) {
-          throw new DuplicateNftCollectionIdDefinitionError(nftCollectionKey, existingByNftCollectionId.name);
-        }
-        this._coinByNftCollectionID.set(nftCollectionKey, coin);
+        this._coinByNftCollectionID.set(`${coin.prefix}${coin.family}:${coin.nftCollectionId}`, coin);
       }
     }
   }
@@ -93,9 +69,9 @@ export class CoinMap {
       }
       if (oldCoin.isToken) {
         if (oldCoin instanceof ContractAddressDefinedToken) {
-          this._coinByContractAddress.delete(CoinMap.contractAddressKey(oldCoin));
+          this._coinByContractAddress.delete(`${oldCoin.family}:${oldCoin.contractAddress}`);
         } else if (oldCoin instanceof NFTCollectionIdDefinedToken) {
-          this._coinByNftCollectionID.delete(CoinMap.nftCollectionIdKey(oldCoin));
+          this._coinByNftCollectionID.delete(`${oldCoin.prefix}${oldCoin.family}:${oldCoin.nftCollectionId}`);
         }
       }
     }

modules/statics/test/unit/coins.ts

@@ -41,7 +41,7 @@ import {
   trimmedDynamicBaseChainConfig,
 } from './resources/amsTokenConfig';
 import { EthLikeErc20Token } from '../../../sdk-coin-evm/src';
-import { ProgramID, taptNFTCollection, terc20 } from '../../src/account';
+import { ProgramID } from '../../src/account';
 import { allCoinsAndTokens } from '../../src/allCoinsAndTokens';
 
 interface DuplicateCoinObject {
@@ -753,70 +753,6 @@ 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);
   });
@@ -847,10 +783,10 @@ describe('CoinMap', function () {
 
   it('should get coin by address', () => {
     const weth = coins.get('weth');
-    const wethByAddress = coins.get(`${weth.family}:${weth.network.type}:${(weth as Erc20Coin).contractAddress}`);
+    const wethByAddress = coins.get(`${weth.family}:${(weth as Erc20Coin).contractAddress}`);
     wethByAddress.should.deepEqual(weth);
     const tweth = coins.get('tweth');
-    const twethByAddress = coins.get(`${tweth.family}:${tweth.network.type}:${(tweth as Erc20Coin).contractAddress}`);
+    const twethByAddress = coins.get(`${tweth.family}:${(tweth as Erc20Coin).contractAddress}`);
     twethByAddress.should.deepEqual(tweth);
   });
 
@@ -859,11 +795,7 @@ describe('CoinMap', function () {
   });
 
   it('should find coin by NFT collection ID', () => {
-    const nftCollectionStatics = coins.get(
-      `tapt:${
-        coins.get('tapt:nftcollection1').network.type
-      }:0xbbc561fbfa5d105efd8dfb06ae3e7e5be46331165b99d518f094c701e40603b5`
-    );
+    const nftCollectionStatics = coins.get('tapt:0xbbc561fbfa5d105efd8dfb06ae3e7e5be46331165b99d518f094c701e40603b5');
     nftCollectionStatics.name.should.eql('tapt:nftcollection1');
   });