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

Eip-1559 - target block gas used as param #1873

Merged
merged 8 commits into from
Oct 1, 2024

Conversation

jewei1997
Copy link
Contributor

@jewei1997 jewei1997 commented Sep 27, 2024

Describe your changes and provide context

Introduce target block gas used as a param instead of just using block gas limit/2. This give us more flexibility to set target gas used independently from the block gas limit. Also, emits metrics for base fee.

Testing performed to validate your change

unit tests, tested on loadtest cluster with prometheus metrics

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 72.22222% with 10 lines in your changes missing coverage. Please review.

Project coverage is 61.30%. Comparing base (91f9abc) to head (a53c9d4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
x/evm/types/params.go 35.71% 6 Missing and 3 partials ⚠️
x/evm/keeper/fee.go 92.85% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1873      +/-   ##
==========================================
+ Coverage   61.24%   61.30%   +0.06%     
==========================================
  Files         263      263              
  Lines       23246    23272      +26     
==========================================
+ Hits        14237    14267      +30     
+ Misses       8006     8000       -6     
- Partials     1003     1005       +2     
Flag Coverage Δ
61.30% <72.22%> (+0.06%) ⬆️

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

Files with missing lines Coverage Δ
evmrpc/filter.go 76.04% <ø> (ø)
x/evm/keeper/evm.go 68.18% <ø> (-0.21%) ⬇️
x/evm/keeper/params.go 76.92% <100.00%> (+1.24%) ⬆️
x/evm/migrations/migrate_eip_1559_params.go 100.00% <100.00%> (ø)
x/evm/module.go 40.40% <100.00%> (+1.22%) ⬆️
x/evm/keeper/fee.go 85.71% <92.85%> (+8.97%) ⬆️
x/evm/types/params.go 69.49% <35.71%> (-4.55%) ⬇️

... and 1 file with indirect coverage changes

@@ -285,7 +286,10 @@
// EndBlock executes all ABCI EndBlock logic respective to the evm module. It
// returns no validator updates.
func (am AppModule) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.ValidatorUpdate {
am.keeper.AdjustDynamicBaseFeePerGas(ctx, uint64(req.BlockGasUsed))
newBaseFee := am.keeper.AdjustDynamicBaseFeePerGas(ctx, uint64(req.BlockGasUsed))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
@jewei1997 jewei1997 force-pushed the target-block-gas-used-as-param branch from 7f0bdb4 to 65b0852 Compare September 28, 2024 21:00
app/upgrades.go Outdated
@@ -110,6 +110,7 @@ var upgradesList = []string{
"v5.7.4",
"v5.7.5",
"v5.8.0",
"v5.9.1-jeremy-eip-1559",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: delete this, was just testing upgrade

@jewei1997 jewei1997 merged commit a2c4683 into main Oct 1, 2024
49 checks passed
@jewei1997 jewei1997 deleted the target-block-gas-used-as-param branch October 1, 2024 13:53
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.

2 participants