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

Fix UFix64 <-> UInt256 conversion #56

Closed
sisyphusSmiling opened this issue Apr 30, 2024 · 2 comments · Fixed by #57, #62 or #68
Closed

Fix UFix64 <-> UInt256 conversion #56

sisyphusSmiling opened this issue Apr 30, 2024 · 2 comments · Fixed by #57, #62 or #68

Comments

@sisyphusSmiling
Copy link
Contributor

Problem

Current behavior simply truncates decimal which is poor user experience, particularly for dust values. In the edge where the total supply of a token is 1 (prior art here in memecoins), users wouldn't be able to bridge any value and worse, if attempted the request would succeed in escrowing the value only to return none on the receiving end.

Acceptance Criteria

Conversions are appropriate for the given amount. E.g. 0.001234 converts to 12340000000000 for decimals == 18. Any implementation should also cover the case where an ERC20's decimal value is less precise than Cadence's UFix64 decimal value of 8, rounding down to prevent overage in either direction.

@sisyphusSmiling
Copy link
Contributor Author

Reopening as new bugs were uncovered

@sisyphusSmiling
Copy link
Contributor Author

Reopening after uncovering another issue. @bluesign shared a fix - would you be able to submit a PR or would you mind if I do on your behalf?

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