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

Allow larger gas limit in non-transactional execution #799

Conversation

nanocryk
Copy link
Contributor

@nanocryk nanocryk commented Aug 1, 2022

Infura allows to use eth_call/eth_estimateGas with up to 10x the block gas limit:
https://docs.infura.io/infura/networks/ethereum/json-rpc-methods/eth_call

To replicate this feature :

  • Runtime gas limit check is made only if call is transactional.
  • Client RPC have a configurable factor, and will ensure the requested gas limit is less than the block gas limit times the factor.

@nanocryk nanocryk requested a review from sorpaas as a code owner August 1, 2022 09:18
Copy link
Contributor

@tgmichel tgmichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm aside from the suggestion. Also, can you please add tests for it?

client/rpc/src/eth/execute.rs Outdated Show resolved Hide resolved
client/rpc/src/eth/execute.rs Outdated Show resolved Hide resolved
@nanocryk
Copy link
Contributor Author

nanocryk commented Aug 2, 2022

Changing the limit for estimateGas changes the estimation. Do we want to avoid that? Also, while I can see the point of having for exemple 10x for eth_call (perform calculations that is meant to be done off-chain), I don't see the point of estimating more than the block gas limit since you're mainly estimating to make a transaction with it.

What do you think @tgmichel @albertov19 @sorpaas ? Should it be only increased for eth_call ?

@albertov19
Copy link

Changing the limit for estimateGas changes the estimation. Do we want to avoid that? Also, while I can see the point of having for exemple 10x for eth_call (perform calculations that is meant to be done off-chain), I don't see the point of estimating more than the block gas limit since you're mainly estimating to make a transaction with it.

What do you think @tgmichel @albertov19 @sorpaas ? Should it be only increased for eth_call ?

I've tried both Alchemy and Infura API (I think both run GETH Clients) and they accept 50M gas units for gas field in the JSON RPC payload. I understand it is a bit pointless but 🤷

@albertov19
Copy link

Moreover, eth_estimateGas did not change the return value when specifying different values as gas (500M, 200M, 50M, 100000). Alchemy has a similar thin in their docs, where they say the limit gas in eth_call and eth_estimateGas to 550M.

@sorpaas sorpaas merged commit ddcf843 into polkadot-evm:master Aug 10, 2022
abhijeetbhagat pushed a commit to web3labs/frontier that referenced this pull request Jan 11, 2023
* move non-transactional executation gas limit check in client

* Fix condition

Co-authored-by: tgmichel <[email protected]>

* typo

* tests

* prettier

* do contract call

* update estimated gas in tests

Co-authored-by: tgmichel <[email protected]>
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 this pull request may close these issues.

4 participants