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

Gas estimation message for EIP1559 txs is extremely misleading #5709

Closed
2 tasks done
simplyoptimistic opened this issue Aug 24, 2023 · 9 comments · Fixed by #7106
Closed
2 tasks done

Gas estimation message for EIP1559 txs is extremely misleading #5709

simplyoptimistic opened this issue Aug 24, 2023 · 9 comments · Fixed by #7106
Assignees
Labels
T-bug Type: bug

Comments

@simplyoptimistic
Copy link

Component

Forge

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

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (1143e57 2023-08-24T00:28:32.561836000Z)

What command(s) is the bug in?

forge script

Operating System

None

Describe the bug

--priority-gas-price was implemented in #5281

My version of foundry is up to date.

Expected behavior: when I use --priority-gas-price, it lowers my gas fee on deployment and doesn't use foundry's default 3 gwei tip.

To reproduce, create a fresh repository and deploy. I ran the following command:

forge script script/Counter.s.sol:CounterScript --priority-gas-price 100 --broadcast --slow --rpc-url optimism -vvvv

This is the output:

Estimated gas price: 3.0000001 gwei

Estimated total gas used for script: 138734

Estimated amount required: 0.0004162020138734 ETH

Following the suggestion in #5281, I use --legacy --with-gas-price:

forge script script/Counter.s.sol:CounterScript --legacy --with-gas-price 100 --broadcast --slow --rpc-url optimism -vvvv

This is the output:

Chain 420

Estimated gas price: 0.0000001 gwei

Estimated total gas used for script: 138734

Estimated amount required: 0.0000000000138734 ETH

I have also tried by using the environment variable alias, it also doesn't work.

@simplyoptimistic simplyoptimistic added the T-bug Type: bug label Aug 24, 2023
@gakonst gakonst added this to Foundry Aug 24, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Aug 24, 2023
@Evalir
Copy link
Member

Evalir commented Aug 30, 2023

cc @Sabnock01

@Sabnock01
Copy link
Contributor

cc @Sabnock01

Taking a look

@simplyoptimistic
Copy link
Author

Are there any updates on this? It is inconvenient to deploy on L2s without being able to specify the priority fee especially if there is network congestion and the base fee is fluctuating.

@Evalir
Copy link
Member

Evalir commented Nov 6, 2023

@simplyoptimistic ack—we thought this had been fixed with new refactors. Looking

@Evalir Evalir self-assigned this Dec 18, 2023
@Evalir
Copy link
Member

Evalir commented Dec 22, 2023

So, this flag is working as intended, but the estimation output is wrong.

This happens because the priority gas price is applied just before sending the transaction ()https://github.com/foundry-rs/foundry/blob/master/crates/forge/bin/cmd/script/broadcast.rs#L106), instead of being applied when the script sequence is being created (https://github.com/foundry-rs/foundry/blob/master/crates/forge/bin/cmd/script/broadcast.rs#L541-L544). This means that when calculating the EIP1559 price, the default estimator is used which has the hardcoded tip, but the transaction is sent with the correct priority gas price and the most up-to-date base fee possible. This does mean though that this leads to massive overpaying on L2s as the default estimator uses the hardcoded tip which is excessive.

The correct fix for this is to make a custom estimator which can take a non-hardcoded tip, but this will require changes upstream on alloy / ethers.

@Evalir Evalir changed the title --priority-gas-price not working in forge script Gas estimation message for EIP1559 txs is extremely misleading Dec 22, 2023
@ultrasecreth
Copy link

The problem is deeper, even when you specify the tip by hand you overpay, and this affect both L1 & L2
The issue is that the tip is set to match the gas price, so you end up paying whatever value you specify on with-gas-price

Exhibit A:

Exhibit B:

Been overspending on deployments/scripts for a while now, I'm pretty sure I raised a separate issue a while ago for this.

@simplyoptimistic
Copy link
Author

Looks like this has been fixed, at least with simulations. Thanks for the hard work! Will report if I have any issues with this.

@advock
Copy link

advock commented Apr 13, 2024

@simplyoptimistic whats the solution ??

@Sabnock01
Copy link
Contributor

@simplyoptimistic whats the solution ??

It simply shows accurate gas estimates now.
image-189.png

@jenpaff jenpaff moved this from Todo to Completed in Foundry Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants