Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

fix order of eip activation #288

Merged
merged 1 commit into from
Jul 14, 2021
Merged

fix order of eip activation #288

merged 1 commit into from
Jul 14, 2021

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Jul 14, 2021

Closes: #287

Description

Remove the extra eips since we already activated Berlin hardfork in chain config.

Why wrong order cause issue:

  • eip1884 set SLOAD's constant gas to 800, without touch dynamic gas
  • eip2929 change SLOAD to use dynamic gas(2100 for cold access) and set constant gas to zero

So in the right order, the end result is 2100, but in reverse order, we get 2900.

Verified in external integration test.


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
Copy link

codecov bot commented Jul 14, 2021

Codecov Report

Merging #288 (c3bf215) into main (aab793e) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #288   +/-   ##
=======================================
  Coverage   47.54%   47.54%           
=======================================
  Files          45       45           
  Lines        3136     3136           
=======================================
  Hits         1491     1491           
  Misses       1570     1570           
  Partials       75       75           
Impacted Files Coverage Δ
x/evm/types/params.go 65.95% <100.00%> (ø)

x/evm/types/params.go Outdated Show resolved Hide resolved
x/evm/types/params.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@fedekunze fedekunze enabled auto-merge (squash) July 14, 2021 10:29
@fedekunze
Copy link
Contributor

seems that a test case is failing @yihuang

@yihuang
Copy link
Contributor Author

yihuang commented Jul 14, 2021

seems that a test case is failing @yihuang

seems that set to nil can fix the failing test.

@fedekunze fedekunze merged commit 84febdd into evmos:main Jul 14, 2021
yihuang referenced this pull request in yihuang/ethermint Jul 15, 2021
Closes: #274

evm: fix `ExtraEIP` activation (#288)

Closes: #287

Update x/evm/types/utils.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

Add `Failed` utility function and changelog
fedekunze pushed a commit that referenced this pull request Jul 15, 2021
Closes: #274

evm: fix `ExtraEIP` activation (#288)

Closes: #287

Update x/evm/types/utils.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

Add `Failed` utility function and changelog
@yihuang yihuang deleted the wrong-gas branch October 27, 2022 07:38
mmsqe referenced this pull request in mmsqe/ethermint Aug 2, 2023
* Problem: abci handshake is not shutdown gracefully

Solution:
- use cancellable node constructor

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* fix grpc-only

* better app close

* log error

---------

Signed-off-by: yihuang <[email protected]>
mmsqe referenced this pull request in mmsqe/ethermint Sep 5, 2023
* Problem: abci handshake is not shutdown gracefully

Solution:
- use cancellable node constructor

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* fix grpc-only

* better app close

* log error

---------

Signed-off-by: yihuang <[email protected]>
mmsqe referenced this pull request in mmsqe/ethermint Sep 6, 2023
* Problem: abci handshake is not shutdown gracefully

Solution:
- use cancellable node constructor

* Update CHANGELOG.md



* fix grpc-only

* better app close

* log error

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: yihuang <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gas cost is different from go-ethereum
2 participants