-
Notifications
You must be signed in to change notification settings - Fork 40
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
React-native bigint64 deserialization problem - refactoring/bigint64le #16
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @Admdebian thanks for contributing. Definitely would be great to fix this code so it works on react-native. Do you understand why the old code fails on react-native? I'm a bit hesitant to accept a fix without understanding what the problem is.
I actually find the bug quite mysterious, because your implementation of readBigUInt64LE looks like it should do exactly the same multiplications as the existing code.
src/readBig.ts
Outdated
|
||
const bufferLenght = Math.min(buffer.length, 8) | ||
|
||
let tot: number = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code loses precision -- this is a floating point number, which has fewer bits of precision than a 64 bit integer (since some of its 64 bits are allocated to the exponent). I think if you make this a BigInt right off the bat, it should be fine.
src/readBig.ts
Outdated
if (first === undefined || last === undefined) boundsError(offset, buffer.length - 8) | ||
buffer = buffer.slice(offset) | ||
|
||
const bufferLenght = Math.min(buffer.length, 8) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in the name. also, i believe this code expects buffer to have 8 bytes storing the number, so you can get rid of the min here and also the Number.MAX_VALUE / maxIndex check below.
I need your help to understand and find the best solution.
Can the shift operator do something wrong on diferent environments? The method Buffer.writeBigInt64LE returns only the 32 bit part of the buffer as the current code does. |
Update: Current pyth code:
if val === 14, I see in logs that val << 32 === 14 Then: val << 31 === 0 If I force BigInt I get same results: BigInt(BigInt(val) << BigInt(32)) I think that the operator works only with 32 bit in react-native. |
From the DOC:
|
Ok, interesting. So when I run:
I get |
LOG BigInt(BigInt(val) << BigInt(32)): "14" Without shim.js and additional modules, Buffer and BigInt are not supported in react-native. We are surprised like you. If this problem has happened to us, it can happen to other projects. |
Oh, I think i know what the problem is. Can you try |
Output:
BigInt.shiftLeft is not defined without a new package. I installed:
Then I imported:
Problem:
|
I think you want to add the module https://www.npmjs.com/package/big-integer , not BigInt . |
@Admdebian sorry for the late reply -- went on vacation and totally forgot about this for a few days. Hopefully the last comment makes sense to you and resolves the issue. lmk if not. |
Same problem. You can try with the last push. It doesn't convert BigInt to bigint. I think if we change bigint to BigInt we introduce a breaking change. In this project they moved from bigint to BigInt: https://github.com/prisma/prisma/pull/5842/files |
Ok, I consulted with some of our javascript gurus, and they said that (1) there is a native (the javascript big integer ecosystem is confusing. appreciate you bearing with me here.) |
I think react-native can't deserialize prices correctly on all devices.
The price is deserialized to its BigInt32 Little Endian value instead of BigInt64 Little endian value.
I wrote a new code to test this problem.
Example on testnet:
Test deserialization (only for unsigned): https://asecuritysite.com/encryption/numbers01
Info from my package.json:
My node version is: v16.13.0
I have a shim configuration to enable bigint support in react-native environment: