Proposer duties backwards compat (#8335)

The beacon API spec wasn't updated to use the Fulu definition of `dependent_root` for the proposer duties endpoint. No other client updated their logic, so to retain backwards compatibility the decision has been made to continue using the block root at the end of epoch `N - 1`, and introduce a new v2 endpoint down the track to use the correct dependent root.

Eth R&D discussion: https://discord.com/channels/595666850260713488/598292067260825641/1433036715848765562


  Change the behaviour of the v1 endpoint back to using the last slot of `N - 1` rather than the last slot of `N - 2`. This introduces the possibility of dependent root false positives (the root can change without changing the shuffling), but causes the least compatibility issues with other clients.


Co-Authored-By: Michael Sproul <[email protected]>

Author: michaelsproul

beacon_node/beacon_chain/src/beacon_proposer_cache.rs

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

beacon_node/beacon_chain/tests/store_tests.rs

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

beacon_node/http_api/src/proposer_duties.rs

@@ -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,
         )
@@ -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)?;
@@ -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(),
             )
@@ -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)?;
 
@@ -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,
     )
@@ -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

beacon_node/http_api/tests/interactive_tests.rs

@@ -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()
     );

consensus/types/src/beacon_state.rs

@@ -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());
+        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.
     ///

testing/ef_tests/src/cases/fork_choice.rs

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