Skip to content

Automatic L1 gas pricing#585

Merged
PlasmaPower merged 77 commits intoredo-devnet-reinit-1from
automatic-l1-gas-pricing
Jun 18, 2022
Merged

Automatic L1 gas pricing#585
PlasmaPower merged 77 commits intoredo-devnet-reinit-1from
automatic-l1-gas-pricing

Conversation

@edfelten
Copy link
Contributor

@edfelten edfelten commented May 9, 2022

Currently pulls in OffchainLabs/go-ethereum#107

@codecov
Copy link

codecov bot commented May 9, 2022

Codecov Report

Merging #585 (d54f53e) into redo-devnet-reinit-1 (0783c83) will decrease coverage by 0.00%.
The diff coverage is 52.32%.

@@                   Coverage Diff                    @@
##           redo-devnet-reinit-1     #585      +/-   ##
========================================================
- Coverage                 51.92%   51.92%   -0.01%     
========================================================
  Files                       223      224       +1     
  Lines                     26073    26432     +359     
  Branches                    486      488       +2     
========================================================
+ Hits                      13538    13724     +186     
- Misses                    11154    11258     +104     
- Partials                   1381     1450      +69     

Copy link
Contributor

@rachel-bousfield rachel-bousfield left a comment

Choose a reason for hiding this comment

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

A few things to fix / consider. I'm making a PR to touch up some of the math

} else {
posterCost, calldataUnits, _ = p.state.L1PricingState().PosterDataCost(p.msg, p.evm.Context.Coinbase)
if calldataUnits > 0 {
if err := p.state.L1PricingState().AddToUnitsSinceUpdate(calldataUnits); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should think about what we want the behavior for eth_calls to be. If we don't want them to include the poster data cost, as this PR changes it to, we need to update the geth code to increase the gas limit for calls by the data cost.

We should also consider having a variable for the poster that we assign to to deduplicate this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored to deduplicate code. eth_call behavior issue is still open for discussion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe in classic we include the poster data cost in eth_call
It keeps values consistent with the return of estimateGas
Also makes the call execution closer to what would be in an actual tx

Copy link
Contributor

@PlasmaPower PlasmaPower left a comment

Choose a reason for hiding this comment

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

LGTM

@PlasmaPower PlasmaPower merged commit 3f4939d into redo-devnet-reinit-1 Jun 18, 2022
@PlasmaPower PlasmaPower deleted the automatic-l1-gas-pricing branch June 18, 2022 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants