Skip to content

chore: fix underlyingAsset for test token and coins #6438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mukeshsp
Copy link
Contributor

@mukeshsp mukeshsp commented Jul 11, 2025

The underlyingAsset should be set to the corresponding testnet asset if the coin belongs to a testnet. This PR refactors such tokens to ensure consistency.

Ticket: WIN-6149

@mukeshsp mukeshsp force-pushed the fix-ofc-decimalPlaces branch 6 times, most recently from 08b245e to 1b1da0d Compare July 11, 2025 11:41
@mukeshsp mukeshsp changed the title chore: fix underlyingAsset and decimalPlaces for test token and coins chore: fix underlyingAsset for test token and coins Jul 11, 2025
@mukeshsp mukeshsp marked this pull request as ready for review July 11, 2025 12:09
@mukeshsp mukeshsp requested review from a team as code owners July 11, 2025 12:09
@mukeshsp mukeshsp force-pushed the fix-ofc-decimalPlaces branch from 1b1da0d to 4f8c3cf Compare July 14, 2025 08:50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a potentially breaking change, several of our user interfaces were previously built under the assumption that the UnderlyingAsset provided an environment agnostic way of defining a coin. We should first do an alpha release and vet this change before merging to master

Copy link
Contributor

@bitgoAaron bitgoAaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. please use conventional commit messages, this type of change is not a chore, it should be a feature or fix.
  2. please include BREAKING CHANGE, as we are now fundamentally re-defining what UnderlyingAsset is used for
  3. please do an alpha release & verify this change in bitgo-ui before proceeding with merges to master

Copy link
Contributor

@lcovar lcovar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with aaron

@mukeshsp mukeshsp force-pushed the fix-ofc-decimalPlaces branch from 4f8c3cf to 63d692f Compare July 15, 2025 05:06
Ticket: WIN-6149

BREAKING CHANGE: The underlyingAsset should be set to the corresponding testnet asset if the coin belongs to a testnet. This PR refactors such tokens to ensure consistency.
@mukeshsp mukeshsp force-pushed the fix-ofc-decimalPlaces branch from 63d692f to 568c2a1 Compare July 15, 2025 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants