Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions beacon_node/beacon_chain/src/beacon_proposer_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,17 @@ impl BeaconProposerCache {
}

/// Compute the proposer duties using the head state without cache.
///
/// Return:
/// - Proposer indices.
/// - True dependent root.
/// - Legacy dependent root (last block of epoch `N - 1`).
/// - Head execution status.
/// - Fork at `request_epoch`.
pub fn compute_proposer_duties_from_head<T: BeaconChainTypes>(
request_epoch: Epoch,
chain: &BeaconChain<T>,
) -> Result<(Vec<usize>, Hash256, ExecutionStatus, Fork), BeaconChainError> {
) -> Result<(Vec<usize>, Hash256, Hash256, ExecutionStatus, Fork), BeaconChainError> {
// Atomically collect information about the head whilst holding the canonical head `Arc` as
// short as possible.
let (mut state, head_state_root, head_block_root) = {
Expand Down Expand Up @@ -203,11 +210,23 @@ pub fn compute_proposer_duties_from_head<T: BeaconChainTypes>(
.proposer_shuffling_decision_root_at_epoch(request_epoch, head_block_root, &chain.spec)
.map_err(BeaconChainError::from)?;

// This is only required because the V1 proposer duties endpoint spec wasn't updated for Fulu. We
// can delete this once the V1 endpoint is deprecated at the Glamsterdam fork.
let legacy_dependent_root = state
.legacy_proposer_shuffling_decision_root_at_epoch(request_epoch, head_block_root)
.map_err(BeaconChainError::from)?;

// Use fork_at_epoch rather than the state's fork, because post-Fulu we may not have advanced
// the state completely into the new epoch.
let fork = chain.spec.fork_at_epoch(request_epoch);

Ok((indices, dependent_root, execution_status, fork))
Ok((
indices,
dependent_root,
legacy_dependent_root,
execution_status,
fork,
))
}

/// If required, advance `state` to the epoch required to determine proposer indices in `target_epoch`.
Expand Down
9 changes: 6 additions & 3 deletions beacon_node/beacon_chain/tests/store_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1561,7 +1561,7 @@ async fn proposer_duties_from_head_fulu() {

// Compute the proposer duties at the next epoch from the head
let next_epoch = head_state.next_epoch().unwrap();
let (_indices, dependent_root, _, fork) =
let (_indices, dependent_root, legacy_dependent_root, _, fork) =
compute_proposer_duties_from_head(next_epoch, &harness.chain).unwrap();

assert_eq!(
Expand All @@ -1570,6 +1570,8 @@ async fn proposer_duties_from_head_fulu() {
.proposer_shuffling_decision_root_at_epoch(next_epoch, head_block_root.into(), spec)
.unwrap()
);
assert_ne!(dependent_root, legacy_dependent_root);
assert_eq!(legacy_dependent_root, Hash256::from(head_block_root));
assert_eq!(fork, head_state.fork());
}

Expand Down Expand Up @@ -1617,7 +1619,7 @@ async fn proposer_lookahead_gloas_fork_epoch() {
assert_eq!(head_state.current_epoch(), gloas_fork_epoch - 1);

// Compute the proposer duties at the fork epoch from the head.
let (indices, dependent_root, _, fork) =
let (indices, dependent_root, legacy_dependent_root, _, fork) =
compute_proposer_duties_from_head(gloas_fork_epoch, &harness.chain).unwrap();

assert_eq!(
Expand All @@ -1630,6 +1632,7 @@ async fn proposer_lookahead_gloas_fork_epoch() {
)
.unwrap()
);
assert_ne!(dependent_root, legacy_dependent_root);
assert_ne!(fork, head_state.fork());
assert_eq!(fork, spec.fork_at_epoch(gloas_fork_epoch));

Expand All @@ -1639,7 +1642,7 @@ async fn proposer_lookahead_gloas_fork_epoch() {
.add_attested_blocks_at_slots(head_state, head_state_root, &gloas_slots, &all_validators)
.await;

let (no_lookahead_indices, no_lookahead_dependent_root, _, no_lookahead_fork) =
let (no_lookahead_indices, no_lookahead_dependent_root, _, _, no_lookahead_fork) =
compute_proposer_duties_from_head(gloas_fork_epoch, &harness.chain).unwrap();

assert_eq!(no_lookahead_indices, indices);
Expand Down
27 changes: 19 additions & 8 deletions beacon_node/http_api/src/proposer_duties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ pub fn proposer_duties<T: BeaconChainTypes>(
.safe_add(1)
.map_err(warp_utils::reject::arith_error)?
{
let (proposers, dependent_root, execution_status, _fork) =
let (proposers, _dependent_root, legacy_dependent_root, execution_status, _fork) =
compute_proposer_duties_from_head(request_epoch, chain)
.map_err(warp_utils::reject::unhandled_error)?;
convert_to_api_response(
chain,
request_epoch,
dependent_root,
legacy_dependent_root,
execution_status.is_optimistic_or_invalid(),
proposers,
)
Expand Down Expand Up @@ -116,6 +116,11 @@ fn try_proposer_duties_from_cache<T: BeaconChainTypes>(
.beacon_state
.proposer_shuffling_decision_root_at_epoch(request_epoch, head_block_root, &chain.spec)
.map_err(warp_utils::reject::beacon_state_error)?;
let legacy_dependent_root = head
.snapshot
.beacon_state
.legacy_proposer_shuffling_decision_root_at_epoch(request_epoch, head_block_root)
.map_err(warp_utils::reject::beacon_state_error)?;
let execution_optimistic = chain
.is_optimistic_or_invalid_head_block(head_block)
.map_err(warp_utils::reject::unhandled_error)?;
Expand All @@ -129,7 +134,7 @@ fn try_proposer_duties_from_cache<T: BeaconChainTypes>(
convert_to_api_response(
chain,
request_epoch,
head_decision_root,
legacy_dependent_root,
execution_optimistic,
indices.to_vec(),
)
Expand All @@ -151,7 +156,7 @@ fn compute_and_cache_proposer_duties<T: BeaconChainTypes>(
current_epoch: Epoch,
chain: &BeaconChain<T>,
) -> Result<ApiDuties, warp::reject::Rejection> {
let (indices, dependent_root, execution_status, fork) =
let (indices, dependent_root, legacy_dependent_root, execution_status, fork) =
compute_proposer_duties_from_head(current_epoch, chain)
.map_err(warp_utils::reject::unhandled_error)?;

Expand All @@ -166,7 +171,7 @@ fn compute_and_cache_proposer_duties<T: BeaconChainTypes>(
convert_to_api_response(
chain,
current_epoch,
dependent_root,
legacy_dependent_root,
execution_status.is_optimistic_or_invalid(),
indices,
)
Expand Down Expand Up @@ -229,12 +234,18 @@ fn compute_historic_proposer_duties<T: BeaconChainTypes>(

// We can supply the genesis block root as the block root since we know that the only block that
// decides its own root is the genesis block.
let dependent_root = state
.proposer_shuffling_decision_root(chain.genesis_block_root, &chain.spec)
let legacy_dependent_root = state
.legacy_proposer_shuffling_decision_root_at_epoch(epoch, chain.genesis_block_root)
.map_err(BeaconChainError::from)
.map_err(warp_utils::reject::unhandled_error)?;

convert_to_api_response(chain, epoch, dependent_root, execution_optimistic, indices)
convert_to_api_response(
chain,
epoch,
legacy_dependent_root,
execution_optimistic,
indices,
)
}

/// Converts the internal representation of proposer duties into one that is compatible with the
Expand Down
3 changes: 1 addition & 2 deletions beacon_node/http_api/tests/interactive_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1017,10 +1017,9 @@ async fn proposer_duties_with_gossip_tolerance() {
assert_eq!(
proposer_duties_tolerant_current_epoch.dependent_root,
head_state
.proposer_shuffling_decision_root_at_epoch(
.legacy_proposer_shuffling_decision_root_at_epoch(
tolerant_current_epoch,
head_block_root,
spec
)
.unwrap()
);
Expand Down
16 changes: 16 additions & 0 deletions consensus/types/src/beacon_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,22 @@ impl<E: EthSpec> BeaconState<E> {
}
}

/// Returns the block root at the last slot of `epoch - 1`.
///
/// This can be deleted after Glamsterdam and the removal of the v1 proposer duties endpoint.
pub fn legacy_proposer_shuffling_decision_root_at_epoch(
&self,
epoch: Epoch,
head_block_root: Hash256,
) -> Result<Hash256, Error> {
let decision_slot = epoch.saturating_sub(1u64).end_slot(E::slots_per_epoch());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine, but just want to note that the previous logic uses min_seed_lookahead, but no live network modifies this config

// Pre-Fulu the proposer shuffling decision slot for epoch N is the slot at the end of
// epoch N - 1 (note: +1 -1 for min_seed_lookahead=1 in all current configs).
epoch
.saturating_add(Epoch::new(1))
.saturating_sub(self.min_seed_lookahead)
.start_slot(E::slots_per_epoch())
.saturating_sub(1_u64)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah good point. Should be OK seeing as we plan to deprecate v1 proposer duties at Glamsterdam

if self.slot() <= decision_slot {
Ok(head_block_root)
} else {
self.get_block_root(decision_slot).copied()
}
}

/// Returns the block root which decided the proposer shuffling for the current epoch. This root
/// can be used to key this proposer shuffling.
///
Expand Down
2 changes: 1 addition & 1 deletion testing/ef_tests/src/cases/fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,7 @@ impl<E: EthSpec> Tester<E> {
let cached_head = self.harness.chain.canonical_head.cached_head();
let next_slot = cached_head.snapshot.beacon_block.slot() + 1;
let next_slot_epoch = next_slot.epoch(E::slots_per_epoch());
let (proposer_indices, decision_root, _, fork) =
let (proposer_indices, decision_root, _, _, fork) =
compute_proposer_duties_from_head(next_slot_epoch, &self.harness.chain).unwrap();
let proposer_index = proposer_indices[next_slot.as_usize() % E::slots_per_epoch() as usize];

Expand Down
Loading