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-1153: Transient storage opcodes #4118

Merged
merged 67 commits into from Feb 3, 2023
Merged

Conversation

codyborn
Copy link
Contributor

@codyborn codyborn commented Jul 16, 2022

PR description

  • Implement EIP-1153: Transient storage opcodes
  • Create new EIP1153 spec for including this feature post Paris.
  • Added a new ByteCodeBuilder class to make it easier to construct the E2E test cases.

Eip1153 Tests 🆕

TStoreEVMOperationTest.java

  • load arbitrary value is 0 at beginning of transaction: TLOAD(x) is 0
  • loading after storing returns the stored value: TSTORE(x, y), TLOAD(x) returns y
  • loading any other slot after storing to a slot returns 0: TSTORE(x, y), TLOAD(x + n) where n > 0 returns 0
  • contracts have separate transient storage—loading a slot in a separate contract after storing returns 0: TSTORE(x, y), CALL(z, ...), TLOAD(x) returns 0
  • reentrant calls access the same transient storage: TSTORE(x, y), CALL(self, ...), TLOAD(x) returns y
  • reentrant calls can manipulate the same transient storage: TSTORE(x, y), CALL(self, ...), TSTORE(x, z), TLOAD(x) returns z
  • successfully returned calls do not revert transient storage writes: TSTORE(x, y), CALL(self, ...), TSTORE(x, z), RETURN, TLOAD(x) returns z
  • revert undoes the transient storage write from the failed call: TSTORE(x, y), CALL(self, ...), TSTORE(x, z), REVERT, TLOAD(x) returns y
  • revert undoes all the transient storage writes to the same key from the failed call, i.e. applies the modifications in reverse order: TSTORE(x, y), CALL(self, ...), TSTORE(x, z), TSTORE(x, z + 1) REVERT, TLOAD(x) returns y
  • revert undoes transient storage writes from inner calls that successfully returned: TSTORE(x, y), CALL(self, ...), CALL(self, ...), TSTORE(x, y + 1), RETURN, REVERT, TLOAD(x) returns y
  • delegatecall manipulates transient storage in the context of the current address: TSTORE(x, y), DELEGATECALL(a, ...), TSTORE(x, z), RETURN, TLOAD(x) returns z
  • delegatecall reads transient storage in the context of the current address: TSTORE(x, y), DELEGATECALL(a, ...), TLOAD(x) returns y
  • transient storage cannot be manipulated in a static context: TSTORE(x, y), STATICCALL(a, ...), TSTORE(x, z) reverts
  • transient storage cannot be manipulated in a static context when calling self: TSTORE(x, y), STATICCALL(self, ...), TSTORE(x, z) reverts
  • transient storage cannot be manipulated in a nested static context: TSTORE(x, y), STATICCALL(self, ...), CALL(self, ...), TSTORE(x, z) reverts
  • Transient storage can be accessed in a static context when calling selfTSTORE(x, y), STATICCALL(self, ...), CALL(self, ...), TLOAD(x) returns y
  • transient storage does not persist beyond a single transaction: TSTORE(x, y), RETURN, (new transaction to same contract), TLOAD(x, y) returns 0

TStoreOperationTest.java

  • Insufficient gas
  • Simple tstore test
  • TLoad uninitialized
  • TStore/TLoad
  • TStore->TStore->TLoad
  • No gas refund

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

Unclear if I should add a new record to the changelog. Please advise.

@siladu siladu added the mainnet label Jul 17, 2022
@siladu
Copy link
Contributor

siladu commented Jul 17, 2022

Thanks for the contribution @codyborn!

Please can you point me to where EIP-1153 is confirmed for Shanghai?
I can't see it in this list: https://github.com/ethereum/execution-specs/blob/master/network-upgrades/mainnet-upgrades/shanghai.md#eips-considered-for-inclusion

Also, you'll need to sign off your commits for the DCO check to pass, see https://wiki.hyperledger.org/display/BESU/DCO

@codyborn
Copy link
Contributor Author

Thanks @siladu! It's not yet confirmed for any hardfork; I just put it in the Merge++ hardfork as a placeholder. Do you have a recommended practice for features like this that aren't yet scheduled?

For the DCO, do I need to recreate the PR entirely with new commits or is there a way to roll all these up into a new commit?

@siladu
Copy link
Contributor

siladu commented Jul 18, 2022

Hi @codyborn,

Do you have a recommended practice for features like this that aren't yet scheduled?

You could put it behind an experimental feature toggle. The toggle code has been removed but something similar was done for EIP-1559, see #641

Since it's not yet planned to go into Shanghai, it would be best to remove the Shanghai hard fork code and just use the toggle. Then it can be added to the appropriate fork when the time comes.

For the DCO, do I need to recreate the PR entirely with new commits or is there a way to roll all these up into a new commit?

Shouldn't need to recreate the PR. You can either amend your existing commits with the sign-off and force push; or squash, sign-off and force push.

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

Good code contrib. A few nits and things to clean up. 👍

codyborn and others added 22 commits July 31, 2022 16:27
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
* Remove backward sync exception recursive nesting

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
hyperledger#4098)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
…at should not be the case (hyperledger#4102)

* ignore 2 tests that assume that the system language is English, if that should not be the case

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
…tion post-merge sync. (hyperledger#4104)

Signed-off-by: Justin Florentine <justin+github@florentine.us>

Co-authored-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
* Add experimental config option to enable v5 discovery

Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net>

Co-authored-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net>
Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
Move Taccat Isid from Active to Emeritus Maintainer.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
…ledger#4105)

* Remove hash to append from the queue only if the step succeeds

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
* Create backward sync retries on demand

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
codyborn and others added 11 commits December 2, 2022 19:45
Signed-off-by:  Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
@codyborn
Copy link
Contributor Author

codyborn commented Dec 5, 2022

Hi @shemnon, I've merged the latest upstream changes into this PR. Let me know if there are any outstanding things to address. Thanks!

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
@shemnon
Copy link
Contributor

shemnon commented Jan 26, 2023

@codyborn I updated the codebase to deal with some large changes underneath relating to optimizations. I also updated the storage to be attached to the MessageFrame instead of the account. It greatly simplified the implication for the EVM library as well, Went from 44 files down to 12.

Let me know what you think.

@shemnon shemnon dismissed their stale review January 26, 2023 13:31

tstore is now CFI for cancun, moved into Cancun spec.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
@codyborn
Copy link
Contributor Author

@codyborn I updated the codebase to deal with some large changes underneath relating to optimizations. I also updated the storage to be attached to the MessageFrame instead of the account. It greatly simplified the implication for the EVM library as well, Went from 44 files down to 12.

Let me know what you think.

Looks great! Just gave it a pass and agree it's much simpler with the MessageFrame approach. Nice work 👏

@shemnon shemnon enabled auto-merge (squash) February 3, 2023 17:06
@shemnon shemnon merged commit 53e11e3 into hyperledger:main Feb 3, 2023
shemnon added a commit to shemnon/besu that referenced this pull request Feb 8, 2023
* Implement EIP-1153: Transient storage opcodes
* Added a new ByteCodeBuilder class to make it easier to construct the E2E test cases.
* currently targeting cancun

Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
Co-authored-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net>
Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Co-authored-by: matkt <karim.t2am@gmail.com>
Co-authored-by: Diego López León <dieguitoll@gmail.com>
Co-authored-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Miguel Rojo <miguelangel.rojofernandez@mastercard.com>
Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
Co-authored-by: mark-terry <36909937+mark-terry@users.noreply.github.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Co-authored-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
ensi321 pushed a commit to ensi321/besu that referenced this pull request Feb 19, 2023
* Implement EIP-1153: Transient storage opcodes
* Added a new ByteCodeBuilder class to make it easier to construct the E2E test cases.
* currently targeting cancun

Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
Co-authored-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net>
Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Co-authored-by: matkt <karim.t2am@gmail.com>
Co-authored-by: Diego López León <dieguitoll@gmail.com>
Co-authored-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Miguel Rojo <miguelangel.rojofernandez@mastercard.com>
Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
Co-authored-by: mark-terry <36909937+mark-terry@users.noreply.github.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Co-authored-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
* Implement EIP-1153: Transient storage opcodes
* Added a new ByteCodeBuilder class to make it easier to construct the E2E test cases.
* currently targeting cancun

Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
Co-authored-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net>
Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Co-authored-by: matkt <karim.t2am@gmail.com>
Co-authored-by: Diego López León <dieguitoll@gmail.com>
Co-authored-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Miguel Rojo <miguelangel.rojofernandez@mastercard.com>
Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
Co-authored-by: mark-terry <36909937+mark-terry@users.noreply.github.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Co-authored-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* Implement EIP-1153: Transient storage opcodes
* Added a new ByteCodeBuilder class to make it easier to construct the E2E test cases.
* currently targeting cancun

Signed-off-by: Cody Born <codyborn@outlook.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Jiri Peinlich <jiri.peinlich@gmail.com>
Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
Co-authored-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net>
Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Co-authored-by: matkt <karim.t2am@gmail.com>
Co-authored-by: Diego López León <dieguitoll@gmail.com>
Co-authored-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Miguel Rojo <miguelangel.rojofernandez@mastercard.com>
Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
Co-authored-by: mark-terry <36909937+mark-terry@users.noreply.github.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Co-authored-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet