Skip to content

Fix the calculation for estimated gas price#3117

Merged
jxom merged 5 commits intowevm:mainfrom
celo-org:piersy/fix_gas_price_estimation
Jan 8, 2025
Merged

Fix the calculation for estimated gas price#3117
jxom merged 5 commits intowevm:mainfrom
celo-org:piersy/fix_gas_price_estimation

Conversation

@piersy
Copy link
Contributor

@piersy piersy commented Dec 12, 2024

Updates estimateFeesPerGas to handle both the values
returned by the Celo L1 and also the values that will be returned
by the L2 Celo implementation.

Also removes the use of the viem multiplier for Celo L1 gas
calculations since the Celo L1 node already doubles the base fee.

Changes explained in detail:

Note the default viemMultiplier is 1.2 and the default celoMultiplier is 2.

The current gas maxFeePerGas suggested by viem (before this PR) is this:

  • Celo L1 - ((baseFee * celoMultiplier) * viemMultiplier) + suggestedTip
  • Op L2 stack - ((baseFee+suggestedTip) * viemMultiplier) + suggestedTip

With this PR we get:

  • Celo L1 - (baseFee * celoMultiplier) + suggestedTip
  • Op L2 stack - (baseFee * viemMultiplier) + suggestedTip

@changeset-bot
Copy link

changeset-bot bot commented Dec 12, 2024

🦋 Changeset detected

Latest commit: 7249a73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
viem Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Dec 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
viem ✅ Ready (Inspect) Visit Preview Dec 12, 2024 1:30pm

@vercel
Copy link

vercel bot commented Dec 12, 2024

@piersy is attempting to deploy a commit to the Wevm Team on Vercel.

A member of the Team first needs to authorize it.

piersy and others added 4 commits January 9, 2025 07:48
The gas price returned by the rpc is base_fee + max_priority_fee.
So we shouldn't add the max_priority_fee onto that value since it's
already included. Also the logic used by viem for applying the
multiplier is to only apply the multiplier to the base fee and not the
priority fee, so we match that approach here.
@jxom jxom force-pushed the piersy/fix_gas_price_estimation branch from 5cb2631 to b7163c2 Compare January 8, 2025 20:57
@jxom jxom merged commit 99706e7 into wevm:main Jan 8, 2025
2 of 3 checks passed
@github-actions github-actions bot mentioned this pull request Jan 8, 2025
ezdac added a commit to celo-org/op-geth that referenced this pull request May 27, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
ezdac added a commit to celo-org/op-geth that referenced this pull request May 27, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
ezdac added a commit to celo-org/op-geth that referenced this pull request May 27, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
ezdac added a commit to celo-org/op-geth that referenced this pull request May 27, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
ezdac added a commit to celo-org/op-geth that referenced this pull request Jun 3, 2025
* chore(ci): update foundry version to v1.1.0

* chore(ci): use new op-stack-base image runner

* chore: set default TERM in e2e test for tput

* chore(ci): determine npm version from package.json

* chore(ci): put setup in composite action

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 20, 2025
* chore(ci): update foundry version to v1.1.0

* chore(ci): use new op-stack-base image runner

* chore: set default TERM in e2e test for tput

* chore(ci): determine npm version from package.json

* chore(ci): put setup in composite action

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 21, 2025
* chore(ci): update foundry version to v1.1.0

* chore(ci): use new op-stack-base image runner

* chore: set default TERM in e2e test for tput

* chore(ci): determine npm version from package.json

* chore(ci): put setup in composite action

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 26, 2025
* chore(ci): update foundry version to v1.1.0

* chore(ci): use new op-stack-base image runner

* chore: set default TERM in e2e test for tput

* chore(ci): determine npm version from package.json

* chore(ci): put setup in composite action

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 26, 2025
* chore(ci): update foundry version to v1.1.0

* chore(ci): use new op-stack-base image runner

* chore: set default TERM in e2e test for tput

* chore(ci): determine npm version from package.json

* chore(ci): put setup in composite action

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 28, 2025
* chore(ci): update foundry version to v1.1.0

* chore(ci): use new op-stack-base image runner

* chore: set default TERM in e2e test for tput

* chore(ci): determine npm version from package.json

* chore(ci): put setup in composite action

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 30, 2025
* chore(ci): update foundry version to v1.1.0

* chore(ci): use new op-stack-base image runner

* chore: set default TERM in e2e test for tput

* chore(ci): determine npm version from package.json

* chore(ci): put setup in composite action

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 30, 2025
* chore: set default TERM in e2e test for tput

* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.

* Loosen error check condition in e2e-test

Due to possible expansion of the responses error string in potential
proxy setups on a live RPC endpoint, the strict error check in the e2e
viem test caused the test to fail, although the error condition was
expected.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 30, 2025
* chore: update e2e test js dependencies

* Adapt viem e2e test's asserts to upstream fix

Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 31, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 31, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Jul 31, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Aug 1, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Aug 1, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Aug 1, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
Kourin1996 pushed a commit to celo-org/op-geth that referenced this pull request Aug 3, 2025
Upstream viem recently merged a fix to the `estimateFeesPerGas`
function (wevm/viem#3117),
and the recent updates to the e2e tests npm packages now includes the
fixed version of viem.

With this the asserted values in the viem e2e test are no longer a
reflection of what es expected to be returned from the viem library
and had to be adjusted.
karlb added a commit to celo-org/op-geth that referenced this pull request Sep 10, 2025
This brings a celoSepolia definition and staying up to date is generally
a good idea.
Due to wevm/viem#3117 being merged in view, we
can (and have to) remove a changed assert in the "test gas price
difference for fee currency" test.
karlb added a commit to celo-org/op-geth that referenced this pull request Sep 10, 2025
This brings a celoSepolia definition and staying up to date is generally
a good idea.
Due to wevm/viem#3117 being merged in view, we
can (and have to) remove a changed assert in the "test gas price
difference for fee currency" test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments