-
Couldn't load subscription status.
- Fork 978
EIP-1153: Transient storage opcodes #4118
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
Conversation
|
Thanks for the contribution @codyborn! Please can you point me to where EIP-1153 is confirmed for Shanghai? Also, you'll need to sign off your commits for the DCO check to pass, see https://wiki.hyperledger.org/display/BESU/DCO |
|
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? |
|
Hi @codyborn,
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.
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. |
There was a problem hiding this 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. 👍
evm/src/main/java/org/hyperledger/besu/evm/gascalculator/ShanghaiGasCalculator.java
Outdated
Show resolved
Hide resolved
evm/src/main/java/org/hyperledger/besu/evm/operation/TLoadOperation.java
Outdated
Show resolved
Hide resolved
evm/src/main/java/org/hyperledger/besu/evm/operation/TStoreOperation.java
Outdated
Show resolved
Hide resolved
evm/src/main/java/org/hyperledger/besu/evm/operation/TStoreOperation.java
Outdated
Show resolved
Hide resolved
evm/src/main/java/org/hyperledger/besu/evm/operation/TLoadOperation.java
Show resolved
Hide resolved
evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleAccount.java
Outdated
Show resolved
Hide resolved
ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/ByteCodeBuilder.java
Outdated
Show resolved
Hide resolved
ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/ByteCodeBuilder.java
Outdated
Show resolved
Hide resolved
ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/ByteCodeBuilder.java
Outdated
Show resolved
Hide resolved
...reum/core/src/test/java/org/hyperledger/besu/ethereum/vm/operations/TStoreOperationTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
* Remove backward sync exception recursive nesting Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Jiri Peinlich <[email protected]> Signed-off-by: Cody Born <[email protected]>
hyperledger#4098) Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Cody Born <[email protected]>
…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 <[email protected]> Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Cody Born <[email protected]>
…tion post-merge sync. (hyperledger#4104) Signed-off-by: Justin Florentine <[email protected]> Co-authored-by: garyschulte <[email protected]> Signed-off-by: Cody Born <[email protected]>
* Add experimental config option to enable v5 discovery Signed-off-by: Gabriel Trintinalia <[email protected]> Co-authored-by: Gabriel Trintinalia <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Cody Born <[email protected]>
Move Taccat Isid from Active to Emeritus Maintainer. Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Cody Born <[email protected]>
…ledger#4105) * Remove hash to append from the queue only if the step succeeds Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Cody Born <[email protected]>
* Create backward sync retries on demand Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Merge1153
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
Signed-off-by: Cody Born <[email protected]>
|
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 <[email protected]>
Signed-off-by: Danno Ferrin <[email protected]>
Signed-off-by: Danno Ferrin <[email protected]>
|
@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. |
tstore is now CFI for cancun, moved into Cancun spec.
Signed-off-by: Danno Ferrin <[email protected]>
Signed-off-by: Danno Ferrin <[email protected]>
Looks great! Just gave it a pass and agree it's much simpler with the MessageFrame approach. Nice work 👏 |
* 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 <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Jiri Peinlich <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: Diego López León <[email protected]> Signed-off-by: Antony Denyer <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Fabio Di Fabio <[email protected]> Co-authored-by: Jiri Peinlich <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Justin Florentine <[email protected]> Co-authored-by: garyschulte <[email protected]> Co-authored-by: Gabriel Trintinalia <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: matkt <[email protected]> Co-authored-by: Diego López León <[email protected]> Co-authored-by: Antony Denyer <[email protected]> Co-authored-by: Miguel Rojo <[email protected]> Co-authored-by: Miguel Angel Rojo <[email protected]> Co-authored-by: mark-terry <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Danno Ferrin <[email protected]>
* 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 <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Jiri Peinlich <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: Diego López León <[email protected]> Signed-off-by: Antony Denyer <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Fabio Di Fabio <[email protected]> Co-authored-by: Jiri Peinlich <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Justin Florentine <[email protected]> Co-authored-by: garyschulte <[email protected]> Co-authored-by: Gabriel Trintinalia <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: matkt <[email protected]> Co-authored-by: Diego López León <[email protected]> Co-authored-by: Antony Denyer <[email protected]> Co-authored-by: Miguel Rojo <[email protected]> Co-authored-by: Miguel Angel Rojo <[email protected]> Co-authored-by: mark-terry <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Danno Ferrin <[email protected]>
* 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 <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Jiri Peinlich <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: Diego López León <[email protected]> Signed-off-by: Antony Denyer <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Fabio Di Fabio <[email protected]> Co-authored-by: Jiri Peinlich <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Justin Florentine <[email protected]> Co-authored-by: garyschulte <[email protected]> Co-authored-by: Gabriel Trintinalia <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: matkt <[email protected]> Co-authored-by: Diego López León <[email protected]> Co-authored-by: Antony Denyer <[email protected]> Co-authored-by: Miguel Rojo <[email protected]> Co-authored-by: Miguel Angel Rojo <[email protected]> Co-authored-by: mark-terry <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Danno Ferrin <[email protected]>
* 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 <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Jiri Peinlich <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: Diego López León <[email protected]> Signed-off-by: Antony Denyer <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Fabio Di Fabio <[email protected]> Co-authored-by: Jiri Peinlich <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Justin Florentine <[email protected]> Co-authored-by: garyschulte <[email protected]> Co-authored-by: Gabriel Trintinalia <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: matkt <[email protected]> Co-authored-by: Diego López León <[email protected]> Co-authored-by: Antony Denyer <[email protected]> Co-authored-by: Miguel Rojo <[email protected]> Co-authored-by: Miguel Angel Rojo <[email protected]> Co-authored-by: mark-terry <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Danno Ferrin <[email protected]>
PR description
ByteCodeBuilderclass to make it easier to construct the E2E test cases.Eip1153 Tests 🆕
TStoreEVMOperationTest.java
TLOAD(x)is0TSTORE(x, y),TLOAD(x)returnsyTSTORE(x, y),TLOAD(x + n)wheren > 0returns0TSTORE(x, y),CALL(z, ...),TLOAD(x)returns0TSTORE(x, y),CALL(self, ...),TLOAD(x)returnsyTSTORE(x, y),CALL(self, ...),TSTORE(x, z),TLOAD(x)returnszTSTORE(x, y),CALL(self, ...),TSTORE(x, z),RETURN,TLOAD(x)returnszTSTORE(x, y),CALL(self, ...),TSTORE(x, z),REVERT,TLOAD(x)returnsyTSTORE(x, y),CALL(self, ...),TSTORE(x, z),TSTORE(x, z + 1)REVERT,TLOAD(x)returnsyTSTORE(x, y),CALL(self, ...),CALL(self, ...),TSTORE(x, y + 1),RETURN,REVERT,TLOAD(x)returnsyTSTORE(x, y),DELEGATECALL(a, ...),TSTORE(x, z),RETURN,TLOAD(x)returnszTSTORE(x, y),DELEGATECALL(a, ...),TLOAD(x)returnsyTSTORE(x, y),STATICCALL(a, ...),TSTORE(x, z)revertsTSTORE(x, y),STATICCALL(self, ...),TSTORE(x, z)revertsTSTORE(x, y),STATICCALL(self, ...),CALL(self, ...),TSTORE(x, z)revertsTSTORE(x, y),STATICCALL(self, ...),CALL(self, ...),TLOAD(x)returnsyTStoreOperationTest.java
Documentation
doc-change-requiredlabel to this PR ifupdates are required.
Changelog
Unclear if I should add a new record to the changelog. Please advise.