From 565c322d5a0234fc81f8277df45efd754e921aa3 Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 19 May 2021 11:30:03 +1000 Subject: [PATCH 01/13] Adjust timeout duration for VC attestation duties --- account_manager/src/validator/exit.rs | 1 + beacon_node/http_api/tests/tests.rs | 3 + common/eth2/src/lib.rs | 81 +++++++++++++++++++++++++-- testing/node_test_rig/src/lib.rs | 3 + validator_client/src/fork_service.rs | 1 + validator_client/src/lib.rs | 9 +-- 6 files changed, 88 insertions(+), 10 deletions(-) diff --git a/account_manager/src/validator/exit.rs b/account_manager/src/validator/exit.rs index 6c15615af46..58738a2b2d4 100644 --- a/account_manager/src/validator/exit.rs +++ b/account_manager/src/validator/exit.rs @@ -81,6 +81,7 @@ pub fn cli_run(matches: &ArgMatches, env: Environment) -> Result< let client = BeaconNodeHttpClient::new( SensitiveUrl::parse(&server_url) .map_err(|e| format!("Failed to parse beacon http server: {:?}", e))?, + env.eth2_config.spec.seconds_per_slot, ); let testnet_config = env diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index fb9e74cec17..d880a3d686e 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -37,6 +37,7 @@ use types::{ type E = MainnetEthSpec; +const SECONDS_PER_SLOT: u64 = 12; const SLOTS_PER_EPOCH: u64 = 32; const VALIDATOR_COUNT: usize = SLOTS_PER_EPOCH as usize; const CHAIN_LENGTH: u64 = SLOTS_PER_EPOCH * 5 - 1; // Make `next_block` an epoch transition @@ -213,6 +214,7 @@ impl ApiTester { listening_socket.port() )) .unwrap(), + SECONDS_PER_SLOT, ); Self { @@ -325,6 +327,7 @@ impl ApiTester { listening_socket.port() )) .unwrap(), + SECONDS_PER_SLOT, ); Self { diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 5e1b434601d..cfc31344ad3 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -25,6 +25,12 @@ use ssz::Decode; use std::convert::TryFrom; use std::fmt; use std::iter::Iterator; +use std::time::Duration; + +/// A specific timeout ratio for HTTP requests involved in producing attestations. +/// This can help reduce missed attestations that occur when an endpoint fallback occurs. +/// A value of 3 would be a timeout of 1/3 the slots_per_second (rounded down) of the network. +const HTTP_ATTESTATION_TIMEOUT_RATIO: u64 = 3; #[derive(Debug)] pub enum Error { @@ -83,6 +89,7 @@ impl fmt::Display for Error { pub struct BeaconNodeHttpClient { client: reqwest::Client, server: SensitiveUrl, + seconds_per_slot: u64, } impl fmt::Display for BeaconNodeHttpClient { @@ -98,15 +105,24 @@ impl AsRef for BeaconNodeHttpClient { } impl BeaconNodeHttpClient { - pub fn new(server: SensitiveUrl) -> Self { + pub fn new(server: SensitiveUrl, seconds_per_slot: u64) -> Self { Self { client: reqwest::Client::new(), server, + seconds_per_slot, } } - pub fn from_components(server: SensitiveUrl, client: reqwest::Client) -> Self { - Self { client, server } + pub fn from_components( + server: SensitiveUrl, + client: reqwest::Client, + seconds_per_slot: u64, + ) -> Self { + Self { + client, + server, + seconds_per_slot, + } } /// Return the path with the standard `/eth1/v1` prefix applied. @@ -131,6 +147,26 @@ impl BeaconNodeHttpClient { .map_err(Error::Reqwest) } + /// Perform a HTTP GET request with a custom timeout. + async fn get_with_timeout( + &self, + url: U, + timeout: Duration, + ) -> Result { + let response = self + .client + .get(url) + .timeout(timeout) + .send() + .await + .map_err(Error::Reqwest)?; + ok_or_error(response) + .await? + .json() + .await + .map_err(Error::Reqwest) + } + /// Perform a HTTP GET request, returning `None` on a 404 error. async fn get_opt(&self, url: U) -> Result, Error> { let response = self.client.get(url).send().await.map_err(Error::Reqwest)?; @@ -146,6 +182,31 @@ impl BeaconNodeHttpClient { } } + /// Perform a HTTP GET request with a custom timeout, returning `None` on a 404 error. + async fn get_opt_with_timeout( + &self, + url: U, + timeout: Duration, + ) -> Result, Error> { + let response = self + .client + .get(url) + .timeout(timeout) + .send() + .await + .map_err(Error::Reqwest)?; + match ok_or_error(response).await { + Ok(resp) => resp.json().await.map(Option::Some).map_err(Error::Reqwest), + Err(err) => { + if err.status() == Some(StatusCode::NOT_FOUND) { + Ok(None) + } else { + Err(err) + } + } + } + } + /// Perform a HTTP GET request using an 'accept' header, returning `None` on a 404 error. pub async fn get_bytes_opt_accept_header( &self, @@ -567,6 +628,9 @@ impl BeaconNodeHttpClient { let response = self .client .post(path) + .timeout(Duration::from_secs( + self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO, + )) .json(attestations) .send() .await @@ -974,10 +1038,11 @@ impl BeaconNodeHttpClient { .append_pair("slot", &slot.to_string()) .append_pair("committee_index", &committee_index.to_string()); - self.get(path).await + let timeout = Duration::from_secs(self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO); + self.get_with_timeout(path, timeout).await } - /// `GET validator/attestation_attestation?slot,attestation_data_root` + /// `GET validator/aggregate_attestation?slot,attestation_data_root` pub async fn get_validator_aggregate_attestation( &self, slot: Slot, @@ -997,7 +1062,8 @@ impl BeaconNodeHttpClient { &format!("{:?}", attestation_data_root), ); - self.get_opt(path).await + let timeout = Duration::from_secs(self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO); + self.get_opt_with_timeout(path, timeout).await } /// `POST validator/duties/attester/{epoch}` @@ -1033,6 +1099,9 @@ impl BeaconNodeHttpClient { let response = self .client .post(path) + .timeout(Duration::from_secs( + self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO, + )) .json(aggregates) .send() .await diff --git a/testing/node_test_rig/src/lib.rs b/testing/node_test_rig/src/lib.rs index 436b852c252..28fd02c303f 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -19,6 +19,8 @@ pub use environment; pub use eth2; pub use validator_client::Config as ValidatorConfig; +const SECONDS_PER_SLOT: u64 = 12; + /// The global timeout for HTTP requests to the beacon node. const HTTP_TIMEOUT: Duration = Duration::from_secs(4); @@ -77,6 +79,7 @@ impl LocalBeaconNode { Ok(BeaconNodeHttpClient::from_components( beacon_node_url, beacon_node_http_client, + SECONDS_PER_SLOT, )) } } diff --git a/validator_client/src/fork_service.rs b/validator_client/src/fork_service.rs index 487038f0607..66e67f2ba52 100644 --- a/validator_client/src/fork_service.rs +++ b/validator_client/src/fork_service.rs @@ -85,6 +85,7 @@ impl ForkServiceBuilder { ); let candidates = vec![CandidateBeaconNode::new(eth2::BeaconNodeHttpClient::new( sensitive_url::SensitiveUrl::parse("http://127.0.0.1").unwrap(), + 12, ))]; let mut beacon_nodes = BeaconNodeFallback::new(candidates, spec, log.clone()); beacon_nodes.set_slot_clock(slot_clock); diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 0f462aca7e2..a6b1a55c979 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -57,9 +57,6 @@ const RETRY_DELAY: Duration = Duration::from_secs(2); /// The time between polls when waiting for genesis. const WAITING_FOR_GENESIS_POLL_TIME: Duration = Duration::from_secs(12); -/// The global timeout for HTTP requests to the beacon node. -const HTTP_TIMEOUT: Duration = Duration::from_secs(12); - #[derive(Clone)] pub struct ProductionValidatorClient { context: RuntimeContext, @@ -228,12 +225,16 @@ impl ProductionValidatorClient { .into_iter() .map(|url| { let beacon_node_http_client = ClientBuilder::new() - .timeout(HTTP_TIMEOUT) + // Set default timeout to be the full slot duration. + .timeout(Duration::from_secs( + context.eth2_config.spec.seconds_per_slot, + )) .build() .map_err(|e| format!("Unable to build HTTP client: {:?}", e))?; Ok(BeaconNodeHttpClient::from_components( url, beacon_node_http_client, + context.eth2_config.spec.seconds_per_slot, )) }) .collect::, String>>()?; From d0af27ae844944b37a6dacc8914f0c1359fcb8cc Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 19 May 2021 12:30:40 +1000 Subject: [PATCH 02/13] Change timeouts to milliseconds for more control --- common/eth2/src/lib.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index cfc31344ad3..eb4454f4d06 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -29,7 +29,7 @@ use std::time::Duration; /// A specific timeout ratio for HTTP requests involved in producing attestations. /// This can help reduce missed attestations that occur when an endpoint fallback occurs. -/// A value of 3 would be a timeout of 1/3 the slots_per_second (rounded down) of the network. +/// A value of 3 would be a timeout of 1/3 the milliseconds per slot (rounded down) of the network. const HTTP_ATTESTATION_TIMEOUT_RATIO: u64 = 3; #[derive(Debug)] @@ -628,8 +628,8 @@ impl BeaconNodeHttpClient { let response = self .client .post(path) - .timeout(Duration::from_secs( - self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO, + .timeout(Duration::from_millis( + (self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO, )) .json(attestations) .send() @@ -1038,7 +1038,8 @@ impl BeaconNodeHttpClient { .append_pair("slot", &slot.to_string()) .append_pair("committee_index", &committee_index.to_string()); - let timeout = Duration::from_secs(self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO); + let timeout = + Duration::from_millis((self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO); self.get_with_timeout(path, timeout).await } @@ -1062,7 +1063,8 @@ impl BeaconNodeHttpClient { &format!("{:?}", attestation_data_root), ); - let timeout = Duration::from_secs(self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO); + let timeout = + Duration::from_millis((self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO); self.get_opt_with_timeout(path, timeout).await } @@ -1099,8 +1101,8 @@ impl BeaconNodeHttpClient { let response = self .client .post(path) - .timeout(Duration::from_secs( - self.seconds_per_slot / HTTP_ATTESTATION_TIMEOUT_RATIO, + .timeout(Duration::from_millis( + (self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO, )) .json(aggregates) .send() From 34181f6750b1027e8956f87f4ea6fc98c79ba5d5 Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 2 Jun 2021 15:40:07 +1000 Subject: [PATCH 03/13] Optimize timeouts by using a dedicated struct --- account_manager/src/validator/exit.rs | 4 +- common/eth2/src/lib.rs | 94 ++++++++++++++++++++------- testing/node_test_rig/src/lib.rs | 10 ++- validator_client/src/cli.rs | 6 ++ validator_client/src/config.rs | 4 ++ validator_client/src/fork_service.rs | 2 +- validator_client/src/lib.rs | 42 ++++++++++-- 7 files changed, 123 insertions(+), 39 deletions(-) diff --git a/account_manager/src/validator/exit.rs b/account_manager/src/validator/exit.rs index 58738a2b2d4..aa305679dde 100644 --- a/account_manager/src/validator/exit.rs +++ b/account_manager/src/validator/exit.rs @@ -4,7 +4,7 @@ use clap::{App, Arg, ArgMatches}; use environment::Environment; use eth2::{ types::{GenesisData, StateId, ValidatorData, ValidatorId, ValidatorStatus}, - BeaconNodeHttpClient, + BeaconNodeHttpClient, Timeouts, }; use eth2_keystore::Keystore; use eth2_network_config::Eth2NetworkConfig; @@ -81,7 +81,7 @@ pub fn cli_run(matches: &ArgMatches, env: Environment) -> Result< let client = BeaconNodeHttpClient::new( SensitiveUrl::parse(&server_url) .map_err(|e| format!("Failed to parse beacon http server: {:?}", e))?, - env.eth2_config.spec.seconds_per_slot, + Timeouts::default(env.eth2_config.spec.seconds_per_slot), ); let testnet_config = env diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index eb4454f4d06..b4f9a092bbc 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -27,11 +27,6 @@ use std::fmt; use std::iter::Iterator; use std::time::Duration; -/// A specific timeout ratio for HTTP requests involved in producing attestations. -/// This can help reduce missed attestations that occur when an endpoint fallback occurs. -/// A value of 3 would be a timeout of 1/3 the milliseconds per slot (rounded down) of the network. -const HTTP_ATTESTATION_TIMEOUT_RATIO: u64 = 3; - #[derive(Debug)] pub enum Error { /// The `reqwest` client raised an error. @@ -83,13 +78,44 @@ impl fmt::Display for Error { } } +/// A struct to define a variety of different timeouts for different validator tasks to ensure +/// proper fallback behaviour. +#[derive(Clone)] +pub struct Timeouts { + attestation: Duration, + attester_duties: Duration, + proposal: Duration, + proposer_duties: Duration, +} + +impl Timeouts { + pub fn new( + attestation: Duration, + attester_duties: Duration, + proposal: Duration, + proposer_duties: Duration, + ) -> Self { + Timeouts { + attestation, + attester_duties, + proposal, + proposer_duties, + } + } + + pub fn default(seconds_per_slot: u64) -> Self { + let timeout = Duration::from_secs(seconds_per_slot); + Timeouts::new(timeout, timeout, timeout, timeout) + } +} + /// A wrapper around `reqwest::Client` which provides convenience methods for interfacing with a /// Lighthouse Beacon Node HTTP server (`http_api`). #[derive(Clone)] pub struct BeaconNodeHttpClient { client: reqwest::Client, server: SensitiveUrl, - seconds_per_slot: u64, + timeouts: Timeouts, } impl fmt::Display for BeaconNodeHttpClient { @@ -105,23 +131,23 @@ impl AsRef for BeaconNodeHttpClient { } impl BeaconNodeHttpClient { - pub fn new(server: SensitiveUrl, seconds_per_slot: u64) -> Self { + pub fn new(server: SensitiveUrl, timeouts: Timeouts) -> Self { Self { client: reqwest::Client::new(), server, - seconds_per_slot, + timeouts, } } pub fn from_components( server: SensitiveUrl, client: reqwest::Client, - seconds_per_slot: u64, + timeouts: Timeouts, ) -> Self { Self { client, server, - seconds_per_slot, + timeouts, } } @@ -251,15 +277,36 @@ impl BeaconNodeHttpClient { Ok(()) } - /// Perform a HTTP POST request, returning a JSON response. - async fn post_with_response( + /// Perform a HTTP POST request with a custom timeout. + async fn post_with_timeout( + &self, + url: U, + body: &T, + timeout: Duration, + ) -> Result<(), Error> { + let response = self + .client + .post(url) + .timeout(timeout) + .json(body) + .send() + .await + .map_err(Error::Reqwest)?; + ok_or_error(response).await?; + Ok(()) + } + + /// Perform a HTTP POST request with a custom timeout, returning a JSON response. + async fn post_with_timeout_and_response( &self, url: U, body: &V, + timeout: Duration, ) -> Result { let response = self .client .post(url) + .timeout(timeout) .json(body) .send() .await @@ -530,7 +577,8 @@ impl BeaconNodeHttpClient { .push("beacon") .push("blocks"); - self.post(path, block).await?; + self.post_with_timeout(path, block, self.timeouts.proposal) + .await?; Ok(()) } @@ -628,9 +676,7 @@ impl BeaconNodeHttpClient { let response = self .client .post(path) - .timeout(Duration::from_millis( - (self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO, - )) + .timeout(self.timeouts.attestation) .json(attestations) .send() .await @@ -992,7 +1038,8 @@ impl BeaconNodeHttpClient { .push("proposer") .push(&epoch.to_string()); - self.get(path).await + self.get_with_timeout(path, self.timeouts.proposer_duties) + .await } /// `GET validator/blocks/{slot}` @@ -1038,8 +1085,7 @@ impl BeaconNodeHttpClient { .append_pair("slot", &slot.to_string()) .append_pair("committee_index", &committee_index.to_string()); - let timeout = - Duration::from_millis((self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO); + let timeout = self.timeouts.attestation; self.get_with_timeout(path, timeout).await } @@ -1063,8 +1109,7 @@ impl BeaconNodeHttpClient { &format!("{:?}", attestation_data_root), ); - let timeout = - Duration::from_millis((self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO); + let timeout = self.timeouts.attestation; self.get_opt_with_timeout(path, timeout).await } @@ -1083,7 +1128,8 @@ impl BeaconNodeHttpClient { .push("attester") .push(&epoch.to_string()); - self.post_with_response(path, &indices).await + self.post_with_timeout_and_response(path, &indices, self.timeouts.attester_duties) + .await } /// `POST validator/aggregate_and_proofs` @@ -1101,9 +1147,7 @@ impl BeaconNodeHttpClient { let response = self .client .post(path) - .timeout(Duration::from_millis( - (self.seconds_per_slot * 1_000) / HTTP_ATTESTATION_TIMEOUT_RATIO, - )) + .timeout(self.timeouts.attestation) .json(aggregates) .send() .await diff --git a/testing/node_test_rig/src/lib.rs b/testing/node_test_rig/src/lib.rs index 28fd02c303f..e1a296846e8 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -4,7 +4,7 @@ use beacon_node::ProductionBeaconNode; use environment::RuntimeContext; -use eth2::{reqwest::ClientBuilder, BeaconNodeHttpClient}; +use eth2::{reqwest::ClientBuilder, BeaconNodeHttpClient, Timeouts}; use sensitive_url::SensitiveUrl; use std::path::PathBuf; use std::time::Duration; @@ -19,10 +19,8 @@ pub use environment; pub use eth2; pub use validator_client::Config as ValidatorConfig; -const SECONDS_PER_SLOT: u64 = 12; - /// The global timeout for HTTP requests to the beacon node. -const HTTP_TIMEOUT: Duration = Duration::from_secs(4); +const HTTP_TIMEOUT: u64 = 4; /// Provides a beacon node that is running in the current process on a given tokio executor (it /// is _local_ to this process). @@ -73,13 +71,13 @@ impl LocalBeaconNode { ) .map_err(|e| format!("Unable to parse beacon node URL: {:?}", e))?; let beacon_node_http_client = ClientBuilder::new() - .timeout(HTTP_TIMEOUT) + .timeout(Duration::from_secs(HTTP_TIMEOUT)) .build() .map_err(|e| format!("Unable to build HTTP client: {:?}", e))?; Ok(BeaconNodeHttpClient::from_components( beacon_node_url, beacon_node_http_client, - SECONDS_PER_SLOT, + Timeouts::default(HTTP_TIMEOUT), )) } } diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 13e4f4e022b..d4c4dc3198f 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -94,6 +94,12 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { node is not synced.", ), ) + .arg( + Arg::with_name("use-long-timeouts") + .long("use-long-timeouts") + .help("If present, the validator client will use longer timeouts for requests \ + made to the beacon node.") + ) // This overwrites the graffiti configured in the beacon node. .arg( Arg::with_name("graffiti") diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index cb5c862dae5..d9a5d7b411e 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -35,6 +35,8 @@ pub struct Config { pub disable_auto_discover: bool, /// If true, re-register existing validators in definitions.yml for slashing protection. pub init_slashing_protection: bool, + /// if true, use longer timeouts for requests made to the beacon node. + pub use_long_timeouts: bool, /// Graffiti to be inserted everytime we create a block. pub graffiti: Option, /// Graffiti file to load per validator graffitis. @@ -68,6 +70,7 @@ impl Default for Config { allow_unsynced_beacon_node: false, disable_auto_discover: false, init_slashing_protection: false, + use_long_timeouts: false, graffiti: None, graffiti_file: None, http_api: <_>::default(), @@ -156,6 +159,7 @@ impl Config { config.allow_unsynced_beacon_node = cli_args.is_present("allow-unsynced"); config.disable_auto_discover = cli_args.is_present("disable-auto-discover"); config.init_slashing_protection = cli_args.is_present("init-slashing-protection"); + config.use_long_timeouts = cli_args.is_present("use-long-timeouts"); if let Some(graffiti_file_path) = cli_args.value_of("graffiti-file") { let mut graffiti_file = GraffitiFile::new(graffiti_file_path.into()); diff --git a/validator_client/src/fork_service.rs b/validator_client/src/fork_service.rs index 66e67f2ba52..a58870867f3 100644 --- a/validator_client/src/fork_service.rs +++ b/validator_client/src/fork_service.rs @@ -85,7 +85,7 @@ impl ForkServiceBuilder { ); let candidates = vec![CandidateBeaconNode::new(eth2::BeaconNodeHttpClient::new( sensitive_url::SensitiveUrl::parse("http://127.0.0.1").unwrap(), - 12, + eth2::Timeouts::default(12), ))]; let mut beacon_nodes = BeaconNodeFallback::new(candidates, spec, log.clone()); beacon_nodes.set_slot_clock(slot_clock); diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index a6b1a55c979..2914a1049f5 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -30,7 +30,7 @@ use clap::ArgMatches; use duties_service::DutiesService; use environment::RuntimeContext; use eth2::types::StateId; -use eth2::{reqwest::ClientBuilder, BeaconNodeHttpClient, StatusCode}; +use eth2::{reqwest::ClientBuilder, BeaconNodeHttpClient, StatusCode, Timeouts}; use fork_service::{ForkService, ForkServiceBuilder}; use http_api::ApiSecret; use initialized_validators::InitializedValidators; @@ -57,6 +57,13 @@ const RETRY_DELAY: Duration = Duration::from_secs(2); /// The time between polls when waiting for genesis. const WAITING_FOR_GENESIS_POLL_TIME: Duration = Duration::from_secs(12); +/// Specific timeout constants for HTTP requests involved in different validator duties. +/// This can help ensure proper endpoint fallback occurs. +const HTTP_ATTESTATION_TIMEOUT: u64 = 1; +const HTTP_ATTESTER_DUTIES_TIMEOUT: u64 = 1; +const HTTP_PROPOSAL_TIMEOUT_QUOTIENT: u64 = 2; +const HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT: u64 = 3; + #[derive(Clone)] pub struct ProductionValidatorClient { context: RuntimeContext, @@ -219,22 +226,47 @@ impl ProductionValidatorClient { })?; } + let last_beacon_node = config + .beacon_nodes + .last() + .ok_or_else(|| "No beacon nodes defined.".to_string())?; + let beacon_nodes: Vec = config .beacon_nodes .clone() .into_iter() .map(|url| { + let secs_per_slot = context.eth2_config.spec.seconds_per_slot; let beacon_node_http_client = ClientBuilder::new() // Set default timeout to be the full slot duration. - .timeout(Duration::from_secs( - context.eth2_config.spec.seconds_per_slot, - )) + .timeout(Duration::from_secs(secs_per_slot)) .build() .map_err(|e| format!("Unable to build HTTP client: {:?}", e))?; + + // Use quicker timeouts if a fallback beacon node exists. + let timeouts = if url.full != last_beacon_node.full && !config.use_long_timeouts { + info!( + log, + "Fallback endpoints are available, using optimized timeouts."; + ); + Timeouts::new( + Duration::from_secs(HTTP_ATTESTATION_TIMEOUT), + Duration::from_secs(HTTP_ATTESTER_DUTIES_TIMEOUT), + Duration::from_millis( + secs_per_slot * 1_000 / HTTP_PROPOSAL_TIMEOUT_QUOTIENT, + ), + Duration::from_millis( + secs_per_slot * 1_000 / HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT, + ), + ) + } else { + Timeouts::default(secs_per_slot) + }; + Ok(BeaconNodeHttpClient::from_components( url, beacon_node_http_client, - context.eth2_config.spec.seconds_per_slot, + timeouts, )) }) .collect::, String>>()?; From 5e7e8f48110daf0933ea6737ddad881f86f51dee Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 2 Jun 2021 16:42:12 +1000 Subject: [PATCH 04/13] Fix tests --- beacon_node/http_api/tests/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index d880a3d686e..aa1e2585d17 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -9,7 +9,7 @@ use discv5::enr::{CombinedKey, EnrBuilder}; use environment::null_logger; use eth2::Error; use eth2::StatusCode; -use eth2::{types::*, BeaconNodeHttpClient}; +use eth2::{types::*, BeaconNodeHttpClient, Timeouts}; use eth2_libp2p::{ rpc::methods::MetaData, types::{EnrBitfield, SyncState}, @@ -214,7 +214,7 @@ impl ApiTester { listening_socket.port() )) .unwrap(), - SECONDS_PER_SLOT, + Timeouts::default(SECONDS_PER_SLOT), ); Self { @@ -327,7 +327,7 @@ impl ApiTester { listening_socket.port() )) .unwrap(), - SECONDS_PER_SLOT, + Timeouts::default(SECONDS_PER_SLOT), ); Self { From 6a419c3e56df4e1692f5662d8e9633bfbf9ddade Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 4 Jun 2021 12:01:04 +1000 Subject: [PATCH 05/13] Adjust comments --- validator_client/src/config.rs | 2 +- validator_client/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index d9a5d7b411e..b299247de20 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -35,7 +35,7 @@ pub struct Config { pub disable_auto_discover: bool, /// If true, re-register existing validators in definitions.yml for slashing protection. pub init_slashing_protection: bool, - /// if true, use longer timeouts for requests made to the beacon node. + /// If true, use longer timeouts for requests made to the beacon node. pub use_long_timeouts: bool, /// Graffiti to be inserted everytime we create a block. pub graffiti: Option, diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 2914a1049f5..459ef3a959a 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -58,7 +58,7 @@ const RETRY_DELAY: Duration = Duration::from_secs(2); const WAITING_FOR_GENESIS_POLL_TIME: Duration = Duration::from_secs(12); /// Specific timeout constants for HTTP requests involved in different validator duties. -/// This can help ensure proper endpoint fallback occurs. +/// This can help ensure that proper endpoint fallback occurs. const HTTP_ATTESTATION_TIMEOUT: u64 = 1; const HTTP_ATTESTER_DUTIES_TIMEOUT: u64 = 1; const HTTP_PROPOSAL_TIMEOUT_QUOTIENT: u64 = 2; From 22279ad34ca55690295148580201923486bec234 Mon Sep 17 00:00:00 2001 From: Mac L Date: Tue, 8 Jun 2021 11:39:49 +1000 Subject: [PATCH 06/13] Add test for new cli flag --- lighthouse/tests/validator_client.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index a0982064b76..a6a644fd4d6 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -194,6 +194,14 @@ fn init_slashing_protections_flag() { .with_config(|config| assert!(config.init_slashing_protection)); } +#[test] +fn use_long_timeouts_flag() { + CommandLineTest::new() + .flag("use-long-timeouts", None) + .run() + .with_config(|config| assert!(config.use_long_timeouts)); +} + // Tests for Graffiti flags. #[test] fn graffiti_flag() { From ff1eae1123d5ffe95f5345acec6989f4670997d5 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 21 Jun 2021 10:58:39 +1000 Subject: [PATCH 07/13] Implement suggestions --- common/eth2/src/lib.rs | 28 +++++++++------------------- validator_client/src/cli.rs | 3 ++- validator_client/src/lib.rs | 33 +++++++++++++++++---------------- 3 files changed, 28 insertions(+), 36 deletions(-) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index b4f9a092bbc..be57df10ab2 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -82,31 +82,21 @@ impl fmt::Display for Error { /// proper fallback behaviour. #[derive(Clone)] pub struct Timeouts { - attestation: Duration, - attester_duties: Duration, - proposal: Duration, - proposer_duties: Duration, + pub attestation: Duration, + pub attester_duties: Duration, + pub proposal: Duration, + pub proposer_duties: Duration, } impl Timeouts { - pub fn new( - attestation: Duration, - attester_duties: Duration, - proposal: Duration, - proposer_duties: Duration, - ) -> Self { + pub fn set_all(timeout: Duration) -> Self { Timeouts { - attestation, - attester_duties, - proposal, - proposer_duties, + attestation: timeout, + attester_duties: timeout, + proposal: timeout, + proposer_duties: timeout, } } - - pub fn default(seconds_per_slot: u64) -> Self { - let timeout = Duration::from_secs(seconds_per_slot); - Timeouts::new(timeout, timeout, timeout, timeout) - } } /// A wrapper around `reqwest::Client` which provides convenience methods for interfacing with a diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index d4c4dc3198f..08d3d9ae56e 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -98,7 +98,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("use-long-timeouts") .long("use-long-timeouts") .help("If present, the validator client will use longer timeouts for requests \ - made to the beacon node.") + made to the beacon node. This flag is generally not recommended, \ + longer timeouts can cause missed duties when fallbacks are used.") ) // This overwrites the graffiti configured in the beacon node. .arg( diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 459ef3a959a..f12f1ddd6cf 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -59,8 +59,8 @@ const WAITING_FOR_GENESIS_POLL_TIME: Duration = Duration::from_secs(12); /// Specific timeout constants for HTTP requests involved in different validator duties. /// This can help ensure that proper endpoint fallback occurs. -const HTTP_ATTESTATION_TIMEOUT: u64 = 1; -const HTTP_ATTESTER_DUTIES_TIMEOUT: u64 = 1; +const HTTP_ATTESTATION_TIMEOUT: Duration = Duration::from_secs(1); +const HTTP_ATTESTER_DUTIES_TIMEOUT: Duration = Duration::from_secs(1); const HTTP_PROPOSAL_TIMEOUT_QUOTIENT: u64 = 2; const HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT: u64 = 3; @@ -226,16 +226,17 @@ impl ProductionValidatorClient { })?; } - let last_beacon_node = config + let last_beacon_node_index = config .beacon_nodes - .last() + .len() + .checked_sub(1) .ok_or_else(|| "No beacon nodes defined.".to_string())?; let beacon_nodes: Vec = config .beacon_nodes - .clone() - .into_iter() - .map(|url| { + .iter() + .enumerate() + .map(|(i, url)| { let secs_per_slot = context.eth2_config.spec.seconds_per_slot; let beacon_node_http_client = ClientBuilder::new() // Set default timeout to be the full slot duration. @@ -244,27 +245,27 @@ impl ProductionValidatorClient { .map_err(|e| format!("Unable to build HTTP client: {:?}", e))?; // Use quicker timeouts if a fallback beacon node exists. - let timeouts = if url.full != last_beacon_node.full && !config.use_long_timeouts { + let timeouts = if i < last_beacon_node_index && !config.use_long_timeouts { info!( log, "Fallback endpoints are available, using optimized timeouts."; ); - Timeouts::new( - Duration::from_secs(HTTP_ATTESTATION_TIMEOUT), - Duration::from_secs(HTTP_ATTESTER_DUTIES_TIMEOUT), - Duration::from_millis( + Timeouts { + attestation: HTTP_ATTESTATION_TIMEOUT, + attester_duties: HTTP_ATTESTER_DUTIES_TIMEOUT, + proposal: Duration::from_millis( secs_per_slot * 1_000 / HTTP_PROPOSAL_TIMEOUT_QUOTIENT, ), - Duration::from_millis( + proposer_duties: Duration::from_millis( secs_per_slot * 1_000 / HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT, ), - ) + } } else { - Timeouts::default(secs_per_slot) + Timeouts::set_all(Duration::from_secs(secs_per_slot)) }; Ok(BeaconNodeHttpClient::from_components( - url, + url.clone(), beacon_node_http_client, timeouts, )) From b340230b6d420cb1c7aa3cfddf9faa09ceb9953d Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 18 Jun 2021 13:59:14 +1000 Subject: [PATCH 08/13] Add metrics for VC HTTP calls --- validator_client/src/attestation_service.rs | 16 ++++++++++++++++ validator_client/src/block_service.rs | 9 +++++++++ validator_client/src/duties_service.rs | 16 ++++++++++++++++ validator_client/src/http_metrics/metrics.rs | 10 ++++++++++ 4 files changed, 51 insertions(+) diff --git a/validator_client/src/attestation_service.rs b/validator_client/src/attestation_service.rs index 9c79e79b4dd..b87760511ca 100644 --- a/validator_client/src/attestation_service.rs +++ b/validator_client/src/attestation_service.rs @@ -340,6 +340,10 @@ impl AttestationService { let attestation_data = self .beacon_nodes .first_success(RequireSynced::No, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::ATTESTATION_SERVICE_TIMES, + &[metrics::ATTESTATIONS_HTTP_GET], + ); beacon_node .get_validator_attestation_data(slot, committee_index) .await @@ -402,6 +406,10 @@ impl AttestationService { match self .beacon_nodes .first_success(RequireSynced::No, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::ATTESTATION_SERVICE_TIMES, + &[metrics::ATTESTATIONS_HTTP_POST], + ); beacon_node .post_beacon_pool_attestations(attestations_slice) .await @@ -454,6 +462,10 @@ impl AttestationService { let aggregated_attestation = self .beacon_nodes .first_success(RequireSynced::No, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::ATTESTATION_SERVICE_TIMES, + &[metrics::AGGREGATES_HTTP_GET], + ); beacon_node .get_validator_aggregate_attestation( attestation_data_ref.slot, @@ -506,6 +518,10 @@ impl AttestationService { match self .beacon_nodes .first_success(RequireSynced::No, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::ATTESTATION_SERVICE_TIMES, + &[metrics::AGGREGATES_HTTP_POST], + ); beacon_node .post_validator_aggregate_and_proof(signed_aggregate_and_proofs_slice) .await diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index 48f9e74894b..95e556a0337 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -263,17 +263,26 @@ impl BlockService { let signed_block = self .beacon_nodes .first_success(RequireSynced::No, |beacon_node| async move { + let get_timer = metrics::start_timer_vec( + &metrics::BLOCK_SERVICE_TIMES, + &[metrics::BEACON_BLOCK_HTTP_GET], + ); let block = beacon_node .get_validator_blocks(slot, randao_reveal_ref, graffiti.as_ref()) .await .map_err(|e| format!("Error from beacon node when producing block: {:?}", e))? .data; + drop(get_timer); let signed_block = self_ref .validator_store .sign_block(validator_pubkey_ref, block, current_slot) .ok_or("Unable to sign block")?; + let _post_timer = metrics::start_timer_vec( + &metrics::BLOCK_SERVICE_TIMES, + &[metrics::BEACON_BLOCK_HTTP_POST], + ); beacon_node .post_beacon_blocks(&signed_block) .await diff --git a/validator_client/src/duties_service.rs b/validator_client/src/duties_service.rs index 0ad85a0ddf8..aa7edd29e5a 100644 --- a/validator_client/src/duties_service.rs +++ b/validator_client/src/duties_service.rs @@ -285,6 +285,10 @@ async fn poll_validator_indices( let download_result = duties_service .beacon_nodes .first_success(duties_service.require_synced, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::DUTIES_SERVICE_TIMES, + &[metrics::VALIDATOR_ID_HTTP_GET], + ); beacon_node .get_beacon_states_validator_id( StateId::Head, @@ -453,6 +457,10 @@ async fn poll_beacon_attesters( if let Err(e) = duties_service .beacon_nodes .first_success(duties_service.require_synced, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::DUTIES_SERVICE_TIMES, + &[metrics::SUBSCRIPTIONS_HTTP_POST], + ); beacon_node .post_validator_beacon_committee_subscriptions(subscriptions_ref) .await @@ -509,6 +517,10 @@ async fn poll_beacon_attesters_for_epoch( let response = duties_service .beacon_nodes .first_success(duties_service.require_synced, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::DUTIES_SERVICE_TIMES, + &[metrics::ATTESTER_DUTIES_HTTP_POST], + ); beacon_node .post_validator_duties_attester(epoch, local_indices) .await @@ -640,6 +652,10 @@ async fn poll_beacon_proposers( let download_result = duties_service .beacon_nodes .first_success(duties_service.require_synced, |beacon_node| async move { + let _timer = metrics::start_timer_vec( + &metrics::DUTIES_SERVICE_TIMES, + &[metrics::PROPOSER_DUTIES_HTTP_GET], + ); beacon_node .get_validator_duties_proposer(current_epoch) .await diff --git a/validator_client/src/http_metrics/metrics.rs b/validator_client/src/http_metrics/metrics.rs index d5e5bf4814a..120e6c2c7e6 100644 --- a/validator_client/src/http_metrics/metrics.rs +++ b/validator_client/src/http_metrics/metrics.rs @@ -9,8 +9,14 @@ pub const SAME_DATA: &str = "same_data"; pub const UNREGISTERED: &str = "unregistered"; pub const FULL_UPDATE: &str = "full_update"; pub const BEACON_BLOCK: &str = "beacon_block"; +pub const BEACON_BLOCK_HTTP_GET: &str = "beacon_block_http_get"; +pub const BEACON_BLOCK_HTTP_POST: &str = "beacon_block_http_post"; pub const ATTESTATIONS: &str = "attestations"; +pub const ATTESTATIONS_HTTP_GET: &str = "attestations_http_get"; +pub const ATTESTATIONS_HTTP_POST: &str = "attestations_http_post"; pub const AGGREGATES: &str = "aggregates"; +pub const AGGREGATES_HTTP_GET: &str = "aggregates_http_get"; +pub const AGGREGATES_HTTP_POST: &str = "aggregates_http_post"; pub const CURRENT_EPOCH: &str = "current_epoch"; pub const NEXT_EPOCH: &str = "next_epoch"; pub const UPDATE_INDICES: &str = "update_indices"; @@ -18,6 +24,10 @@ pub const UPDATE_ATTESTERS_CURRENT_EPOCH: &str = "update_attesters_current_epoch pub const UPDATE_ATTESTERS_NEXT_EPOCH: &str = "update_attesters_next_epoch"; pub const UPDATE_ATTESTERS_FETCH: &str = "update_attesters_fetch"; pub const UPDATE_ATTESTERS_STORE: &str = "update_attesters_store"; +pub const ATTESTER_DUTIES_HTTP_POST: &str = "attester_duties_http_post"; +pub const PROPOSER_DUTIES_HTTP_GET: &str = "proposer_duties_http_get"; +pub const VALIDATOR_ID_HTTP_GET: &str = "validator_id_http_get"; +pub const SUBSCRIPTIONS_HTTP_POST: &str = "subscriptions_http_post"; pub const UPDATE_PROPOSERS: &str = "update_proposers"; pub const SUBSCRIPTIONS: &str = "subscriptions"; From 531b2db2eba883fde6d9d54f3b21db0109fa5149 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 21 Jun 2021 11:23:37 +1000 Subject: [PATCH 09/13] Fix naming --- account_manager/src/validator/exit.rs | 2 +- beacon_node/http_api/tests/tests.rs | 4 ++-- testing/node_test_rig/src/lib.rs | 2 +- validator_client/src/fork_service.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/account_manager/src/validator/exit.rs b/account_manager/src/validator/exit.rs index aa305679dde..648601616b9 100644 --- a/account_manager/src/validator/exit.rs +++ b/account_manager/src/validator/exit.rs @@ -81,7 +81,7 @@ pub fn cli_run(matches: &ArgMatches, env: Environment) -> Result< let client = BeaconNodeHttpClient::new( SensitiveUrl::parse(&server_url) .map_err(|e| format!("Failed to parse beacon http server: {:?}", e))?, - Timeouts::default(env.eth2_config.spec.seconds_per_slot), + Timeouts::set_all(Duration::from_secs(env.eth2_config.spec.seconds_per_slot)), ); let testnet_config = env diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index aa1e2585d17..66ea49d9ece 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -214,7 +214,7 @@ impl ApiTester { listening_socket.port() )) .unwrap(), - Timeouts::default(SECONDS_PER_SLOT), + Timeouts::set_all(Duration::from_secs(SECONDS_PER_SLOT)), ); Self { @@ -327,7 +327,7 @@ impl ApiTester { listening_socket.port() )) .unwrap(), - Timeouts::default(SECONDS_PER_SLOT), + Timeouts::set_all(Duration::from_secs(SECONDS_PER_SLOT)), ); Self { diff --git a/testing/node_test_rig/src/lib.rs b/testing/node_test_rig/src/lib.rs index e1a296846e8..6d350f2ac2f 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -77,7 +77,7 @@ impl LocalBeaconNode { Ok(BeaconNodeHttpClient::from_components( beacon_node_url, beacon_node_http_client, - Timeouts::default(HTTP_TIMEOUT), + Timeouts::set_all(Duration::from_secs(HTTP_TIMEOUT)), )) } } diff --git a/validator_client/src/fork_service.rs b/validator_client/src/fork_service.rs index a58870867f3..5c284823d22 100644 --- a/validator_client/src/fork_service.rs +++ b/validator_client/src/fork_service.rs @@ -85,7 +85,7 @@ impl ForkServiceBuilder { ); let candidates = vec![CandidateBeaconNode::new(eth2::BeaconNodeHttpClient::new( sensitive_url::SensitiveUrl::parse("http://127.0.0.1").unwrap(), - eth2::Timeouts::default(12), + eth2::Timeouts::set_all(Duration::from_secs(12)), ))]; let mut beacon_nodes = BeaconNodeFallback::new(candidates, spec, log.clone()); beacon_nodes.set_slot_clock(slot_clock); From 1ac0abd386408c38cfc803b3b6ec6c8e9adb0c7d Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 21 Jun 2021 19:11:36 +1000 Subject: [PATCH 10/13] Adjust timeouts --- testing/node_test_rig/src/lib.rs | 6 +++--- validator_client/src/lib.rs | 27 ++++++++++++--------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/testing/node_test_rig/src/lib.rs b/testing/node_test_rig/src/lib.rs index 6d350f2ac2f..acf9bb9e686 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -20,7 +20,7 @@ pub use eth2; pub use validator_client::Config as ValidatorConfig; /// The global timeout for HTTP requests to the beacon node. -const HTTP_TIMEOUT: u64 = 4; +const HTTP_TIMEOUT: Duration = Duration::from_secs(4); /// Provides a beacon node that is running in the current process on a given tokio executor (it /// is _local_ to this process). @@ -71,13 +71,13 @@ impl LocalBeaconNode { ) .map_err(|e| format!("Unable to parse beacon node URL: {:?}", e))?; let beacon_node_http_client = ClientBuilder::new() - .timeout(Duration::from_secs(HTTP_TIMEOUT)) + .timeout(HTTP_TIMEOUT) .build() .map_err(|e| format!("Unable to build HTTP client: {:?}", e))?; Ok(BeaconNodeHttpClient::from_components( beacon_node_url, beacon_node_http_client, - Timeouts::set_all(Duration::from_secs(HTTP_TIMEOUT)), + Timeouts::set_all(HTTP_TIMEOUT), )) } } diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index f12f1ddd6cf..89c0fb771d5 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -59,10 +59,10 @@ const WAITING_FOR_GENESIS_POLL_TIME: Duration = Duration::from_secs(12); /// Specific timeout constants for HTTP requests involved in different validator duties. /// This can help ensure that proper endpoint fallback occurs. -const HTTP_ATTESTATION_TIMEOUT: Duration = Duration::from_secs(1); -const HTTP_ATTESTER_DUTIES_TIMEOUT: Duration = Duration::from_secs(1); -const HTTP_PROPOSAL_TIMEOUT_QUOTIENT: u64 = 2; -const HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT: u64 = 3; +const HTTP_ATTESTATION_TIMEOUT_QUOTIENT: u32 = 4; +const HTTP_ATTESTER_DUTIES_TIMEOUT_QUOTIENT: u32 = 4; +const HTTP_PROPOSAL_TIMEOUT_QUOTIENT: u32 = 2; +const HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT: u32 = 4; #[derive(Clone)] pub struct ProductionValidatorClient { @@ -237,10 +237,11 @@ impl ProductionValidatorClient { .iter() .enumerate() .map(|(i, url)| { - let secs_per_slot = context.eth2_config.spec.seconds_per_slot; + let slot_duration = Duration::from_secs(context.eth2_config.spec.seconds_per_slot); + let beacon_node_http_client = ClientBuilder::new() // Set default timeout to be the full slot duration. - .timeout(Duration::from_secs(secs_per_slot)) + .timeout(slot_duration) .build() .map_err(|e| format!("Unable to build HTTP client: {:?}", e))?; @@ -251,17 +252,13 @@ impl ProductionValidatorClient { "Fallback endpoints are available, using optimized timeouts."; ); Timeouts { - attestation: HTTP_ATTESTATION_TIMEOUT, - attester_duties: HTTP_ATTESTER_DUTIES_TIMEOUT, - proposal: Duration::from_millis( - secs_per_slot * 1_000 / HTTP_PROPOSAL_TIMEOUT_QUOTIENT, - ), - proposer_duties: Duration::from_millis( - secs_per_slot * 1_000 / HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT, - ), + attestation: slot_duration / HTTP_ATTESTATION_TIMEOUT_QUOTIENT, + attester_duties: slot_duration / HTTP_ATTESTER_DUTIES_TIMEOUT_QUOTIENT, + proposal: slot_duration / HTTP_PROPOSAL_TIMEOUT_QUOTIENT, + proposer_duties: slot_duration / HTTP_PROPOSER_DUTIES_TIMEOUT_QUOTIENT, } } else { - Timeouts::set_all(Duration::from_secs(secs_per_slot)) + Timeouts::set_all(slot_duration) }; Ok(BeaconNodeHttpClient::from_components( From 05d463cecbe9c6ca7f2c5f677c23e8e0d052ec09 Mon Sep 17 00:00:00 2001 From: Mac L Date: Tue, 22 Jun 2021 11:04:44 +1000 Subject: [PATCH 11/13] Structure function calls consistently --- common/eth2/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index be57df10ab2..54e950a4370 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1075,8 +1075,7 @@ impl BeaconNodeHttpClient { .append_pair("slot", &slot.to_string()) .append_pair("committee_index", &committee_index.to_string()); - let timeout = self.timeouts.attestation; - self.get_with_timeout(path, timeout).await + self.get_with_timeout(path, self.timeouts.attestation).await } /// `GET validator/aggregate_attestation?slot,attestation_data_root` @@ -1099,8 +1098,7 @@ impl BeaconNodeHttpClient { &format!("{:?}", attestation_data_root), ); - let timeout = self.timeouts.attestation; - self.get_opt_with_timeout(path, timeout).await + self.get_opt_with_timeout(path, self.timeouts.attestation).await } /// `POST validator/duties/attester/{epoch}` From 5c5cabf71126a4e33ddcedc3af8ef163d50b4e42 Mon Sep 17 00:00:00 2001 From: Mac L Date: Tue, 22 Jun 2021 11:11:48 +1000 Subject: [PATCH 12/13] Fix formatting --- common/eth2/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 54e950a4370..7a0a09b05cf 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1098,7 +1098,8 @@ impl BeaconNodeHttpClient { &format!("{:?}", attestation_data_root), ); - self.get_opt_with_timeout(path, self.timeouts.attestation).await + self.get_opt_with_timeout(path, self.timeouts.attestation) + .await } /// `POST validator/duties/attester/{epoch}` From 5d8e11d30ad113d0034e7674229f7d9ac08fc75f Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 25 Jun 2021 11:48:57 +1000 Subject: [PATCH 13/13] Re-trigger CI --- validator_client/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 89c0fb771d5..d9fe21111be 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -275,7 +275,7 @@ impl ProductionValidatorClient { .map(CandidateBeaconNode::new) .collect(); - // Set the count for beacon node fallbacks excluding the primary beacon node + // Set the count for beacon node fallbacks excluding the primary beacon node. set_gauge( &http_metrics::metrics::ETH2_FALLBACK_CONFIGURED, num_nodes.saturating_sub(1) as i64,