From 957e6dc79772c6ab6f3a5f1c60f7723d044a8c6b Mon Sep 17 00:00:00 2001 From: neha-kri Date: Mon, 8 Jun 2026 20:36:21 +0530 Subject: [PATCH] fix(statics): reject duplicate contract address and NFT index keys in CoinMap TICKET: CGD-715 TICKET: CGD-715 --- modules/statics/src/coins.ts | 5 +- modules/statics/src/coins/erc7984Tokens.ts | 4 +- modules/statics/src/errors.ts | 14 +++ modules/statics/src/map.ts | 61 +++++++++- modules/statics/test/unit/coins.ts | 126 ++++++++++++++++++++- 5 files changed, 201 insertions(+), 9 deletions(-) diff --git a/modules/statics/src/coins.ts b/modules/statics/src/coins.ts index c7f49e2d30..678e7e8930 100644 --- a/modules/statics/src/coins.ts +++ b/modules/statics/src/coins.ts @@ -476,7 +476,10 @@ export function createTokenMapUsingConfigDetails(tokenConfigMap: Record): boolean { + if (coin instanceof ContractAddressDefinedToken) { + const key = CoinMap.contractAddressKey(coin); + const existing = this._coinByContractAddress.get(key); + return existing !== undefined && existing.network.type === coin.network.type; + } + if (coin instanceof NFTCollectionIdDefinedToken) { + const key = CoinMap.nftCollectionIdKey(coin); + const existing = this._coinByNftCollectionID.get(key); + return existing !== undefined && existing.network.type === coin.network.type; + } + return false; + } + static fromCoins(coins: Readonly[]): CoinMap { const coinMap = new CoinMap(); coins.forEach((coin) => { @@ -47,9 +82,25 @@ 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) { + if (existingByContractAddress.network.type === coin.network.type) { + throw new DuplicateContractAddressDefinitionError(contractAddressKey, existingByContractAddress.name); + } + } else { + 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) { + if (existingByNftCollectionId.network.type === coin.network.type) { + throw new DuplicateNftCollectionIdDefinitionError(nftCollectionKey, existingByNftCollectionId.name); + } + } else { + this._coinByNftCollectionID.set(nftCollectionKey, coin); + } } } } @@ -69,9 +120,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)); } } } diff --git a/modules/statics/test/unit/coins.ts b/modules/statics/test/unit/coins.ts index 8ebbda4fa1..ebe3a52ba4 100644 --- a/modules/statics/test/unit/coins.ts +++ b/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') as Erc20Coin; + const contractAddress = template.contractAddress.toString(); + const tokenA = terc20( + '11111111-1111-4111-8111-111111111111', + 'token-a', + 'Token A', + 6, + contractAddress, + template.asset, + template.features, + template.prefix, + template.suffix, + template.network as EthereumNetwork + ); + const tokenB = terc20( + '22222222-2222-4222-8222-222222222222', + 'token-b', + 'Token B', + 18, + contractAddress, + template.asset, + template.features, + template.prefix, + template.suffix, + template.network as EthereumNetwork + ); + const contractAddressKey = `${tokenA.family}:${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}:${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); }); @@ -1447,6 +1511,66 @@ describe('create token map using config details', () => { }); }); +describe('create token map contract address de-duplication', () => { + function firstStaticErc20(): Readonly { + for (const [, coin] of coins) { + if (coin instanceof Erc20Coin) { + return coin as Readonly; + } + } + throw new Error('expected at least one static ERC20 token in the coin map'); + } + + function collidingAmsConfig( + staticToken: Readonly, + name: string, + id: string + ): Parameters[0] { + return { + [name]: [ + { + id, + fullName: 'Colliding AMS Token', + name, + prefix: '', + suffix: name.toUpperCase(), + baseUnit: 'wei', + kind: 'crypto', + family: staticToken.family, + isToken: true, + features: [...staticToken.features], + decimalPlaces: staticToken.decimalPlaces, + asset: name, + network: staticToken.network, + primaryKeyCurve: 'secp256k1', + contractAddress: staticToken.contractAddress, + }, + ], + } as unknown as Parameters[0]; + } + + const collidingName = 'eth:cshld976colliding'; + const collidingId = 'aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee'; + + it('uses a name and id not already in the static coin map', () => { + coins.has(collidingName).should.eql(false); + coins.has(collidingId).should.eql(false); + }); + + it('skips an AMS token that reuses an existing static contract address under a different name', () => { + const staticToken = firstStaticErc20(); + const config = collidingAmsConfig(staticToken, collidingName, collidingId); + + let tokenMap: CoinMap | undefined; + (() => { + tokenMap = createTokenMapUsingConfigDetails(config); + }).should.not.throw(); + + (tokenMap as CoinMap).has(collidingName).should.eql(false); + (tokenMap as CoinMap).has(staticToken.name).should.eql(true); + }); +}); + describe('DynamicCoin and dynamic base chain support', function () { describe('createToken with dynamic base chain', function () { it('should return a DynamicCoin when isToken is false with a BaseNetwork instance', function () {