feat(beacon): --builder and register_validator

bin/ethlambda/src/cli.rs

@@ -307,6 +307,44 @@ pub(crate) struct MainnetOptions {
     /// `eth-clients` publishes and kurtosis mounts at `/network-configs`.
     #[arg(long, default_value = crate::network::DEFAULT_NETWORK)]
     pub(crate) network: String,
+
+    /// Base URL of a builder API endpoint, normally MEV-Boost (for example
+    /// `http://127.0.0.1:18550`) or a relay directly. Without it every block
+    /// is built locally, and `register_validator` answers 500 so a validator
+    /// client configured for a builder hears that it has none (the fee
+    /// recipients it names are still recorded).
+    #[arg(long, value_parser = parse_builder_url)]
+    pub(crate) builder: Option<String>,
+
+    /// Follow the builder's 307 and 308 redirects, up to ten per call and
+    /// never from https to http. A 301, 302 or 303 is not followed even then:
+    /// following one resends a POST as a GET without its body.
+    ///
+    /// Off by default, so registrations and signed blocks go only to the URL
+    /// `--builder` names. A redirect not followed fails the call, with an
+    /// error naming where it pointed.
+    #[arg(long, requires = "builder")]
+    pub(crate) builder_follow_redirects: bool,
+
+    /// How long block production waits for the builder's bid, in
+    /// milliseconds. Also sent to the builder as `X-Timeout-Ms`, so it can
+    /// hold its answer for as long as this node will wait.
+    #[arg(
+        long,
+        default_value_t = 1000,
+        requires = "builder",
+        value_parser = clap::value_parser!(u64).range(1..),
+    )]
+    pub(crate) builder_header_timeout: u64,
+}
+
+/// `--builder`'s value, refused while the flags are parsed unless it is an
+/// http or https URL: the same check the builder client makes, so the error
+/// names the flag instead of surfacing at startup.
+fn parse_builder_url(value: &str) -> Result<String, String> {
+    ethlambda_rpc::BuilderClient::check_url(value)
+        .map(|()| value.to_owned())
+        .map_err(|err| err.to_string())
 }
 
 /// Flags only the lean chain takes.
@@ -1020,6 +1058,91 @@ mod tests {
         );
     }
 
+    #[test]
+    fn the_builder_is_off_by_default() {
+        let beacon = parse_beacon(beacon_args());
+
+        assert_eq!(beacon.mainnet.builder, None);
+        assert_eq!(beacon.mainnet.builder_header_timeout, 1000);
+    }
+
+    #[test]
+    fn the_builder_flags_parse() {
+        let mut args = beacon_args();
+        args.extend([
+            "--builder",
+            "http://127.0.0.1:18550",
+            "--builder-header-timeout",
+            "750",
+        ]);
+
+        let options: Options = parse_beacon(args).into();
+
+        match options.network {
+            Network::Mainnet { mainnet, .. } => {
+                assert_eq!(mainnet.builder.as_deref(), Some("http://127.0.0.1:18550"));
+                assert_eq!(mainnet.builder_header_timeout, 750);
+            }
+            other => panic!("expected the mainnet network, got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn a_zero_header_timeout_is_refused() {
+        let mut args = beacon_args();
+        args.extend([
+            "--builder",
+            "http://127.0.0.1:18550",
+            "--builder-header-timeout",
+            "0",
+        ]);
+
+        try_parse_from(args).expect_err("a zero timeout must not parse");
+    }
+
+    #[test]
+    fn a_header_timeout_needs_a_builder() {
+        let mut args = beacon_args();
+        args.extend(["--builder-header-timeout", "750"]);
+
+        try_parse_from(args).expect_err("a timeout without a builder must not parse");
+    }
+
+    #[test]
+    fn a_builder_url_is_checked_while_parsing() {
+        for bad in ["not a url", "localhost:18550"] {
+            let mut args = beacon_args();
+            args.extend(["--builder", bad]);
+
+            let err = try_parse_from(args).expect_err("a bad builder url must not parse");
+
+            assert!(err.to_string().contains("--builder"), "{err}");
+        }
+    }
+
+    #[test]
+    fn redirects_are_off_unless_asked_for() {
+        let mut args = beacon_args();
+        args.extend(["--builder", "http://127.0.0.1:18550"]);
+        assert!(!parse_beacon(args).mainnet.builder_follow_redirects);
+
+        let mut args = beacon_args();
+        args.extend([
+            "--builder",
+            "http://127.0.0.1:18550",
+            "--builder-follow-redirects",
+        ]);
+        assert!(parse_beacon(args).mainnet.builder_follow_redirects);
+    }
+
+    #[test]
+    fn following_redirects_needs_a_builder() {
+        let mut args = beacon_args();
+        args.push("--builder-follow-redirects");
+
+        try_parse_from(args).expect_err("following redirects without a builder must not parse");
+    }
+
     #[test]
     fn both_execution_flags_together_pair_into_one_value() {
         let mut args = beacon_args();

bin/ethlambda/src/main.rs

@@ -454,6 +454,38 @@ async fn run_node(options: Options) -> eyre::Result<()> {
         Network::Mainnet { .. } => false,
     });
 
+    // A malformed URL fails here, at startup, rather than at the first
+    // proposal; an unreachable builder only warns, since it may come up
+    // after this node does.
+    let builder = match &network {
+        Network::Lean(_) => None,
+        Network::Mainnet { mainnet, .. } => mainnet
+            .builder
+            .as_deref()
+            .map(|url| {
+                ethlambda_rpc::BuilderClient::with_redirects(
+                    url,
+                    std::time::Duration::from_millis(mainnet.builder_header_timeout),
+                    version::client_version(),
+                    mainnet.builder_follow_redirects,
+                )
+            })
+            .transpose()?,
+    };
+    // A one-off probe, for the log only: nothing waits on it or reads its
+    // answer, and every later call to the builder is tried on its own.
+    if let Some(builder) = builder.clone() {
+        tokio::spawn(async move {
+            let _ = builder
+                .status()
+                .await
+                .inspect(|()| info!("Builder is up"))
+                .inspect_err(|err| {
+                    warn!(%err, "Builder did not answer the startup status check; each call to it is still tried")
+                });
+        });
+    }
+
     // Shared, runtime-readable sync status. The blockchain actor writes it each
     // tick (alongside the `lean_node_sync_status` metric); the RPC
     // `/lean/v0/node/syncing` endpoint reads it. Seeded to Idle, matching the
@@ -848,6 +880,7 @@ async fn run_node(options: Options) -> eyre::Result<()> {
                     events: rpc_events,
                     client_version: version::engine_client_version(),
                     identity: rpc_identity,
+                    builder,
                 },
                 local_peer_id,
                 rpc_shutdown,

crates/net/rpc/src/beacon/builder_client/error.rs

@@ -2,7 +2,8 @@
 
 use std::time::Duration;
 
-use reqwest::StatusCode;
+use reqwest::header;
+use reqwest::{StatusCode, Url};
 
 use super::{read_body, truncate_error_text};
 
@@ -26,6 +27,15 @@ pub enum BuilderError {
     /// A status the call does not expect.
     #[error("builder answered {status}: {body}")]
     Status { status: u16, body: String },
+    /// A redirect the client did not follow: any redirect without
+    /// `--builder-follow-redirects`, and with it a 301, 302 or 303, an https
+    /// to http downgrade, or the hop past the tenth.
+    #[error(
+        "builder redirected the request to {location} ({status}), which was not followed; \
+         only --builder-follow-redirects follows a redirect, and then only a 307 or 308, \
+         up to ten, never from https to http"
+    )]
+    Redirected { status: u16, location: String },
     /// A body that does not decode as what the call returns.
     #[error("builder response did not decode: {0}")]
     Decode(String),
@@ -60,8 +70,15 @@ impl BuilderError {
 
     /// The error a status other than the call's success stands for. A 400
     /// carries the builder's `message`, or its raw body if it sent no JSON.
+    /// A redirect carries its target.
     pub(super) async fn from_status(response: reqwest::Response, deadline: Duration) -> Self {
         let status = response.status();
+        if is_redirect(status) {
+            return Self::Redirected {
+                status: status.as_u16(),
+                location: shown_location(&response),
+            };
+        }
         let body = read_body(response, deadline).await.unwrap_or_default();
         let body = String::from_utf8_lossy(&body).into_owned();
         if status != StatusCode::BAD_REQUEST {
@@ -83,6 +100,7 @@ impl BuilderError {
             Self::Transport(_) => "unreachable",
             Self::BadRequest(_) => "bad_request",
             Self::Status { .. } => "unexpected_status",
+            Self::Redirected { .. } => "redirected",
             Self::Decode(_) | Self::Incomplete(_) | Self::TooLarge { .. } => "invalid_response",
             Self::InvalidUrl(_) => "invalid_url",
             Self::UnsupportedFork(_) => "unsupported_fork",
@@ -91,6 +109,62 @@ impl BuilderError {
     }
 }
 
+/// Whether `status` is a redirect: one of the five statuses a client
+/// follows, and the only ones reqwest consults its redirect policy on. Any
+/// other 3xx (a 304 answering a conditional request, a 300 offering choices)
+/// sends the client nowhere, so it is an ordinary unexpected status.
+fn is_redirect(status: StatusCode) -> bool {
+    matches!(
+        status,
+        StatusCode::MOVED_PERMANENTLY
+            | StatusCode::FOUND
+            | StatusCode::SEE_OTHER
+            | StatusCode::TEMPORARY_REDIRECT
+            | StatusCode::PERMANENT_REDIRECT
+    )
+}
+
+/// What a redirect shows when its target cannot be shown safely.
+const UNSHOWN_LOCATION: &str = "a Location that does not resolve to a URL with a host";
+
+/// A redirect's target as an error shows it.
+fn shown_location(response: &reqwest::Response) -> String {
+    let Some(value) = response.headers().get(header::LOCATION) else {
+        return "nowhere (no Location header)".to_owned();
+    };
+    shown_target(response.url(), value.as_bytes())
+}
+
+/// `location` resolved against `base`, the URL that answered with it, as a
+/// client following it would, then shown without any user or password and
+/// cut to the error text cap: the error ends up in logs and response bodies
+/// (see `from_reqwest`).
+///
+/// Never the raw value. `//user:pass@host/x` is a relative reference, so it
+/// does not parse as a URL on its own, yet it carries credentials; resolved,
+/// it is an absolute URL whose credentials can be removed. Resolving also
+/// strips any `--builder` credentials a relative target would inherit from
+/// `base`. A value that does not resolve, or resolves to a URL with no host
+/// to hold credentials (`mailto:`), is shown as a fixed placeholder.
+pub(super) fn shown_target(base: &Url, location: &[u8]) -> String {
+    let location = String::from_utf8_lossy(location);
+    base.join(&location)
+        .ok()
+        .and_then(without_credentials)
+        .map_or_else(
+            || UNSHOWN_LOCATION.to_owned(),
+            |url| truncate_error_text(url.into()),
+        )
+}
+
+/// `url` without its user and password, or `None` when it cannot hold them
+/// and so cannot have them removed either.
+pub(super) fn without_credentials(mut url: Url) -> Option<Url> {
+    url.set_username("").ok()?;
+    url.set_password(None).ok()?;
+    Some(url)
+}
+
 /// builder-specs' `ErrorMessage`, of which only the message is kept.
 #[derive(serde::Deserialize)]
 struct ErrorMessage {