Skip to content

fix: display token supply with correct decimals #287

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
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

@rafaelcr rafaelcr commented Dec 16, 2024

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
Copy link

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

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

@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
@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

diwakergupta commented Dec 16, 2024

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

diwakergupta commented Dec 16, 2024

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

rafaelcr commented Dec 16, 2024

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
Contributor

blockstack-devops commented Dec 17, 2024

🎉 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

1 more reviewer

@zone117x zone117x zone117x approved these changes

Reviewers whose approvals may not affect merge requirements

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.

5 participants

@rafaelcr @diwakergupta @blockstack-devops @zone117x