solana-labs / solana-web3.js

Solana JavaScript SDK
https://solana-labs.github.io/solana-web3.js
MIT License
2.12k stars 851 forks source link

JSON RPC API using (u)int64 whereas JS can only handle int53 #1116

Open linuskendall opened 2 years ago

linuskendall commented 2 years ago

Problem

The JSON API uses int64 for fields like lamports. However, the maximum safe integer in browsers is 2^53-1

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER

Proposed Solution

Make lamports a string in the HTTP/RPC API and parse with BigInt on the web3.js client side?

vpontis commented 2 years ago

@linuskendall thanks for opening this!

Yea, we've switched to passing around lamports and token amounts as strings internally and then parsing with BN.js. But we still get number from most of the JSON RPC endpoints.

While it's much nicer to get a number, I fear this will lead to some bad bugs...

mvines commented 2 years ago

We've switch over to strings in a couple of the methods already, and marked the Number field as deprecated. Example: https://docs.solana.com/developing/clients/jsonrpc-api#token-balances-structure

It would be helpful to enumerate the remaining problematic methods to make this issue easier to pick up by somebody willing to clean them up too!

vpontis commented 2 years ago

@mvines The biggest offender right now is lamports which is consistently a number in the web3.js API.

emdoyle commented 2 years ago

@mvines This also occurs when using getBalance, and if I understand correctly it's because the value is coerced by superstruct into a number. Took me a long time to realize this was the reason balances were off by ~40 lamports in my tests client-side :/

This also happens with getAccountInfo (lamports as @vpontis mentioned) so I don't think there is a safe way to retrieve native SOL balances in web3.js at the moment unless I'm missing some other method.

steveluscher commented 2 years ago

The use of JS numbers is terrifying, and has been added as a principle in solana-labs/solana-web3.js#1111 as we consider a rewrite of web3.js.

steveluscher commented 1 year ago

I removed the ‘good first issue’ tag. I think this is rather extremely complicated, and will require a months' (years'?) long deprecation strategy for the old fields, and possibly a version bump of the RPC itself.

Some of the groundwork is already being laid in the new web3 rewrite through the use of bigint at the outer edge of the interface. This should let people who program apps using the new RPC to seamlessly upgrade to the new RPC methods (when we cut them over) without changing their app's code or upgrading their other dependencies.