feat(protocol)!: improve protocol based on Brecht's internal review

[Brechtpd]

These are some simplifications/optimization/small fixes/questions I have after doing a first pass review of the code, though certainly still parts that require more looking into. Nothing super important though here.

It's certainly not my goal to get all these small modifications in the current code base, but maybe some of them could and some of them could be done in a later version if deemed not important enough to still change the code at this point. I do also think that unnecessary changes should be avoided at this point.

I will also add comments to the different parts in this PR with some more context where needed.

Overall impression: it's hard to review because of the complexity of the different systems. There's a lot of code and the different optimizations that work on top of each other that just make things difficult to verify. The amount of different tricks you need to keep in your head just for all the gas optimizations to store the list of blocks and transitions is already big, and if you add the contestation part on top of it, it makes it hard. While the rollup basically just stores a list of blocks and a mapping of transitions, the current implementation makes this a challenge to easily understand. There's also a lot of features that result in a lot of conditional branches which does not make things easier.

There seems to be quite some tests missing I feel. Which I can understand very well because the amount of features and if/else cases that need to be tested is big. Take for example just the AssignmentHook mechanism and the amount of different things that would need to be tested to truly test all the features: payment with ETH, payment with ERC20 token, limited by time/block id/parent hash/meta hash, paying an ETH tip to coinbase, multiple hooks which also impact some of there features in non-trivial ways,... And this is only for the successful paths, you would still have to test for all the error paths as well. It's a lot of work.

I know a lot of my "keep things simple" comments have been ignored before, so I don't have any illusions or really goals to change things at this point. But I don't come with only complaints today because I actually have a simplified Taiko implementation that I started working on for the booster rollups: Brechtpd#1. It also contains a new design for how to do multi-provers and making sure buggy provers are automatically disabled. The mechanism used there would already greatly decrease the complexity compared to the current contestation system (and fixes issues I have raised in the past with that).

[dantaik]

I got this warning so I have to spend some time to learn the transient storage.

Warning (2394): Transient storage as defined by EIP-1153 can break the composability of smart contracts: Since transient storage is cleared only at the end of the transaction and not at the end of the outermost call frame to the contract within a transaction, your contract may unintentionally misbehave when invoked multiple times in a complex transaction. To avoid this, be sure to clear all transient storage at the end of any call to your contract. The use of transient storage for reentrancy guards that are cleared at the end of the call is safe.

Not sure if my understanding is correct: Previously we can call proposeBlock multiple times in one L1 transaction, but with the new implementation of reentrance guard, this is not possible. Same for many other nonReentrant functions.

I think a valuable feedback is about msg.sender in 1) mesage ownership, and 2) block.coinbase, and 3) transition.prover. We probably need to work on these improvements.

[adaki2004]

I got this warning so I have to spend some time to learn the transient storage.

Great article here.

Quote:
"An expected canonical use case for transient storage is cheaper reentrancy locks, which can be readily implemented with the opcodes as showcased below. However, given the caveats mentioned in the specification of EIP-1153, utmost care has to be taken for more advanced use cases of transient storage to preserve the composability of your smart contract. To raise awareness of this issue, for the time being, the compiler will emit a warning on use of tstore in assembly."

[adaki2004]

Just as a benchmark, gas tested and improvements for propose/prove/verify:

(non-tiered - tiered tests)
Propose: 9 - 10%
Prove: 1.8 - 3.5 %
Verify: 5 - 8%

[dantaik]

Just as a benchmark, gas tested and improvements for propose/prove/verify:
 (non-tiered - tiered tests) Propose: 9 - 10% Prove: 1.8 - 3.5 % Verify: 5 - 8%

not sure I understand these numbers...

[adaki2004]

Just as a benchmark, gas tested and improvements for propose/prove/verify:
> (non-tiered - tiered tests) Propose: 9 - 10% Prove: 1.8 - 3.5 % Verify: 5 - 8%

not sure I understand these numbers...

Old code gas report:

Brecht's code gas report: