Interop: Ban Deposits that Trigger Executing Messages for Devnet

[tynes]

Problem Statement

It is possible to include a deposit that has an invalid executing message. Deposits are force include and invalid executing messages are reorg'd out, so this means an attacker would be able to force reorgs on the chain. There are a few solutions to this problem, but we need to research them and there are higher priority things in the short term.

Design

We want to ban deposit transactions completely from triggering executing messages to prevent this attack in the short term. We can reenable them in the future (after mainnet release via another hardfork). The design that we have come to consensus on is as follows:

• add an isDeposit()(bool) function to L1Block
• add a new function L1Block.setL1BlockValuesInterop that sets isDeposit = true then calls setL1BlockValuesEcotone
• add a new function depositsComplete() which sets isDeposit = false
• add a new deposit tx to derivation pipeline that calls depositsComplete()
• the CrossL2Inbox does require(L1Block.isDeposit() == false)

It was determined in #10870 that there was a missing edge case with deposit handling in the context of interop without the "only EOA" invariant. The semantics are being defined in ethereum-optimism/design-docs#13 but it was decided that we should not implement this as part of the devnet release and instead just ban deposits that trigger executing messages completely.

The only way to do this reliably is by hacking into the state transition function. Since we don't have an explicit goal of being multiclient and we want to run the devnet to see how stable our software is, we can modify the STF temporarily to ban deposits and then remove this hack when we have a proper solution implemented.

An easy way to hack this solution is to hook into someplace around applyTransaction and revert deposit transactions that trigger an executing message event.

While this code is temporary and not going to live forever, it will help us in the short term be able to reliably test the end to end flow of the system on a devnet.

[protolambda]

Based on conversations last week around the deposit-tx milestone: we can make the "is deposit" boolean visible to the inbox contract, to disable force-inclusion of interop messages, which could otherwise pass through invalid messages.

Proposal to not modify the execution engine, and instead use system-deposits to add the behavior, and use statically pre-declared messages as mechanism to verify interop messages:

Functionality to add now for first devnet:

• modify CrossL2Inbox with an authenticated function that disables it.
• flip CrossL2Inbox "off" with a system deposit
• process deposits, they won't be able to use the inbox. Instead just revert.
• flip CrossL2Inbox back "on" with a system deposit

---

Below this is still in a design phase and is not being prioritized for the first mainnet release

Functionality to add next (later devnet iteration):

• flip CrossL2Inbox to "deposits" mode.
• derivation should mutate deposits:
  • If the deposit directly targets the cross-l2-inbox, then:
    • Check if a valid message is being inserted. If not valid, then replace the sender with address X which is not authorized. If valid, then address Y which is authorized.
  • Modify the CrossL2Inbox contract to authenticate the insertions. If it is X then event-log it as invalid. If it is Y then event-log it as valid, and store a message-hash in the state.
  • Other deposits are as-is, and can interact with the inbox. The inbox should check its storage for a pre-validated message hash. If a deposit, but no message hash, then revert (and maybe event-log to clarify the failure, and support retry schemes).
• flip CrossL2Inbox back to "regular" mode.

If there is an external reorg, or unavailable data, we can't immediately verify the message declared in a deposit. To mitigate this, we can enforce the statically declared message deposit to only pull in a message older than a sequence-window, and otherwise mark the deposit as invalid (change sender to X).

[tynes]

We will want to spec this new behavior in the predeploys. The L1Block contract would need a new getter, something like isDeposit()(bool). The CrossL2Inbox would add an invariant if (l1Block.isDeposit() == false) revert Unauthorized(); and then we can relax that restriction in the future like @protolambda said

One major question is whether the isDeposit() function should be only callable by the CrossL2Inbox. Do we want arbitrary smart contracts to be able to know this information? It would be used to build censorship features like ethereum-optimism/design-docs#30

[tynes]

We could avoid mutating deposits by adding another function on the OptimismPortal that emits the deposit transaction event but this still does not solve the problem with the sequence window. So my current understanding of the proposed solution is fully in derivation, so the deposit is mutated if 1) the identifier doesn't point to the message or 2) the message is newer than the sequencing window of the remote chain.

This would mean we need to standardize the sequencing window between chains, since there is no way to know what the value is, unless we use the superchain registry config or assume that they are always the same. I don't love using the superchain registry approach as that is not scalable, perhaps we hardfork in a constant value and then make it configurable in the future (for quality of life for L3s)

The function that registers the events in the CrossL2Inbox, would that be onlyDeposit() && onlyEOA()? This would allow the derivation pipeline to do static analysis, this would be a reliable way to ensure static analysis

[Inphi]

One major question is whether the isDeposit() function should be only callable by the CrossL2Inbox. Do we want arbitrary smart contracts to be able to know this information? It would be used to build censorship features like ethereum-optimism/design-docs#30

I'd lean towards not expanding ABI use scope in principle. If there's demand for a valid use-case (non-censoring, etc) for a no-auth isDeposit then we can add it in later.

[tynes]

In my mind there are 2 ways to think about this problem, one is completely within the smart contracts and one is completely within the derivation pipeline. The smart contract based solution was partially described as well as the derivation pipeline based solution. We should explore the tradeoffs between these two, once we introduce mutating deposits then it may become tech debt eventually as its a new class of behavior

[tynes]

Specs in progress: ethereum-optimism/specs#258

[skeletor-spaceman]

I'll propose an alternative approach:
Instead of having to upgrade the sensitive L1Block contract, and having to turn on isDeposit at the start of the block, while also having to turn it off after "all" deposits have been processed.
Have you previously discussed the following:
on L2CrossDomainMessenger.relayMessage(...) inside if (_isOtherMessenger()) { we can set a transient isDeposit flag.
then, on CrossL2Inbox before emitting the ExecutingMessage a call to L2CrossDomainMessenger.isDeposit()(bool) is done, if the return value is true CrossL2Inbox will revert.
this revert is then caught by the L2CrossDomainMessenger and the message could be manually re-relayed at a later date, or eventually expired back to L1.

I think this gets rid of the problem of having the isDeposit flag linger to the next block. what was the rationale for this? maybe I'm missing something.

this will only require an quick and easy upgrade to the L2CrossDomainMessenger which we already are planning to upgrade.
plus minimal changes to the CrossL2Inbox.

EDIT: There is no way to enforce a single entry point nor a single source of the txs on-chain.

[tynes]

I'll propose an alternative approach: Instead of having to upgrade the sensitive L1Block contract, and having to turn on isDeposit at the start of the block, while also having to turn it off after "all" deposits have been processed. Have you previously discussed the following: on L2CrossDomainMessenger.relayMessage(...) inside if (_isOtherMessenger()) { we can set a transient isDeposit flag. then, on CrossL2Inbox before emitting the ExecutingMessage a call to L2CrossDomainMessenger.isDeposit()(bool) is done, if the return value is true CrossL2Inbox will revert. this revert is then caught by the L2CrossDomainMessenger and the message could be manually re-relayed at a later date, or eventually expired back to L1.

I think this gets rid of the problem of having the isDeposit flag linger to the next block. what was the rationale for this? maybe I'm missing something.

this will only require an quick and easy upgrade to the L2CrossDomainMessenger which we already are planning to upgrade. plus minimal changes to the CrossL2Inbox.

This is an interesting approach but not all deposits MUST go through the L2CrossDomainMessenger. Somebody can call OptimismPortal.depositTransaction with any L2 target, meaning there isn't a common entrypoint to hook into

[tynes]

Needs a review and a few comments to be resolved. Smart contract wise, everything is done. Very close to being complete