search

graphprotocol / mission-control-indexer

Technical indexer documentation and infrastructure templates for the Mission Control testnet
21 stars 4 forks source link

Graph Node v0.20.0 makes eth_getBlockByNumber calls - it should be only eth_getBlockByHash #269

Open Chainode opened 3 years ago

Chainode commented 3 years ago

The current graph node v0.20.0 makes eth_getBlockByNumber calls which should actually not happen, especially not per default as an Ethereum node with EIP-1898 is required for Mainnet to avoid unpredictable subgraph failures, inconsistent indexing results - implicit slashing.

Even setting GRAPH_ETH_CALL_BY_NUMBER to false didn't changed this. export GRAPH_ETH_CALL_BY_NUMBER=false

Expected is to see only eth_getBlockByHash calls. Otherwise what would be the purpose of using an Ethereum node with EIP-1898.

image

trader-payne commented 3 years ago

Unfortunately that's the expected behavior. The way it works is that you just need to remove the env var completely. Setting it up to ANY value, enables it. It's not a boolean.

Chainode commented 3 years ago

So is expected to see eth_getBlockByNumber calls even though the node has EIP 1898? Shouldn't be only calls eth_getBlockByHash? Other asked, why the mix then?

trader-payne commented 3 years ago

Having that env var present, enables call by number function.

Chainode commented 3 years ago

Not really, it calls it anyway. That's what I also said in my initial message. I started without any variable. Then I noticed that eth_getBlockByNumber is used. Then I stopped, deleted DB, created new one, added the env var and started again. Same behaviour.

leoyvens commented 3 years ago

Hi @Chainode and @trader-payne. @trader-payne is correct you should entirely unset GRAPH_ETH_CALL_BY_NUMBER. And @Chainode you're also right that the node will continue to make eth_getBlockByNumber requests, but our use of the Ethereum API is reorg safe. The implementation pre-dates EIP-1898, which would have made the job easier, and it is always aware that a block returned by eth_getBlockByNumber might be reorged if it is a recent block.

For full disclosure, we have a determinsim bug which will be fixed by this PR https://github.com/graphprotocol/graph-node/pull/2062. Until this is fixed and the affected subgraphs reindexed, we are aware that inconsistencies may happen. The general policy is that inconsistencies due to graph node bugs should not lead to slashing.

Chainode commented 3 years ago

Hi @leoyvens - yes, is not about using GRAPH_ETH_CALL_BY_NUMBER as I didn't planed to use it anyway. I used that only as a test to see if it will make a difference, as specified in the issue.

This got clarified in the meantime. The determinism bug is something independent from call by the number. I think it would be interesting to hear more about how the slashing actually works at protocol level and when it gets triggered and how we proceed in undesired slashing cases. Maybe governance is here an idea.