Discussion: differences in graphQL responses between Besu and Geth due to number encoding

[renaynay]

The hive test suite GraphQL tests found inconsistencies in the responses between the besu and geth clients in the number encoding, where Geth returns hex for certain fields as opposed to Besu which returns number/float.

I've created a gist documenting the queries for which the responses of Geth and Besu differ due to different number encoding here. (The GraphQL test logs can also be viewed on our hive test run visualizer).

Geth uses hex strings because JSON numbers are often implemented as floats, which cannot reliably represent all possible 64-bit integers. It would be nice to have a discussion about this to see if Besu wants to potentially switch to using hex as well.

cc @fjl

[shemnon]

Any change to GraphQL output would involve changing the spec and the schema - https://eips.ethereum.org/EIPS/eip-1767. Our GraphQL library strongly maps the types in the schema with it's output, and any change to the output requires a change in the schema.

[renaynay]

Okay thank you for your response @shemnon , we are working on fixing some query responses to align better with the spec.

I noticed however that we're failing to serve a couple of queries on Account, but I do not see that queries on Account are defined in the spec. There was some discussion about it over a year ago in this thread: https://ethereum-magicians.org/t/graphql-interface-to-ethereum-node-data/2710/40 , but beyond that, I don't see that the spec has been formally updated to include queries on Account.

Do you have any further insights on this?

[atoulme]

I had a chat with Danno:
-no need to move away from numbers, as most values are well within the valid range.
-hive should update tests to remove queries to Account, replacing them by querying for account in the latest or pending block.
-Besu will deprecate and remove the top level account query, with low priority.

[renaynay]

Thanks @atoulme and @shemnon, will update hive to remove the queries on Account from the graphql tests.

[renaynay]

Okay, closing this issue as geth has been working on getting spec-compliant. Thanks for your help @shemnon @atoulme