refactor(params): store all params under one key in evm module#1617

GAtom22 · 2023-01-19T23:02:34Z

Closes: #XXX

Description

Refactor evm module's parameters to be stored on a single store key.
This refactor was done to overcome a performance issue observed with the previous logic.

For contributor use:

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

For admin use:

Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
Reviewers assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

codecov · 2023-01-19T23:12:13Z

Codecov Report

Merging #1617 (46418fe) into main (9305788) will decrease coverage by 0.20%.
The diff coverage is 70.47%.

❗ Current head 46418fe differs from pull request most recent head 35261e2. Consider uploading reports for the commit 35261e2 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1617      +/-   ##
==========================================
- Coverage   68.33%   68.13%   -0.20%     
==========================================
  Files         110      112       +2     
  Lines       10184    10146      -38     
==========================================
- Hits         6959     6913      -46     
- Misses       2821     2828       +7     
- Partials      404      405       +1

Impacted Files	Coverage Δ
x/evm/keeper/migrations.go	`77.77% <0.00%> (-22.23%)`	⬇️
x/evm/types/key.go	`0.00% <ø> (ø)`
x/evm/types/params_legacy.go	`0.00% <0.00%> (ø)`
x/evm/module.go	`52.17% <33.33%> (-1.56%)`	⬇️
x/evm/keeper/params.go	`41.66% <50.00%> (-42.80%)`	⬇️
x/evm/migrations/v4/migrate.go	`52.63% <50.00%> (-25.94%)`	⬇️
app/ante/fees.go	`82.83% <85.71%> (+0.39%)`	⬆️
x/evm/migrations/v5/migrate.go	`92.10% <92.10%> (ø)`
app/ante/eth.go	`80.17% <100.00%> (+0.08%)`	⬆️
app/ante/fee_market.go	`76.92% <100.00%> (+0.92%)`	⬆️
... and 7 more

Vvaradinov

Overall looking good @GAtom22. One additional change I have in mind is moving the extra_eips back into the Params message as we won't be setting it individually anymore. Check this #1472 to see how it was before.

x/evm/migrations/v4/migrate.go

x/evm/keeper/params.go

fedekunze

Great start. See comments below

x/evm/types/params.go

x/evm/keeper/params.go

x/evm/migrations/v4/migrate.go

…er functions

… into GAtom22/evm-params

x/evm/types/params.go

… into GAtom22/evm-params

fedekunze

LGTM, pending benchmark results

CHANGELOG.md

x/evm/types/params_legacy.go

… into GAtom22/evm-params

x/evm/types/params_legacy.go

* fix(evm): handle RC1 params during migration * migration * fix: test case updated for RC1 * v5 migration * tests * tests pt2 * comment * execute make proto-all Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org>

x/evm/migrations/v4/migrate.go

x/evm/migrations/v5/migrate.go

x/evm/keeper/params.go

addressed

MalteHerrmann · 2023-01-23T18:25:30Z

https://github.com/Mergifyio backport release/v0.21.x

* refactor(params): store all params under one key in evm module * refactor(params): add changelog entry * refactor(params): update based on review comments. Remove params getter functions * refactor(params): refactor params store key * refactor(params): remove unnecessary store keys * refactor(params): add paramSetPairs for backwards compatibility * Update CHANGELOG.md * refactor(params): add license to params_legacy file * Apply suggestions from code review * fix(evm): handle RC1 params during migration (#1624) * fix(evm): handle RC1 params during migration * migration * fix: test case updated for RC1 * v5 migration * tests * tests pt2 * comment * execute make proto-all Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org> * Apply suggestions from code review * rm dup vars Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org> (cherry picked from commit f07b14f) # Conflicts: # CHANGELOG.md

mergify · 2023-01-23T18:26:14Z

backport release/v0.21.x

✅ Backports have been created

Details

#1626 refactor(params): store all params under one key in evm module (backport #1617) has been created for branch release/v0.21.x

…ort #1617) (#1626) * refactor(params): store all params under one key in evm module (#1617) * refactor(params): store all params under one key in evm module * refactor(params): add changelog entry * refactor(params): update based on review comments. Remove params getter functions * refactor(params): refactor params store key * refactor(params): remove unnecessary store keys * refactor(params): add paramSetPairs for backwards compatibility * Update CHANGELOG.md * refactor(params): add license to params_legacy file * Apply suggestions from code review * fix(evm): handle RC1 params during migration (#1624) * fix(evm): handle RC1 params during migration * migration * fix: test case updated for RC1 * v5 migration * tests * tests pt2 * comment * execute make proto-all Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org> * Apply suggestions from code review * rm dup vars Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org> (cherry picked from commit f07b14f) # Conflicts: # CHANGELOG.md * update changelog Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com> Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com> Co-authored-by: MalteHerrmann <malte@evmos.org>

* release: v0.21.0-rc1 changelog (#1606) * changelog: added rc-1 changelog entry * fix: typo fix rc1 * fix(rpc): bind default json-rpc listen address to localhost (backport #1613) (#1621) * fix(rpc): bind default json-rpc listen address to localhost (#1613) * update nix * bind default json-rpc to 127.0.0.1 * add change doc * Update CHANGELOG.md Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> (cherry picked from commit 3587015) # Conflicts: # CHANGELOG.md * address merge conflicts Co-authored-by: mmsqe <tqd0800210105@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org> * fix(rpc): align fee history (backport #1611) (#1620) * fix(rpc): align fee history (#1611) * update nix * add next fee in fee history * fix test * add change doc * height + 1 for next fee * cross check baseFeePerGas len * Update tests/integration_tests/test_fee_history.py Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com> * fix oldestBlock & align earliest input as eth * update doc * update nix * isort test_fee_history.py * fix test * align rpc res as eth * add cross check * add baseFeePerGas len check * add oldestBlock check Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com> Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> (cherry picked from commit 47fdfd3) # Conflicts: # CHANGELOG.md * address merge conflicts Co-authored-by: mmsqe <mavis@crypto.com> Co-authored-by: MalteHerrmann <malte@evmos.org> Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * support customize db opener in `StartCmd` (backport #1615) (#1622) * support customize db opener in `StartCmd` (#1615) * support customize db opener in `StartCmd` * Update CHANGELOG.md * pass to dbOpener, so we can control different options through cli flags * add comments (cherry picked from commit f1337aa) # Conflicts: # CHANGELOG.md * conflicts Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * tests(evm): add benchmark tests setup for params (#1623) (#1627) * tests: add benchmark tests setup * fix: localized benchmark tests to keeper * update benchmark Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> (cherry picked from commit 9305788) Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> * refactor(params): store all params under one key in evm module (backport #1617) (#1626) * refactor(params): store all params under one key in evm module (#1617) * refactor(params): store all params under one key in evm module * refactor(params): add changelog entry * refactor(params): update based on review comments. Remove params getter functions * refactor(params): refactor params store key * refactor(params): remove unnecessary store keys * refactor(params): add paramSetPairs for backwards compatibility * Update CHANGELOG.md * refactor(params): add license to params_legacy file * Apply suggestions from code review * fix(evm): handle RC1 params during migration (#1624) * fix(evm): handle RC1 params during migration * migration * fix: test case updated for RC1 * v5 migration * tests * tests pt2 * comment * execute make proto-all Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org> * Apply suggestions from code review * rm dup vars Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: MalteHerrmann <malte@evmos.org> (cherry picked from commit f07b14f) # Conflicts: # CHANGELOG.md * update changelog Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com> Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com> Co-authored-by: MalteHerrmann <malte@evmos.org> * fix(evm): revert v4 migration changes (#1625) (#1628) * fix(evm): revert v4 migration changes * add check for chainID * fix: renamed vars to avoid duplicate proto * test * fix: set default extraEIPs to nil * fix: updated tests Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> (cherry picked from commit 9bfa1ff) Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * minor improvement to changelog (#1629) * adjust year to 2023 for v0.21.0-rc2 release (#1630) * Update CHANGELOG.md Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: mmsqe <tqd0800210105@gmail.com> Co-authored-by: mmsqe <mavis@crypto.com> Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com>

GAtom22 added 2 commits January 19, 2023 19:36

refactor(params): store all params under one key in evm module

fde18d9

refactor(params): add changelog entry

c606869

GAtom22 marked this pull request as ready for review January 19, 2023 23:17

GAtom22 requested a review from a team as a code owner January 19, 2023 23:17

GAtom22 requested review from Vvaradinov, facs95 and fedekunze and removed request for a team January 19, 2023 23:17

Vvaradinov previously requested changes Jan 20, 2023

View reviewed changes

x/evm/migrations/v4/migrate.go Outdated Show resolved Hide resolved

x/evm/keeper/params.go Outdated Show resolved Hide resolved

fedekunze suggested changes Jan 20, 2023

View reviewed changes

x/evm/types/params.go Outdated Show resolved Hide resolved

x/evm/keeper/params.go Outdated Show resolved Hide resolved

x/evm/keeper/params.go Outdated Show resolved Hide resolved

x/evm/migrations/v4/migrate.go Show resolved Hide resolved

GAtom22 and others added 4 commits January 20, 2023 09:22

refactor(params): update based on review comments. Remove params gett…

0a491ef

…er functions

Merge branch 'main' into GAtom22/evm-params

83eecc0

refactor(params): refactor params store key

1f1ce8f

Merge branch 'GAtom22/evm-params' of https://github.com/evmos/ethermint…

af9d799

… into GAtom22/evm-params

GAtom22 requested review from Vvaradinov and fedekunze January 20, 2023 12:48

GAtom22 and others added 2 commits January 20, 2023 10:04

refactor(params): remove unnecessary store keys

fff184c

Merge branch 'main' into GAtom22/evm-params

ed7efb7

fedekunze reviewed Jan 20, 2023

View reviewed changes

x/evm/types/params.go Outdated Show resolved Hide resolved

GAtom22 added 2 commits January 20, 2023 11:35

refactor(params): add paramSetPairs for backwards compatibility

e419274

Merge branch 'GAtom22/evm-params' of https://github.com/evmos/ethermint…

9530e19

… into GAtom22/evm-params

fedekunze approved these changes Jan 20, 2023

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

x/evm/types/params_legacy.go Show resolved Hide resolved

fedekunze and others added 4 commits January 21, 2023 12:19

Update CHANGELOG.md

f2e899f

refactor(params): add license to params_legacy file

7d75104

Merge branch 'GAtom22/evm-params' of https://github.com/evmos/ethermint…

3d276ec

… into GAtom22/evm-params

Merge branch 'main' into GAtom22/evm-params

110cd8e

fedekunze reviewed Jan 22, 2023

View reviewed changes

x/evm/types/params_legacy.go Outdated Show resolved Hide resolved

x/evm/types/params_legacy.go Outdated Show resolved Hide resolved

x/evm/types/params_legacy.go Outdated Show resolved Hide resolved

fedekunze and others added 2 commits January 22, 2023 13:09

Apply suggestions from code review

2c50d32

Merge branch 'main' into GAtom22/evm-params

46418fe

github-actions bot added the C:Proto protobuf files (*.pb.go) label Jan 23, 2023

fedekunze reviewed Jan 23, 2023

View reviewed changes

x/evm/migrations/v4/migrate.go Outdated Show resolved Hide resolved

x/evm/migrations/v5/migrate.go Outdated Show resolved Hide resolved

x/evm/keeper/params.go Outdated Show resolved Hide resolved

fedekunze added 2 commits January 23, 2023 16:29

Apply suggestions from code review

d23fcee

rm dup vars

35261e2

fedekunze approved these changes Jan 23, 2023

View reviewed changes

fedekunze enabled auto-merge (squash) January 23, 2023 15:33

fedekunze merged commit f07b14f into main Jan 23, 2023

fedekunze deleted the GAtom22/evm-params branch January 23, 2023 15:38

mergify bot mentioned this pull request Jan 23, 2023

refactor(params): store all params under one key in evm module (backport #1617) #1626

Merged

This was referenced Jan 24, 2023

chore: merge release/v11.0.x back into main evmos/evmos#1273

Closed

chore: sync main with v0.21.x branch #1636

Merged

Conversation

GAtom22 commented Jan 19, 2023

Description

Uh oh!

codecov bot commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Vvaradinov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fedekunze left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedekunze left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MalteHerrmann commented Jan 23, 2023

Uh oh!

mergify bot commented Jan 23, 2023

✅ Backports have been created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jan 19, 2023 •

edited

Loading