The JSON RPC `getBlockCommitment` method will almost never produce correct lamport values

[steveluscher]

This issue is really just an escalation of #30741, because this RPC method is pretty much wrong, 100% of the time – at least on mainnet.

 % curl https://api.mainnet-beta.solana.com -X POST -H "Content-Type: application/json" -d '                                                                                                  
  {
    "jsonrpc": "2.0", "id": 1,
    "method": "getBlockCommitment",
    "params":[208151977]
  }
'
{"jsonrpc":"2.0","result":{"commitment":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,382219010785520358],"totalStake":383784289656803160},"id":1}

Those numbers are way beyond the max safe JavaScript integer. As soon as you parse that string as JSON, the values change.

JSON.parse('{"jsonrpc":"2.0","result":{"commitment":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,382219010785520358],"totalStake":383784289656803160},"id":1}');
//          {"jsonrpc":"2.0","result":{"commitment":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,382219010785520400],"totalStake":383784289656803140},"id":1}

[mcintyre94]

As a shorter term workaround could we have the RPC return an additional field containing the string representation of totalStake? We have some other RPC methods that return numbers encoded as strings like that.

[steveluscher]

Oh, that wouldn't even be a short term workaround – that's where we need to end up.

Not a string, though, we should ship ‘value objects’ through the RPC. These are objects with some content and a tag that denotes their 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.

[mcintyre94]

Cool got you, that makes sense. Would be great to have that on the web3js side for sure! Is there value in adding a new field as a value object like that to the getBlockCommitment for now? So backward compatible, not making any wider changes on the Rust side.

[buffalojoec]

I think this is a good approach, and it's not going to cause a lot of friction since it's going into a new library that will require migration paths for other new tooling as well.

One question I have is where exactly do we draw the line on which return types should be value objects and which are suitable for numbers? Sure, the answer is obviously "if it overflows Number" but how do we know which methods may or may not produce responses that could overflow? It's not always as obvious as with getBlockCommitment

Kind of like a "do we just do this to all of them" or not type of question (pertaining to the U64UnsafeBeyond2Pow53Minus1 or lamports types)

[steveluscher]

Sure, the answer is obviously "if it overflows Number" but how do we know which methods may or may not produce responses that could overflow?

Must be BigInt:

• Rust usize (example)
• Rust u64 (example)

Should be Number:

• Rust u8 (example)
• Rust u32 (example)

Basically less than 53 bits: we're good.

[steveluscher]

Is there value in adding a new field as a value object like that to the getBlockCommitment for now?

Just to be super clear, the dApp side of the web3.js API will never see these value objects. They're wrappers for the transport only.

Goes something like this:

1. Rust tries to crap out a u64
2. Rust RPC converts that to the JSON string {"_":0,"v":"123456789"}
3. web3.js gets that value and notices the "_"
4. web3.js deserializes it according to the value object deserializer that corresponds to 0
5. The dApp receives a bigint and is none the wiser.

[buffalojoec]

Circling back on this issue, it seems like this will be very difficult to implement without breaking everyone's downstream APIs.

In the example above, anyone who's built on top of Web3 JS would be none the wiser. However, anyone using the Rust client or the raw JSON payloads would be nuked.

Curious if anyone has thoughts on how we might achieve what we're trying to do without massive fallout?

[steveluscher]

When it comes time to do the next major piece of work on the JSON-RPC, we'll advance the version of the JSON-RPC (2.0) and fix the return values at the same time. Consumers will have to switch to the new version to get the ‘doesn't mutilate your u64s’ behavior.

Worth bringing up in #jsonrpc on Discord https://discord.com/channels/428295358100013066/1129079940365697104