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

Add bigint support to microalgosToAlgos() and algosToMicroalgos() #907

Open
No-Cash-7970 opened this issue Sep 18, 2024 · 0 comments
Open
Labels
new-feature-request Feature request that needs triage

Comments

@No-Cash-7970
Copy link

No-Cash-7970 commented Sep 18, 2024

Problem

The Algod and Indexer clients now use bigint for microAlgo amounts in v3. However, the microalgosToAlgos() and algosToMicroalgos() only support numbers.

Solution

Workaround No. 1

SDK users can workaround this issue for now by converting a bigint microAlgo value to a number before using microalgosToAlgos() or algosToMicroalgos(), but this can result in a loss of precision for large Algo values.

Workaround No. 2

Do not use microalgosToAlgos() or algosToMicroalgos() and do the conversion calculations manually with bigints.

Solution for microalgosToAlgos()

Accept either a bigint or number argument. Return a number (like before). This solution is backwards compatible and would not require SDK users to change their existing code.

Solution No. 1 for algosToMicroalgos()

Can continue to accept only a number argument, but I think it would be good for consistency to also allow a bigint argument like proposed solution for microalgosToAlgos(). Return a bigint, which may be another breaking change.

Solution No. 2 for algosToMicroalgos()

Accept the same argument types as Solution No. 1, but have an optional returnBigInt boolean argument that is false by default. When returnBigInt is true, a bigint is returned, otherwise a number is returned (like before). This would keep backwards compatibility, but is slightly more work to implement and maintain.

Dependencies

The INVALID_MICROALGOS_ERROR_MSG constant will need to be updated to reflect microalgosToAlgos() being able to accept integers larger than 2^53 - 1.

Urgency

Medium.

Web and mobile apps use microalgosToAlgos() and algosToMicroalgos() frequently to display Algo amounts and to receive an Algo amount from the user respectively.

Misc.

I think "Solution for microalgosToAlgos()" and "Solution No. 2 for algosToMicroalgos()" are the best options. I can create a pull request for implementing those solutions.

@No-Cash-7970 No-Cash-7970 added the new-feature-request Feature request that needs triage label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature-request Feature request that needs triage
Projects
None yet
Development

No branches or pull requests

1 participant