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

[FEATURE] Use smart contract-based binary search to find userop gas limits #211

Open
0xSulpiride opened this issue Jul 1, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@0xSulpiride
Copy link
Member

Is your feature request related to a problem? Please describe.
Skandha started to misestimate gas limits of userops that use EntryPoint v0.7.0.
We need to switch to smart-contract based binary search algorithm for better estimation of gas limits.

Describe the solution you'd like
We should implement the same approach that Rundler does.
They use this contract: https://github.com/alchemyplatform/rundler/blob/307e8c2cbeab80518a56a2f309e994de3b1631f0/crates/types/contracts/src/v0_7/CallGasEstimationProxy.sol

@0xSulpiride 0xSulpiride added the enhancement New feature or request label Jul 1, 2024
@nikhilkumar1612
Copy link

I'm taking a look into this.

@ch4r10t33r
Copy link
Member

Thank you @nikhilkumar1612

Do give us a shout if you have any questions.

@ch4r10t33r ch4r10t33r added the good first issue Good for newcomers label Jul 2, 2024
@nikhilkumar1612
Copy link

nikhilkumar1612 commented Jul 2, 2024

hey i have a few questions:

  1. If a eth_call is made to entrypoint contract's address after overriding its state to CallGasEstimateProxy's bytecode, how will the call even reach simulateHandleOps ? because both contracts are not meant for deployment. I understand that this proxy based approach is done to satisfy the require(address(this) == msg.sender) inside estimateCallGas() but since the deployed instance of an entrypoint will not have both the functionalities how will this work ?
  2. IMPLEMENTATION_ADDRESS_MARKER - constant in CallGasEstimationProxyTypes is this address entrypoint ?
  3. what exactly does this line mean - L32.

@0xSulpiride
Copy link
Member Author

Hi @nikhilkumar1612 thanks for picking up this issue!

  1. CallGasEstimationProxy is a proxy that delegates all calls to the _implementation() contract (if no function from it's body matches the selector). So we need 2 overrides in eth_call. One that overrides EntryPoint's address to CallGasEstimationProxy's code and another to override PROXY_IMPLEMENTATION_ADDRESS_MARKER to EntryPoint's code. This way when we call simulateHandleOps on CallGasEstimationProxy, proxy will delegatecall it to EntryPoint
  2. It's just a random address
  3. Explained in 1.

Let me know if you have more questions!

@nikhilkumar1612
Copy link

thanks @0xSulpiride . I have completed the changes needed for the types,
are these changes relavant to v6 as well ? or is this enhancement only relavant to v7 ?

@0xSulpiride
Copy link
Member Author

@nikhilkumar1612 this is only relevant to v7

@ch4r10t33r
Copy link
Member

thanks @0xSulpiride . I have completed the changes needed for the types, are these changes relavant to v6 as well ? or is this enhancement only relavant to v7 ?

Thank you @nikhilkumar1612 Please create a PR when ready.

@nikhilkumar1612
Copy link

hi guys please find my PR and let me know the changes.
@0xSulpiride @ch4r10t33r

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Development

No branches or pull requests

3 participants