fix(statics): skip AMS tokens that reuse a static contract address#8959
Closed
zahin-mohammad wants to merge 1 commit into
Closed
fix(statics): skip AMS tokens that reuse a static contract address#8959zahin-mohammad wants to merge 1 commit into
zahin-mohammad wants to merge 1 commit into
Conversation
The UI builds a combined coin map by merging the static coin map with the live AMS token config (createTokenMapUsingConfigDetails). The dedup guard isCoinPresentInCoinMap only matches on name/id/alias, so an AMS token that reuses a contract address already claimed by a static token under a different name slipped through, and CoinMap.fromCoins then threw DuplicateContractAddressDefinitionError — crashing the UI. Add CoinMap.hasTokenAddressConflict, which reuses the map's own contract-address and NFT-collection key derivation, and use it in the merge loop to skip a colliding token instead of crashing. Ticket: CSHLD-976 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Will let the coins team take this forward. |
Closed
4 tasks
mmcshinsky-bitgo
added a commit
that referenced
this pull request
Jun 5, 2026
… nft collection id The ams<->static merge in createTokenMapUsingConfigDetails dedups via isCoinPresentInCoinMap, which only matches on name/id/alias -- never the contract address. An AMS-served token that reuses a contract address (or NFT collection id) already claimed by a static token under a different name slips through, and CoinMap.fromCoins -> addCoin then throws DuplicateContractAddressDefinitionError, aborting the entire token-map build for every consumer. Add CoinMap.hasTokenAddressConflict(coin) -- reusing the map's own contractAddressKey / nftCollectionIdKey derivation against its populated address/NFT indexes -- and use it in the merge loop to skip a colliding token instead of crashing. Statics coins are seeded first, so statics stays the source of truth and the colliding AMS token is the one dropped. Covers both contract-address and NFT-collection-id collisions. Surfaced by the eth:at bot token (0x0581ccdf...aa, eth testnet) added in #8885; the live AMS config serves a token at the same address under a different name. Supersedes the handoff reference in #8959 (closed). CSHLD-976
mmcshinsky-bitgo
added a commit
that referenced
this pull request
Jun 5, 2026
… nft collection id The ams<->static merge in createTokenMapUsingConfigDetails dedups via isCoinPresentInCoinMap, which only matches on name/id/alias -- never the contract address. An AMS-served token that reuses a contract address (or NFT collection id) already claimed by a static token under a different name slips through, and CoinMap.fromCoins -> addCoin then throws DuplicateContractAddressDefinitionError, aborting the entire token-map build for every consumer. Add CoinMap.hasTokenAddressConflict(coin) -- reusing the map's own contractAddressKey / nftCollectionIdKey derivation against its populated address/NFT indexes -- and use it in the merge loop to skip a colliding token instead of crashing. Statics coins are seeded first, so statics stays the source of truth and the colliding AMS token is the one dropped. Covers both contract-address and NFT-collection-id collisions. Surfaced by the eth:at bot token (0x0581ccdf...aa, eth testnet) added in #8885; the live AMS config serves a token at the same address under a different name. Supersedes the handoff reference in #8959 (closed). CSHLD-976
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DuplicateContractAddressDefinitionErrorthat crashes the BitGo UI when an AMS-served token reuses a contract address already claimed by a static token under a different name.createTokenMapUsingConfigDetailsdedups viaisCoinPresentInCoinMap, which only matches on name/id/alias — never the contract address. The colliding token slipped through andCoinMap.fromCoins→addCointhen threw.CoinMap.hasTokenAddressConflict(coin)(reusing the map's owncontractAddressKey/nftCollectionIdKeyderivation) and uses it in the merge loop to skip a colliding token instead of crashing. Checking the built coin means the key uses the same address normalization as static tokens, so it matches reliably. Covers both contract-address and NFT-collection-id collisions.Surfaced by the
eth:atbot token (0x0581ccdf…aa, eth testnet) added in #8885; the live AMS config serves a token at the same address under a different name. Observed crash path:syncAmsCoinsToPresenter → createTokenMapUsingTrimmedConfigDetails → createTokenMapUsingConfigDetails → CoinMap.fromCoins → addCoin.Ships forward (no statics pin, no lost coins) — unlike the bitgo-retail revert (BitGo/bitgo-retail#6932), which stepped statics back only 3 beta builds and still shipped
eth:at, so it would not have fixed the crash.Test plan
modules/statics/test/unit/coins.ts— fails before the fix (throwsDuplicateContractAddressDefinitionError), passes after (token skipped, static token preserved).tsc --noEmitclean; ESLint 0 errors on changed files.Scope note
Targets the static-vs-AMS collision (the incident). A hypothetical AMS-vs-AMS duplicate (two server tokens at the same address, neither in statics) would still throw at
fromCoins— a separate, pre-existing AMS data-integrity concern, out of scope here.Ticket: CSHLD-976
🤖 Generated with Claude Code