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.

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

feat: units utils #1277

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

feat: units utils #1277

wants to merge 1 commit into from

Conversation

tabaktoni
Copy link
Collaborator

Motivation and Resolution

Requested by dev from HH Bangkok similar to https://viem.sh/docs/utilities/formatUnits

Convert fri to strk

units(1n, 'fri')

convert strk to fri

units('0.3', 'strk')

Checklist:

  • Performed a self-review of the code
  • Rebased to the last commit of the target branch (or merged it into my branch)
  • Linked the issues which this PR resolves
  • Documented the changes in code (API docs will be generated automatically)
  • Updated the tests
  • All tests are passing

@tabaktoni tabaktoni requested a review from penovicp November 29, 2024 13:59
@tabaktoni
Copy link
Collaborator Author

@penovicp plc check did i overengineer and did I miss some cases

Copy link
Collaborator

@PhilippeR26 PhilippeR26 left a comment

Choose a reason for hiding this comment

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

lgtm.
Just the orthography ofsimbol

Copy link
Collaborator

@penovicp penovicp left a comment

Choose a reason for hiding this comment

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

Aside from the simbol/symbol issue that Phil mentioned, the PR mostly looks good to me.

The only thing I would suggest is to split the new function into two complementary ones, something like formatStrkToFri() and formatFriToStrk(). I feel that approach would be less ambiguous and more discoverable.

I'll approve the PR and let you decide.

@PhilippeR26
Copy link
Collaborator

The current function signature is just a copy of Ethers ones.
It allows an easy transition to Starknet for EVM guys.

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.

3 participants