-
Notifications
You must be signed in to change notification settings - Fork 50
Adding L1SLOAD values in the end of L2 batches when writing to the DA layer #64
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
base: master
Are you sure you want to change the base?
Conversation
… layer This improves the following two issues: 1. L2 nodes replaying transactions during validation no longer need an archive node and don't need to call eth_getStorageAt for each L1SLOAD they encounter in the batch, saving the extra RPC call for each of them, at the expense of an extra 32 bytes written to DA, per call to the L1SLOAD call in the batch. Removing the need for archive node and improving validation speed seems worth it, especially when using blob transactions. 2. L2 nodes don't need to worry about consistency when replaying transaction during batch validation.
@@ -61,7 +61,8 @@ The introduction of the `L1SLOAD` precompile may increase the requirement for th | |||
|
|||
**Prerequisite 2**: The L2 sequencer has a notion of the *latest seen L1 block*, which is deterministic over all L2 nodes, i.e. it is part of the L2 state machine. The exact mechanism is not in scope for this RIP. | |||
|
|||
**Implementation**: When the L2 node encounters a call to the `L1SLOAD` precompiled contract, it first verifies that its input is well-formed. It then retrieves its latest seen L1 block number `l1BlockNumber` and sends an RPC query `eth_getStorageAt(address, storageKey, l1BlockNumber)` to the L1 node. Finally, it writes the received storage value to the designated output buffer. | |||
**Implementation**: When the L2 sequencer encounters a call to the `L1SLOAD` precompiled contract, it first verifies that its input is well-formed. It then retrieves its latest seen L1 block number `l1BlockNumber` and sends an RPC query `eth_getStorageAt(address, storageKey, l1BlockNumber)` to the L1 node, and writes the received storage value to the designated output buffer. Finally, it writes all the received storage values of all `L1SLOAD` it encountered in the block, in order, to the end of the L2 batch it submits on L1. \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting suggestion and it does indeed have some benefits.
My concern is, wouldn't this make "L2 follower nodes" (the ones that receive unsafe blocks on L2 p2p) impossible? Since batch submission delay for most current rollups is 30s or more, they usually sync on L2 p2p, not directly from L1.
On a more general note, I think this RIP should just define the interface and semantics of L1SLOAD, and the underlying implementation can be left to the individual rollups.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Thegaram thanks for the feedback!
While the RIP can have only the interface and the expected behavior, I think that it would be nice to have a suggested implementation that lets rollups have the opcode, without needing their l2 nodes to run l1 archive nodes, as it is a pain point for several rollups right now. Wdyt?
It can definitely be moved to a different section as a recommendation, though I'm not sure it's in the scope of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Thegaram regarding "L2 follower nodes", since only the sequencer appends the values to the batch and writing to L1, I think they'll work exactly as they would without this change. They'll need to get the values from L1 just like the sequencer would, and don't benefit from this change. However, they don't need an archive node since they're using recent state. This change was meant primarily to solve the problem for validators that sync from L1, i.e. validating the entire chain. Without this change they need an archive node and multiple RPC calls to it. With this change they don't. Correct?
And as @shahafn said, I think it's good to suggest an implementation that doesn't require an archive node even though it's an implementation detail and each rollup can decide for itself how to handle it. The only "mandatory" part is the interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yoavw, thank you for a well-written explanation of the benefits this brings. IMHO this is pretty good idea and I'm keen on this solution.
As everyone here already knows, the biggest issue with this RIP was archive node requirement, and unless I'm missing something, this fixes the issue completely :)
@Thegaram could you provide an update the changes LGTM |
@@ -61,7 +61,8 @@ The introduction of the `L1SLOAD` precompile may increase the requirement for th | |||
|
|||
**Prerequisite 2**: The L2 sequencer has a notion of the *latest seen L1 block*, which is deterministic over all L2 nodes, i.e. it is part of the L2 state machine. The exact mechanism is not in scope for this RIP. | |||
|
|||
**Implementation**: When the L2 node encounters a call to the `L1SLOAD` precompiled contract, it first verifies that its input is well-formed. It then retrieves its latest seen L1 block number `l1BlockNumber` and sends an RPC query `eth_getStorageAt(address, storageKey, l1BlockNumber)` to the L1 node. Finally, it writes the received storage value to the designated output buffer. | |||
**Implementation**: When the L2 sequencer encounters a call to the `L1SLOAD` precompiled contract, it first verifies that its input is well-formed. It then retrieves its latest seen L1 block number `l1BlockNumber` and sends an RPC query `eth_getStorageAt(address, storageKey, l1BlockNumber)` to the L1 node, and writes the received storage value to the designated output buffer. Finally, it writes all the received storage values of all `L1SLOAD` it encountered in the block, in order, to the end of the L2 batch it submits on L1. \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should restructure the RIP to put more emphasis on the standard interface, and leave the underlying implementation up to the rollup. We can include examples on how one would implement this. We can do this in another PR.
@@ -98,13 +99,10 @@ function loadFromL1(address l1Address, uint256 key1, uint256 key2) public view r | |||
|
|||
### Which L1 block does `L1SLOAD` read the storage value at? | |||
|
|||
According to the specification defined above, `L1SLOAD` returns the storage value at the latest L1 block known to the L2 sequencer. There are two related issues: | |||
- How to guarantee the consistent return value of `L1SLOAD` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the consistency note should be kept for these reasons.
- Nodes syncing from L2 p2p instead of L1 need to fetch from the same L1 height as the sequencer.
- For proving the values appended to the batch, we need to know which L1 height they were fetched from.
That said, I don't think consistency is an issue, this section just emphasizes that the L2 needs to have a notion of "latest known L1 block" that is consistent across multiple nodes, that's all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@forshtat up
This improves the following two issues: