From 3cac6d9ed50d14e9aec0d90cc0fb1bc484c10605 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 22 Jun 2023 02:14:56 +0000 Subject: [PATCH] Configure the `validator/register_validator` batch size via the CLI (#4399) ## Issue Addressed NA ## Proposed Changes Adds the `--validator-registration-batch-size` flag to the VC to allow runtime configuration of the number of validators POSTed to the [`validator/register_validator`](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/registerValidator) endpoint. There are builders (Agnostic and Eden) that are timing out with `regsiterValidator` requests with ~400 validators, even with a 9 second timeout. Exposing the batch size will help tune batch sizes to (hopefully) avoid this. This PR should not change the behavior of Lighthouse when the new flag is not provided (i.e., the same default value is used). ## Additional Info NA --- lighthouse/tests/validator_client.rs | 21 +++++++++++++++++++++ validator_client/src/cli.rs | 10 ++++++++++ validator_client/src/config.rs | 9 +++++++++ validator_client/src/lib.rs | 1 + validator_client/src/preparation_service.rs | 21 ++++++++++++++++----- 5 files changed, 57 insertions(+), 5 deletions(-) diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 8c1f0477c4..27d7c10e96 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -499,3 +499,24 @@ fn latency_measurement_service() { assert!(!config.enable_latency_measurement_service); }); } + +#[test] +fn validator_registration_batch_size() { + CommandLineTest::new().run().with_config(|config| { + assert_eq!(config.validator_registration_batch_size, 500); + }); + CommandLineTest::new() + .flag("validator-registration-batch-size", Some("100")) + .run() + .with_config(|config| { + assert_eq!(config.validator_registration_batch_size, 100); + }); +} + +#[test] +#[should_panic] +fn validator_registration_batch_size_zero_value() { + CommandLineTest::new() + .flag("validator-registration-batch-size", Some("0")) + .run(); +} diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 6e199cb173..436b8eb4d5 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -333,6 +333,16 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .default_value("true") .takes_value(true), ) + .arg( + Arg::with_name("validator-registration-batch-size") + .long("validator-registration-batch-size") + .value_name("INTEGER") + .help("Defines the number of validators per \ + validator/register_validator request sent to the BN. This value \ + can be reduced to avoid timeouts from builders.") + .default_value("500") + .takes_value(true), + ) /* * Experimental/development options. */ diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index fa297dcfed..e0dd12e100 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -77,6 +77,8 @@ pub struct Config { pub disable_run_on_all: bool, /// Enables a service which attempts to measure latency between the VC and BNs. pub enable_latency_measurement_service: bool, + /// Defines the number of validators per `validator/register_validator` request sent to the BN. + pub validator_registration_batch_size: usize, } impl Default for Config { @@ -117,6 +119,7 @@ impl Default for Config { gas_limit: None, disable_run_on_all: false, enable_latency_measurement_service: true, + validator_registration_batch_size: 500, } } } @@ -380,6 +383,12 @@ impl Config { config.enable_latency_measurement_service = parse_optional(cli_args, "latency-measurement-service")?.unwrap_or(true); + config.validator_registration_batch_size = + parse_required(cli_args, "validator-registration-batch-size")?; + if config.validator_registration_batch_size == 0 { + return Err("validator-registration-batch-size cannot be 0".to_string()); + } + /* * Experimental */ diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 6e4a8da6ac..60943a260c 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -487,6 +487,7 @@ impl ProductionValidatorClient { .beacon_nodes(beacon_nodes.clone()) .runtime_context(context.service_context("preparation".into())) .builder_registration_timestamp_override(config.builder_registration_timestamp_override) + .validator_registration_batch_size(config.validator_registration_batch_size) .build()?; let sync_committee_service = SyncCommitteeService::new( diff --git a/validator_client/src/preparation_service.rs b/validator_client/src/preparation_service.rs index 5bd93a5053..7d6e1744c8 100644 --- a/validator_client/src/preparation_service.rs +++ b/validator_client/src/preparation_service.rs @@ -23,9 +23,6 @@ const PROPOSER_PREPARATION_LOOKAHEAD_EPOCHS: u64 = 2; /// Number of epochs to wait before re-submitting validator registration. const EPOCHS_PER_VALIDATOR_REGISTRATION_SUBMISSION: u64 = 1; -/// The number of validator registrations to include per request to the beacon node. -const VALIDATOR_REGISTRATION_BATCH_SIZE: usize = 500; - /// Builds an `PreparationService`. pub struct PreparationServiceBuilder { validator_store: Option>>, @@ -33,6 +30,7 @@ pub struct PreparationServiceBuilder { beacon_nodes: Option>>, context: Option>, builder_registration_timestamp_override: Option, + validator_registration_batch_size: Option, } impl PreparationServiceBuilder { @@ -43,6 +41,7 @@ impl PreparationServiceBuilder { beacon_nodes: None, context: None, builder_registration_timestamp_override: None, + validator_registration_batch_size: None, } } @@ -74,6 +73,14 @@ impl PreparationServiceBuilder { self } + pub fn validator_registration_batch_size( + mut self, + validator_registration_batch_size: usize, + ) -> Self { + self.validator_registration_batch_size = Some(validator_registration_batch_size); + self + } + pub fn build(self) -> Result, String> { Ok(PreparationService { inner: Arc::new(Inner { @@ -91,6 +98,9 @@ impl PreparationServiceBuilder { .ok_or("Cannot build PreparationService without runtime_context")?, builder_registration_timestamp_override: self .builder_registration_timestamp_override, + validator_registration_batch_size: self.validator_registration_batch_size.ok_or( + "Cannot build PreparationService without validator_registration_batch_size", + )?, validator_registration_cache: RwLock::new(HashMap::new()), }), }) @@ -107,6 +117,7 @@ pub struct Inner { // Used to track unpublished validator registration changes. validator_registration_cache: RwLock>, + validator_registration_batch_size: usize, } #[derive(Hash, Eq, PartialEq, Debug, Clone)] @@ -447,7 +458,7 @@ impl PreparationService { } if !signed.is_empty() { - for batch in signed.chunks(VALIDATOR_REGISTRATION_BATCH_SIZE) { + for batch in signed.chunks(self.validator_registration_batch_size) { match self .beacon_nodes .first_success( @@ -462,7 +473,7 @@ impl PreparationService { Ok(()) => info!( log, "Published validator registrations to the builder network"; - "count" => registration_data_len, + "count" => batch.len(), ), Err(e) => warn!( log,