EIP-1153: Transient storage opcodes

[codyborn]

Changes:

• Implement EIP-1153: Transient storage opcodes
• Create new Shanghai spec for including this feature post ArrowGlacier
• Adds abstraction to StorageProvider classes to re-use most of the original StorageProvider implementation.
  
  Note that the old StorageProvider.cs code is now split across PartialStorageProviderBase.cs and PersistentStorageProvider.cs (no new code in these two files).

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Other (please describe):

Testing

Requires testing

• Yes
• No

In case you checked yes, did you write tests??

• Yes
• No

Eip1153 Tests 🆕

• 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

VirtualMachine Tests

• loading and storage are not subject to gas refunds
• load arbitrary slot costs 100 gas: TLOAD(x) costs 100 gas
  • more specifically, it should cost same as warm SLOAD
• store arbitrary value in arbitrary slot costs 100 gas: TSTORE(x, y) costs 100 gas
  • more specifically, it should cost same as warm dirty SSTORE

StorageProvider Tests

• TLOAD uninitialized location
• TLOAD successful after TSTORE
• TLOAD after restore
• Commit resets transient state
• Reset resets transient state
• Transient state changes do not impact Persistent state
• Persistent state changes do not impact Transient state

Invalid Opcode Tests

• Added Shanghai config test

Further comments (optional)

I evaluated various designs implementing TransientStorageProvider and optimized for code reuse and limiting impact across code-base. StorageProvider maintains the same interface, thus keeping the caller logic mostly unchanged.

[LukaszRozmej]

WOW massive! We need a thorough review of this. With merge going on as highest priority it might take us a bit of time. But awesome!

[LukaszRozmej]

One major issue - I don't like the break down of StorageProvider into more classes, we probably want the contrary to have State and Storage unified in the future (Verkle tries) and this will make the code harder to refactor. It is also unnecessary as transient storage is way simpler than persistent one and doesn't need all its bells and whistles. If I am wrong and don't see something that is needed and comes from EIP spec, please do tell me, I might have missed something.

Other comments are minor omissions or nitpicks that would be good to address but I am open to discussion if you don't agree with them.

Also we are trying to add some more comments to the code to having documenting comments in most important places would help a bit.

Other than that still great PR!

[willpote]

Not too familiar with the codebase but this is very cool. Thanks for putting this together.

[LukaszRozmej]

I think you accidently updated submodules.

[LukaszRozmej]

Pushed my refactorings to: https://github.com/NethermindEth/nethermind/tree/1153 and commit: d0e8d0d

[codyborn]

Pushed my refactorings to: https://github.com/NethermindEth/nethermind/tree/1153 and commit: d0e8d0d

Thanks! Just reviewed your changes and they look great.

[LukaszRozmej]

Thanks! Just reviewed your changes and they look great.

Can you merge it to your PR?

[codyborn]

Thanks! Just reviewed your changes and they look great.

Can you merge it to your PR?

Just merged! I'll go through and add more comments to the files that I touched.