feat: implement eth_getBlockReceipts #423
Conversation
core/src/execution/mod.rs
Outdated
| @@ -197,6 +197,7 @@ impl<N: NetworkSpec, R: ExecutionRpc<N>> ExecutionClient<N, R> { | |||
|
|
|||
| let tx_hashes = block.transactions.hashes(); | |||
|
|
|||
| // TODO: replace this with rpc.get_block_receipts? | |||
There was a problem hiding this comment.
@ncitron; thoughts on this?
Using getBlockReceipts instead of getTransactionReceipt would significantly reduce the number of HTTP calls made to the RPC.
There was a problem hiding this comment.
Is getBlockReceipts widely supported (at least by most the major clients and rpc providers)? If so, we should be using it instead.
There was a problem hiding this comment.
erigon, geth, nethermind, besu, nimbus, reth,
infura, quicknode, alchemy, chainstack
yep, seems to be widely supported.
There was a problem hiding this comment.
Yeah in that case lets do it. Thanks for finding this optimization!
There was a problem hiding this comment.
Ah. So there is one peculiar issue:
Some RPC providers return different response in eth_getTransactionReceipt vs eth_getBlockReceipts. This is primarily due to ethereum/execution-apis#295 not being finalized. For example, https://ethereum-rpc.publicnode.com includes the logs[].blockTimestamp field in eth_getBlockReceipts but not in eth_getTransactionReceipt. This makes the N::receipt_contains equality check break for such providers.
A way to curb this is by comparing encoded receipt which I've added as a fallback with a note in comment. LMK if you have a better approach.
|
The
|
|
Don't worry about those tests. They are failing in CI because CI runs triggered from forks don't have access to gh secrets (where we keep our alchemy api key) for security reasons. |
| @@ -337,6 +337,7 @@ impl<N: NetworkSpec, R: ExecutionRpc<N>> ExecutionClient<N, R> { | |||
| .collect::<Result<HashSet<_>, _>>()?; | |||
|
|
|||
| // Collect all (proven) tx receipts as a map of tx hash to receipt | |||
| // TODO: use get_block_receipts instead to reduce the number of RPC calls? | |||
There was a problem hiding this comment.
Can significantly optimize verify_logs too. Will create a separate issue for it once this PR is merged.
Partially Closes: #200.
Related: #200 (comment)