Execution-api JSON-RPC master tracker issue

[kdeme]

Tracker issue for all execution-api JSON RPC issues.

Most of these are visible / discovered via hive tests, but some are additional ones discovered (and more are likely to be discovered as these get worked on).

Some are also just spec / test vector / hive PITAs

For latest hive results on rpc-compat: https://hive.ethpandaops.io/#/test/generic/1769753159-b9b9d40703a67f0737e0eb880873ff13

Recurring issues over several methods:

block tag not parsing hash

Some calls are named "...ByNumber" or "...ByHash"

These will parse either:

• latest/safe/finalized and number
• block hash

Other calls without this specific naming however are often supposed parse latest/safe/finalized + number + hash.
Several failures are due to nimbus not implementing the hash part.

• Execution API: block tag not parsing hash #3981

null vs omit vs error vs [ ]

null vs omitting the fields:

Our current JSON flavor is set to NOT omit null fields (= Opt.none(T)). This is because this behavior is actually expected/specified in engine_api (see status-im/nim-json-rpc#238).
I think ideally Hive would just not see this as an error, but just as an alternative, correct response.
But we could also adjust our JSON-RPC flavor for eth_ and debug_ method namespace. For this we need #3914 so that we can define different flavors per api.

null vs error:

We often return an error when a block (header, etc) is not found. The tests (and spec) however expect a null for not found and error for actual errors.

null vs empty array

In some cases there is also an empty array returned instead of null and vice versa.

Unimplemented EIP-1898

• Implement EIP-1898 for execution JSON-RPC API #4088 : add support for eip-1898 #4084

This will hit several endpoints as it is currently not implemented. No issues on hive however as no test vectors exist.

Execution API JSON-RPC issues per method

debug_getRawBlock

• wrong error code invalid param: "code": -- -32000 ++ -32602

debug_getRawHeader

• wrong error code invalid param: "code": -- -32000 ++ -32602

debug_getRawReceipts

• wrong error code invalid param: "code": -- -32000 ++ -32602

debug_getRawReceipts

• returns empty receipts on test case

debug_getRawTransaction

• wrong error code invalid param: "code": -- -32000 ++ -32602,

eth_blobBaseFee

• wrong blobBaseFee calculation: response differs from expected (-- client, ++ test): fixed in Fix eth_blobBaseFee JSON RPC method #3934

eth_call

• invalid result on test case
• reverts not happening on abi error and panic test cases

eth_createAccessList

• all 3 hive errors are errors because of null fields -> all 3 fixed with null omit

additionalProperties seems to be not there in our implementation? But also not in the test vectors? Just in the spec, wth is this..?

eth_estimateGas

• wrong error code + revert data missing: fixed in Fix JSON error object for eth_estimateGas #3940
• Other issues (not showing in hive):
  • eth_estimateGas estimating 22 million gas when the other els are estimating 164 thousand #3763
  • the rpc method implementation is ok. but the actual implementation of rpcEstimateGas is incorrect  #2773
  • Fix EstimateGas to add gas cost for EIP-7702 authorization #3945
  • Improve eth_estimateGas binary search by doing first some short circuits #3946

eth_feeHistory

• Not implemented: Implement Execution api method eth_feeHistory #3944

eth_getBalance

• Need support for block tag parsing hash (see top)

eth_getBlockByHash / eth_getBlockByNumber

Several issues:

• null fields
• size mismatch
• pre-merged TD value: Make specs PR on this, if not accepted, we just diverge here.
• TransactionObject has several failures too:
  • -- "maxFeePerGas": "0x0",
  • -- "maxPriorityFeePerGas": "0x0",
  • "to": -- "0x0000000000000000000000000000000000000000" ++ null, -> strange, here they do expect null?
  • "yParity": -- null ++ "0x1"
  • "chainId": -- null ++ "0xc72dd9d5e883e",
  • Missing recently added blockTimestamp field: Add blockTimestamp field to tx object in json-rpc methods #4111

eth_getBlockReceipts

• Need support for block tag parsing hash (see top)
• on empty: "result": -- [] ++ null -> is that failure correct though? Why set it to null in that case? Check spec.
• block not found -> needs null value
• "to": -- "0x0000000000000000000000000000000000000000" ++ null, -> strange, here they do expect null?
• invalid log index

eth_getLogs

• invalid log index

• Block not found error code: "code": -- -32000 ++ -32602,
  This is however so inconsistent, for example for getReceipts a null value is expected here. Not sure if this specified at all.

• Need to error on: ++ "message": "invalid argument 0: cannot specify both BlockHash and FromBlock/ToBlock, choose one or the other"

• Need to error on: ++ "message": "invalid block range params" -> on invalid range

eth_getProof

• Need support for block tag parsing hash (see top)

eth_getStorageAt

• Need to error on invalid param instead of returning 0x00...: ++ "message": "storage key too long (want at most 32 bytes):
• Need 32602 error code
• Need support for block tag parsing hash (see top)

eth_getTransactionByBlockHashAndIndex

Some same TransactionObject issues as in eth_getBlockByHash

• null fields
• "to": -- "0x0000000000000000000000000000000000000000" ++ null, -> strange, here they do expect null?
• Missing recently added blockTimestamp field

eth_getTransactionByBlockNumberAndIndex

same issues as for eth_getTransactionByBlockHashAndIndex

eth_getTransactionByHash

same issues as for eth_getTransactionByBlockHashAndIndex

And:

• "yParity": -- null ++ "0x1"
• gas price issue: "gasPrice": -- "0x0" ++ "0x3b9aca01",
• -- "blobVersionedHashes": [],

And:

• Missing recently added blockTimestamp field

eth_getTransactionReceipt

• null fields
• "to": -- "0x0000000000000000000000000000000000000000" ++ null, -> strange, here they do expect null?

eth_sendRawTransaction

• txErrorBasicValidation errors on calls that should pass in tests

eth_simulateV1

• Not implemented: Implement Execution api method eth_simulateV1 #3943

eth_maxPriorityFeePerGas

• Not implemented: implement eth_maxPriorityFeePerGas #3505

Not being tested in hive rpc-compat, so no failures there.

eth_getStorageValues

• Not implemented: Implement Execution api method eth_getStorageValues #4110

testing_buildBlockV1

• Not implemented. This is however a pure testing endpoint. I believe this should be part of a separate (non mandatory) list of hive tests.

[kdeme]

Linking fix PRs inside the main issue, but here are some other related recent fixes:

• Bump nim-web3 and fill in LogObject blockTimestamp field #3930
• Uncomment & rework debug json-rpc endpoints #3929

cc @chirag-parmar As you requested to follow these fixes in case there is anything related also for NVP

[RazorClient]

adding this here from
#4112 (comment)

We do raise ValueError all over the place (mostly for errors that don't actually have a defined code in the execution-apis specifications), so it is understandable for this PR to follow that example.

But I do want to make the remark that this results in the -32000 code error for all those errors. I think we should move to raising specifically ApplicationError and pick a consistent default number for those that are not defined by the specs, e.g. 38000.

Reason: JSON-RPC specs:

The error codes from and including -32768 to -32000 are reserved for pre-defined errors. Any code within this range, but not defined explicitly below is reserved for future use.

The remainder of the space is available for application-defined errors.

as I think it would be wise to have another issue and fix all the error numbers abuse together rather than at one single place