Indicate that invalid blocks are optimistic (#3383)

## Issue Addressed

NA

## Proposed Changes

This PR will make Lighthouse return blocks with invalid payloads via the API with `execution_optimistic = true`. This seems a bit awkward, however I think it's better than returning a 404 or some other error.

Let's consider the case where the only possible head is invalid (#3370 deals with this). In such a scenario all of the duties endpoints will start failing because the head is invalid. I think it would be better if the duties endpoints continue to work, because it's likely that even though the head is invalid the duties are still based upon valid blocks and we want the VC to have them cached. There's no risk to the VC here because we won't actually produce an attestation pointing to an invalid head.

Ultimately, I don't think it's particularly important for us to distinguish between optimistic and invalid blocks on the API. Neither should be trusted and the only *real* reason that we track this is so we can try and fork around the invalid blocks.


## Additional Info

- ~~Blocked on #3370~~
This commit is contained in:
Paul Hauner 2022-07-30 05:08:57 +00:00
parent fdfdb9b57c
commit bcfde6e7df
11 changed files with 91 additions and 65 deletions

View File

@ -4131,8 +4131,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
/// 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<Payload: ExecPayload<T::EthSpec>>(
/// Returns `Ok(true)` if the block has `ExecutionStatus::Optimistic` or has
/// `ExecutionStatus::Invalid`.
pub fn is_optimistic_or_invalid_block<Payload: ExecPayload<T::EthSpec>>(
&self,
block: &SignedBeaconBlock<T::EthSpec, Payload>,
) -> Result<bool, BeaconChainError> {
@ -4142,7 +4143,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
} 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<T: BeaconChainTypes> BeaconChain<T> {
/// 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<T: BeaconChainTypes> BeaconChain<T> {
///
/// 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<Payload: ExecPayload<T::EthSpec>>(
pub fn is_optimistic_or_invalid_head_block<Payload: ExecPayload<T::EthSpec>>(
&self,
head_block: &SignedBeaconBlock<T::EthSpec, Payload>,
) -> Result<bool, BeaconChainError> {
@ -4168,7 +4169,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
} 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<T: BeaconChainTypes> BeaconChain<T> {
/// 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<bool, BeaconChainError> {
pub fn is_optimistic_or_invalid_head(&self) -> Result<bool, BeaconChainError> {
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<T: BeaconChainTypes> BeaconChain<T> {
} 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)
}
}

View File

@ -752,7 +752,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
) -> 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<T: BeaconChainTypes> BeaconChain<T> {
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<E: EthSpec, S: SlotClock>(
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);

View File

@ -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"
);
}

View File

@ -68,7 +68,7 @@ fn cached_attestation_duties<T: BeaconChainTypes>(
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<T: BeaconChainTypes>(
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

View File

@ -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) => {

View File

@ -894,7 +894,7 @@ pub fn serve<T: BeaconChainTypes>(
(
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<T: BeaconChainTypes>(
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<T: BeaconChainTypes>(
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 {

View File

@ -62,7 +62,7 @@ pub fn proposer_duties<T: BeaconChainTypes>(
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<T: BeaconChainTypes>(
.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<T: BeaconChainTypes>(
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<T: BeaconChainTypes>(
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

View File

@ -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<T: BeaconChainTypes>(
};
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)?;

View File

@ -40,7 +40,7 @@ pub fn sync_committee_duties<T: BeaconChainTypes>(
// 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

View File

@ -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<bool, Error<T::Error>> {
pub fn is_optimistic_or_invalid_block(
&self,
block_root: &Hash256,
) -> Result<bool, Error<T::Error>> {
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<bool, Error<T::Error>> {
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))
}

View File

@ -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