Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions crates/app/src/node/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,9 @@ pub(crate) async fn wire_p2p(

// TODO: also send the post-#4130 `Cluster-Uuid` header (relay-side load
// balancing), pending in pluto-p2p's `new_relays`.
let relay_addrs = bootnode::relay_addrs_for_resolution(&p2p_config.relays);
let relays = bootnode::new_relays(
cancellation.clone(),
&relay_addrs,
&p2p_config.relays,
&crate::utils::hex_7(&lock_hash),
)
.await?;
Expand Down
97 changes: 92 additions & 5 deletions crates/cli/src/commands/common.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! Shared helpers for CLI commands.

use std::str::FromStr;
use pluto_p2p::config::RelayAddr;
use tracing::warn;

use libp2p::{Multiaddr, multiaddr};
use crate::error::CliError;

/// Shared license notice shown by long-running commands.
pub const LICENSE: &str = concat!(
Expand Down Expand Up @@ -51,7 +52,93 @@ pub fn build_console_tracing_config(
builder.override_env_filter(level.into()).build()
}

/// Parses a relay string as either a relay URL or a raw multiaddr.
pub fn parse_relay_addr(relay: &str) -> std::result::Result<Multiaddr, libp2p::multiaddr::Error> {
multiaddr::from_url(relay).or_else(|_| Multiaddr::from_str(relay))
/// Parses the configured relay addresses, warning about insecure ones.
///
/// Exactly one empty value (`--p2p-relays=""`) means "no relays". That is the
/// only accepted empty form: every other empty is an error, including interior
/// ones (`a,,b`, `,`) and an empty value repeated or mixed with real addresses
/// (`--p2p-relays="" --p2p-relays=https://x`, which flattens to the same list
/// as `--p2p-relays=,https://x` and so cannot be told apart from it). Each of
/// those is a field that was meant to hold an address; dropping them would
/// silently leave fewer relays configured than requested.
pub fn parse_relay_addrs(relays: &[String]) -> std::result::Result<Vec<RelayAddr>, CliError> {
if let [only] = relays
&& only.is_empty()
{
return Ok(Vec::new());
}

let mut parsed = Vec::with_capacity(relays.len());

for relay in relays {
let addr: RelayAddr = relay.parse().map_err(|source| CliError::InvalidRelayAddr {
addr: relay.clone(),
source,
})?;

// Warn once per plain-http relay while validating flags, before the P2P
// stack starts resolving them.
if addr.is_insecure_url() {
warn!(address = %relay, "Insecure relay address provided, not HTTPS");
}

parsed.push(addr);
}

Ok(parsed)
}

#[cfg(test)]
mod tests {
use super::*;

// Per-address parsing is covered by `RelayAddr`'s own tests; what is left
// to check here is the empty-value contract and the error wrapping.

#[test]
fn treats_a_lone_empty_value_as_no_relays() {
// `--p2p-relays=""` is how relaying is turned off.
assert!(
parse_relay_addrs(&["".to_string()])
.expect("relays")
.is_empty()
);
assert!(parse_relay_addrs(&[]).expect("relays").is_empty());
}

#[test]
fn rejects_interior_empty_values() {
// `a,,b` splits to ["a", "", "b"] and `,` to ["", ""]. Dropping those
// empties would silently turn a typo'd flag into fewer relays, or none
// at all.
for relays in [
vec!["https://relay.one".to_string(), String::new()],
vec![
"https://relay.one".to_string(),
String::new(),
"https://relay.two".to_string(),
],
vec![String::new(), String::new()],
] {
let err = parse_relay_addrs(&relays).expect_err("empty entry should be rejected");

assert!(
err.to_string().contains("empty relay address"),
"unexpected error: {err}"
);
}
}

#[test]
fn rejects_invalid_relays() {
let err = parse_relay_addrs(&["not-an-address".to_string()])
.expect_err("invalid relay should be rejected");

// The offending address must be named; the old error was just
// "Invalid multiaddr: invalid multiaddr".
assert!(
err.to_string().contains("not-an-address"),
"unexpected error: {err}"
);
}
}
25 changes: 7 additions & 18 deletions crates/cli/src/commands/dkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@
use std::{future::Future, path::PathBuf};

use crate::{
commands::common::{ConsoleColor, LICENSE, build_console_tracing_config, parse_relay_addr},
commands::common::{ConsoleColor, LICENSE, build_console_tracing_config, parse_relay_addrs},
duration::Duration,
error::{CliError, Result},
};
use libp2p::multiaddr::Protocol;
use tokio_util::sync::CancellationToken;
use tracing::{info, warn};
use tracing::info;

/// Arguments for the `dkg` command.
#[derive(clap::Args, Clone, Debug)]
Expand Down Expand Up @@ -127,17 +126,7 @@ impl TryFrom<DkgArgs> for pluto_dkg::dkg::Config {
let tracing_config =
build_console_tracing_config(args.log.level.clone(), &args.log.color, None);
let p2p_config = {
let mut relays = Vec::new();

for relay in &args.p2p.relays {
let multiaddr = parse_relay_addr(relay)?;

if multiaddr.iter().any(|protocol| protocol == Protocol::Http) {
warn!(address = %relay, "Insecure relay address provided, not HTTPS");
}

relays.push(multiaddr);
}
let relays = parse_relay_addrs(&args.p2p.relays)?;

pluto_p2p::config::P2PConfig {
relays,
Expand Down Expand Up @@ -296,8 +285,8 @@ mod tests {
use super::*;
use crate::cli::{Cli, Commands};
use clap::Parser;
use libp2p::{Multiaddr, multiaddr};
use std::{str::FromStr, sync::Arc, time::Duration as StdDuration};
use pluto_p2p::config::RelayAddr;
use std::{sync::Arc, time::Duration as StdDuration};

#[test]
fn dkg_is_registered_as_top_level_subcommand() {
Expand Down Expand Up @@ -432,8 +421,8 @@ mod tests {
assert_eq!(
config.p2p.relays,
vec![
multiaddr::from_url("https://relay.one").expect("relay url"),
Multiaddr::from_str("/ip4/127.0.0.1/tcp/9000").expect("relay multiaddr")
RelayAddr::Url("https://relay.one".parse().expect("relay url")),
RelayAddr::Multiaddr("/ip4/127.0.0.1/tcp/9000".parse().expect("relay multiaddr")),
]
);
assert_eq!(config.p2p.external_ip.as_deref(), Some("1.2.3.4"));
Expand Down
18 changes: 2 additions & 16 deletions crates/cli/src/commands/relay.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::{
commands::common::{ConsoleColor, LICENSE, build_console_tracing_config, parse_relay_addr},
commands::common::{ConsoleColor, LICENSE, build_console_tracing_config, parse_relay_addrs},
error::CliError,
};
use libp2p::multiaddr::Protocol;
use pluto_p2p::k1;
use std::{collections::HashMap, path::PathBuf, time::Duration};
use tokio_util::sync::CancellationToken;
Expand Down Expand Up @@ -39,20 +38,7 @@ impl TryInto<pluto_relay_server::config::Config> for RelayArgs {

fn try_into(self) -> std::result::Result<pluto_relay_server::config::Config, Self::Error> {
let p2p_config = {
let mut relays = Vec::new();

for relay in &self.p2p.relays {
let multiaddr = parse_relay_addr(relay)?;

if multiaddr.iter().any(|protocol| protocol == Protocol::Http) {
tracing::warn!(
address = %relay,
"Insecure relay address provided, not HTTPS"
);
}

relays.push(multiaddr);
}
let relays = parse_relay_addrs(&self.p2p.relays)?;

pluto_p2p::config::P2PConfig {
relays,
Expand Down
56 changes: 37 additions & 19 deletions crates/cli/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,13 @@ use std::{
time::Duration as StdDuration,
};

use libp2p::multiaddr::Protocol;
use pluto_eth2util::helpers::validate_http_headers;
use pluto_featureset::{Feature, FeaturesetError, Status};
use tokio_util::sync::CancellationToken;
use tracing::{error, info, warn};

use crate::{
commands::common::{ConsoleColor, LICENSE, build_console_tracing_config, parse_relay_addr},
commands::common::{ConsoleColor, LICENSE, build_console_tracing_config, parse_relay_addrs},
duration::Duration,
error::{CliError, Result},
};
Expand Down Expand Up @@ -726,23 +725,7 @@ impl TryFrom<RunArgs> for RunConfig {
// --- p2p validation ---
validate_hostname(p2p.external_host.as_deref())?;

let mut relays = Vec::with_capacity(p2p.relays.len());
for relay in &p2p.relays {
// Charon treats `--p2p-relays=""` as "no relays"; clap's comma parser
// yields a single empty string, so skip empties to match (also handles
// stray empties like `a,,b`).
if relay.is_empty() {
continue;
}

let multiaddr = parse_relay_addr(relay)?;

if multiaddr.iter().any(|protocol| protocol == Protocol::Http) {
warn!(address = %relay, "Insecure relay address provided, not HTTPS");
}

relays.push(multiaddr);
}
let relays = parse_relay_addrs(&p2p.relays)?;

// --- run-level validation ---
if general.beacon_node_endpoints.is_empty() && !general.simnet_beacon_mock {
Expand Down Expand Up @@ -1497,6 +1480,41 @@ mod tests {
);
}

#[test]
fn run_accepts_relay_urls_with_a_path() {
// `http://relay:3640/enr` is the form the docker-compose relay serves;
// the path must reach the config intact rather than being rejected or
// truncated by a multiaddr round-trip.
let cases = [
"http://relay:3640/enr",
"http://relay:3640",
"https://relay.example.org/enr",
"/ip4/127.0.0.1/tcp/3610/p2p/16Uiu2HAm7ULrTMdiEmQCJ2N9nsuGvfUDvfDGgHXJ4vNjrCwCzGDs",
];

for case in cases {
let config =
parse_run(&[&format!("--p2p-relays={case}")]).expect("relay should be accepted");

assert_eq!(
config.p2p.relays,
vec![case.parse().expect("relay addr")],
"unexpected relays for {case}"
);
}
}

#[test]
fn run_rejects_invalid_relays() {
let err = parse_run(&["--p2p-relays=not-an-address"])
.expect_err("invalid relay should be rejected");

assert!(
err.to_string().contains("not-an-address"),
"unexpected error: {err}"
);
}

#[test]
fn run_simnet_beacon_mock_satisfies_beacon_requirement() {
let cli = Cli::try_parse_from(["pluto", "run", "--simnet-beacon-mock"])
Expand Down
Loading
Loading