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

Change eth_getLogs reply to be consistent with Ethereum when querying for non-existent block hashes #2350

Merged
merged 3 commits into from Jun 14, 2023

Conversation

tgmichel
Copy link
Contributor

@tgmichel tgmichel commented Jun 14, 2023

What does it do?

On Moonbeam clients, eth_getLogs returns empty results when asking for a random, non-existent, blockhash. On Ethereum, it returns an error with the message unknown block

What important points should reviewers know?

To replicate run the following curl commands:

MOONBASE ALPHA:
Request:

curl --location 'https://rpc.api.moonbase.moonbeam.network' \
--header 'Content-Type: application/json' \
--data '{
    "jsonrpc":"2.0",
    "id":1,
    "method":"eth_getLogs",
    "params": [{"blockHash":"0x5f273a37594f87b34d1e329d644f977fe56afaa6078d144e98d6106baf076875"}]
  }'

Response:

{
    "jsonrpc": "2.0",
    "result": [],
    "id": 1
}

INFURA SEPOLIA

Request:

curl --location 'https://sepolia.infura.io/v3/bf0136b4b47b423f89d0547938f9d65d' \
--header 'Content-Type: application/json' \
--data '{
    "jsonrpc":"2.0",
    "id":1,
    "method":"eth_getLogs",
    "params": [{"blockHash":"0x5f273a37594f87b34d1e329d644f977fe56afaa6078d144e98d6106baf076875"}]
  }'

Response:

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -32000,
        "message": "unknown block"
    }
}

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

⚠️ Breaking Changes ⚠️

❗ Changes the jsonrpc response when querying for not existing block hashes from

{
    "jsonrpc": "2.0",
    "id": 1,
    "result": []
}

to

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -32000,
        "message": "unknown block"
    }
}

@tgmichel tgmichel added B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit breaking Needs to be mentioned in breaking changes labels Jun 14, 2023
@tgmichel tgmichel requested a review from crystalin June 14, 2023 13:53
@github-actions
Copy link
Contributor

Coverage generated "Wed Jun 14 14:41:51 UTC 2023":
https://s3.amazonaws.com/moonbeam-coverage/pulls/2350/html/index.html

Master coverage: 71.22%
Pull coverage: 71.22%

@crystalin crystalin marked this pull request as ready for review June 14, 2023 15:32
@crystalin crystalin merged commit 79a1c1e into master Jun 14, 2023
26 checks passed
@crystalin crystalin deleted the tgm-pin-frontier-49dc9b4 branch June 14, 2023 15:32
@noandrea noandrea changed the title eth_getLogs return expected error/message for unknown hash Change eth_getLogs reply to be consistent with Etherum when querying for non-existent block hashes Aug 30, 2023
@noandrea noandrea changed the title Change eth_getLogs reply to be consistent with Etherum when querying for non-existent block hashes Change eth_getLogs reply to be consistent with Ethereum when querying for non-existent block hashes Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes breaking Needs to be mentioned in breaking changes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants