diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 10506f3038..fec7fe25ff 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4131,8 +4131,9 @@ impl BeaconChain { /// Returns the value of `execution_optimistic` for `block`. /// /// Returns `Ok(false)` if the block is pre-Bellatrix, or has `ExecutionStatus::Valid`. - /// Returns `Ok(true)` if the block has `ExecutionStatus::Optimistic`. - pub fn is_optimistic_block>( + /// Returns `Ok(true)` if the block has `ExecutionStatus::Optimistic` or has + /// `ExecutionStatus::Invalid`. + pub fn is_optimistic_or_invalid_block>( &self, block: &SignedBeaconBlock, ) -> Result { @@ -4142,7 +4143,7 @@ impl BeaconChain { } else { self.canonical_head .fork_choice_read_lock() - .is_optimistic_block(&block.canonical_root()) + .is_optimistic_or_invalid_block(&block.canonical_root()) .map_err(BeaconChainError::ForkChoiceError) } } @@ -4150,7 +4151,7 @@ impl BeaconChain { /// Returns the value of `execution_optimistic` for `head_block`. /// /// Returns `Ok(false)` if the block is pre-Bellatrix, or has `ExecutionStatus::Valid`. - /// Returns `Ok(true)` if the block has `ExecutionStatus::Optimistic`. + /// Returns `Ok(true)` if the block has `ExecutionStatus::Optimistic` or `ExecutionStatus::Invalid`. /// /// This function will return an error if `head_block` is not present in the fork choice store /// and so should only be used on the head block or when the block *should* be present in the @@ -4158,7 +4159,7 @@ impl BeaconChain { /// /// There is a potential race condition when syncing where the block_root of `head_block` could /// be pruned from the fork choice store before being read. - pub fn is_optimistic_head_block>( + pub fn is_optimistic_or_invalid_head_block>( &self, head_block: &SignedBeaconBlock, ) -> Result { @@ -4168,7 +4169,7 @@ impl BeaconChain { } else { self.canonical_head .fork_choice_read_lock() - .is_optimistic_block_no_fallback(&head_block.canonical_root()) + .is_optimistic_or_invalid_block_no_fallback(&head_block.canonical_root()) .map_err(BeaconChainError::ForkChoiceError) } } @@ -4177,17 +4178,17 @@ impl BeaconChain { /// You can optionally provide `head_info` if it was computed previously. /// /// Returns `Ok(false)` if the head block is pre-Bellatrix, or has `ExecutionStatus::Valid`. - /// Returns `Ok(true)` if the head block has `ExecutionStatus::Optimistic`. + /// Returns `Ok(true)` if the head block has `ExecutionStatus::Optimistic` or `ExecutionStatus::Invalid`. /// /// There is a potential race condition when syncing where the block root of `head_info` could /// be pruned from the fork choice store before being read. - pub fn is_optimistic_head(&self) -> Result { + pub fn is_optimistic_or_invalid_head(&self) -> Result { self.canonical_head .head_execution_status() - .map(|status| status.is_optimistic()) + .map(|status| status.is_optimistic_or_invalid()) } - pub fn is_optimistic_block_root( + pub fn is_optimistic_or_invalid_block_root( &self, block_slot: Slot, block_root: &Hash256, @@ -4198,7 +4199,7 @@ impl BeaconChain { } else { self.canonical_head .fork_choice_read_lock() - .is_optimistic_block_no_fallback(block_root) + .is_optimistic_or_invalid_block_no_fallback(block_root) .map_err(BeaconChainError::ForkChoiceError) } } diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index c37f266824..709382f05b 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -752,7 +752,9 @@ impl BeaconChain { ) -> Result<(), Error> { let old_snapshot = &old_cached_head.snapshot; let new_snapshot = &new_cached_head.snapshot; - let new_head_is_optimistic = new_head_proto_block.execution_status.is_optimistic(); + let new_head_is_optimistic = new_head_proto_block + .execution_status + .is_optimistic_or_invalid(); // Detect and potentially report any re-orgs. let reorg_distance = detect_reorg( @@ -883,7 +885,9 @@ impl BeaconChain { finalized_proto_block: ProtoBlock, ) -> Result<(), Error> { let new_snapshot = &new_cached_head.snapshot; - let finalized_block_is_optimistic = finalized_proto_block.execution_status.is_optimistic(); + let finalized_block_is_optimistic = finalized_proto_block + .execution_status + .is_optimistic_or_invalid(); self.op_pool .prune_all(&new_snapshot.beacon_state, self.epoch()?); @@ -1260,7 +1264,7 @@ fn observe_head_block_delays( let block_time_set_as_head = timestamp_now(); let head_block_root = head_block.root; let head_block_slot = head_block.slot; - let head_block_is_optimistic = head_block.execution_status.is_optimistic(); + let head_block_is_optimistic = head_block.execution_status.is_optimistic_or_invalid(); // Calculate the total delay between the start of the slot and when it was set as head. let block_delay_total = get_slot_delay_ms(block_time_set_as_head, head_block_slot, slot_clock); diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index df0c61f532..5e03ef2335 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -291,7 +291,7 @@ impl InvalidPayloadRig { let execution_status = self.execution_status(root.into()); match forkchoice_response { - Payload::Syncing => assert!(execution_status.is_optimistic()), + Payload::Syncing => assert!(execution_status.is_strictly_optimistic()), Payload::Valid => assert!(execution_status.is_valid_and_post_bellatrix()), Payload::Invalid { .. } | Payload::InvalidBlockHash @@ -421,7 +421,7 @@ async fn invalid_payload_invalidates_parent() { }) .await; - assert!(rig.execution_status(roots[0]).is_optimistic()); + assert!(rig.execution_status(roots[0]).is_strictly_optimistic()); assert!(rig.execution_status(roots[1]).is_invalid()); assert!(rig.execution_status(roots[2]).is_invalid()); @@ -555,7 +555,7 @@ async fn pre_finalized_latest_valid_hash() { if slot == 1 { assert!(rig.execution_status(root).is_valid_and_post_bellatrix()); } else { - assert!(rig.execution_status(root).is_optimistic()); + assert!(rig.execution_status(root).is_strictly_optimistic()); } } } @@ -605,7 +605,7 @@ async fn latest_valid_hash_will_not_validate() { } else if slot == 1 { assert!(execution_status.is_valid_and_post_bellatrix()) } else { - assert!(execution_status.is_optimistic()) + assert!(execution_status.is_strictly_optimistic()) } } } @@ -646,7 +646,7 @@ async fn latest_valid_hash_is_junk() { if slot == 1 { assert!(rig.execution_status(root).is_valid_and_post_bellatrix()); } else { - assert!(rig.execution_status(root).is_optimistic()); + assert!(rig.execution_status(root).is_strictly_optimistic()); } } } @@ -734,7 +734,7 @@ async fn invalidates_all_descendants() { assert!(execution_status.is_valid_and_post_bellatrix()); } else if slot <= latest_valid_slot { // Blocks prior to and included the latest valid hash are not marked as valid. - assert!(execution_status.is_optimistic()); + assert!(execution_status.is_strictly_optimistic()); } else { // Blocks after the latest valid hash are invalid. assert!(execution_status.is_invalid()); @@ -791,7 +791,9 @@ async fn switches_heads() { assert_eq!(rig.harness.head_block_root(), fork_block_root); // The fork block has not yet been validated. - assert!(rig.execution_status(fork_block_root).is_optimistic()); + assert!(rig + .execution_status(fork_block_root) + .is_strictly_optimistic()); for root in blocks { let slot = rig @@ -816,7 +818,7 @@ async fn switches_heads() { assert!(execution_status.is_valid_and_post_bellatrix()); } else if slot <= latest_valid_slot { // Blocks prior to and included the latest valid hash are not marked as valid. - assert!(execution_status.is_optimistic()); + assert!(execution_status.is_strictly_optimistic()); } else { // Blocks after the latest valid hash are invalid. assert!(execution_status.is_invalid()); @@ -899,8 +901,8 @@ async fn manually_validate_child() { let parent = rig.import_block(Payload::Syncing).await; let child = rig.import_block(Payload::Syncing).await; - assert!(rig.execution_status(parent).is_optimistic()); - assert!(rig.execution_status(child).is_optimistic()); + assert!(rig.execution_status(parent).is_strictly_optimistic()); + assert!(rig.execution_status(child).is_strictly_optimistic()); rig.validate_manually(child); @@ -917,13 +919,13 @@ async fn manually_validate_parent() { let parent = rig.import_block(Payload::Syncing).await; let child = rig.import_block(Payload::Syncing).await; - assert!(rig.execution_status(parent).is_optimistic()); - assert!(rig.execution_status(child).is_optimistic()); + assert!(rig.execution_status(parent).is_strictly_optimistic()); + assert!(rig.execution_status(child).is_strictly_optimistic()); rig.validate_manually(parent); assert!(rig.execution_status(parent).is_valid_and_post_bellatrix()); - assert!(rig.execution_status(child).is_optimistic()); + assert!(rig.execution_status(child).is_strictly_optimistic()); } #[tokio::test] @@ -1124,7 +1126,7 @@ async fn attesting_to_optimistic_head() { "the head should be the latest imported block" ); assert!( - rig.execution_status(root).is_optimistic(), + rig.execution_status(root).is_strictly_optimistic(), "the head should be optimistic" ); @@ -1371,7 +1373,7 @@ async fn build_optimistic_chain( .chain .canonical_head .fork_choice_read_lock() - .is_optimistic_block(&post_transition_block_root) + .is_optimistic_or_invalid_block(&post_transition_block_root) .unwrap(), "the transition block should be imported optimistically" ); @@ -1636,7 +1638,7 @@ async fn optimistic_transition_block_invalid_unfinalized_syncing_ee() { // It should still be marked as optimistic. assert!(rig .execution_status(post_transition_block_root) - .is_optimistic()); + .is_strictly_optimistic()); // the optimistic merge transition block should NOT have been removed from the database let otbs = load_optimistic_transition_blocks(&rig.harness.chain) @@ -1913,8 +1915,9 @@ async fn recover_from_invalid_head_after_persist_and_reboot() { .chain .canonical_head .fork_choice_read_lock() - .is_optimistic_block(&resumed_head.head_block_root()) - .unwrap(), + .get_block_execution_status(&resumed_head.head_block_root()) + .unwrap() + .is_strictly_optimistic(), "the invalid block should have become optimistic" ); } diff --git a/beacon_node/http_api/src/attester_duties.rs b/beacon_node/http_api/src/attester_duties.rs index 6805d7104c..9febae5b19 100644 --- a/beacon_node/http_api/src/attester_duties.rs +++ b/beacon_node/http_api/src/attester_duties.rs @@ -68,7 +68,7 @@ fn cached_attestation_duties( duties, request_indices, dependent_root, - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), chain, ) } @@ -95,7 +95,7 @@ fn compute_historic_attester_duties( head.beacon_state_root(), head.beacon_state .clone_with(CloneConfig::committee_caches_only()), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), )) } else { None diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index 91425e2f10..e418849040 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -33,7 +33,7 @@ impl BlockId { .map_err(warp_utils::reject::beacon_chain_error)?; Ok(( cached_head.head_block_root(), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), )) } CoreBlockId::Genesis => Ok((chain.genesis_block_root, false)), @@ -53,7 +53,7 @@ impl BlockId { } CoreBlockId::Slot(slot) => { let execution_optimistic = chain - .is_optimistic_head() + .is_optimistic_or_invalid_head() .map_err(warp_utils::reject::beacon_chain_error)?; let root = chain .block_root_at_slot(*slot, WhenSlotSkipped::None) @@ -85,7 +85,7 @@ impl BlockId { let execution_optimistic = chain .canonical_head .fork_choice_read_lock() - .is_optimistic_block(root) + .is_optimistic_or_invalid_block(root) .map_err(BeaconChainError::ForkChoiceError) .map_err(warp_utils::reject::beacon_chain_error)?; Ok((*root, execution_optimistic)) @@ -112,7 +112,7 @@ impl BlockId { .map_err(warp_utils::reject::beacon_chain_error)?; Ok(( cached_head.snapshot.beacon_block.clone_as_blinded(), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), )) } CoreBlockId::Slot(slot) => { @@ -167,7 +167,7 @@ impl BlockId { .map_err(warp_utils::reject::beacon_chain_error)?; Ok(( cached_head.snapshot.beacon_block.clone(), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), )) } CoreBlockId::Slot(slot) => { diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index a1b23c7f03..a8e305f3c1 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -894,7 +894,7 @@ pub fn serve( ( cached_head.head_block_root(), cached_head.snapshot.beacon_block.clone_as_blinded(), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), ) } // Only the parent root parameter, do a forwards-iterator lookup. @@ -1608,7 +1608,7 @@ pub fn serve( chain .canonical_head .fork_choice_read_lock() - .is_optimistic_block(&root) + .is_optimistic_or_invalid_block(&root) .ok() } else { return Err(unsupported_version_rejection(endpoint_version)); @@ -1699,7 +1699,7 @@ pub fn serve( let sync_distance = current_slot - head_slot; let is_optimistic = chain - .is_optimistic_head() + .is_optimistic_or_invalid_head() .map_err(warp_utils::reject::beacon_chain_error)?; let syncing_data = api_types::SyncingData { diff --git a/beacon_node/http_api/src/proposer_duties.rs b/beacon_node/http_api/src/proposer_duties.rs index 13788a07b2..877d64e20f 100644 --- a/beacon_node/http_api/src/proposer_duties.rs +++ b/beacon_node/http_api/src/proposer_duties.rs @@ -62,7 +62,7 @@ pub fn proposer_duties( chain, request_epoch, dependent_root, - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), proposers, ) } else if request_epoch @@ -104,7 +104,7 @@ fn try_proposer_duties_from_cache( .map_err(warp_utils::reject::beacon_state_error)?; let head_epoch = head_block.slot().epoch(T::EthSpec::slots_per_epoch()); let execution_optimistic = chain - .is_optimistic_head_block(head_block) + .is_optimistic_or_invalid_head_block(head_block) .map_err(warp_utils::reject::beacon_chain_error)?; let dependent_root = match head_epoch.cmp(&request_epoch) { @@ -168,7 +168,7 @@ fn compute_and_cache_proposer_duties( chain, current_epoch, dependent_root, - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), indices, ) } @@ -194,7 +194,7 @@ fn compute_historic_proposer_duties( head.beacon_state_root(), head.beacon_state .clone_with(CloneConfig::committee_caches_only()), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), )) } else { None diff --git a/beacon_node/http_api/src/state_id.rs b/beacon_node/http_api/src/state_id.rs index af47c242d6..051789c953 100644 --- a/beacon_node/http_api/src/state_id.rs +++ b/beacon_node/http_api/src/state_id.rs @@ -28,7 +28,7 @@ impl StateId { .map_err(warp_utils::reject::beacon_chain_error)?; return Ok(( cached_head.head_state_root(), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), )); } CoreStateId::Genesis => return Ok((chain.genesis_state_root, false)), @@ -45,7 +45,7 @@ impl StateId { CoreStateId::Slot(slot) => ( *slot, chain - .is_optimistic_head() + .is_optimistic_or_invalid_head() .map_err(warp_utils::reject::beacon_chain_error)?, ), CoreStateId::Root(root) => { @@ -58,7 +58,7 @@ impl StateId { let execution_optimistic = chain .canonical_head .fork_choice_read_lock() - .is_optimistic_block_no_fallback(&hot_summary.latest_block_root) + .is_optimistic_or_invalid_block_no_fallback(&hot_summary.latest_block_root) .map_err(BeaconChainError::ForkChoiceError) .map_err(warp_utils::reject::beacon_chain_error)?; return Ok((*root, execution_optimistic)); @@ -74,7 +74,7 @@ impl StateId { .finalized_checkpoint .root; let execution_optimistic = fork_choice - .is_optimistic_block_no_fallback(&finalized_root) + .is_optimistic_or_invalid_block_no_fallback(&finalized_root) .map_err(BeaconChainError::ForkChoiceError) .map_err(warp_utils::reject::beacon_chain_error)?; return Ok((*root, execution_optimistic)); @@ -133,7 +133,7 @@ impl StateId { .snapshot .beacon_state .clone_with_only_committee_caches(), - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), )); } CoreStateId::Slot(slot) => (self.root(chain)?, Some(*slot)), @@ -198,7 +198,7 @@ impl StateId { .map_err(warp_utils::reject::beacon_chain_error)?; return func( &head.snapshot.beacon_state, - execution_status.is_optimistic(), + execution_status.is_optimistic_or_invalid(), ); } _ => self.state(chain)?, @@ -241,7 +241,7 @@ pub fn checkpoint_slot_and_execution_optimistic( }; let execution_optimistic = fork_choice - .is_optimistic_block_no_fallback(root) + .is_optimistic_or_invalid_block_no_fallback(root) .map_err(BeaconChainError::ForkChoiceError) .map_err(warp_utils::reject::beacon_chain_error)?; diff --git a/beacon_node/http_api/src/sync_committees.rs b/beacon_node/http_api/src/sync_committees.rs index 54a3e075d3..77becef7df 100644 --- a/beacon_node/http_api/src/sync_committees.rs +++ b/beacon_node/http_api/src/sync_committees.rs @@ -40,7 +40,7 @@ pub fn sync_committee_duties( // Even when computing duties from state, any block roots pulled using the request epoch are // still dependent on the head. So using `is_optimistic_head` is fine for both cases. let execution_optimistic = chain - .is_optimistic_head() + .is_optimistic_or_invalid_head() .map_err(warp_utils::reject::beacon_chain_error)?; // Try using the head's sync committees to satisfy the request. This should be sufficient for diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index c17c46a777..5438aaf62b 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1269,34 +1269,40 @@ where .is_descendant(self.fc_store.finalized_checkpoint().root, block_root) } - /// Returns `Ok(true)` if `block_root` has been imported optimistically. That is, the - /// execution payload has not been verified. + /// Returns `Ok(true)` if `block_root` has been imported optimistically or deemed invalid. /// - /// Returns `Ok(false)` if `block_root`'s execution payload has been verfied, if it is a - /// pre-Bellatrix block or if it is before the PoW terminal block. + /// Returns `Ok(false)` if `block_root`'s execution payload has been elected as fully VALID, if + /// it is a pre-Bellatrix block or if it is before the PoW terminal block. /// /// In the case where the block could not be found in fork-choice, it returns the /// `execution_status` of the current finalized block. /// /// This function assumes the `block_root` exists. - pub fn is_optimistic_block(&self, block_root: &Hash256) -> Result> { + pub fn is_optimistic_or_invalid_block( + &self, + block_root: &Hash256, + ) -> Result> { if let Some(status) = self.get_block_execution_status(block_root) { - Ok(status.is_optimistic()) + Ok(status.is_optimistic_or_invalid()) } else { - Ok(self.get_finalized_block()?.execution_status.is_optimistic()) + Ok(self + .get_finalized_block()? + .execution_status + .is_optimistic_or_invalid()) } } /// The same as `is_optimistic_block` but does not fallback to `self.get_finalized_block` /// when the block cannot be found. /// - /// Intended to be used when checking if the head has been imported optimistically. - pub fn is_optimistic_block_no_fallback( + /// Intended to be used when checking if the head has been imported optimistically or is + /// invalid. + pub fn is_optimistic_or_invalid_block_no_fallback( &self, block_root: &Hash256, ) -> Result> { if let Some(status) = self.get_block_execution_status(block_root) { - Ok(status.is_optimistic()) + Ok(status.is_optimistic_or_invalid()) } else { Err(Error::MissingProtoArrayBlock(*block_root)) } diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 3ecdc68a2e..306c986018 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -89,10 +89,22 @@ impl ExecutionStatus { /// /// - Has execution enabled, AND /// - Has a payload that has not yet been verified by an EL. - pub fn is_optimistic(&self) -> bool { + pub fn is_strictly_optimistic(&self) -> bool { matches!(self, ExecutionStatus::Optimistic(_)) } + /// Returns `true` if the block: + /// + /// - Has execution enabled, AND + /// - Has a payload that has not yet been verified by an EL, OR. + /// - Has a payload that has been deemed invalid by an EL. + pub fn is_optimistic_or_invalid(&self) -> bool { + matches!( + self, + ExecutionStatus::Optimistic(_) | ExecutionStatus::Invalid(_) + ) + } + /// Returns `true` if the block: /// /// - Has execution enabled, AND