Skip to content

feat: more accurate gas-oracle gas metering#1813

Merged
tynes merged 2 commits intodevelopfrom
fix/gpo-gas-adding
Nov 30, 2021
Merged

feat: more accurate gas-oracle gas metering#1813
tynes merged 2 commits intodevelopfrom
fix/gpo-gas-adding

Conversation

@tynes
Copy link
Contributor

@tynes tynes commented Nov 24, 2021

Description
Compute the gas used in an epoch based on the
actual amount of gas used instead of assuming
that the full block's worth of gas was used in
each block.

Convert the average block gas limit to a uint64 in the
codebase instead of it being used as a float64.

Additional context
Minimal changes were done in a way where the existing tests
could ensure there is no regression. The gas-oracle is not
part of the integration-tests so thorough review and observing
its behavior on kovan is necessary before rolling this out to mainnet.
#1736 is a prerequisite
for getting the gas-oracle into the itests

Metadata

  • Fixes ENG-1681

@changeset-bot
Copy link

changeset-bot bot commented Nov 24, 2021

🦋 Changeset detected

Latest commit: ab2378e

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

This PR includes changesets to release 1 package
Name Type
@eth-optimism/gas-oracle 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

@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2021

Codecov Report

Merging #1813 (ab2378e) into develop (84513e7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1813   +/-   ##
========================================
  Coverage    71.99%   71.99%           
========================================
  Files           70       70           
  Lines         2321     2321           
  Branches       346      346           
========================================
  Hits          1671     1671           
  Misses         650      650           
Flag Coverage Δ
batch-submitter 62.07% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 57.50% <ø> (ø)
data-transport-layer 38.64% <ø> (ø)
message-relayer 70.86% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84513e7...ab2378e. Read the comment docs.

@mslipper
Copy link
Collaborator

mslipper commented Nov 29, 2021

LGTM from my POV - may want get Karl to take a look as well for sanity.

@tynes tynes force-pushed the fix/gpo-gas-adding branch from 155918a to f265212 Compare November 30, 2021 17:32
Copy link
Contributor

@karlfloersch karlfloersch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left a tiny bit of feedback but overall all of the logic looks right.

Compute the gas used in an epoch based on the
actual amount of gas used instead of assuming
that the full block's worth of gas was used in
each block.
Convert the average block gas limit to a `uint64` in the
codebase instead of it being used as a `float64`.
@tynes tynes force-pushed the fix/gpo-gas-adding branch from f265212 to ab2378e Compare November 30, 2021 18:46
@tynes tynes merged commit 0fb44b3 into develop Nov 30, 2021
@tynes tynes deleted the fix/gpo-gas-adding branch November 30, 2021 23:07
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.

4 participants