Skip to content

Commit

Permalink
Avoid recomputing head
Browse files Browse the repository at this point in the history
  • Loading branch information
macladson committed Apr 12, 2022
1 parent 19a6bd7 commit e56d93b
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 17 deletions.
47 changes: 38 additions & 9 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
})
.collect();

let execution_optimistic = self.is_optimistic_head()?;
let execution_optimistic = self.is_optimistic_head(None)?;

Ok((duties, dependent_root, execution_optimistic))
})
Expand Down Expand Up @@ -3484,6 +3484,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
.beacon_state
.attester_shuffling_decision_root(self.genesis_block_root, RelativeEpoch::Current);

let execution_optimistic = self.is_optimistic_head_block(&new_head.beacon_block)?;

drop(lag_timer);

// Clear the early attester cache in case it conflicts with `self.canonical_head`.
Expand Down Expand Up @@ -3640,7 +3642,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
current_duty_dependent_root,
previous_duty_dependent_root,
epoch_transition: is_epoch_transition,
execution_optimistic: self.is_optimistic_head()?,
execution_optimistic,
}));
}
(Err(e), _) | (_, Err(e)) => {
Expand All @@ -3662,7 +3664,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
new_head_block: beacon_block_root,
new_head_state: state_root,
epoch: head_slot.epoch(T::EthSpec::slots_per_epoch()),
execution_optimistic: self.is_optimistic_head()?,
execution_optimistic,
}));
}

Expand All @@ -3688,7 +3690,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
observed_delay: block_delays.observed,
imported_delay: block_delays.imported,
set_as_head_delay: block_delays.set_as_head,
execution_optimistic: self.is_optimistic_head()?,
execution_optimistic,
}));
}
}
Expand Down Expand Up @@ -4197,20 +4199,47 @@ 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::Unknown`.
pub fn is_optimistic_head_block(
&self,
head_block: &SignedBeaconBlock<T::EthSpec>,
) -> Result<bool, BeaconChainError> {
// Check if the block is pre-Bellatrix.
if head_block.message().execution_payload().is_err() {
Ok(false)
} else {
self.fork_choice
.read()
.is_optimistic_block_no_fallback(&head_block.canonical_root())
.map_err(BeaconChainError::ForkChoiceError)
}
}

/// Returns the value of `execution_optimistic` for the current head block.
/// 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::Unknown`.
pub fn is_optimistic_head(&self) -> Result<bool, BeaconChainError> {
let head_info = self.head_info()?;
let head_block_root = head_info.block_root;
pub fn is_optimistic_head(
&self,
head_info: Option<&HeadInfo>,
) -> Result<bool, BeaconChainError> {
head_info
.map(|head| self.is_optimistic_head_internal(head))
.unwrap_or_else(|| self.is_optimistic_head_internal(&self.head_info()?))
}

fn is_optimistic_head_internal(&self, head_info: &HeadInfo) -> Result<bool, BeaconChainError> {
// Check if the block is pre-Bellatrix.
if head_info.execution_payload_block_hash.is_none() {
Ok(false)
} else {
self.fork_choice
.read()
.is_optimistic_block_no_fallback(&head_block_root)
.is_optimistic_block_no_fallback(&head_info.block_root)
.map_err(BeaconChainError::ForkChoiceError)
}
}
Expand Down Expand Up @@ -4354,7 +4383,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
epoch: new_finalized_checkpoint.epoch,
block: new_finalized_checkpoint.root,
state: new_finalized_state_root,
execution_optimistic: self.is_optimistic_head()?,
execution_optimistic: self.is_optimistic_head(None)?,
}));
}
}
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/beacon_proposer_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ pub fn compute_proposer_duties_from_head<T: BeaconChainTypes>(
.proposer_shuffling_decision_root(chain.genesis_block_root)
.map_err(BeaconChainError::from)?;

let execution_optimistic = chain.is_optimistic_head()?;
let execution_optimistic = chain.is_optimistic_head(None)?;

Ok((indices, dependent_root, execution_optimistic, state.fork()))
}
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/http_api/src/block_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,11 @@ impl BlockId {
// Genesis block is inherently verified.
CoreBlockId::Genesis => false,
CoreBlockId::Head => chain
.is_optimistic_head()
.is_optimistic_head_block(&block)
.map_err(warp_utils::reject::beacon_chain_error)?,
// Slot, Finalized and Justified are determined based on the current head.
CoreBlockId::Slot(_) | CoreBlockId::Finalized | CoreBlockId::Justified => chain
.is_optimistic_head()
.is_optimistic_head_block(&block)
.map_err(warp_utils::reject::beacon_chain_error)?,
// If the root is explicitly given, we can determine based on fork-choice.
CoreBlockId::Root(_) => chain
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ pub fn serve<T: BeaconChainTypes>(
// compute the response is dependent on the head block.
let execution_optimistic = match uses_head {
false => chain.is_optimistic_block(&block),
true => chain.is_optimistic_head(),
true => chain.is_optimistic_head(None),
}
.map_err(warp_utils::reject::beacon_chain_error)?;

Expand Down
2 changes: 1 addition & 1 deletion beacon_node/http_api/src/proposer_duties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ fn try_proposer_duties_from_cache<T: BeaconChainTypes>(
};

let execution_optimistic = chain
.is_optimistic_head()
.is_optimistic_head(Some(&head))
.map_err(warp_utils::reject::beacon_chain_error)?;

chain
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/http_api/src/state_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl StateId {
})
}

/// Convienience function to compute `fork` when `execution_optimistic` isn't desired.
/// Convenience function to compute `fork` when `execution_optimistic` isn't desired.
pub fn fork<T: BeaconChainTypes>(
&self,
chain: &BeaconChain<T>,
Expand Down Expand Up @@ -158,7 +158,7 @@ impl StateId {
| CoreStateId::Slot(_)
| CoreStateId::Finalized
| CoreStateId::Justified => chain
.is_optimistic_head()
.is_optimistic_head(None)
.map_err(warp_utils::reject::beacon_chain_error)?,
CoreStateId::Root(_) => {
let state_root = self.root(chain)?;
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/http_api/src/sync_committees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_head(None)
.map_err(warp_utils::reject::beacon_chain_error)?;

// Try using the head's sync committees to satisfy the request. This should be sufficient for
Expand Down

0 comments on commit e56d93b

Please sign in to comment.