Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

experimental: allow the batch inbox to be a contract #284

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

blockchaindevsh
Copy link

@blockchaindevsh blockchaindevsh commented Jul 23, 2024

Description

This specification aims to allow the batch inbox to be a contract, enabling customized batch submission conditions such as:

  • Requiring the batch transaction to be signed by a quorum of sequencers in a decentralized sequencing network; or
  • Mandating that the batch transaction call a BLOB storage contract (e.g., EthStorage) with a long-term storage fee, which is then distributed to data nodes that prove BLOB storage over time.

Note: This specification can serve as a preliminary step towards addressing #221.

Copy link
Contributor

@tynes tynes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not detailed enough to be implemented solely from the spec. If you are serious about this, please update this spec to be detailed to the point where it is possible to implement by just looking at the spec. We will also need the ability to make the batch inbox dynamic so that chains that use empty accounts today can migrate to using smart contract based batch inboxes in the future

@blockchaindevsh
Copy link
Author

blockchaindevsh commented Jul 24, 2024

@tynes Thank you for your insightful advice and for highlighting the migration issue—it's an excellent point. We'll update the specification with more comprehensive details.

However, we have a question regarding the OP Stack upgrade process:
Is the upgrade performed by manually ensuring all software components are up-to-date before applying changes to the SystemConfig contract? We understand that the op-node scans for SystemConfig updates in each L1 block and applies them to its rollup.Config accordingly. However, it appears that the op-batcher only fetches rollup.Config during initRollupConfig, potentially missing dynamic updates to rollup.Config.

Could you clarify the upgrade mechanism and how it ensures all components remain synchronized with SystemConfig changes?

@tynes
Copy link
Contributor

tynes commented Jul 24, 2024

@blockchaindevsh The batcher will need to be aware of the fact that the batch inbox is dynamic. It will need to do one of the following:

  1. Listen to events on the system config
  2. Poll the op-node for changes
  3. Poll the system config periodically
  4. Write the batch inbox address into the L2 state similar to how custom gas token works and use that as the source of truth

Option 4 is better than 3 because it removes the need for an L1 archive node, it also reduces complexity around missed events with option 1. Everything is tradeoffs.

@blockchaindevsh
Copy link
Author

@tynes Upon reviewing the custom gas token implementation in the develop branch, it appears that it only takes effect during the initial contract deployment and subsequently operates solely within the contract itself. Neither the op-node nor the op-batcher perform any processing related to this feature. I'm uncertain if I've overlooked any aspects of its implementation.

Let's assume that the batcher should submit to the new inbox immediately upon its change in the SystemConfig contract on L1.

We extensively discussed the batch submission process when the inbox changes. It appears that regardless of the strategy employed by the batcher to fetch inbox address, there remains a possibility of inadvertently submitting to a stale inbox.

(Bn is L1 block, bn is L2 block, the graph is simplified so that bn always chooses Bn as l1 origin.)

0: 
B0 =========>  B1 (batch 0)
|              |
v              v
b0(batch 0) => b1(batch 1 submitting)
where batch 0, which contains b0, is submitted and included in B1; batch 1, which contains b1, is being submitted

1:
B0 =========>  B1 (batch 0)    ========> B2(x)
|              |                          |
v              v                          v
b0(batch 0) => b1 (batch 1 submitting) => b2 (set L2 state)
where batch 1 is still being submitted but not included on L1 yet, and x is the tx to change inbox addr

2:
B0 =========>  B1 (batch 0)    ========> B2(x)            => B3(batch 1, but stale inbox)
|              |                          |
v              v                          v
b0(batch 0) => b1 (batch 1)           => b2 (set L2 state)
where batch 1 is included in B3 but submitted to a stale inbox, which breaks our assumption that all batches after inbox change transaction (x) should be submitted to the new address. 

To mitigate this, after a transaction is submitted, the batcher should verify whether the inbox has changed at the L1 block that included the batch. If a change is detected, it should resubmit the transaction to the updated inbox.

3:
B0 =========>  B1 (batch 0)    ========> B2(x)             => B3(batch 1, but stale inbox)
|              |                          |                    |
v              v                          v                    v
b0(batch 0) => b1 (batch 1)           => b2 (set L2 state) => b3
when batch 1 is included in B3, batcher found B3 inbox address changed on L1, and resubmit to new inbox.

4:
B0 =========>  B1 (batch 0)    ========> B2(x)             => B3(batch 1, but stale inbox) => B4 (batch 1, new inbox)
|              |                          |                    |
v              v                          v                    v
b0(batch 0) => b1 (batch 1)           => b2 (set L2 state) => b3

We concluded that it would be simpler and less prone to errors if the batcher queries the latest inbox directly from L1 immediately before submitting each batch(similar to option 3, but on demand instead of periodically).

@tynes
Copy link
Contributor

tynes commented Jul 26, 2024

How would op-node know which batch inbox is canonical?

@qzhodl
Copy link
Contributor

qzhodl commented Jul 26, 2024

@blockchaindevsh The batcher will need to be aware of the fact that the batch inbox is dynamic. It will need to do one of the following:

  1. Listen to events on the system config
  2. Poll the op-node for changes
  3. Poll the system config periodically
  4. Write the batch inbox address into the L2 state similar to how custom gas token works and use that as the source of truth

Option 4 is better than 3 because it removes the need for an L1 archive node, it also reduces complexity around missed events with option 1. Everything is tradeoffs.

Does the option 4 mean that the DepositContractAddress is currently part of L2 state while the BatchInboxAddress is not?

@blockchaindevsh
Copy link
Author

blockchaindevsh commented Jul 26, 2024

@tynes op-node would know the canonical batch inbox in a manner similar to the batcherAddr parameter: it's passed as an argument to the DataSourceFactory.OpenData function.

The caller is responsible for passing a canonical value:

image image
  • When called from Reset, the value will be correspond to the L1 origin of the L2 block that was reset to.
  • When called from NextData, the value will be correspond to the L1 block that included the batch.

@tynes
Copy link
Contributor

tynes commented Jul 26, 2024

@blockchaindevsh You are caught up in implementation details. You did not answer my question.

If you are interested in working on a spec for this feature, please ensure that it is fully specified. Right now it is very unclear how this feature works based on your description. If you aren't sure how to write a spec, look at other documents in this repo, such as the custom gas token feature.

@qzhodl The batch inbox address and the deposit contract address are the same thing, just with different names. That is left over tech debt

@blockchaindevsh
Copy link
Author

blockchaindevsh commented Aug 3, 2024

@tynes I've updated the specification with additional details, drawing heavily from the custom gas token specification.

@blockchaindevsh
Copy link
Author

@tynes , I was wondering if you could provide any further advice regarding the inbox specification ?

The `marshalBinaryInboxFork` function is added to [`L1BlockInfo`](https://github.com/ethereum-optimism/optimism/blob/5e317379fae65b76f5a6ee27581f0e62d2fe017a/op-node/rollup/derive/l1_block_info.go#L41). This new function incorporates an additional `batchInbox` parameter for the [`setL1BlockValuesEcotone`](https://github.com/ethereum-optimism/optimism/blob/5e317379fae65b76f5a6ee27581f0e62d2fe017a/packages/contracts-bedrock/src/L2/L1Block.sol#L136) function:

```golang
func (info *L1BlockInfo) marshalBinaryInboxFork() ([]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should define the spec in terms of the serialization instead of source code. Its not super clear exactly what this is doing without knowing the implementation details of various functions used

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is gone after following the new design pattern in f5b8178.

@tynes
Copy link
Contributor

tynes commented Aug 27, 2024

I think we want to make this spec more generic and move it into the holocene folder per ethereum-optimism/optimism#11600 (comment)

@blockchaindevsh
Copy link
Author

I think we want to make this spec more generic and move it into the holocene folder per ethereum-optimism/optimism#11600 (comment)

I've updated the spec to follow the design pattern in #122 and moved it into the holocene folder.

@tynes
Copy link
Contributor

tynes commented Aug 29, 2024

@blockchaindevsh I started a PR that we can build on in #358

This document is good but it focuses a lot on particular implementation details. These are important but not necessarily for the specs - we want to describe what should happen and some why but not links to source code

I would also like to understand a bit better on how your proposal around the dynamic batch inbox impacts the fault proof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants