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

Migrate from JavaScript number to string in JSON RPC API when representing u64 values #30741

Open
steveluscher opened this issue Feb 15, 2023 · 3 comments
Labels
do-not-close Add this tag to exempt an issue/PR from being closed by the stalebot

Comments

@steveluscher
Copy link
Contributor

steveluscher commented Feb 15, 2023

Note: this will require a massive multi-year migration plan; this issue is just a stub now to point some TODOs to.

Problem

Responses from our JSON RPC API represent u64 numbers as JavaScript numbers. Since JavaScript numbers are IEEE 754 floating point, this will result in truncation above 2^53 (ie. 9007199254740992).

Example: If you call getAccountInfo on an account with more than 9,007,199.254740992◎, the lamports field will be truncated. You can convince yourself of this by executing the following statement in a JS repl:

JSON.parse('{"lamports":9007199254740993}');
> 9007199254740992

Proposed Solution

Return u64 as a value object through the JSON RPC API.

A value object is one with some content and a tag that denotes its type.

{
  "jsonrpc":"2.0",
  "result": {
    "commitment":[
      { "_": 0, "v": "0" },
      { "_": 0, "v": "0" },
      { "_": 0, "v": "0" },
      /* ... */
      { "_": 0, "v": "382219010785520358" },
    ],
    "totalStake": { "_": 0, "v": "383784289656803160" },
  },
  "id":1,
}

Where _ is an enum denoting the type of the value (eg. here 0 would be mapped to bigint) then the rest of the object is the data (eg. here v is the bigint value as a string).

Using value objects makes it so that the client doesn't have to keep a giant map of keypaths that are supposed to actually be strings vs. are strings but need to be cast to bigint. All it needs to do is to look for the tag (_) and deserialize the value accordingly.

Deprecation Plan

There are many ways to do a deprecation like this. This GitHub issue is only a stub for now; we will discuss how to move forward in the coming months.

@steveluscher steveluscher added the do-not-close Add this tag to exempt an issue/PR from being closed by the stalebot label Feb 15, 2023
@steveluscher steveluscher changed the title Migrate from JavaScript number to bigint in JSON RPC API Migrate from JavaScript number to string in JSON RPC API when representing u64 values Feb 15, 2023
@steveluscher steveluscher transferred this issue from solana-labs/solana Mar 16, 2023
@steveluscher steveluscher transferred this issue from solana-labs/solana-web3.js Mar 16, 2023
steveluscher added a commit to solana-labs/solana-web3.js that referenced this issue Mar 16, 2023
…ed for the Solana JSON RPC

## Summary

`u64` values, in the Solana JSON RPC, are implemented as JavaScript numbers. This means that any number greater than `Number.MAX_SAFE_INTEGER` (9007199254740991) can't be trusted. This is tracked in solana-labs/solana#30741.

In this PR we create a serializer that converts `bigint` values coming in from `@solana/rpc-core` to JavaScript `number`, as a concession.

## Test Plan

```shell
pnpm test:unit:node
```
steveluscher added a commit to solana-labs/solana-web3.js that referenced this issue Mar 16, 2023
…ed for the Solana JSON RPC

## Summary

`u64` values, in the Solana JSON RPC, are implemented as JavaScript numbers. This means that any number greater than `Number.MAX_SAFE_INTEGER` (9007199254740991) can't be trusted. This is tracked in solana-labs/solana#30741.

In this PR we create a serializer that converts `bigint` values coming in from `@solana/rpc-core` to JavaScript `number`, as a concession.

## Test Plan

```shell
pnpm test:unit:node
```
steveluscher added a commit to solana-labs/solana-web3.js that referenced this issue Mar 16, 2023
…ed for the Solana JSON RPC

## Summary

`u64` values, in the Solana JSON RPC, are implemented as JavaScript numbers. This means that any number greater than `Number.MAX_SAFE_INTEGER` (9007199254740991) can't be trusted. This is tracked in solana-labs/solana#30741.

In this PR we create a serializer that converts `bigint` values coming in from `@solana/rpc-core` to JavaScript `number`, as a concession.

## Test Plan

```shell
pnpm test:unit:node
```
steveluscher added a commit to solana-labs/solana-web3.js that referenced this issue Mar 17, 2023
…ed for the Solana JSON RPC

## Summary

`u64` values, in the Solana JSON RPC, are implemented as JavaScript numbers. This means that any number greater than `Number.MAX_SAFE_INTEGER` (9007199254740991) can't be trusted. This is tracked in solana-labs/solana#30741.

In this PR we create a serializer that converts `bigint` values coming in from `@solana/rpc-core` to JavaScript `number`, as a concession.

## Test Plan

```shell
pnpm test:unit:node
```
steveluscher added a commit to solana-labs/solana-web3.js that referenced this issue Mar 17, 2023
…ed for the Solana JSON RPC (#1204)

## Summary

`u64` values, in the Solana JSON RPC, are implemented as JavaScript numbers. This means that any number greater than `Number.MAX_SAFE_INTEGER` (9007199254740991) can't be trusted. This is tracked in solana-labs/solana#30741.

In this PR we create a serializer that converts `bigint` values coming in from `@solana/rpc-core` to JavaScript `number`, as a concession.

## Test Plan

```shell
pnpm test:unit:node
```
@mschneider
Copy link
Contributor

massive multi-year migration plan

If solana can break jsonrpc compatibility in minor versions once a year, an important issue like this one, shouldn't be held back.

@steveluscher
Copy link
Contributor Author

@buffalojoec
Copy link
Contributor

Just noting this strategy should apply to f64 as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-close Add this tag to exempt an issue/PR from being closed by the stalebot
Projects
None yet
Development

No branches or pull requests

3 participants