Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: display token supply with correct decimals #287

Merged
merged 3 commits into from
Dec 16, 2024
Merged

fix: display token supply with correct decimals #287

merged 3 commits into from
Dec 16, 2024

Conversation

rafaelcr
Copy link
Collaborator

sBTC example:

Before

  "decimals": 8,
  "total_supply": "23190",

After

  "decimals": 8,
  "total_supply": "0.00023190",

@rafaelcr rafaelcr requested a review from zone117x December 16, 2024 16:45
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 16, 2024 •
edited
Loading

Vercel deployment URL: https://token-metadata-9oz4ujojs-hirosystems.vercel.app 🚀

@codecov Codecov
Copy link

codecov bot commented Dec 16, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/index.ts 0.00% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

zone117x
zone117x reviewed Dec 16, 2024
View reviewed changes
src/api/routes/ft.ts Outdated Show resolved Hide resolved
@rafaelcr rafaelcr merged commit 94c7b4a into develop Dec 16, 2024
8 of 9 checks passed
@rafaelcr rafaelcr deleted the fix/supply branch December 16, 2024 17:13
@diwakergupta
Copy link
Member

Why do we need this change at all? API already returns the decimal value so clients can / should use that to adjust the display as needed

@rafaelcr
Copy link
Collaborator Author

rafaelcr commented Dec 16, 2024 •
edited
Loading

I think there was confusion around this for sBTC in the Explorer @diwakergupta but we could roll this back if you think it would be out of scope for the API and let the Explorer deal with the formatting

EDIT: We could also just move the formatted value to a new field for readability and leave the current one as is

@diwakergupta
Copy link
Member

I think there was confusion around this for sBTC in the Explorer @diwakergupta but we could roll this back if you think it would be out of scope for the API and let the Explorer deal with the formatting

Yeah I think that's best, since this is a potentially breaking change and easily handled client-side. I would maybe expand the OpenAPI docs for this endpoint though to clarify total_supply and how to use decimals with it

@rafaelcr
Copy link
Collaborator Author

OK sounds good

rafaelcr added a commit that referenced this pull request Dec 16, 2024
@rafaelcr
Revert "fix: display token supply with correct decimals (#287)"
51a734d 
This reverts commit 94c7b4a.
@rafaelcr rafaelcr mentioned this pull request Dec 16, 2024
Merged
rafaelcr added a commit that referenced this pull request Dec 16, 2024
@rafaelcr
fix: roll back FT supply display, update docs instead (#288)
080176e 
* Revert "fix: display token supply with correct decimals (#287)"

This reverts commit 94c7b4a.

* docs: update
blockstack-devops pushed a commit that referenced this pull request Dec 17, 2024
@semantic-release-bot
chore(release): 1.1.6 [skip ci]
6340895 
## [1.1.6](v1.1.5...v1.1.6) (2024-12-17)

### Bug Fixes

* display token supply with correct decimals ([#287](#287)) ([94c7b4a](94c7b4a))
* roll back FT supply display, update docs instead ([#288](#288)) ([080176e](080176e)), closes [#287](#287)
* update ft date on supply change ([#289](#289)) ([94b9486](94b9486))
@blockstack-devops
Copy link

🎉 This PR is included in version 1.1.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zone117x zone117x zone117x approved these changes

Assignees
No one assigned
Labels
released
Projects
API Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rafaelcr @diwakergupta @blockstack-devops @zone117x