Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Add EIP-1884 SLOAD_GAS change to EIP-2200 #11474

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

GregTheGreek
Copy link

@GregTheGreek GregTheGreek commented Feb 10, 2020

I was doing some bench marking and analysis surrounding some of the recent EIPs. While tinkering with EIP-1884 I accidentally created a split on a private network between Parity, Besu and Geth, where Parity and Geth forked together with SLOAD_GAS of 200 rather than 800. After investigating, I discovered that EIP-2200 was "technically" incorrectly implemented.

EIP-1884 and EIP-2200 both adjust SLOAD_GAS from 200 to 800 - but the current implementation of EIP-2200 assumes that EIP-1884 is already implemented, and only adjusts SSTORE.

This PR adds the EIP-1884 SLOAD_GAS changes to EIP-2200.

There is a Sibling PR in Geth

@parity-cla-bot
Copy link

It looks like @GregTheGreek signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@ordian
Copy link
Collaborator

ordian commented Feb 10, 2020

EIP-1884 and EIP-2200 both adjust SLOAD_GAS from 200 to 800 - but the current implementation of EIP-2200 assumes that EIP-1884 is already implemented, and only adjusts SSTORE.

My understanding that eip2200_advanced_transition is not the same as eip2200_transition, which is eip1283_transition + eip1706_transition + eip1884_transition: #11347

cc @sorpaas

Copy link
Collaborator

@sorpaas sorpaas left a comment

Choose a reason for hiding this comment

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

The SLOAD_GAS in EIP-2200 actually has nothing to do with SLOAD -- it's simply the dirty gas for SSTORE. Geth has the naming of it more precisely. So "technically" the current implementations in Parity and Geth are correct.

But anyway having it or not should not make much difference, given we always activate EIP-2200 with EIP-1884.

@sorpaas sorpaas changed the title Update params.rs Add EIP-1884 SLOAD_GAS change to EIP-2200 Feb 10, 2020
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.

4 participants