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

[pool] Insufficient preVerificationGas throws off latency tracking metric #744

Closed
dancoombs opened this issue Jun 26, 2024 · 2 comments
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@dancoombs
Copy link
Collaborator

Describe the bug
On L2s that contain dynamic preVerificationGas, one of the filters for being eligible to mine is having a sufficient PVG value for the current network gas prices. If below, the UO will not get mined. This should not count against its time to mine latency in the mempool.

To reproduce
Steps to reproduce the behavior:

  • Submit a UO that has an underpriced PVG
  • Watch time to mine metrics

Expected behavior
A UO should only have its time to mine increased when it is eligible to be mined.

Additional context
One way to solve this would be to re-calculate PVG on every block. This is already done by the builder. The solution here is likely to move this calculation down into the pool in a batched manner, limiting the amount of eth_call requests.

Its possible that we can start calculating the dynamic PVG without making an eth_call per UO. The calculation is only based on network constants that need to be queried once per block. Until we move the PVG calculation into Rundler its unlikely we'll be able to efficiently serve this. This move is healthy for Rundler in general as it limits RPC calls.

For example on optimism we can do this calculation directly: https://github.com/ethereum-optimism/optimism/blob/7f403ea5f316a9085061bc63b6618afcdbca1278/packages/contracts-bedrock/src/L2/GasPriceOracle.sol#L226 by only querying a few constants that only change once per block.

@dancoombs dancoombs added the bug Something isn't working label Jun 26, 2024
@dancoombs
Copy link
Collaborator Author

@dancoombs dancoombs changed the title [pool] Insufficient preVerificationGas throws of latency tracking metric [pool] Insufficient preVerificationGas throws off latency tracking metric Jun 27, 2024
@dancoombs dancoombs added this to the v0.5 milestone Jun 27, 2024
@dancoombs dancoombs modified the milestones: v0.5, v0.4 Sep 10, 2024
@dancoombs dancoombs self-assigned this Sep 10, 2024
@dancoombs
Copy link
Collaborator Author

This not only throws off the latency tracking metric, but has a worse bug where it can stall the bundler.

If the mempool becomes full of UOs that have underpriced PVG, but have high enough priority fees to be in the top 128 (our default max bundle size) best operations, they can be crowding out UOs that have sufficiently priced PVG.

The fix here should fix both of these issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant