From efa6ba37bb89c562034a5d472245a70f77c163e3 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Tue, 4 Mar 2025 17:53:00 -0800 Subject: [PATCH] Make ExecutionBlock::total_difficulty Optional (#7050) This change makes the `total_difficulty` field in `ExecutionBlock` an `Option` since newer clients are no longer including the `totalDifficulty` field. I think this will fix https://github.com/sigp/lighthouse/issues/6937 but I was actually more focused on the builder registration case described below. In our [builder-playground](https://github.com/flashbots/builder-playground) we setup a local devnet using lighthouse, reth, and mev-boost-relay. After upgrading to reth 1.2.0 and lighthouse v7.0.0.beta.0 for Pectra, we noticed that the validator registration process was _sometimes_ failing with: ``` Feb 25 23:35:25.038 ERRO Unable to publish proposer preparation to all beacon nodes, error: Some endpoints failed, num_failed: 1 http://localhost:3500/ => RequestFailed(ServerMessage(ErrorMessage { code: 400, message: "BAD_REQUEST: error updating proposer preparations: ForkchoiceUpdate(EngineError(Api { error: Json(Error(\"missing field `totalDifficulty`\", line: 0, column: 0)) }))", stacktraces: [] })), service: preparation Feb 25 23:35:25.099 WARN Unable to publish validator registrations to the builder network, error: Some endpoints failed, num_failed: 1 http://localhost:3500/ => RequestFailed(ServerMessage(ErrorMessage { code: 400, message: "BAD_REQUEST: error updating proposer preparations: ForkchoiceUpdate(EngineError(Api { error: Json(Error(\"missing field `totalDifficulty`\", line: 0, column: 0)) }))", stacktraces: [] })), service: preparation ``` What was even more confusing, was that it was sometimes working, which actually led to a wild goose chase thinking it was a networking issue. However, when tracing through the LH code, I came across this comment: https://github.com/sigp/lighthouse/blob/70194dfc6a3f4d10c9059610f889ff5a4e863a6a/beacon_node/beacon_chain/src/beacon_chain.rs#L6048-L6049 This explained why it sometimes worked, in our playground we run lighthouse with `--prepare-payload-lookahead 8000` thus there was always a 4-second window where the call wasn't made. But, if the call was made, then this code would 100% fail with updated reth: https://github.com/sigp/lighthouse/blob/unstable/beacon_node/execution_layer/src/lib.rs#L1688-L1692 Which would then mapped to a `Error::ForkchoiceUpdate` in `update_execution_engine_forkchoice`. Anyways, the fix was to make `total_difficulty` Optional, and then to update any code paths where it was used. In doing so, I assume that if the EL doesn't include total difficulty then the chain is already post-merge. --- .../beacon_chain/src/bellatrix_readiness.rs | 2 +- beacon_node/execution_layer/src/engine_api.rs | 9 ++++++++- beacon_node/execution_layer/src/lib.rs | 16 ++++++++++------ .../src/test_utils/execution_block_generator.rs | 4 ++-- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/bellatrix_readiness.rs b/beacon_node/beacon_chain/src/bellatrix_readiness.rs index 500588953f..412870354b 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 b9d878b1f8..aed6cdba67 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 + .is_none_or(|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 4fd7188c20..cde6cc6f48 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,11 @@ 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 5a5c9e1fa9..81fb9bd7b8 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(), }, }