Conversation
… CoinMap CoinMap.addCoin() now throws on collision for contract-address and NFT-collection indexes instead of silently overwriting the existing entry. Index keys include network type (family:networkType:address) so mainnet and testnet tokens on the same family no longer share one slot. Use distinct placeholder addresses for eth:ctkn and eth:cusdt until mainnet contracts are defined. CGD-715 TICKET: CGD-715
mullapudipruthvik
approved these changes
Jun 3, 2026
| const contractAddressKey = CoinMap.contractAddressKey(coin); | ||
| const existingByContractAddress = this._coinByContractAddress.get(contractAddressKey); | ||
| if (existingByContractAddress) { | ||
| throw new DuplicateContractAddressDefinitionError(contractAddressKey, existingByContractAddress.name); |
Contributor
There was a problem hiding this comment.
when does this run ?
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
2 participants
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.
Description
Fixes CGD-715 / BTGO-13: CoinMap.addCoin() enforced uniqueness for coin name, id, and alias, but silently overwrote entries in the contract-address and NFT-collection secondary indexes when two tokens shared the same lookup key. A later-registered token could shadow an existing one for coins.get(family:networkType:contractAddress) lookups, leading to wrong token metadata (e.g. decimals) during transaction construction.
Changes:
Throw DuplicateContractAddressDefinitionError / DuplicateNftCollectionIdDefinitionError on index collision (same pattern as name/id checks).
Include network.type in index keys (family:mainnet|testnet:address) so mainnet and testnet tokens on the same chain family no longer share one slot.
Assign unique placeholder contract addresses for eth:ctkn and eth:cusdt (both previously used 0x000…000).
Add unit tests for duplicate contract/NFT keys and update address/NFT lookup tests for the new key format.
Motivation: Sigma Prime audit finding — secondary indexes must not allow silent overwrite.
Issue Number
CGD-715 (BTGO-13)
Type of change
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
Breaking change: Contract-address and NFT-collection lookups via CoinMap.get() must use the new composite key format, e.g. eth:mainnet:0x… instead of eth:0x…. Coin lookup by name is unchanged.