Add proxy RPC daemon by mslipper · Pull Request #1638 · ethereum-optimism/optimism

mslipper · 2021-10-27T07:13:34Z

Description

Adds a replacement for the Lua-based Nginx RPC router.

Additional context

The Lua-based router isn't flexible enough for our needs, so I offered to rewrite it in Go. This rewrite offers the following additional functionality over what we had before:

We can declaratively route RPC methods to different groups backends.
Failed requests to a backend will be retried up a configurable number of times.
Prometheus metrics.

I've included a full README as well as an annotated config file for reference. Below, I've excerpted the most important bits from the readme to make review easier:

Defining RPC routes in config:

[backends]
# A map of backends by name.
[backends.infura]
# The URL to contact the backend at.
base_url = "url-here"
# HTTP basic auth username to use with the backend.
username = ""
# HTTP basic auth password to use with the backend.
password = ""

[backend_groups]
# A map of backend groups by name.
[backend_groups.main]
# A list of backend names to place in the group.
backends = ["infura", "alchemy"]

[method_mappings]
# A mapping between RPC methods and the backend groups that should serve them.
eth_call = "main"
eth_chainId = "main"
# other mappings go here

Exposed metrics:

Name	Description	Flags
`proxyd_backend_requests_total`	Count of all successful requests to a backend.	backend_name: The name of the backend.
`proxyd_backend_errors_total`	Count of all backend errors.	backend_name: The name of the backend
`proxyd_http_requests_total`	Count of all HTTP requests, successful or not.
`proxyd_http_request_duration_histogram_seconds`	Histogram of HTTP request durations.
`proxyd_rpc_requests_total`	Count of all RPC requests.	method_name: The RPC method requested.
`proxyd_blocked_rpc_requests_total`	Count of all RPC requests with a blacklisted method.	method_name: The RPC method requested.
`proxyd_rpc_errors_total`	Count of all RPC errors. NOTE: Does not include errors sent from the backend to the client.

Requested feedback:

Code standards & structure
Are there any additional features we'd like to add?
What does everyone think of the routing config? Is it flexible enough for our needs?

Next steps:

If this PR looks good, I'll add some unit tests and do some load testing on a GCP instance. Once that's done I'll add it to k8s with Ben's approval.

changeset-bot · 2021-10-27T07:13:37Z

⚠️ No Changeset found

Latest commit: 3ec03a2

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

codecov-commenter · 2021-10-27T07:24:56Z

Codecov Report

Merging #1638 (094d475) into develop (2a7d347) will not change coverage.
The diff coverage is n/a.

❗ Current head 094d475 differs from pull request most recent head 3ec03a2. Consider uploading reports for the commit 3ec03a2 to get more accurate results

@@           Coverage Diff            @@
##           develop    #1638   +/-   ##
========================================
  Coverage    76.52%   76.52%           
========================================
  Files           82       82           
  Lines         3041     3041           
  Branches       466      466           
========================================
  Hits          2327     2327           
  Misses         714      714

Flag	Coverage Δ
batch-submitter	`61.74% <ø> (ø)`
contracts	`86.05% <ø> (ø)`
core-utils	`63.27% <ø> (ø)`
data-transport-layer	`37.86% <ø> (ø)`
message-relayer	`83.48% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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 2a7d347...3ec03a2. Read the comment docs.

tynes · 2021-10-27T16:05:59Z

Do you mind moving this into the go directory? So it would be go/rpc-proxy

tynes · 2021-10-27T16:16:15Z

rpc-proxy/server.go

Do you think that it would be easy to add some sort of fallback/retry logic? One common problem is that we send a request to one eth api provider and they are down and then we want to fall back to another eth api provider. This would prevent losing a connection to L1

Got it. Will update to retry the next backend in the list on failure rather than retry the current one.

rpc-proxy/backend.go

rpc-proxy/server.go

rpc-proxy/proxyd.go

tynes · 2021-10-27T16:38:07Z

Could you add this to the top level package.json .packages list? This will allow releases to be managed by changesets. Ideally our CI automatically detects new packages and does the same kind of publish logic for each, but for now we need to manually add new listings to the CI

optimism/package.json

Line 7 in fc0616e

"packages": [

rpc-proxy/package.json

rpc-proxy/proxyd.go

tynes · 2021-10-27T17:45:50Z

Code standards & structure

Generally looks good to me!

Are there any additional features we'd like to add?

I mentioned it in an above comment, but just the ability to fallback to the next backend in a backend group. This would make it seamless for when one backend goes down (altho this adds a consistency risk)
Mitigate potential consistency risks by sending requests to each backend in a backend group in parallel and then compare the results? Would use a lot of requests when it comes to infura/alchemy but we have self hosted nodes running and ideally we fallback to in infura/alchemy after trying our local node

What does everyone think of the routing config? Is it flexible enough for our needs?

The routing config looks flexible enough 👍

tynes

Left various comments requesting changes, will re-review when pinged

go/proxyd/cmd/proxyd/main.go

tynes · 2021-10-28T21:29:18Z

go/proxyd/proxyd.go

Realizing that this could leak API keys in the base_url since infura puts the api key in the path

tynes

Generally looks good to me

tynes · 2021-10-28T21:32:34Z

In a follow up PR we are going to need to add the CI publishing, see these commits:

The first one contains the logic and the next 2 are bugfixes

mslipper · 2021-10-29T00:07:04Z

Release configs are set up now.

tynes · 2021-10-29T00:17:36Z

Do you mind rebasing into a couple of commits and then I think this is ready for merging

mslipper · 2021-10-29T00:19:35Z

Rebase done.

mslipper · 2021-10-29T00:21:46Z

Oh wait you mean squash the commits, my bad. 1 moment.

mslipper · 2021-10-29T00:22:15Z

Ok, done for real this time.

mslipper requested review from optimisticben and tynes October 27, 2021 07:13