Remove redundancy from execution_layer
#3118
Labels
bellatrix
Required to support the Bellatrix Upgrade
Comments
This was referenced Jun 23, 2022
Closed
bors bot
pushed a commit
that referenced
this issue
Jun 29, 2022
## Issue Addressed Resolves #3069 ## Proposed Changes Unify the `eth1-endpoints` and `execution-endpoints` flags in a backwards compatible way as described in #3069 (comment) Users have 2 options: 1. Use multiple non auth execution endpoints for deposit processing pre-merge 2. Use a single jwt authenticated execution endpoint for both execution layer and deposit processing post merge Related #3118 To enable jwt authenticated deposit processing, this PR removes the calls to `net_version` as the `net` namespace is not exposed in the auth server in execution clients. Moving away from using `networkId` is a good step in my opinion as it doesn't provide us with any added guarantees over `chainId`. See ethereum/consensus-specs#2163 and #2115 Co-authored-by: Paul Hauner <[email protected]>
|
This has been achieved in #3214. There's still some redundancy code lingering in the background that will be cleaned up by #3284 and similar PRs. I'm going to close this since I think that restricting this feature from users is the must have task, will the corresponding clean-ups are nice to have (and already underway). |
bors bot
pushed a commit
that referenced
this issue
Jul 4, 2022
## Issue Addressed Part of #3118, continuation of #3257 ## Proposed Changes - the [ `first_success_without_retry` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L348-L351) function returns a single error. - the [`first_success`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L324) function returns a single error. - [ `EngineErrors` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/lib.rs#L69) carries a single error. - [`EngineError`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L173-L177) now does not need to carry an Id - [`process_multiple_payload_statuses`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/payload_status.rs#L46-L50) now doesn't need to receive an iterator of statuses and weight in different errors ## Additional Info This is built on top of #3294
bors bot
pushed a commit
that referenced
this issue
Jul 11, 2022
…#3284) ## Issue Addressed Part of #3118, continuation of #3257 and #3283 ## Proposed Changes - Merge the [`Engines`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L161-L165) struct and [`Engine` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L62-L67) - Remove unnecessary generics ## Additional Info There is more cleanup to do that will come in subsequent PRs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Overview
Presently, we have support for multiple EEs in the
execution_layercrate. Users specify these EEs via the--execution-enginesflag.I propose that we remove support for multiple EEs and only support a single EE.
Why
I added support for multiple EEs in the early days of merge specification in the interests of providing redundancy like we do with
--eth1-endpointson the BN or--beacon-nodeson the VC. As the spec has evolved, it's that redundancy for EEs isn't going to be the same as it is for BNs or "eth1" nodes.With redundant BN endpoints or Eth1 endpoints, users can have another endpoint "standing by" that's ready to serve requests should the primary endpoints failed. These "backup" endpoints, in my practical experience, are some third-party service (Infura, Alchemy, etc) or another self-hosted node that's shared between many nodes. Since an EE can only be driven by a single CE, this shared-backup is not feasible. Therefore, if a user has 5x BNs and wants redundancy with EEs, they need to run 5x EEs (rather than just 1x EE with a shared backup). For small-scale setups, there's no option to use centralized services (Infura, at the very least) as backups. For large-scale setups, having redundancy for many EEs is much more expensive that for BNs or Eth1 nodes. It's clear that redundancy for EEs is less appealing economically than for the existing places that we provide redundancy.
So, the benefits of redundancy with EEs is dubious.
Another reason we might use multiple EEs is because we want to compare the results of multiple EE implementations to detect consensus faults. I'd argue this is a taks for developers, not for users.
Proxies
Interestingly, we could support multiple EEs for whatever purposes (redundancy, consensus fault detection, etc) via an external prupose-built proxy. Point Lighthouse at the proxy and the proxy at multiple EEs. This would be suitable for all client implementations and remove the complexity from Lighthouse.
Summary
Since the benefits for multiple EEs isn't as clear as the other reasons we've added multiple endpoints for other services, and we can do the same thing with an external proxy, I propose we just go with the proxy approach. Whether or not we actually implement the proxy (or leave it up to someone else) is another question.
Another point to consider is that we need to add support for MEV solutions (see #3062). This means supporting another type of EE; one that should produce but not verify payloads. Add this complexity to supporting multiple EEs and we've got ourselves quite a lot of complexity.
The text was updated successfully, but these errors were encountered: