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

eth_call gas limit 30 million incompatible with graph-node 50 million requirement #705

Closed
e00dan opened this issue Sep 14, 2022 · 4 comments · Fixed by #1133
Closed

eth_call gas limit 30 million incompatible with graph-node 50 million requirement #705

e00dan opened this issue Sep 14, 2022 · 4 comments · Fixed by #1133
Assignees

Comments

@e00dan
Copy link

e00dan commented Sep 14, 2022

What happened:

Graph Node eth_call calls by default don't work with Axon because Axon default eth_call gas limit is 30 million.

What you expected to happen:

Axon default eth_call gas limit is at least 50 million.

How to reproduce it (as minimally and precisely as possible):

Run any subgraph on graph-node that does eth_calls or send eth_call with gas limit = 50 million. Axon will error.

Additional information

/// Gas limit for eth_call. The value of 50_000_000 is a protocol-wide parameter so this
/// should be changed only for debugging purposes and never on an indexer in the network. This
/// value was chosen because it is the Geth default
/// https://github.com/ethereum/go-ethereum/blob/e4b687cf462870538743b3218906940ae590e7fd/eth/ethconfig/config.go#L91.
/// It is not safe to set something higher because Geth will silently override the gas limit
/// with the default. This means that we do not support indexing against a Geth node with
/// RPCGasCap set below 50 million.

Source: https://github.com/graphprotocol/graph-node/blob/5009effd4b5947d45fb76506cdb34c5b9fcb4b5a/chain/ethereum/src/ethereum_adapter.rs#L77

Changing Axon gas limit to 50 million results in working graph-node. I've tested it with Moloch V2 events.
image

@KaoImin
Copy link
Contributor

KaoImin commented Sep 24, 2022

Yes, we have limited the gas limit to a maximum of 30 million when calling the eth_call interface. And now ethereum block limit of 30 million gas.

@e00dan
Copy link
Author

e00dan commented Mar 30, 2023

@KaoImin

Maybe we could make MAX_BLOCK_GAS_LIMIT an environment variable or passed as a config value?

It seems both Geth and Erigon (80% of Ethereum clients according to https://ethernodes.org/) have 50 million gas default eth_call gas limit.

  1. Geth: https://github.com/ethereum/go-ethereum/blob/master/eth/ethconfig/config.go#L89
  2. Erigon: https://github.com/ledgerwatch/erigon/blob/devel/cmd/rpcdaemon/cli/config.go#L95

I didn't check other clients. I'm not sure if Axon wants to be EVM compatible or EVM equivalent, but if the latter then it would be good to align with major Etheruem clients and graph-node so it's easier for developers to use tools they already use for other chains.

@serejke
Copy link

serejke commented Mar 31, 2023

After a short research of popular nodes (Geth, Nethermind, Erigon) it seems that each node has a custom Gas Limit for the eth_call, which is big enough to support different EVM chains. Currently, Ethereum has a theoretical limit for block 30M but the Geth's eth_call allows for 50M, even though such a transaction will never fit into a single block.

I think Axon needs to decouple 2 configurations:

  • one for the the protocol level block size cap MAX_BLOCK_GAS_LIMIT — may keep it as is 30M
  • make another constant 50M, or better off a configuration variable, for the Ethereum RPC that is being validated at
    if req.gas.unwrap_or_default() > U256::from(MAX_BLOCK_GAS_LIMIT) {

This way the existing tooling calling Axon RPCs will be compatible, and no protocol-level changes are introduced.

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

Hello @ahonn.

This issue was closed.
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

Successfully merging a pull request may close this issue.

4 participants