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

3.0.0: Remove API response int decoding option, support BigInt everywhere #816

Closed
wants to merge 4 commits into from

Conversation

jasonpaulos
Copy link
Contributor

@jasonpaulos jasonpaulos commented Aug 17, 2023

Summary

In release 3.0.0, we are introducing typed responses for algod API calls. One area where these responses could be improved is with respect to integers, since all response integer types are currently number | bigint. This union type is not very useful, as you can perform virtually no operations against it without casting to one of the component types (which potential loss in accuracy for bigint -> number conversions).

The idea behind this PR is that we will parse everything as a bigint, since that is the most general integer type available to us, and unfortunately the algod API does not distinguish between full uint64s and smaller integer types. There are two advantages to making this change:

  1. The typed responses will use bigint, which is more useful than a union type.
  2. We no longer use the builtin JSON.parse by default for API responses. This parser is unable to produce bigints, so it will provide inaccurate data for large integers, of which there are plenty in our responses. We introduced the IntDecoding option on requests to provide support for larger integers without breaking backwards compatibility in v2, but there is no reason to keep this around any longer, as it involves unnecessary complexity.

Work required

  • Modify generator to make properties bigint (PR TypeScript: Use bigints for internal property types generator#67)
  • Expose algosdk.parseJSON and algosdk.stringifyJSON, which support bigints
  • Remove setIntDecoding functions from client and request classes
  • Allow transactions to accept bigints for all numeric values (required because suggested params now returns fee/round info as bigints)
  • Update tests to expected bigints instead of numbers
    • SDK unit tests
    • Cucumber unit tests
    • Cucumber integration tests

@jasonpaulos
Copy link
Contributor Author

Closing in favor of #852

@jasonpaulos jasonpaulos deleted the bigint-everywhere branch January 10, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant