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

Use msgpack fork that supports bigints. #229

Merged

EvanJRichard merged 2 commits into develop from evan/new_msgpack

Oct 13, 2020

Contributor

EvanJRichard commented Oct 13, 2020

Summary

See #130. This PR proposes to change the js-sdk to use a fork that supports BigInt<->uint64 encode/decode.

See also

Resolves #130


          Check in unit tests passing.

41f5d07

EvanJRichard self-assigned this

jasonpaulos reviewed

View reviewed changes

Member

jasonpaulos left a comment

@EvanJRichard could you add a small test to tests/2.Encoding.js that verifies encoding/decoding a BigInt works as expected?

Pending that, LGTM.


          add encoding unit test for: x = 0; number_max < x < uint64_max; and x…

492a88e

… > uint64_max

jasonpaulos approved these changes

View reviewed changes

EvanJRichard merged commit 1a31f97 into develop

EvanJRichard deleted the evan/new_msgpack branch

October 13, 2020 19:05

jasonpaulos pushed a commit that referenced this pull request


          Use msgpack fork that supports bigints. (#229)

10da502

A bigint is used only when the decoded number cannot fit in to a Number.

Resolves: #130

mxmauro commented Oct 19, 2020

Sorry @EvanJRichard my mistake, now found the correct code but still couldn't find the repo on GitHub

Member

jasonpaulos commented Oct 20, 2020

@mxmauro the repo for algo-msgpack-with-bigint is https://github.com/EvanJRichard/msgpack-javascript.

@EvanJRichard, we should probably update our fork's package.json to point to its repo.

jasonpaulos mentioned this pull request

Merged

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