Add specs package by maurelian · Pull Request #514 · ethereum-optimism/optimism

maurelian · 2021-04-20T18:24:29Z

Description

This PR moves specifications from a private repo to here. Things I think reviewers should focus on:

Identifying factually incorrect information.
Identifying information that is going to change in the near future (I will add a note in the specs to reflect it).
If you respond with 'Request changes', please indicate what you would require to approve. :)

Responses to some comments/questions I anticipate

1. What's up with these files that have no substantive content?

I have included a couple files that are effectively empty (or have their content commented out). Per the note in these files, I would like to keep them as a place holder.

2. Where is the content to describe how X works?

Please DO leave these comments so I can prioritize the areas most in need of specification. However, IMO completeness is shouldn't be a requirement to merge this in.

3. What going on in /l2geth?

The content there was written by others in the previous repo. I think it's useful to bring this in for the geth team. I'm open to splitting it out into another location, if there is a clear proposal, otherwise would prefer just to get it in here.

fix: accurately describe ovmL1TXORIGIN more specs

words

fix location of package.json

words

docs: polish

changeset-bot · 2021-04-20T18:24:31Z

🦋 Changeset detected

Latest commit: 0a3caa3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@eth-optimism/specs	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

maurelian · 2021-04-20T18:26:18Z

.github/CODEOWNERS

 l2geth/                             @smartcontracts @tynes @karlfloersch
+packages/specs/l2geth/              @smartcontracts @tynes @karlfloersch
+packages/contracts/                 @smartcontracts @ben-chain @maurelian
+packages/specs/protocol/            @smartcontracts @ben-chain @maurelian


I moved packages/contracts/ up to make the relationship between the specs and the implementation locations more obvious.

maurelian · 2021-04-20T18:26:58Z

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

-     * @notice Specifies from which L1 rollup queue this transaction originated from.
-     * @return _queueOrigin Address of the ovmL1QUEUEORIGIN within the current message context.
+     * @notice Specifies from which source (Sequencer or Queue) this transaction originated from.
+     * @return _queueOrigin Enum indicating the ovmL1QUEUEORIGIN within the current message context.


Just a comment I noticed needed improvement while doing this work.

maurelian · 2021-04-20T18:31:45Z

packages/specs/l2geth/sequencer/README.md

@@ -0,0 +1,109 @@
+# Sequencer specs


/seqencer had it's own top level dir in the specs repo. I moved it into /l2geth here. I supposed I could have done:

mv l2geth/sequencer/* l2geth rm -rf sequencer

But this felt like a more conservative approach to content written by others.

maurelian · 2021-04-20T18:38:43Z

packages/specs/package.json

+  },
+  "scripts": {
+    "format": "yarn prettier --write ./**/*.md",
+    "format:check": "yarn prettier --check ./**/*.md"


I intend for format:check to run in CI, but haven't set that up yet.

I know that yarn prettier --check ./**/*.md looks funny. Why not just prettier --check **/*.md, honestly I'm not sure why, but that and every other variation of this script I tried failed to run on all md files up and down the tree.

maurelian · 2021-04-20T18:54:57Z

packages/specs/package.json

+  },
+  "scripts": {
+    "format": "yarn prettier --write \"{l2geth,protocol}/**/*.md\"",
+    "format:check": "yarn prettier --check \"{l2geth,protocol}/**/*.md\"",


I realize the glob string seems like overkill, but I couldn't get anything else to hit all the md files up and down the tree.

my intention is for format:check to run in CI, but I haven't set that up.

maurelian · 2021-04-20T18:55:59Z

packages/specs/protocol/components/accounts.md

@@ -0,0 +1,19 @@
+# Accounts
+
+This document is a WIP. It is kept here to maintain a consistent structure mirroring the [implementation directory](../../../contracts/contracts/optimistic-ethereum/OVM/).


I hope to complete this document thoroughly in the near future, but would like to include it as a placeholder.
If it's a blocker, I can remove it.

packages/specs/protocol/components/execution.md

maurelian · 2021-04-20T18:57:42Z

packages/specs/protocol/data-structures.md

@@ -0,0 +1,348 @@
+# Optimistic Ethereum Data Structures


Feel free to leave a comment here where you think a variable name should change, but IMO actually updating it should be reserved for a new PR.

tynes · 2021-04-20T23:44:43Z

The content there was written by others in the previous repo. I think it's useful to bring this in for the geth team. I'm open to splitting it out into another location, if there is a clear proposal, otherwise would prefer just to get it in here.

I'm on board with leaving it in and agree with renaming it to l2geth

tynes · 2021-04-20T23:56:42Z

I made an update to l2geth here: ethereum-optimism/specs#31

I can open a PR against this or wait until this is merged in, whatever is easier for @maurelian getting this in

gakonst

Please:

pull this to the top level directory (not under packages), and add it to the global yarn workspace.
mark the package as private so that it does not get published by accident

Have the protocol changes been approved by @ben-chain? ethereum-optimism/specs#30 is still a draft.

Separately:
It seems like transaction-indexer refers to the DTL so should be moved outside of l2geth, and the transaction-ingestor refers to L2geth's SyncService so should be renamed to that?

maurelian · 2021-04-21T15:37:47Z

OK, will move to root of repo.

Have the protocol changes been approved by @ben-chain? ethereum-optimism/specs#30 is still a draft.

There should not be protocol changes in here yet. For now I've aimed to describe the protocol as is.
Changes to the spec can be made and reviewed in separate PRs.

maurelian · 2021-04-22T03:49:23Z

package.json

    "l2geth",
-    "integration-tests"
+    "integration-tests",
+    "specs"


@gakonst is this change sufficient to "add it to the global yarn workspace" as requested?

gakonst · 2021-04-22T08:54:53Z

package.json

    "l2geth",
-    "integration-tests"
+    "integration-tests",
+    "specs"


specs/package.json

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

ben-chain

Woot woot, Let's Get This Merged!

* chore: import specs fix: accurately describe ovmL1TXORIGIN more specs * chore: rename to match structure of contracts words * chore: move to packages * chore: add package.json fix location of package.json * docs: rename processes dir to components words * docs: remove empty specs dir * docs: move sequencer into l2geth * chore: assign code owners to specs * chore: add prettier * docs: polish bridge and actors section docs: polish * chore: add format:check script * docs: fixup readme * chore: fix prettier config * fix: json linting * chore: add changeset * Update packages/specs/protocol/components/execution.md * chore: move specs to root * chore: make private, add to yarn workspaces * Update specs/package.json Co-authored-by: Georgios Konstantopoulos <me@gakonst.com> * fix: formatting Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

…)` (ethereum-optimism#514)

fixes #513 --------- Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

Closes #461

maurelian added 11 commits April 20, 2021 14:09

chore: import specs

6d322b3

fix: accurately describe ovmL1TXORIGIN more specs

chore: rename to match structure of contracts

73c2234

words

chore: move to packages

e9ab984

chore: add package.json

2792d50

fix location of package.json

docs: rename processes dir to components

2a5108e

words

docs: remove empty specs dir

c692ae1

docs: move sequencer into l2geth

de2c50b

chore: assign code owners to specs

1ddb240

chore: add prettier

bc821cf

docs: polish bridge and actors section

8bc4139

docs: polish

chore: add format:check script

53f791a

maurelian requested review from ben-chain, karlfloersch, smartcontracts and tynes as code owners April 20, 2021 18:24

maurelian changed the title ~~Feat/specs~~ Add a specification package Apr 20, 2021

maurelian added 2 commits April 20, 2021 14:52

docs: fixup readme

469180f

chore: fix prettier config

4977757

maurelian marked this pull request as draft April 20, 2021 18:54

maurelian commented Apr 20, 2021

View reviewed changes

maurelian marked this pull request as ready for review April 20, 2021 18:58

maurelian added 3 commits April 20, 2021 15:05

fix: json linting

117f207

chore: add changeset

bfb9084

Update packages/specs/protocol/components/execution.md

f8e88b4

maurelian requested review from gakonst and snario April 20, 2021 19:07

maurelian changed the title ~~Add a specification package~~ Add a specifics package Apr 20, 2021

maurelian changed the title ~~Add a specifics package~~ Add specs package Apr 20, 2021

gakonst suggested changes Apr 21, 2021

View reviewed changes

maurelian added 2 commits April 21, 2021 23:43

chore: move specs to root

8defd1e

chore: make private, add to yarn workspaces

5817a19

maurelian commented Apr 22, 2021

View reviewed changes

maurelian requested a review from gakonst April 22, 2021 03:49

gakonst approved these changes Apr 22, 2021

View reviewed changes

maurelian and others added 2 commits April 22, 2021 08:09

Update specs/package.json

c01d7e3

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

fix: formatting

0a3caa3

ben-chain approved these changes Apr 22, 2021

View reviewed changes

maurelian merged commit 318857e into master Apr 22, 2021

maurelian deleted the feat/specs branch April 22, 2021 19:41

bap2pecs pushed a commit to babylonlabs-io/optimism that referenced this pull request Jul 31, 2024

fix: data race in GetLastProcessedHeight() and `GetLastVotedHeight(…

1f34c36

…)` (ethereum-optimism#514)

theochap pushed a commit that referenced this pull request Jan 15, 2026

feat: made TxDeposit's mint field non-optional (#514)

cfe1e81

fixes #513 --------- Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

emhane pushed a commit that referenced this pull request Feb 4, 2026

test: extended prune e2e with getProof consistency (#514)

abf16c6

Closes #461

		@@ -0,0 +1,19 @@
		# Accounts

		This document is a WIP. It is kept here to maintain a consistent structure mirroring the [implementation directory](../../../contracts/contracts/optimistic-ethereum/OVM/).

Comments

Conversation

maurelian commented Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Responses to some comments/questions I anticipate

Uh oh!

changeset-bot bot commented Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tynes commented Apr 20, 2021

Uh oh!

tynes commented Apr 20, 2021

Uh oh!

gakonst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maurelian commented Apr 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ben-chain left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maurelian commented Apr 20, 2021 •

edited

Loading

changeset-bot bot commented Apr 20, 2021 •

edited

Loading

gakonst left a comment •

edited

Loading

maurelian commented Apr 21, 2021 •

edited

Loading