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

Inaccurate gas estimation #418

Closed
2 tasks done
Karrq opened this issue Jun 5, 2024 · 4 comments
Closed
2 tasks done

Inaccurate gas estimation #418

Karrq opened this issue Jun 5, 2024 · 4 comments
Assignees
Labels
bug 🐛 Something isn't working p1 🟠 Indicates high priority item

Comments

@Karrq
Copy link
Contributor

Karrq commented Jun 5, 2024

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.0.2 (8cb46fa 2024-06-05T13:34:46.413496000Z)

What command(s) is the bug in?

forge script

Operating System

None

Describe the bug

When using forge script to deploy contracts, forge attempts to calculate the exact gas required to execute a given transaction on the target chain (with --broadcast flag).

Currently, this estimation is inaccurate due to differing gas parameters, which sometimes can lead to failures when broadcasting these transactions on chain.

@Karrq Karrq added the T-bug label Jun 5, 2024
@Karrq Karrq pinned this issue Jun 6, 2024
@Karrq Karrq mentioned this issue Jul 4, 2024
2 tasks
@dutterbutter dutterbutter added bug 🐛 Something isn't working and removed T-bug labels Oct 18, 2024
@dutterbutter
Copy link
Collaborator

@Karrq is this still relevant, if so what is the status?

@dutterbutter dutterbutter added the p1 🟠 Indicates high priority item label Oct 22, 2024
@dutterbutter dutterbutter unpinned this issue Oct 22, 2024
@Karrq
Copy link
Contributor Author

Karrq commented Oct 28, 2024

We did improve gas estimation considerably recently, but haven't been able to check the results of the estimation personally, so we need to run some tests before updating the issue

/cc @elfedy

@elfedy elfedy self-assigned this Nov 19, 2024
@elfedy
Copy link
Contributor

elfedy commented Dec 4, 2024

As of today, the estimation (which involves txs running locally using the rpc fork parameters) is in fact innacurate because the fee parameters are not being correctly used in the local vm, however the gas limit of the tx is overriden with the estimation from an rpc call, which currently renders the simulation useless for us. We should at least show the right gas parameters and also do some tests comparing the estimation vs rpc_call, maybe it makes sense to use the estimation directly.

@dutterbutter
Copy link
Collaborator

@elfedy is this closed from #773?

@elfedy elfedy closed this as completed Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working p1 🟠 Indicates high priority item
Projects
None yet
Development

No branches or pull requests

3 participants