chore(protocol): fix genesis tests and add minor changes

[dantaik]

      - name: protocol - Deploy L1 Contracts
        working-directory: ./packages/protocol
        run: |
          anvil &
          while ! nc -z localhost 8545; do
            sleep 1
          done
          pnpm deploy:foundry

still fails.

Reverting nonReentrant implementation to the previous non-transient approach can make the test pass. Therefore, I guess the new nonReentrant implementation does have some side effect we need to figure out.

@Brechtpd may I suggest to keep the origiinal nonReentrant implementation in your PR then create a seperate PR for it?

Update

I removed some uncessary nonReentrant guards from some function, now the above script succeeds. It does reflect that the behavior of the new nonReentrant is different and we need to care. Please review carefully if these nonReentrant usage can be removed.

[Brechtpd]

Reverting nonReentrant implementation to the previous non-transient approach can make the test pass. Therefore, I guess the new nonReentrant implementation does have some side effect we need to figure out.

Oh that would be bad! If it is indeed different we have to check because it really should behave 100% identical (except for gas costs of course).

I removed some uncessary nonReentrant guards from some function, now the above script succeeds. It does reflect that the behavior of the new nonReentrant is different and we need to care. Please review carefully if these nonReentrant usage can be removed.

I think it's safer to keep all functions nonReentrant just in case. Even if a specific function does not need it itself, there can still be strange behavior when a nonReentrant function that can call an external contract can call into a function of itself that does state changes and doesn't have the guard. So ideally any function that does state changes should have the non-reentrant guard to make sure self calls never can have unexpected consequences.

[adaki2004]

Reverting nonReentrant implementation to the previous non-transient approach can make the test pass. Therefore, I guess the new nonReentrant implementation does have some side effect we need to figure out.

Oh that would be bad! If it is indeed different we have to check because it really should behave 100% identical (except for gas costs of course).

Maybe bc. foundry does not suportts it yet ? It seems anvil does not.

#15741 (comment)

[Brechtpd]

I also implemented transient storage for the bridge call context and that seems to work fine in the tests at least.

[dantaik]

I'll revert the removal of nonEntrancy guards, but maybe Brecht you can revert the new nonEntrancy

I also implemented transient storage for the bridge call context and that seems to work fine in the tests at least.

The genesis tests depends on Anvil, I commented out a line to see if it works.

[dantaik]

New all tests pass (removed a line from genesis test)

[Brechtpd]

Looks like we may need to explicitly enable cancun in anvil like we currently have to do in forge to be able to use the latest opcodes.

But I have never run the deployment script so not sure how to best test this out.

[dantaik]

Looks like we may need to explicitly enable cancun in anvil like we currently have to do in forge to be able to use the latest opcodes.

But I have never run the deployment script so not sure how to best test this out.

change the work flow to use anvil --hardfork cancun