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

fix(l2): prover_server eth_estimateGas reverts #1573

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fborello-lambda
Copy link
Contributor

@fborello-lambda fborello-lambda commented Dec 27, 2024

Motivation

There was a bug with block 288 while using the prover_server in dev mode. While debugging the problem, i faced with several possible problems regarding gas estimations.

At the end, the problem was caused by how the calldata was structured. I am not fully aware of the root cause, since it happened just with blocks higher than 288 == 0x120.

To replicate the issue, we can send an eth_estimateGas request:

 curl -X POST \ 
  http://localhost:8545 \
  -H "Content-Type: application/json" \
  -d '{
    "jsonrpc": "2.0",
    "method": "eth_estimateGas",
    "params": [{
      "to": <OnChainProposer Address>,
      "input": "0xcae120a6000000000000000000000000000000000000000000000000000000000000012300000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000",
      "from": <Verifier Address>,
      "value": "0x0"
    }],
    "id": 1
  }'

If we change the 123 with a number <=120 we will get a custom error message triggered by the require statements in the OnChainProposer.sol, otherwise we will get a message that says: "reverted: unknown reason".

Description

  • Build the calldata in order
  • Change min with max in the calc_gas_limit() function
  • Do the as_u64() after checking the number to avoid panics
  • The field gas in an eth_estimateGas request is optional, if we set it to 0 in the request, the highest_gas_limit would have been 0, now with the change, if the gas is 0, the highest_gas_limit is equal to the block.gas_limit.

@fborello-lambda fborello-lambda self-assigned this Dec 27, 2024
@fborello-lambda fborello-lambda requested a review from a team as a code owner December 27, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant