Skip to content

op-node: l2 rpc refactor#3291

Merged
mergify[bot] merged 3 commits intodevelopfrom
l2-engine-rpc
Aug 23, 2022
Merged

op-node: l2 rpc refactor#3291
mergify[bot] merged 3 commits intodevelopfrom
l2-engine-rpc

Conversation

@protolambda
Copy link
Copy Markdown
Contributor

Description

  • Refactors previous l2.Source into sources.L2Client, now sharing a lot of commons with sources.L1Client through sources.EthClient
  • Remove l2.ReadOnlySource: this was duplicate legacy code from the first op-proposer iteration. We have sources.L2Client now with caching + deduplicated code with L1 client.
  • Add sources.EngineClient: wraps the sources.L2Client with the 3 engine API methods we use. (note: upstream execution APIs state that the eth namespace is part of the engine API endpoint in addition to the engine namespace, so this works).
  • Refactor sources/types.go to:
    • Be more strict about blocks retrieved from post-merge RPC sources. Execution payloads are a subset of the valid block responses we can get.
      • As part of the above: fix the genesis setup in e2e to set the PoW nonce value to 0
    • Define a json definition for the header type. Could improve this more, but the intention is to remove big.Int out of the decoding/encoding picture, make the type flat instead of a pointer structure, and have more control to implement stricter checks on the contents.
  • Port over and optimize Payload retrieval methods to EthClient. (Hopefully we can use this instead of block retrieval for L1 at some point. Especially if we can read raw transactions with some future RPC improvements, instead of the slow transaction encoding/decoding roundtrips we are forced to have now).
  • Remove duplciate/outdated mockL2Client: testutils has everything any test may need for RPC mocking.

See previous L1/eth RPC refactor PR for more context: #3288

Metadata

Fix ENG-2280

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Aug 23, 2022

⚠️ No Changeset found

Latest commit: afe8652

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 23, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

Copy link
Copy Markdown
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.

Generally looks good to me, left some comments

Copy link
Copy Markdown
Contributor

@mslipper mslipper left a comment

Choose a reason for hiding this comment

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

My feedback is non blocking. OK to merge as-is.

Copy link
Copy Markdown
Contributor

@tuxcanfly tuxcanfly left a comment

Choose a reason for hiding this comment

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

OK.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 23, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 23, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot merged commit 48049ab into develop Aug 23, 2022
@mergify mergify bot deleted the l2-engine-rpc branch August 23, 2022 22:26
@mergify mergify bot removed the on-merge-train label Aug 23, 2022
maurelian pushed a commit that referenced this pull request Sep 15, 2022
* op-node: l2 rpc refactor

* op-node: implement l2 rpc review suggestions

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
sam-goldman pushed a commit that referenced this pull request Sep 15, 2022
* op-node: l2 rpc refactor

* op-node: implement l2 rpc review suggestions

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.

4 participants