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 1884 v3 #19743

Merged
merged 5 commits into from
Aug 8, 2019
Merged

Eip 1884 v3 #19743

merged 5 commits into from
Aug 8, 2019

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Jun 20, 2019

This PR is on top of #19735 (and is actually quite small). It adds support for EIP 1884, and makes use of the simplified chain management from #19735. It makes it possible to run statetests with e.g. Constantinople+1884, and thus apply 1884 without having Istanbul defined.

This PR creates core/vm/eips.go where any minor new eips can be added. Minor == eips that only

  • Add or remove opcodes, or
  • redefine opcode costs

For more invasive EIPs, that would modify the block validation or execution engine outside of the evm (e.g. progpow), this refactoring won't help much.

@holiman
Copy link
Contributor Author

holiman commented Jul 7, 2019

This PR has now been updated according to ethereum/EIPs#2180

  • Move SELFBALANCE opcode ,
  • Also include EXTCODEHASH into gas increase

@holiman holiman force-pushed the eip_1884_v3 branch 2 times, most recently from 10e85c0 to df2ec6d Compare August 5, 2019 21:22
@holiman
Copy link
Contributor Author

holiman commented Aug 5, 2019

Rebased, squashed.
Due to some convoluted logic, the ExtraEips section was added to the vm.Config, since that's something that gets passed down into the function that eventually creates the vm.

This does add (a small) overhead, but that would be easily fixed if we reuse vm across all txs in a block.

@karalabe karalabe mentioned this pull request Aug 6, 2019
10 tasks
@holiman
Copy link
Contributor Author

holiman commented Aug 6, 2019

My rebase happened to drop a commit where I moved SELFBALANCE to be according to EIP updates, and also repriced EXTCODEHASH. Now added back again.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

Generally LGTM, minor nitpicks

core/vm/eips.go Outdated Show resolved Hide resolved
core/vm/eips.go Outdated Show resolved Hide resolved
core/vm/eips.go Outdated Show resolved Hide resolved
core/vm/eips.go Show resolved Hide resolved
core/vm/eips.go Outdated Show resolved Hide resolved
core/vm/jump_table.go Show resolved Hide resolved
core/vm/opcodes.go Show resolved Hide resolved
@holiman
Copy link
Contributor Author

holiman commented Aug 7, 2019

Fixed, PTAL

core/vm/interpreter.go Outdated Show resolved Hide resolved
Co-Authored-By: Péter Szilágyi <[email protected]>
Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants