Rollup Node Specification Revamp #144

norswap · 2022-01-21T14:04:14Z

This started as a pull-request to address #87 (standardize languages around deposits), but due to a small communication problem, I was faced with a pretty big merge conflict, which also lead to some interesting discussions with @protolambda about how to structure this spec.

Since I was going to introduce a huge unreadable diff anyway, I've also used to opportunity to knock off #74 (derivation pseudocode) and #106 (sync spec with implementation, though proto had pretty much taken care of that already).

The best way to review the changes to the rollup node specs is to use this links to visualize it rendered directly.

So this now closes #87 and #74

… links

…nd clarify content

norswap · 2022-01-24T16:55:10Z

@protolambda I tried to synthesize our approaches to make the rollup node spec both more friendly to newcomers & more concise. Let me know if you have any feedback!

norswap · 2022-01-24T16:56:11Z

specs/rollup-node.md

+    algorithm from step 2.
+  - Note: if `currentL1` does not exist, it means we are in a re-org to a shorter L1 chain.
+  - Note: as an optimization, we can cache `currentL1` and reuse it as the next value of `nextRefL1` to avoid an
+        extra lookup.


I've tried simplifying this as much as possible while preserve the semantics. @protolambda could you look it over to see if it seems alright?

New version looks good!

norswap · 2022-01-24T16:57:00Z

specs/rollup-node.md

+    - If the next L1 block does not exist yet, there is no re-org, and nothing new to derive, and we can abort the
+        process.
+- Otherwise, if `refL2` is the L2 genesis block, we have re-orged past the genesis block, which is an error that
+    requires a re-genesis of the L2 chain to fix (i.e. creating a new genesis configuration).


I'm not 100% sure about this, but this my best guess. What happens in practice if we re-org past genesis? Wouldn't it be better to specify that a re-org past genesis is allowed, in which case we just wait for the L2 chain inception L1 block height to restart the whole process?

If the L1 reorgs past expected anchor point on L1 then we should just manually redeploy the L2 chain (new L2 genesis) and not build any blocks on top. Wiping the whole L2 chain with a reorg without intervention is pretty severe.

If we start a L2 chain we can probably pick a finalized/confirmed L1 anchor point in the past, to not risk a reorg, at the cost of just a few empty deposit-blocks at the start of the L2 chain.

Good point, I'll add a footnote saying just that.

norswap · 2022-01-24T16:57:46Z

specs/rollup-node.md


-L1 Ethereum [reaches finality approximately every 12 minutes][l1-finality], so these L2 blocks can safely be considered
-to be final: they will never disappear from the chain's history because of a re-org.
+# Handling L1 Re-Orgs



I want to move this to a implementation notes directory, cf. #141

Any objections?

norswap · 2022-01-24T16:58:12Z

specs/rollup-node.md

@@ -175,58 +235,77 @@ The following scenarios are assimilated to errors:
 - [`engine_executePayloadV1`] returning a `status` of `"SYNCING"` or `"INVALID"` whenever passed an execution payload
  that was obtained by a previous call to [`engine_getPayloadV1`].

-## Handling L1 Re-Orgs
+### Finalization Guarantees



I added mentions to fault proofs here because this section makes little sense without it. I'm also okay to remove this this section altogether to preserve consistency (though doing so make the prescriptions on the forkchoiceState seemingly arbitrary).

norswap · 2022-01-24T16:58:34Z

specs/deposits.md


 1. A contract creation deposit, which is indicated by setting the `isCreation` flag to `true`.
   In the event that the `to` address is non-zero, the contract will revert.
 2. A call from a contract account, in which case the `from` value is transformed to its L2
   [alias][address-aliasing].

-> **TODO** Define if/how ETH withdrawals occur.


Removed since it's out of scope for the deposit rollup.

norswap · 2022-01-24T16:59:47Z

specs/deposits.md


-> **TODO** Specify and link to deposit blocks


This is defined in the rollup node, and already implicitly in this file (first tx is l1 attributes, then user-deposited), so I don't think it's necessary to add something here.

Also we link to the glossary.

norswap · 2022-01-24T17:00:43Z

specs/deposits.md

-In the event a deposit transaction is included which is not derived by the [execution
-engine][execution-engine] using the correct derivation algorithm, the resulting state transition
-would be invalid.
-


Removed this part because it was erroneous (it's not about the execution engine but the rollup node) and also because we can't really speak to the consequences at this stage (since we don't submit roots on L1) - what does it mean to be invalid? A bad transaction being included can also not lead to any state transition (i.e. the only bad output is in the block hash, but the state root is correct). Also if you don't follow the prescribed rules, then the result is invalid, which is sort of tautological.

The "valid"/"invalid" refers to following the verifier rules I think. The local rollup-node can insert any deposit into their execution-engine, but if it does not originate from the specified derivation, other nodes will regard it as invalid. You're right that while we don't have block propagation or state-root posting it's not applicable (as other nodes cannot consume the invalid data in the first place).

The "valid"/"invalid" refers to following the verifier rules I think

That's correct.

norswap · 2022-01-24T17:05:33Z

specs/deposits.md


-   <!-- TODO: Define how this account pays gas on these transactions. -->
+No gas is paid for L1 attributes deposited transactions.


Maybe change this to say that "gas" is paid for on L1? Should be done when we transcribe the deposit fee design in the spec.

Technically it's not paid for on L1 either, it's the rollup itself as system inserting the deposit. The rate-limit is simply the L1 block production.

protolambda

Looks great! Not sure about the pseudocode style/language, but ok to defer to when we have more code and can inform that style/language choice better.

protolambda · 2022-01-25T18:28:07Z

meta/markdown-style.md

+- Use links to the [glossary] liberally.
+- Include the references to all the glossary links at the top of the file, under the top-level
+  title.
+- A glossary link reference should start with the `g-` prefix. This enables to see what links to the


good idea, I was having trouble with this before

protolambda · 2022-01-25T18:29:17Z

specs/deposits.md

+**Vocabulary note**: *deposited transaction* refers specifically to an L2 transaction, while
+*deposit* can refer to the transaction at various stages (for instance when it is deposited on L1).
+
+## Table of Contents


Good to see the ToC update, I will automate this with doctoc in a new PR 👍

protolambda · 2022-01-25T18:34:21Z

specs/deposits.md

-In the event a deposit transaction is included which is not derived by the [execution
-engine][execution-engine] using the correct derivation algorithm, the resulting state transition
-would be invalid.
-


The "valid"/"invalid" refers to following the verifier rules I think. The local rollup-node can insert any deposit into their execution-engine, but if it does not originate from the specified derivation, other nodes will regard it as invalid. You're right that while we don't have block propagation or state-root posting it's not applicable (as other nodes cannot consume the invalid data in the first place).

protolambda · 2022-01-25T18:39:56Z

specs/deposits.md


-   <!-- TODO: Define how this account pays gas on these transactions. -->
+No gas is paid for L1 attributes deposited transactions.


Technically it's not paid for on L1 either, it's the rollup itself as system inserting the deposit. The rate-limit is simply the L1 block production.

protolambda · 2022-01-25T18:44:43Z

specs/rollup-node.md

+    - If the next L1 block does not exist yet, there is no re-org, and nothing new to derive, and we can abort the
+        process.
+- Otherwise, if `refL2` is the L2 genesis block, we have re-orged past the genesis block, which is an error that
+    requires a re-genesis of the L2 chain to fix (i.e. creating a new genesis configuration).


If the L1 reorgs past expected anchor point on L1 then we should just manually redeploy the L2 chain (new L2 genesis) and not build any blocks on top. Wiping the whole L2 chain with a reorg without intervention is pretty severe.

If we start a L2 chain we can probably pick a finalized/confirmed L1 anchor point in the past, to not risk a reorg, at the cost of just a few empty deposit-blocks at the start of the L2 chain.

protolambda · 2022-01-25T18:49:17Z

specs/rollup-node.md

+    algorithm from step 2.
+  - Note: if `currentL1` does not exist, it means we are in a re-org to a shorter L1 chain.
+  - Note: as an optimization, we can cache `currentL1` and reuse it as the next value of `nextRefL1` to avoid an
+        extra lookup.


New version looks good!

protolambda · 2022-01-25T18:51:55Z

specs/rollup-node.md

+
+Then we can apply the following pseudocode logic to update the state of both the rollup driver and execution engine:
+
+```pseudocode


This is fine with me, but there's no syntax highlighting so it may be difficult to read. If Go is too verbose for snippets in the spec, we can try python.

Agree with Python, it seems to be more or less the standard for pseudocode in ethereum

I'll try using javascript since we use JSON notation at the top, I think it should highlight well then.

Wrote my answer before seeing maurelian's. Feel free to suggest a modification to the syntax if you feel like it's warranted. I personally don't really see the need to change anything here.

maurelian

A few suggestions, but LGTM

maurelian · 2022-01-27T12:38:25Z

specs/deposits.md

-processed will only derive transactions with a `from` address attested to by the logs of the [L1
+As noted above, the deposited transaction type does not include a signature for validation. Rather,
+authorization is handled by the [L2 chain derivation][g-derivation] process, which when correctly
+applied will only derive transactions with a `from` address attested to by the logs of the [L1
 deposit feed contract][deposit-feed-contract].


This link seems to be broken now.

maurelian · 2022-01-27T12:38:42Z

specs/deposits.md

-In the event a deposit transaction is included which is not derived by the [execution
-engine][execution-engine] using the correct derivation algorithm, the resulting state transition
-would be invalid.
-


The "valid"/"invalid" refers to following the verifier rules I think

That's correct.

specs/deposits.md

maurelian · 2022-01-27T13:38:58Z

specs/rollup-node.md

+
+Then we can apply the following pseudocode logic to update the state of both the rollup driver and execution engine:
+
+```pseudocode


Agree with Python, it seems to be more or less the standard for pseudocode in ethereum

maurelian · 2022-01-27T13:40:39Z

specs/rollup-node.md

-  first L1 block for which an L2 block was produced). See the [Finalization Guarantees][finalization] section for more
-  details.
+Once the [payload attributes] for a given L1 block `B` have been built, and if we have already derived an L2 block from
+`B`'s parent block, then we can use the payload attributes to derive a new L2 block.


This 'and if' really stands out to me. What if we haven't?

Then we can't use the payload attribute to derive a new L2 block :)
Feel free to suggest a clarification if needed!

Co-authored-by: Maurelian <maurelian@protonmail.ch>

codecov-commenter · 2022-01-28T00:12:07Z

Codecov Report

Merging #144 (04309c9) into main (6cf5e20) will increase coverage by 7.84%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
+ Coverage   34.26%   42.11%   +7.84%     
==========================================
  Files          11       13       +2     
  Lines         750      900     +150     
==========================================
+ Hits          257      379     +122     
- Misses        468      488      +20     
- Partials       25       33       +8

Impacted Files	Coverage Δ
opnode/test/geth.go	`50.00% <0.00%> (ø)`
opnode/test/setup.go	`96.07% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cf5e20...04309c9. Read the comment docs.

norswap · 2022-01-28T00:17:41Z

Okay to merge on my end unless they are further suggestions!

norswap added 8 commits January 24, 2022 17:07

add new terms to glossary, fixing internal links but not yet external…

fe6e6e7

… links

add table of contents to deposit spec

449d02e

update deposits.md to use standardized terms

ed90cf1

add g-prefix to glossary links in deposits.md

5c2c043

rewrite rollup node spec to standardize terminology, add pseudocode a…

2e21031

…nd clarify content

update execution engine specs

4bf2bf1

simplify/clarify sync start algorithm

ec8aab3

lint

9d54470

norswap force-pushed the nl/standardize-deposits branch from fa802ef to a92c5d7 Compare January 24, 2022 16:44

norswap added 2 commits January 24, 2022 17:46

switch glossary links in rollup node spec to use g-prefix

ddfe868

add specifications for markdown style

a9087ce

norswap force-pushed the nl/standardize-deposits branch from a92c5d7 to a9087ce Compare January 24, 2022 16:46

norswap changed the title ~~Standardize language around deposits~~ Rollup Node specification make-over Jan 24, 2022

norswap changed the title ~~Rollup Node specification make-over~~ Rollup Node Specification Revamp Jan 24, 2022

norswap requested a review from maurelian January 24, 2022 16:53

norswap commented Jan 24, 2022

View reviewed changes

protolambda approved these changes Jan 25, 2022

View reviewed changes

maurelian approved these changes Jan 27, 2022

View reviewed changes

terminology quickfix

310ff92

Co-authored-by: Maurelian <maurelian@protonmail.ch>

add precisions regarding the re-org past genesis scenario

04309c9

maurelian merged commit 0cdade8 into main Jan 28, 2022

maurelian deleted the nl/standardize-deposits branch January 28, 2022 02:05

This was referenced Jan 28, 2022

Rework execution engine specs to match the implementation #106

Open

Doc: Block-derivation pseudo code for rollup node spec #74

Closed

norswap mentioned this pull request Jan 28, 2022

Standardize language around deposits #61

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollup Node Specification Revamp #144

Rollup Node Specification Revamp #144

norswap commented Jan 21, 2022 •

edited

Loading

norswap commented Jan 24, 2022

norswap Jan 24, 2022

protolambda Jan 25, 2022

norswap Jan 24, 2022

protolambda Jan 25, 2022

norswap Jan 27, 2022

norswap Jan 24, 2022

norswap Jan 24, 2022

norswap Jan 24, 2022

norswap Jan 24, 2022

norswap Jan 24, 2022

protolambda Jan 25, 2022

maurelian Jan 27, 2022

norswap Jan 24, 2022

protolambda Jan 25, 2022

protolambda left a comment

protolambda Jan 25, 2022

protolambda Jan 25, 2022

protolambda Jan 25, 2022

protolambda Jan 25, 2022

protolambda Jan 25, 2022

protolambda Jan 25, 2022

protolambda Jan 25, 2022

maurelian Jan 27, 2022

norswap Jan 27, 2022

norswap Jan 28, 2022

maurelian left a comment

maurelian Jan 27, 2022

maurelian Jan 27, 2022

maurelian Jan 27, 2022

maurelian Jan 27, 2022

norswap Jan 28, 2022

codecov-commenter commented Jan 28, 2022 •

edited

Loading

norswap commented Jan 28, 2022


		<!-- TODO: Define how this account pays gas on these transactions. -->
		No gas is paid for L1 attributes deposited transactions.


		Then we can apply the following pseudocode logic to update the state of both the rollup driver and execution engine:

		```pseudocode

Rollup Node Specification Revamp #144

Rollup Node Specification Revamp #144

Conversation

norswap commented Jan 21, 2022 • edited Loading

norswap commented Jan 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

protolambda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maurelian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jan 28, 2022 • edited Loading

Codecov Report

norswap commented Jan 28, 2022

norswap commented Jan 21, 2022 •

edited

Loading

codecov-commenter commented Jan 28, 2022 •

edited

Loading