From a568186f34db4b435ea17de5de92622a8cae62ca Mon Sep 17 00:00:00 2001 From: Prajwal U Date: Wed, 14 Oct 2026 19:49:22 +0530 Subject: [PATCH] revert(statics): remove replaceExistingCoin coin replacement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit problem/goal: Revert PR #9922 (commit 787e7189), which allowed AMS payloads to update already-registered coins via the identity-guarded CoinMap.replace/replaceExistingCoin path. Restores the prior behavior where statics wins and an AMS payload cannot change features of an already-registered coin. implementation details: git revert --no-commit of 787e7189; removes replaceExistingCoin and its related error type from modules/statics (coins.ts, errors.ts, map.ts) along with the unit tests added by the original PR. verification: yarn run unit-test --scope @bitgo/statics — 39690 passing. Ticket: CSHLD-863 --- modules/statics/src/coins.ts | 83 +++++++------- modules/statics/src/errors.ts | 7 -- modules/statics/src/map.ts | 36 ++----- modules/statics/test/unit/coins.ts | 168 ++++------------------------- 4 files changed, 66 insertions(+), 228 deletions(-) diff --git a/modules/statics/src/coins.ts b/modules/statics/src/coins.ts index 03b0db2272b..426c867b418 100644 --- a/modules/statics/src/coins.ts +++ b/modules/statics/src/coins.ts @@ -84,34 +84,15 @@ allCoinsAndTokens.forEach((coin) => { // onboarding doesn't require hand-maintaining that map for every new EVM family. registerErc20Families(erc20FamilySupportsEip1559); -function findExistingCoin({ name, id, alias }: AmsTokenConfig): Readonly | undefined { - return coins.getOrUndefined(name) ?? coins.getOrUndefined(id) ?? (alias ? coins.getOrUndefined(alias) : undefined); -} - export function createToken(token: AmsTokenConfig): Readonly | undefined { - const existingCoin = findExistingCoin(token); - if (!existingCoin) { - if (!token.isToken) { - try { - return buildDynamicCoin(token); - } catch { - return undefined; - } - } - return constructToken(token); - } - try { - const refreshedCoin = token.isToken ? constructToken(token) : buildDynamicCoin(token); - if (refreshedCoin) { - coins.replace(refreshedCoin); + if (!token.isToken) { + try { + return buildDynamicCoin(token); + } catch (error) { + console.warn(`Failed to build dynamic coin for ${token.name} (${token.id}):`, error); + return undefined; } - } catch { - // an AMS config that fails validation must not break a coin we already know } - return findExistingCoin(token); -} - -function constructToken(token: AmsTokenConfig): Readonly | undefined { const initializerMap: Record = { algo: algoToken, apt: aptToken, @@ -169,6 +150,18 @@ function constructToken(token: AmsTokenConfig): Readonly | undefined { initializerMap[key] = erc721Token; }); + //return the BaseCoin from default coin map if present + if (isCoinPresentInCoinMap({ ...token })) { + if (coins.has(token.name)) { + return coins.get(token.name); + } + if (coins.has(token.id)) { + return coins.get(token.id); + } + if (token.alias && coins.has(token.alias)) { + return coins.get(token.alias); + } + } const family = token.family; const initializer = initializerMap[family] as (...args: unknown[]) => Readonly; if (!initializer) { @@ -509,27 +502,27 @@ export function createTokenMapUsingConfigDetails(tokenConfigMap: Record} coin new coin object - * @throws {CoinReplacementNotAllowedError} when an immutable field differs from the existing record */ public replace(coin: Readonly): void { if (this.has(coin.id)) { const oldCoin = this.get(coin.id); - CoinMap.assertImmutableFieldsUnchanged(oldCoin, coin); this._map.delete(oldCoin.name); this._coinByIds.delete(oldCoin.id); if (oldCoin.alias) { this._coinByAliases.delete(oldCoin.alias); } - if (oldCoin instanceof ContractAddressDefinedToken) { - this._coinByContractAddress.delete(CoinMap.contractAddressKey(oldCoin)); - } else if (oldCoin instanceof NFTCollectionIdDefinedToken) { - this._coinByNftCollectionID.delete(CoinMap.nftCollectionIdKey(oldCoin)); + if (oldCoin.isToken) { + if (oldCoin instanceof ContractAddressDefinedToken) { + this._coinByContractAddress.delete(CoinMap.contractAddressKey(oldCoin)); + } else if (oldCoin instanceof NFTCollectionIdDefinedToken) { + this._coinByNftCollectionID.delete(CoinMap.nftCollectionIdKey(oldCoin)); + } } } this.addCoin(coin); } - private static assertImmutableFieldsUnchanged(oldCoin: Readonly, newCoin: Readonly): void { - const oldContractAddress = oldCoin instanceof ContractAddressDefinedToken ? oldCoin.contractAddress : undefined; - const newContractAddress = newCoin instanceof ContractAddressDefinedToken ? newCoin.contractAddress : undefined; - if (oldCoin.id !== newCoin.id) { - throw new CoinReplacementNotAllowedError(oldCoin.name, 'immutable field id changed'); - } - if (oldCoin.name !== newCoin.name) { - throw new CoinReplacementNotAllowedError(oldCoin.name, 'immutable field name changed'); - } - if (oldContractAddress !== newContractAddress) { - throw new CoinReplacementNotAllowedError(oldCoin.name, 'immutable field contractAddress changed'); - } - if (oldCoin.alias !== newCoin.alias) { - throw new CoinReplacementNotAllowedError(oldCoin.name, 'immutable field alias changed'); - } - } - /** * Hardcoded mapping for backward compatibility. */ diff --git a/modules/statics/test/unit/coins.ts b/modules/statics/test/unit/coins.ts index f234b255d26..5c1c067df90 100644 --- a/modules/statics/test/unit/coins.ts +++ b/modules/statics/test/unit/coins.ts @@ -1,6 +1,5 @@ import 'should'; import { - BaseCoin, BaseNetwork, BaseUnit, CoinFamily, @@ -981,125 +980,13 @@ describe('CoinMap', function () { coinMap.has(token.name).should.be.true(); }); - describe('replace', () => { - // Replacement is only allowed for tokens whose immutable fields (id, name, contract address, alias) - // are unchanged; a mismatch must throw and leave the map untouched so a refreshed AMS record - // can never silently redefine a different asset. - const template = coins.get('tusdc') as Erc20Coin; - const buildToken = (overrides: { - id?: string; - name?: string; - fullName?: string; - contractAddress?: string; - suffix?: string; - }) => - terc20( - overrides.id ?? template.id, - overrides.name ?? template.name, - overrides.fullName ?? template.fullName, - template.decimalPlaces, - overrides.contractAddress ?? template.contractAddress.toString(), - template.asset, - template.features, - template.prefix, - overrides.suffix ?? template.suffix, - template.network as EthereumNetwork - ); - - it('should add a coin that is not in the map', () => { - const coinMap = CoinMap.fromCoins([]); - coinMap.replace(template); - coinMap.get(template.name).should.equal(template); - }); - - it('should replace a token when immutable fields match', () => { - const coinMap = CoinMap.fromCoins([template]); - const refreshed = buildToken({ fullName: 'refreshed name' }); - coinMap.replace(refreshed); - coinMap.get(template.name).should.equal(refreshed); - coinMap.get(template.id).should.equal(refreshed); - coinMap.get(`${template.family}:${template.contractAddress}`).should.equal(refreshed); - }); - - it('should replace a token when only the prefix and suffix change', () => { - const coinMap = CoinMap.fromCoins([template]); - const refreshed = buildToken({ suffix: 'OTHER' }); - coinMap.replace(refreshed); - coinMap.get(template.name).should.equal(refreshed); - }); - - it('should throw and keep the existing token when the name differs', () => { - const coinMap = CoinMap.fromCoins([template]); - (() => coinMap.replace(buildToken({ name: 'tusdc2' }))).should.throw( - `coin '${template.name}' cannot be replaced: immutable field name changed` - ); - coinMap.get(template.name).should.equal(template); - coinMap.has('tusdc2').should.be.false(); - }); - - it('should throw and keep the existing token when the contract address differs', () => { - const coinMap = CoinMap.fromCoins([template]); - (() => - coinMap.replace(buildToken({ contractAddress: '0x0000000000000000000000000000000000000001' }))).should.throw( - `coin '${template.name}' cannot be replaced: immutable field contractAddress changed` - ); - coinMap.get(template.name).should.equal(template); - }); - - it('should throw and keep the existing token when the alias differs', () => { - const coinMap = CoinMap.fromCoins([template]); - // keep the prototype so the candidate is still a contract-address token and only the alias differs - const candidate = Object.assign(Object.create(Object.getPrototypeOf(template)), template, { - alias: 'legacyusdc', - }); - (() => coinMap.replace(candidate)).should.throw( - `coin '${template.name}' cannot be replaced: immutable field alias changed` - ); - coinMap.get(template.name).should.equal(template); - coinMap.has('legacyusdc').should.be.false(); - }); - - it('should treat a token with a different id as a new coin and reject the duplicate name', () => { - const coinMap = CoinMap.fromCoins([template]); - (() => coinMap.replace(buildToken({ id: '11111111-1111-4111-8111-111111111111' }))).should.throw( - `coin '${template.name}' is already defined` - ); - coinMap.get(template.name).should.equal(template); - }); - - it('should replace a base coin when immutable fields match', () => { - const btc = coins.get('btc'); - const coinMap = CoinMap.fromCoins([btc]); - const refreshed = { ...btc, fullName: 'Bitcoin renamed' }; - coinMap.replace(refreshed); - coinMap.get('btc').should.equal(refreshed); - coinMap.get(btc.id).should.equal(refreshed); - }); - - it('should throw and keep the existing base coin when the name differs', () => { - const btc = coins.get('btc'); - const coinMap = CoinMap.fromCoins([btc]); - (() => coinMap.replace({ ...btc, name: 'btc2' })).should.throw( - `coin 'btc' cannot be replaced: immutable field name changed` - ); - coinMap.get('btc').should.equal(btc); - coinMap.has('btc2').should.be.false(); - }); - - it('should throw and keep the existing coin when a contract-address token tries to replace a base coin', () => { - // isToken itself isn't checked directly; a token replacing a base coin is still rejected here - // because the base coin has no contract address while the token does. - const btc = coins.get('btc'); - const coinMap = CoinMap.fromCoins([btc]); - (() => - coinMap.replace( - buildToken({ - id: btc.id, - name: btc.name, - }) - )).should.throw(`coin '${btc.name}' cannot be replaced: immutable field contractAddress changed`); - coinMap.get('btc').should.equal(btc); - }); + it('should replace a coin', () => { + const coinMap = CoinMap.fromCoins([]); + const coin = coins.get('btc'); + const newCoin = { ...coin, name: 'btc2' }; + coinMap.replace(newCoin); + coinMap.has(coin.name).should.be.false(); + coinMap.has(newCoin.name).should.be.true(); }); describe('coinNameFromChainId', function () { @@ -1678,20 +1565,6 @@ describe('ADA RealFi USDrf staking', () => { }); describe('create token map using config details', () => { - // Known tokens are refreshed in the global coin map, and these tests feed deliberately wrong configs - // for real tokens, so put the originals back for the suites that read the global map. - const staticCoinsBeforeAmsTests = new Map>(); - before(() => { - coins.forEach((coin, name) => staticCoinsBeforeAmsTests.set(name, coin)); - }); - after(() => { - staticCoinsBeforeAmsTests.forEach((coin, name) => { - if (coins.get(name) !== coin) { - coins.replace(coin); - } - }); - }); - it('should create a valid token map from AmsTokenConfig', () => { const tokenMap = createTokenMapUsingConfigDetails(amsTokenConfig); Object.keys(amsTokenConfig).forEach((tokenName) => { @@ -1704,19 +1577,14 @@ describe('create token map using config details', () => { }); }); - it('should refresh modifiable fields of a known token from the ams config', () => { - const tokenName = 'thbar:usdc'; + it('should give precedence to static coin map over ams coin map', () => { const tokenMap = createTokenMapUsingConfigDetails(incorrectAmsTokenConfig); - const token = tokenMap.get(tokenName); - token.decimalPlaces.should.eql(incorrectAmsTokenConfig[tokenName][0].decimalPlaces); - }); - - it('should keep the static token when the ams config diverges on an identity field', () => { const tokenName = 'thbar:usdc'; - const divergentConfig = { [tokenName]: [{ ...incorrectAmsTokenConfig[tokenName][0], contractAddress: '0.0.1' }] }; - const token = createTokenMapUsingConfigDetails(divergentConfig).get(tokenName); + const token = tokenMap.get(tokenName); token.decimalPlaces.should.eql(coins.get(tokenName).decimalPlaces); token.baseUnit.should.eql(coins.get(tokenName).baseUnit); + token.decimalPlaces.should.not.eql(incorrectAmsTokenConfig[tokenName][0].decimalPlaces); + token.baseUnit.should.not.eql(incorrectAmsTokenConfig[tokenName][0].baseUnit); }); it('should create a coin map and get formatted tokens from it', () => { @@ -1776,12 +1644,14 @@ describe('create token map using config details', () => { }); }); - it('should refresh a known token through createToken and return the record held by the coin map', () => { + it('should return the base coin present in default coin map', () => { const tokenName = 'thbar:usdc'; const token = createToken(incorrectAmsTokenConfig[tokenName][0]); token?.should.not.be.undefined(); - token?.decimalPlaces.should.eql(incorrectAmsTokenConfig[tokenName][0].decimalPlaces); - coins.get(tokenName).should.equal(token); + token?.decimalPlaces.should.eql(coins.get(tokenName).decimalPlaces); + token?.baseUnit.should.eql(coins.get(tokenName).baseUnit); + token?.decimalPlaces.should.not.eql(incorrectAmsTokenConfig[tokenName][0].decimalPlaces); + token?.baseUnit.should.not.eql(incorrectAmsTokenConfig[tokenName][0].baseUnit); }); it('should be able to create base coin from trimmed token config', () => { @@ -2197,9 +2067,7 @@ describe('DynamicCoin and dynamic base chain support', function () { coinMap.has('mydynchain2').should.be.false(); }); - it('should keep the existing coin when a refresh attempt for a known base chain fails validation', function () { - // id 'btc' is not a valid UUID, so the refreshed DynamicCoin fails validation and the - // pre-existing static btc coin is kept and returned instead of the (invalid) refreshed one. + it('should return undefined when the coin already exists in the static coin map', function () { const config = { ...trimmedDynamicBaseChainConfig['mydynchain'][0], name: 'btc', @@ -2207,7 +2075,7 @@ describe('DynamicCoin and dynamic base chain support', function () { isToken: false, }; const coin = createTokenUsingTrimmedConfigDetails(config); - coin.should.equal(coins.get('btc')); + should(coin).be.undefined(); }); }); });