graphql: standardize test outputs

[s1na]

There are multiple GraphQL hive tests where due to historical reasons 2 responses are accepted. This is unfortunate. The aim of this issue to discuss and agree on one output. The following tests have 2 acceptable responses:

• 17_eth_getBlock_byNumberInvalid.json tries to get a block by a number that doesn't exist in the chain (88888888)
• 07_eth_gasPrice.json accepts both 0x10 and 0x1 as valid responses
• 30_eth_getTransaction_byHash.json this returns a transaction object. it accepts 2 responses. As far as I can tell these are the differences:
  • index being 0 vs 0x0. We should use 0x0 here now that all integer values are hex-encoded
  • log.index being 0 vs 0x0. Same as above.
  • status being null vs 0x0. I'm not sure why status should be null here. IMO we should return 0x0.
• 33_eth_getTransactionReceipt.json as far as I can tell only difference is index being 0 vs 0x0. Should use 0x0.

The following testcase also accepts 2 responses. Either an error, or simply a nil value. It gets the balance of an account against a block which shouldnt exist. However since we've added new block the query has now actually a correct response. It needs to be updated with another block number.

• 12_eth_getBalance_toobig_bn.json

We need to agree what to do for 12 and 17. It boils down to this: when a resource does not exist, return null or error. IMO to stay consistent with JSON-RPC we should return null. Also this is not really an exception.

Not sure what to do for 07_eth_gasPrice.

[s1na]

As for status, as @shemnon pointed out it goes back to pre-byzantium blocks where status was not a part of the block. Therefore clients should return null for pre-byzantium and an integer value if the block is post-byzantium.

[s1na]

Here is a proposal for a single response for the test cases mentioned above:

• 17_eth_getBlock_byNumberInvalid.json: To stay consistent with JSON-RPC return null in case the requested block doesn't exist. Status code 200.
• 07_eth_gasPrice.json: gas price is an estimate and clients can produce a different estimate. I believe we should remove this test case.
• 30_eth_getTransaction_byHash.json:
  • index: hex encoded integer
  • log.index: hex-encoded integer
  • status: null before byzantium
• 33_eth_getTransactionReceipt.json:
  • index: hex encoded integer
• 12_eth_getBalance_toobig_bn.json: geth returns an error "header not found" when using eth_getBalance on a block that doesn't exist. Not sure about other clients. Propose to return an error. Status code 400. Also update the block number in the test case to a non-existent block.

Note for geth:

• 43_graphql_blocks_byWrongRange.json: Return an error as indicated by the test.

[jsvisa]

• 17_eth_getBlock_byNumberInvalid.json: To stay consistent with JSON-RPC return null in case the requested block doesn't exist. Status code 200.

updated, as below:

>> {block(number: 88888888) {number}}
<< {"data":{"block":null}}

• 07_eth_gasPrice.json: gas price is an estimate and clients can produce a different estimate. I believe we should remove this test case.

removed

• 30_eth_getTransaction_byHash.json:

  • index: hex encoded integer
  • log.index: hex-encoded integer
  • status: null before byzantium

index, log.index now is hex encoded, seems this chain.rlp is very old, the tx rereceipt is before byzantium, so the status is null, maybe we can update the chain.rlp to a new one, and update the status to 0x1

• 33_eth_getTransactionReceipt.json:

  • index: hex encoded integer

updated

• 12_eth_getBalance_toobig_bn.json: geth returns an error "header not found" when using eth_getBalance on a block that doesn't exist. Not sure about other clients. Propose to return an error. Status code 400. Also update the block number in the test case to a non-existent block.

besu and go-etherum return null when the block doesn't exist, so we update the test case to null, as below:

>> {block(number: 133) {account(address: "0x6295ee1b4f6dd65047762f924ecd367c17eabf8f") {balance}}}
<< {"data":{"block":null}}

Note for geth:

• 43_graphql_blocks_byWrongRange.json: Return an error as indicated by the test.

as besu and go-ethereum all return empty arry, so I adjust it into:

>> {blocks(from: "0x1e", to: "0x1c") {number gasUsed gasLimit hash nonce stateRoot receiptsRoot transactionCount}}
<< {"data":{"blocks":[]}}

[shemnon]

I don't think the 07_eth_gasPrice.json test should be removed. Instead I think we need to write the response in such a way that it accepts any value. We need to verify it does return a value rather than an error. Perhaps regexp?

[shemnon]

43_graphql_blocks_byWrongRange.json - a bad range should return an error, not an empty array. we may need to figure out a way to encode that in the response without dictating a specific error. But it should not return anything interpretable as a valid response.

[shemnon]

30_eth_getTransaction_byHash.json The genesis is indeed frontier until block 33. As long as querying ancient blocks is permissible I think this test should remain to prove status=null for pre byzantium and a new test that tests after block 33 be used.