Unify execution layer endpoints (#3214)

## Issue Addressed

Resolves #3069 

## Proposed Changes

Unify the `eth1-endpoints` and `execution-endpoints` flags in a backwards compatible way as described in https://github.com/sigp/lighthouse/issues/3069#issuecomment-1134219221

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 https://github.com/sigp/lighthouse/issues/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 https://github.com/ethereum/consensus-specs/issues/2163 and https://github.com/sigp/lighthouse/issues/2115


Co-authored-by: Paul Hauner <paul@paulhauner.com>
This commit is contained in:
Pawan Dhananjay
2022-06-29 09:07:09 +00:00
parent 53b2b500db
commit 5de00b7ee8
31 changed files with 1113 additions and 992 deletions

View File

@@ -409,45 +409,46 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("merge")
.long("merge")
.help("Enable the features necessary to run merge testnets. This feature \
is unstable and is for developers only.")
.takes_value(false),
.help("Deprecated. The feature activates automatically when --execution-endpoint \
is supplied.")
.takes_value(false)
)
.arg(
Arg::with_name("execution-endpoints")
.long("execution-endpoints")
.value_name("EXECUTION-ENDPOINTS")
.help("One or more comma-delimited server endpoints for HTTP JSON-RPC connection. \
If multiple endpoints are given the endpoints are used as fallback in the \
given order. Also enables the --merge flag. \
If this flag is omitted and the --eth1-endpoints is supplied, those values \
will be used. Defaults to http://127.0.0.1:8545.")
Arg::with_name("execution-endpoint")
.long("execution-endpoint")
.value_name("EXECUTION-ENDPOINT")
.alias("execution-endpoints")
.help("Server endpoint for an execution layer jwt authenticated HTTP \
JSON-RPC connection. Uses the same endpoint to populate the \
deposit cache. Also enables the --merge flag.\
If not provided, uses the default value of http://127.0.0.1:8551")
.takes_value(true)
.requires("execution-jwt")
)
.arg(
Arg::with_name("execution-jwt")
.long("execution-jwt")
.value_name("EXECUTION-JWT")
.alias("jwt-secrets")
.help("File path which contains the hex-encoded JWT secret for the \
execution endpoint provided in the --execution-endpoint flag.")
.takes_value(true)
)
.arg(
Arg::with_name("jwt-secrets")
.long("jwt-secrets")
.value_name("JWT-SECRETS")
.help("One or more comma-delimited file paths which contain the corresponding hex-encoded \
JWT secrets for each execution endpoint provided in the --execution-endpoints flag. \
The number of paths should be in the same order and strictly equal to the number \
of execution endpoints provided.")
.takes_value(true)
.requires("execution-endpoints")
)
.arg(
Arg::with_name("jwt-id")
.long("jwt-id")
.value_name("JWT-ID")
Arg::with_name("execution-jwt-id")
.long("execution-jwt-id")
.value_name("EXECUTION-JWT-ID")
.alias("jwt-id")
.help("Used by the beacon node to communicate a unique identifier to execution nodes \
during JWT authentication. It corresponds to the 'id' field in the JWT claims object.\
Set to empty by deafult")
.takes_value(true)
)
.arg(
Arg::with_name("jwt-version")
.long("jwt-version")
.value_name("JWT-VERSION")
Arg::with_name("execution-jwt-version")
.long("execution-jwt-version")
.value_name("EXECUTION-JWT-VERSION")
.alias("jwt-version")
.help("Used by the beacon node to communicate a client version to execution nodes \
during JWT authentication. It corresponds to the 'clv' field in the JWT claims object.\
Set to empty by deafult")
@@ -461,14 +462,15 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
collected from any blocks produced by this node. Defaults to a junk \
address whilst the merge is in development stages. THE DEFAULT VALUE \
WILL BE REMOVED BEFORE THE MERGE ENTERS PRODUCTION")
.requires("merge")
.requires("execution-endpoint")
.takes_value(true)
)
.arg(
Arg::with_name("payload-builders")
.long("payload-builders")
Arg::with_name("payload-builder")
.long("payload-builder")
.alias("payload-builders")
.help("The URL of a service compatible with the MEV-boost API.")
.requires("merge")
.requires("execution-endpoint")
.takes_value(true)
)

View File

@@ -3,12 +3,14 @@ use clap_utils::flags::DISABLE_MALLOC_TUNING_FLAG;
use client::{ClientConfig, ClientGenesis};
use directory::{DEFAULT_BEACON_NODE_DIR, DEFAULT_NETWORK_DIR, DEFAULT_ROOT_DIR};
use environment::RuntimeContext;
use genesis::Eth1Endpoint;
use http_api::TlsConfig;
use lighthouse_network::{multiaddr::Protocol, Enr, Multiaddr, NetworkConfig, PeerIdSerialized};
use sensitive_url::SensitiveUrl;
use slog::{info, warn, Logger};
use std::cmp;
use std::cmp::max;
use std::fmt::Debug;
use std::fs;
use std::net::{IpAddr, Ipv4Addr, ToSocketAddrs};
use std::path::{Path, PathBuf};
@@ -215,15 +217,18 @@ pub fn get_config<E: EthSpec>(
"msg" => "please use --eth1-endpoints instead"
);
client_config.sync_eth1_chain = true;
client_config.eth1.endpoints = vec![SensitiveUrl::parse(endpoint)
let endpoints = vec![SensitiveUrl::parse(endpoint)
.map_err(|e| format!("eth1-endpoint was an invalid URL: {:?}", e))?];
client_config.eth1.endpoints = Eth1Endpoint::NoAuth(endpoints);
} else if let Some(endpoints) = cli_args.value_of("eth1-endpoints") {
client_config.sync_eth1_chain = true;
client_config.eth1.endpoints = endpoints
let endpoints = endpoints
.split(',')
.map(SensitiveUrl::parse)
.collect::<Result<_, _>>()
.map_err(|e| format!("eth1-endpoints contains an invalid URL {:?}", e))?;
client_config.eth1.endpoints = Eth1Endpoint::NoAuth(endpoints);
}
if let Some(val) = cli_args.value_of("eth1-blocks-per-log-query") {
@@ -242,47 +247,79 @@ pub fn get_config<E: EthSpec>(
client_config.eth1.cache_follow_distance = Some(follow_distance);
}
if cli_args.is_present("merge") || cli_args.is_present("execution-endpoints") {
if cli_args.is_present("merge") {
if cli_args.is_present("execution-endpoint") {
warn!(
log,
"The --merge flag is deprecated";
"info" => "the --execution-endpoint flag automatically enables this feature"
)
} else {
return Err("The --merge flag is deprecated. \
Supply a value to --execution-endpoint instead."
.into());
}
}
if let Some(endpoints) = cli_args.value_of("execution-endpoint") {
let mut el_config = execution_layer::Config::default();
if let Some(endpoints) = cli_args.value_of("execution-endpoints") {
client_config.sync_eth1_chain = true;
el_config.execution_endpoints = endpoints
.split(',')
.map(SensitiveUrl::parse)
.collect::<Result<_, _>>()
.map_err(|e| format!("execution-endpoints contains an invalid URL {:?}", e))?;
} else if cli_args.is_present("merge") {
el_config.execution_endpoints = client_config.eth1.endpoints.clone();
}
if let Some(endpoints) = cli_args.value_of("payload-builders") {
el_config.builder_endpoints = endpoints
.split(',')
.map(SensitiveUrl::parse)
.collect::<Result<_, _>>()
.map_err(|e| format!("payload-builders contains an invalid URL {:?}", e))?;
}
if let Some(secrets) = cli_args.value_of("jwt-secrets") {
let secret_files: Vec<_> = secrets.split(',').map(PathBuf::from).collect();
if !secret_files.is_empty() && secret_files.len() != el_config.execution_endpoints.len()
{
return Err(format!(
"{} execution-endpoints supplied with {} jwt-secrets. Lengths \
must match or jwt-secrets must be empty.",
el_config.execution_endpoints.len(),
secret_files.len(),
));
}
el_config.secret_files = secret_files;
// Always follow the deposit contract when there is an execution endpoint.
//
// This is wasteful for non-staking nodes as they have no need to process deposit contract
// logs and build an "eth1" cache. The alternative is to explicitly require the `--eth1` or
// `--staking` flags, however that poses a risk to stakers since they cannot produce blocks
// without "eth1".
//
// The waste for non-staking nodes is relatively small so we err on the side of safety for
// stakers. The merge is already complicated enough.
client_config.sync_eth1_chain = true;
// Parse a single execution endpoint, logging warnings if multiple endpoints are supplied.
let execution_endpoint =
parse_only_one_value(endpoints, SensitiveUrl::parse, "--execution-endpoint", log)?;
// Parse a single JWT secret, logging warnings if multiple are supplied.
//
// JWTs are required if `--execution-endpoint` is supplied.
let secret_files: String = clap_utils::parse_required(cli_args, "execution-jwt")?;
let secret_file =
parse_only_one_value(&secret_files, PathBuf::from_str, "--execution-jwt", log)?;
// Parse and set the payload builder, if any.
if let Some(endpoints) = cli_args.value_of("payload-builder") {
let payload_builder =
parse_only_one_value(endpoints, SensitiveUrl::parse, "--payload-builder", log)?;
el_config.builder_endpoints = vec![payload_builder];
}
// Set config values from parse values.
el_config.secret_files = vec![secret_file.clone()];
el_config.execution_endpoints = vec![execution_endpoint.clone()];
el_config.suggested_fee_recipient =
clap_utils::parse_optional(cli_args, "suggested-fee-recipient")?;
el_config.jwt_id = clap_utils::parse_optional(cli_args, "jwt-id")?;
el_config.jwt_version = clap_utils::parse_optional(cli_args, "jwt-version")?;
el_config.jwt_id = clap_utils::parse_optional(cli_args, "execution-jwt-id")?;
el_config.jwt_version = clap_utils::parse_optional(cli_args, "execution-jwt-version")?;
el_config.default_datadir = client_config.data_dir.clone();
// If `--execution-endpoint` is provided, we should ignore any `--eth1-endpoints` values and
// use `--execution-endpoint` instead. Also, log a deprecation warning.
if cli_args.is_present("eth1-endpoints") || cli_args.is_present("eth1-endpoint") {
warn!(
log,
"Ignoring --eth1-endpoints flag";
"info" => "the value for --execution-endpoint will be used instead. \
--eth1-endpoints has been deprecated for post-merge configurations"
);
}
client_config.eth1.endpoints = Eth1Endpoint::Auth {
endpoint: execution_endpoint,
jwt_path: secret_file,
jwt_id: el_config.jwt_id.clone(),
jwt_version: el_config.jwt_version.clone(),
};
// Store the EL config in the client config.
client_config.execution_layer = Some(el_config);
}
@@ -344,7 +381,6 @@ pub fn get_config<E: EthSpec>(
client_config.eth1.follow_distance = spec.eth1_follow_distance;
client_config.eth1.node_far_behind_seconds =
max(5, spec.eth1_follow_distance / 2) * spec.seconds_per_eth1_block;
client_config.eth1.network_id = spec.deposit_network_id.into();
client_config.eth1.chain_id = spec.deposit_chain_id.into();
client_config.eth1.set_block_cache_truncation::<E>(spec);
@@ -844,3 +880,38 @@ pub fn get_slots_per_restore_point<E: EthSpec>(
Ok((default, false))
}
}
/// Parses the `cli_value` as a comma-separated string of values to be parsed with `parser`.
///
/// If there is more than one value, log a warning. If there are no values, return an error.
pub fn parse_only_one_value<F, T, E>(
cli_value: &str,
parser: F,
flag_name: &str,
log: &Logger,
) -> Result<T, String>
where
F: Fn(&str) -> Result<T, E>,
E: Debug,
{
let values = cli_value
.split(',')
.map(parser)
.collect::<Result<Vec<_>, _>>()
.map_err(|e| format!("{} contains an invalid value {:?}", flag_name, e))?;
if values.len() > 1 {
warn!(
log,
"Multiple values provided";
"info" => "multiple values are deprecated, only the first value will be used",
"count" => values.len(),
"flag" => flag_name
);
}
values
.into_iter()
.next()
.ok_or(format!("Must provide at least one value to {}", flag_name))
}