Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch JSON RPC requests are returned out of order #2540

Closed
banteg opened this issue Aug 17, 2021 · 6 comments
Closed

Batch JSON RPC requests are returned out of order #2540

banteg opened this issue Aug 17, 2021 · 6 comments

Comments

@banteg
Copy link
Contributor

banteg commented Aug 17, 2021

System information

Erigon version: erigon version 2021.08.2-alpha

OS & Version: Linux

Commit hash : 155186a

Expected behaviour

When I do a synchronous batch JSON RPC request with ids going 0, 1, 2, 3, 4, …, it would make sense if the responses came back in the same order. It's not enforced by the spec, but it would help users to avoid hard-to-catch bugs.

Actual behaviour

The results come out of order like 36, 32, 34, 29, 39, 28, …, which is unexpected. This wasn't the case before and I believe it's not the case in other clients. I think this subtlety should be at least addressed in the rpcdaemon docs.

Steps to reproduce the behaviour

import requests

batch = [
    {
        'jsonrpc': '2.0',
        'id': id,
        'method': 'eth_call',
        'params': [
            {'to': '0x6B175474E89094C44Da98b954EedeAC495271d0F', 'data': "0x18160ddd"},
            block,
        ],
    }
    for id, block in enumerate(range(8928158, chain.height, 100000))
]

response = requests.post('http://127.0.0.1:8545', json=batch).json()
returned_order = [res['id'] for res in response]
print(returned_order)
print(returned_order == sorted(returned_order))
@floatcoder
Copy link

floatcoder commented Aug 17, 2021

We've also been experiencing this along with POKT Network over the past few days. web3.js#4250 assumes that a batch response is in order when parsing.

This is also assumed in the Experimental JsonRpcBatchProvider in ethers.js

@tjayrush
Copy link
Contributor

Sorting should be optional.

By forcing a sort on the server side, you're eliminating the benefit of parallelism. If the calling code also wants to process the returned values in parallel (because each block is independent for example), the benefit disappears if it has to wait while the server sorts. Not to mention sorting means the server must accumulate all the data which puts unnecessary memory pressure on the server for large batches.

I'm pretty sure this is why the spec doesn't require sorting.

@AskAlexSharov
Copy link
Collaborator

Fix: #2541

@mdben1247
Copy link

Out-of-order breaks rust-web3 implementation as well.

The specs does not specify the order, but it does specify "The Server should respond ... after all of the batch Request objects have been processed". So regardless of the order, there is no gain to be had here from parallelism, as everything is returned in a single message anyway. In practice, if results are independent, it makes sense to just send separate requests or batch dependent results together in separate batches.

@AskAlexSharov
Copy link
Collaborator

@mdben1247 , @floatcoder - there is corner case - notification requests have no response (nil responses do not return to client:
https://github.com/ethereum/go-ethereum/blob/master/rpc/handler.go#L119
https://github.com/ethereum/go-ethereum/blob/master/rpc/testdata/reqresp-batch.js#L3

It means if some notification request will be in the middle of batch then geth will return shifted results - means geth can return non-ordered response general :-) have fun

@AskAlexSharov
Copy link
Collaborator

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants