Add option to include transaction receipts in eth_getBlockByHash #9075

[shamardy]

closes #9075

[parity-cla-bot]

It looks like this contributor signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

[sorpaas]

Do we need proper EIPs for this eth_ namespace change?

[shamardy]

@sorpaas @5chdn any update here?

[sorpaas]

@shamardy Code looks good to me. The issue I see is whether we would need an EIP for this.

The current design can be confusing -- we have too booleans called in a line, and users can easily mix them up. Just an idea: maybe we can change this to accept either a boolean (which it is the old behavior), or a JSON dict with two optional fields { fullTransactions: bool, fullReceipts: bool }.

[tjayrush]

Personally, coming from the user's perspective, I think the original design works perfectly. Two bools is easy to understand, not confusing.

If users mix the inputs up during the call, they will quickly discover the problem because the returned data won't parse. I'd rather see a simple, straightforward interface.

Also, I think this issue #9279 (comment) is trying to accomplish almost exactly the same thing. They should be combined.

Finally, this should be an EIP not an issue against Parity. In that way, it would get added to other clients (particularly geth). We should be moving towards more consistent RPC interfaces across clients as opposed to less consistent.

[debris]

Do we need proper EIPs for this eth_ namespace change?

This should definitely be an EIP fist. The issue was incorrectly labeled. Please open an EIP here and reopen the issue if accepted. I'm sorry @shamardy