From 276b5e461f8b4596505ce7b83ec561f3201aa46b Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 26 Feb 2025 20:21:51 +1100 Subject: [PATCH 1/8] Add long-timeouts-multiplier cli flag --- validator_client/src/cli.rs | 15 +++++++++++++++ validator_client/src/config.rs | 4 ++++ validator_client/src/lib.rs | 2 +- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index dfcd2064e52..f63451b1944 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -107,6 +107,21 @@ pub struct ValidatorClient { )] pub use_long_timeouts: bool, + #[clap( + long, + requires = "use_long_timeouts", + default_value_t = 1, + help = "If present, the validator client will use a multiplier for the timeout \ + when making requests to the beacon node. This only takes effect when \ + the `--use-long_timeouts` flag is present. The timeouts will be the slot \ + duration multiplied by this value. + This flag is generally not recommended, longer timeouts can cause missed \ + duties when fallbacks are used.", + display_order = 0, + help_heading = FLAG_HEADER, + )] + pub long_timeouts_multiplier: u32, + #[clap( long, value_name = "CERTIFICATE-FILES", diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 2a848e20225..e3173e694c0 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -49,6 +49,8 @@ pub struct Config { pub init_slashing_protection: bool, /// If true, use longer timeouts for requests made to the beacon node. pub use_long_timeouts: bool, + /// Multiplier to use for long timeouts. + pub long_timeouts_multiplier: u32, /// Graffiti to be inserted everytime we create a block. pub graffiti: Option, /// Graffiti file to load per validator graffitis. @@ -111,6 +113,7 @@ impl Default for Config { disable_auto_discover: false, init_slashing_protection: false, use_long_timeouts: false, + long_timeouts_multiplier: 1, graffiti: None, graffiti_file: None, http_api: <_>::default(), @@ -194,6 +197,7 @@ impl Config { config.disable_auto_discover = validator_client_config.disable_auto_discover; config.init_slashing_protection = validator_client_config.init_slashing_protection; config.use_long_timeouts = validator_client_config.use_long_timeouts; + config.long_timeouts_multiplier = validator_client_config.long_timeouts_multiplier; if let Some(graffiti_file_path) = validator_client_config.graffiti_file.as_ref() { let mut graffiti_file = GraffitiFile::new(graffiti_file_path.into()); diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 70236d6a3cc..cb09f9c541a 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -325,7 +325,7 @@ impl ProductionValidatorClient { get_validator_block: slot_duration / HTTP_GET_VALIDATOR_BLOCK_TIMEOUT_QUOTIENT, } } else { - Timeouts::set_all(slot_duration) + Timeouts::set_all(slot_duration.saturating_mul(config.long_timeouts_multiplier)) }; Ok(BeaconNodeHttpClient::from_components( From 359c17e41aa5f919f8441073124e24a80ca77c7d Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 26 Feb 2025 20:37:05 +1100 Subject: [PATCH 2/8] Fix cli docs --- book/src/help_vc.md | 7 +++++++ validator_client/src/cli.rs | 5 ++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 948a09f44d3..3523fcfdd19 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -247,6 +247,13 @@ Flags: contain sensitive information about your validator and so this flag should be used with caution. For Windows users, the log file permissions will be inherited from the parent folder. + --long-timeouts-multiplier + If present, the validator client will use a multiplier for the timeout + when making requests to the beacon node. This only takes effect when + the `--use-long_timeouts` flag is present. The timeouts will be the + slot duration multiplied by this value. This flag is generally not + recommended, longer timeouts can cause missed duties when fallbacks + are used. [default: 1] --metrics Enable the Prometheus metrics HTTP server. Disabled by default. --prefer-builder-proposals diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index f63451b1944..39ccf2b5b21 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -114,9 +114,8 @@ pub struct ValidatorClient { help = "If present, the validator client will use a multiplier for the timeout \ when making requests to the beacon node. This only takes effect when \ the `--use-long_timeouts` flag is present. The timeouts will be the slot \ - duration multiplied by this value. - This flag is generally not recommended, longer timeouts can cause missed \ - duties when fallbacks are used.", + duration multiplied by this value. This flag is generally not recommended, \ + longer timeouts can cause missed duties when fallbacks are used.", display_order = 0, help_heading = FLAG_HEADER, )] From 95eabff8f963aa9f1114d7dfcd2ad2f039dbf05b Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 27 Feb 2025 08:19:23 +1100 Subject: [PATCH 3/8] Update book/src/help_vc.md Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com> --- book/src/help_vc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 3523fcfdd19..5d7d4520d84 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -250,7 +250,7 @@ Flags: --long-timeouts-multiplier If present, the validator client will use a multiplier for the timeout when making requests to the beacon node. This only takes effect when - the `--use-long_timeouts` flag is present. The timeouts will be the + the `--use-long-timeouts` flag is present. The timeouts will be the slot duration multiplied by this value. This flag is generally not recommended, longer timeouts can cause missed duties when fallbacks are used. [default: 1] From e335d04b3fddc30ab746b55f43ad6196357c2a5e Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 27 Feb 2025 08:19:44 +1100 Subject: [PATCH 4/8] Update validator_client/src/cli.rs --- validator_client/src/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 39ccf2b5b21..e0e8e668002 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -113,7 +113,7 @@ pub struct ValidatorClient { default_value_t = 1, help = "If present, the validator client will use a multiplier for the timeout \ when making requests to the beacon node. This only takes effect when \ - the `--use-long_timeouts` flag is present. The timeouts will be the slot \ + the `--use-long-timeouts` flag is present. The timeouts will be the slot \ duration multiplied by this value. This flag is generally not recommended, \ longer timeouts can cause missed duties when fallbacks are used.", display_order = 0, From 2b6c539682970ff5fb113f531296ff1090b2226a Mon Sep 17 00:00:00 2001 From: Mac L Date: Thu, 27 Feb 2025 16:03:32 +1100 Subject: [PATCH 5/8] Add tests --- lighthouse/tests/validator_client.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index f28e7d98298..eccd97d4864 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -129,6 +129,22 @@ fn use_long_timeouts_flag() { .with_config(|config| assert!(config.use_long_timeouts)); } +#[test] +fn long_timeouts_multiplier_flag_default() { + CommandLineTest::new() + .run() + .with_config(|config| assert_eq!(config.long_timeouts_multiplier, 1)); +} + +#[test] +fn long_timeouts_multiplier_flag() { + CommandLineTest::new() + .flag("use-long-timeouts", None) + .flag("long-timeouts-multiplier", Some("10")) + .run() + .with_config(|config| assert_eq!(config.long_timeouts_multiplier, 10)); +} + #[test] fn beacon_nodes_tls_certs_flag() { let dir = TempDir::new().expect("Unable to create temporary directory"); From efa44da10c9d76a03d8f36d09056128072689788 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 26 Feb 2025 12:10:19 -0800 Subject: [PATCH 6/8] Make ExecutionBlock::total_difficulty Optional --- .../beacon_chain/src/bellatrix_readiness.rs | 2 +- beacon_node/execution_layer/src/engine_api.rs | 9 ++++++++- beacon_node/execution_layer/src/lib.rs | 15 +++++++++------ .../src/test_utils/execution_block_generator.rs | 4 ++-- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/bellatrix_readiness.rs b/beacon_node/beacon_chain/src/bellatrix_readiness.rs index 500588953f3..412870354b9 100644 --- a/beacon_node/beacon_chain/src/bellatrix_readiness.rs +++ b/beacon_node/beacon_chain/src/bellatrix_readiness.rs @@ -171,7 +171,7 @@ impl BeaconChain { return BellatrixReadiness::NotSynced; } let params = MergeConfig::from_chainspec(&self.spec); - let current_difficulty = el.get_current_difficulty().await.ok(); + let current_difficulty = el.get_current_difficulty().await.ok().flatten(); BellatrixReadiness::Ready { config: params, current_difficulty, diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index b9d878b1f86..356ac5567c6 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -142,11 +142,18 @@ pub struct ExecutionBlock { pub block_number: u64, pub parent_hash: ExecutionBlockHash, - pub total_difficulty: Uint256, + pub total_difficulty: Option, #[serde(with = "serde_utils::u64_hex_be")] pub timestamp: u64, } +impl ExecutionBlock { + pub fn terminal_total_difficulty_reached(&self, terminal_total_difficulty: Uint256) -> bool { + self.total_difficulty + .map_or(true, |td| td >= terminal_total_difficulty) + } +} + #[superstruct( variants(V1, V2, V3), variant_attributes(derive(Clone, Debug, Eq, Hash, PartialEq),), diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 4fd7188c206..439a8810086 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -621,7 +621,7 @@ impl ExecutionLayer { } /// Get the current difficulty of the PoW chain. - pub async fn get_current_difficulty(&self) -> Result { + pub async fn get_current_difficulty(&self) -> Result, ApiError> { let block = self .engine() .api @@ -1680,7 +1680,8 @@ impl ExecutionLayer { self.execution_blocks().await.put(block.block_hash, block); loop { - let block_reached_ttd = block.total_difficulty >= spec.terminal_total_difficulty; + let block_reached_ttd = + block.terminal_total_difficulty_reached(spec.terminal_total_difficulty); if block_reached_ttd { if block.parent_hash == ExecutionBlockHash::zero() { return Ok(Some(block)); @@ -1689,7 +1690,8 @@ impl ExecutionLayer { .get_pow_block(engine, block.parent_hash) .await? .ok_or(ApiError::ExecutionBlockNotFound(block.parent_hash))?; - let parent_reached_ttd = parent.total_difficulty >= spec.terminal_total_difficulty; + let parent_reached_ttd = + parent.terminal_total_difficulty_reached(spec.terminal_total_difficulty); if block_reached_ttd && !parent_reached_ttd { return Ok(Some(block)); @@ -1765,9 +1767,10 @@ impl ExecutionLayer { parent: ExecutionBlock, spec: &ChainSpec, ) -> bool { - let is_total_difficulty_reached = block.total_difficulty >= spec.terminal_total_difficulty; - let is_parent_total_difficulty_valid = - parent.total_difficulty < spec.terminal_total_difficulty; + let is_total_difficulty_reached = + block.terminal_total_difficulty_reached(spec.terminal_total_difficulty); + let is_parent_total_difficulty_valid = parent + .total_difficulty.is_some_and(|td| td < spec.terminal_total_difficulty); is_total_difficulty_reached && is_parent_total_difficulty_valid } diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index 5a5c9e1fa9a..81fb9bd7b8d 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -84,14 +84,14 @@ impl Block { block_hash: block.block_hash, block_number: block.block_number, parent_hash: block.parent_hash, - total_difficulty: block.total_difficulty, + total_difficulty: Some(block.total_difficulty), timestamp: block.timestamp, }, Block::PoS(payload) => ExecutionBlock { block_hash: payload.block_hash(), block_number: payload.block_number(), parent_hash: payload.parent_hash(), - total_difficulty, + total_difficulty: Some(total_difficulty), timestamp: payload.timestamp(), }, } From 3bb41064d208ebe06a212222fb799e04c7b182a1 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 26 Feb 2025 13:06:09 -0800 Subject: [PATCH 7/8] fixup: fmt fix --- beacon_node/execution_layer/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 439a8810086..cde6cc6f486 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -1770,7 +1770,8 @@ impl ExecutionLayer { let is_total_difficulty_reached = block.terminal_total_difficulty_reached(spec.terminal_total_difficulty); let is_parent_total_difficulty_valid = parent - .total_difficulty.is_some_and(|td| td < spec.terminal_total_difficulty); + .total_difficulty + .is_some_and(|td| td < spec.terminal_total_difficulty); is_total_difficulty_reached && is_parent_total_difficulty_valid } From 015d454d9dc7687babd9219ba1abca38563f4595 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Thu, 27 Feb 2025 06:51:09 -0800 Subject: [PATCH 8/8] fix lint --- beacon_node/execution_layer/src/engine_api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 356ac5567c6..aed6cdba670 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -150,7 +150,7 @@ pub struct ExecutionBlock { impl ExecutionBlock { pub fn terminal_total_difficulty_reached(&self, terminal_total_difficulty: Uint256) -> bool { self.total_difficulty - .map_or(true, |td| td >= terminal_total_difficulty) + .is_none_or(|td| td >= terminal_total_difficulty) } }