Skip to content
This repository was archived by the owner on Sep 8, 2025. It is now read-only.

Conversation

@carver
Copy link
Contributor

@carver carver commented Dec 9, 2019

What was wrong?

Currently, because we don't do this, there is a bug in EIP-2200, net gas
metering. When looking up the "original value", the EVM must get the
storage value at the beginning of the transaction, not the beginning of
the block.

Fixes #1893

How was it fixed?

So we lock in changes at the beginning of every transaction,
to make sure we don't read values from older transactions, when reading
the original value.

To-Do

  • Clean up commit history

Cute Animal Picture

put a cute animal picture link inside the parentheses

Currently, because we don't do this, there is a bug in EIP-2200, net gas
metering. When looking up the "original value", the EVM must get the
storage value at the beginning of the transaction, not the beginning of
the block. So we lock in changes at the beginning of every transaction,
to make sure we don't read values from older transactions, when reading
the original value.
@lithp
Copy link
Contributor

lithp commented Dec 9, 2019

Just confirmed that this allows us to successfully process block 9069000, the Istanbul fork block

Storage value lookups with from_journal=False must pin to the value at
the time of the latest lock_changes() call.
@carver carver requested a review from lithp December 9, 2019 23:51
"""
Locks in changes to storage, typically just as a transaction starts.
This is used, for example, to look up the storage value from the start
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternative: "This is used to ensure that reads which skip the journal read state as of the beginning of the transaction, rather than state as of the beginning of the block"

Copy link
Contributor

@lithp lithp left a comment

Choose a reason for hiding this comment

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

Over DM I mentioned that this is aesthetically a little sub-optimal, we already have a mechanism for doing snapshots that this is kind of reproducing. Let's ship it though! py-evm should support Istanbul and getting this to work using snapshots would require a fair amount of more work.

@carver carver merged commit 3aa3de6 into ethereum:master Dec 10, 2019
@carver carver deleted the net-sstore-original-value-bugfix branch December 10, 2019 00:07
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.

py-evm fails to verify the Istanbul fork

2 participants