EIP-1153: Transient storage opcodes

[codyborn]

trafficstars

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

• [x] load arbitrary value is 0 at beginning of transaction: TLOAD(x) is 0
• [x] loading after storing returns the stored value: TSTORE(x, y), TLOAD(x) returns y
• [x] loading any other slot after storing to a slot returns 0: TSTORE(x, y), TLOAD(x + n) where n > 0 returns 0
• [x] 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
• [x] reentrant calls access the same transient storage: TSTORE(x, y), CALL(self, ...), TLOAD(x) returns y
• [x] reentrant calls can manipulate the same transient storage: TSTORE(x, y), CALL(self, ...), TSTORE(x, z), TLOAD(x) returns z
• [x] successfully returned calls do not revert transient storage writes: TSTORE(x, y), CALL(self, ...), TSTORE(x, z), RETURN, TLOAD(x) returns z
• [x] revert undoes the transient storage write from the failed call: TSTORE(x, y), CALL(self, ...), TSTORE(x, z), REVERT, TLOAD(x) returns y
• [x] 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
• [x] 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
• [x] delegatecall manipulates transient storage in the context of the current address: TSTORE(x, y), DELEGATECALL(a, ...), TSTORE(x, z), RETURN, TLOAD(x) returns z
• [x] delegatecall reads transient storage in the context of the current address: TSTORE(x, y), DELEGATECALL(a, ...), TLOAD(x) returns y
• [x] transient storage cannot be manipulated in a static context: TSTORE(x, y), STATICCALL(a, ...), TSTORE(x, z) reverts
• [x] transient storage cannot be manipulated in a static context when calling self: TSTORE(x, y), STATICCALL(self, ...), TSTORE(x, z) reverts
• [x] transient storage cannot be manipulated in a nested static context: TSTORE(x, y), STATICCALL(self, ...), CALL(self, ...), TSTORE(x, z) reverts
• [x] Transient storage can be accessed in a static context when calling selfTSTORE(x, y), STATICCALL(self, ...), CALL(self, ...), TLOAD(x) returns y
• [x] 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

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

Documentation

• [x] 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.

• [x] I thought about the changelog and included a changelog update if required.

[siladu]

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]

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]

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 https://github.com/hyperledger/besu/pull/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.

[codyborn]

Ready for a final review @siladu @atoulme. I think everything has been addressed.

[codyborn]

Also, I've ran @snreynolds new EIP1153-specific Ethereum tests and they all pass.

[shemnon]

Also, considering (a) the merge is about to happen and (b) this EIP has not gone into "Eligible for Inclusion" (mostly due to no ACD call bandwidth) I'm inclined to slow roll this merging until we see the merge successfully land.

[codyborn]

Also, considering (a) the merge is about to happen and (b) this EIP has not gone into "Eligible for Inclusion" (mostly due to no ACD call bandwidth) I'm inclined to slow roll this merging until we see the merge successfully land.

Thanks for the review @shemnon! Totally fine with the slow roll.. wouldn't want to slow down the merge in any way :)

[codyborn]

Hey @shemnon! Now that the merge has passed (congrats!) and we're looking more seriously at 1153, can we look at getting this PR merged in? I have one open question regarding unit test location. LMK what you think.

[codyborn]

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

[shemnon]

@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.

[codyborn]

@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 👏