Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

ref impl: main node process (connect with RPC, start drivers, handle shutdown) #132

Merged
merged 5 commits into from
Jan 21, 2022
Merged

ref impl: main node process (connect with RPC, start drivers, handle shutdown) #132

merged 5 commits into from
Jan 21, 2022

Conversation

protolambda
Copy link
Collaborator

@protolambda protolambda commented Jan 10, 2022 •
edited
Loading

Part of #119: staging -> main migration

This:

  • Defines the run subcommand, a.k.a. go run ./opnode/cmd run
  • Connects with L1 and L2 RPC connections (or at least creates client binding, initial connection may be broken)
  • Initializes everything
  • Runs node process
  • Handles Close of the node
  • readme how to run the reference implementation, explaining the options

Depends on #131 (merged)

Review: systems team preferred. No spec affected.

@protolambda protolambda self-assigned this Jan 10, 2022
@protolambda protolambda changed the base branch from impl-driver to tmp-126-131 January 10, 2022 23:47
This was referenced Jan 10, 2022
Closed
Merged
@protolambda
Copy link
Collaborator Author

TODO: rebase on main once other PRs are merged. And adapt to minor interface changes. E.g. ctx addition to Drive method and removal from EngineDriver struct (153473a)

@protolambda protolambda mentioned this pull request Jan 13, 2022
Merged
trianglesphere
trianglesphere reviewed Jan 14, 2022
View reviewed changes
opnode/node/node.go Outdated Show resolved Hide resolved
norswap
norswap approved these changes Jan 18, 2022
View reviewed changes
Copy link
Contributor

@norswap norswap left a comment

Choose a reason for hiding this comment

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

LGTM!

opnode/README.md
# websockets or IPC preferred for event notifications to improve sync, http RPC works with adaptive polling.
op node run \
--l1=ws://localhost:8546 --l2=ws//localhost:9001 \
--genesis.l1-num=.... --genesis.l1-hash=..... --genesis.l2-hash=....
Copy link
Contributor

Choose a reason for hiding this comment

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

Actual values would be great. e.g. do the hash need to start with 0x?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Parsing is flexible, the 0x is optional

opnode/node/node.go Show resolved Hide resolved
opnode/node/node.go Outdated Show resolved Hide resolved
@protolambda protolambda changed the base branch from tmp-126-131 to impl-driver January 19, 2022 18:33
@protolambda
Copy link
Collaborator Author

Rebased on top of the driver PR (all dependencies rebased on main)

@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2022 •
edited
Loading

Codecov Report

Merging #132 (59acf60) into main (4493ff4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #132   +/-   ##
=======================================
  Coverage   34.26%   34.26%           
=======================================
  Files          11       11           
  Lines         750      750           
=======================================
  Hits          257      257           
  Misses        468      468           
  Partials       25       25

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 4493ff4...59acf60. Read the comment docs.

@protolambda protolambda mentioned this pull request Jan 20, 2022
Closed
Base automatically changed from impl-driver to main January 21, 2022 15:54
@protolambda
Copy link
Collaborator Author

Rebased on main + made minor changes to adapt to the separated state/actions of the driver in PR 131

@protolambda protolambda merged commit d5f9046 into main Jan 21, 2022
@protolambda protolambda deleted the impl-node branch January 21, 2022 18:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@trianglesphere trianglesphere trianglesphere left review comments

@norswap norswap norswap approved these changes

Assignees

@protolambda protolambda

Labels
implementation
Projects
None yet
Milestone
Deposit Rollup Release Candidate
Development

Successfully merging this pull request may close these issues.
4 participants
@protolambda @codecov-commenter @norswap @trianglesphere