Skip to content

Commit b207727

Browse files
eserileveserilev
committed
2 parents 8d06310 + 092aaae commit b207727

File tree

23 files changed

+879
-167
lines changed

23 files changed

+879
-167
lines changed

‎Cargo.lock‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎beacon_node/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ path = "src/lib.rs"
1515
write_ssz_files = [
1616
"beacon_chain/write_ssz_files",
1717
] # Writes debugging .ssz files to /tmp during block processing.
18+
testing = [] # Enables testing-only CLI flags
1819

1920
[dependencies]
2021
account_utils = { workspace = true }

‎beacon_node/beacon_chain/src/beacon_chain.rs‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4726,6 +4726,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
47264726
// efficient packing of execution blocks.
47274727
Err(Error::SkipProposerPreparation)
47284728
} else {
4729+
debug!(
4730+
?shuffling_decision_root,
4731+
epoch = %proposal_epoch,
4732+
"Proposer shuffling cache miss for proposer prep"
4733+
);
47294734
let head = self.canonical_head.cached_head();
47304735
Ok((
47314736
head.head_state_root(),
@@ -6557,6 +6562,26 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
65576562
}
65586563
}
65596564

6565+
/// This function provides safe and efficient multi-threaded access to the beacon proposer cache.
6566+
///
6567+
/// The arguments are:
6568+
///
6569+
/// - `shuffling_decision_block`: The block root of the decision block for the desired proposer
6570+
/// shuffling. This should be computed using one of the methods for computing proposer
6571+
/// shuffling decision roots, e.g. `BeaconState::proposer_shuffling_decision_root_at_epoch`.
6572+
/// - `proposal_epoch`: The epoch at which the proposer shuffling is required.
6573+
/// - `accessor`: A closure to run against the proposers for the selected epoch. Usually this
6574+
/// closure just grabs a single proposer, or takes the vec of proposers for the epoch.
6575+
/// - `state_provider`: A closure to compute a state suitable for determining the shuffling.
6576+
/// This closure is evaluated lazily ONLY in the case that a cache miss occurs. It is
6577+
/// recommended for code that wants to keep track of cache misses to produce a log and/or
6578+
/// increment a metric inside this closure .
6579+
///
6580+
/// This function makes use of closures in order to efficiently handle concurrent accesses to
6581+
/// the cache.
6582+
///
6583+
/// The error type is polymorphic, if in doubt you can use `BeaconChainError`. You might need
6584+
/// to use a turbofish if type inference can't work it out.
65606585
pub fn with_proposer_cache<V, E: From<BeaconChainError> + From<BeaconStateError>>(
65616586
&self,
65626587
shuffling_decision_block: Hash256,
@@ -6575,12 +6600,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
65756600
// If it is already initialised, then `get_or_try_init` will return immediately without
65766601
// executing the initialisation code at all.
65776602
let epoch_block_proposers = cache_entry.get_or_try_init(|| {
6578-
debug!(
6579-
?shuffling_decision_block,
6580-
%proposal_epoch,
6581-
"Proposer shuffling cache miss"
6582-
);
6583-
65846603
// Fetch the state on-demand if the required epoch was missing from the cache.
65856604
// If the caller wants to not compute the state they must return an error here and then
65866605
// catch it at the call site.
@@ -6610,11 +6629,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
66106629
}
66116630

66126631
let proposers = state.get_beacon_proposer_indices(proposal_epoch, &self.spec)?;
6613-
Ok::<_, E>(EpochBlockProposers::new(
6614-
proposal_epoch,
6615-
state.fork(),
6616-
proposers,
6617-
))
6632+
6633+
// Use fork_at_epoch rather than the state's fork, because post-Fulu we may not have
6634+
// advanced the state completely into the new epoch.
6635+
let fork = self.spec.fork_at_epoch(proposal_epoch);
6636+
6637+
debug!(
6638+
?shuffling_decision_block,
6639+
epoch = %proposal_epoch,
6640+
"Priming proposer shuffling cache"
6641+
);
6642+
6643+
Ok::<_, E>(EpochBlockProposers::new(proposal_epoch, fork, proposers))
66186644
})?;
66196645

66206646
// Run the accessor function on the computed epoch proposers.

‎beacon_node/beacon_chain/src/beacon_proposer_cache.rs‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use smallvec::SmallVec;
1717
use state_processing::state_advance::partial_state_advance;
1818
use std::num::NonZeroUsize;
1919
use std::sync::Arc;
20+
use tracing::instrument;
2021
use types::non_zero_usize::new_non_zero_usize;
2122
use types::{
2223
BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, Fork, Hash256, Slot, Unsigned,
@@ -199,11 +200,14 @@ pub fn compute_proposer_duties_from_head<T: BeaconChainTypes>(
199200
.map_err(BeaconChainError::from)?;
200201

201202
let dependent_root = state
202-
// The only block which decides its own shuffling is the genesis block.
203-
.proposer_shuffling_decision_root(chain.genesis_block_root, &chain.spec)
203+
.proposer_shuffling_decision_root_at_epoch(request_epoch, head_block_root, &chain.spec)
204204
.map_err(BeaconChainError::from)?;
205205

206-
Ok((indices, dependent_root, execution_status, state.fork()))
206+
// Use fork_at_epoch rather than the state's fork, because post-Fulu we may not have advanced
207+
// the state completely into the new epoch.
208+
let fork = chain.spec.fork_at_epoch(request_epoch);
209+
210+
Ok((indices, dependent_root, execution_status, fork))
207211
}
208212

209213
/// If required, advance `state` to the epoch required to determine proposer indices in `target_epoch`.
@@ -214,6 +218,7 @@ pub fn compute_proposer_duties_from_head<T: BeaconChainTypes>(
214218
/// - No-op if `state.current_epoch() == target_epoch`.
215219
/// - It must be the case that `state.canonical_root() == state_root`, but this function will not
216220
/// check that.
221+
#[instrument(skip_all, fields(?state_root, %target_epoch, state_slot = %state.slot()), level = "debug")]
217222
pub fn ensure_state_can_determine_proposers_for_epoch<E: EthSpec>(
218223
state: &mut BeaconState<E>,
219224
state_root: Hash256,
@@ -234,14 +239,6 @@ pub fn ensure_state_can_determine_proposers_for_epoch<E: EthSpec>(
234239
if state.current_epoch() > maximum_epoch {
235240
Err(BeaconStateError::SlotOutOfBounds.into())
236241
} else if state.current_epoch() >= minimum_epoch {
237-
if target_epoch > state.current_epoch() {
238-
let target_slot = target_epoch.start_slot(E::slots_per_epoch());
239-
240-
// Advance the state into the same epoch as the block. Use the "partial" method since state
241-
// roots are not important for proposer/attester shuffling.
242-
partial_state_advance(state, Some(state_root), target_slot, spec)
243-
.map_err(BeaconChainError::from)?;
244-
}
245242
Ok(())
246243
} else {
247244
// State's current epoch is less than the minimum epoch.

‎beacon_node/beacon_chain/src/block_verification.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -950,8 +950,6 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
950950
let proposer_shuffling_decision_block =
951951
parent_block.proposer_shuffling_root_for_child_block(block_epoch, &chain.spec);
952952

953-
// We assign to a variable instead of using `if let Some` directly to ensure we drop the
954-
// write lock before trying to acquire it again in the `else` clause.
955953
let block_slot = block.slot();
956954
let mut opt_parent = None;
957955
let proposer = chain.with_proposer_cache::<_, BlockError>(

‎beacon_node/beacon_chain/src/state_advance_timer.rs‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -333,25 +333,54 @@ fn advance_head<T: BeaconChainTypes>(beacon_chain: &Arc<BeaconChain<T>>) -> Resu
333333
.build_committee_cache(RelativeEpoch::Next, &beacon_chain.spec)
334334
.map_err(BeaconChainError::from)?;
335335

336-
// If the `pre_state` is in a later epoch than `state`, pre-emptively add the proposer shuffling
337-
// for the state's current epoch and the committee cache for the state's next epoch.
336+
// The state root is required to prime the proposer cache AND for writing it to disk.
337+
let advanced_state_root = state.update_tree_hash_cache()?;
338+
339+
// If the `pre_state` is in a later epoch than `state`, pre-emptively update the proposer
340+
// shuffling and attester shuffling caches.
338341
if initial_epoch < state.current_epoch() {
339-
// Update the proposer cache.
340-
//
341-
// We supply the `head_block_root` as the decision block since the prior `if` statement guarantees
342-
// the head root is the latest block from the prior epoch.
343-
beacon_chain
344-
.beacon_proposer_cache
345-
.lock()
346-
.insert(
347-
state.current_epoch(),
348-
head_block_root,
349-
state
350-
.get_beacon_proposer_indices(state.current_epoch(), &beacon_chain.spec)
351-
.map_err(BeaconChainError::from)?,
352-
state.fork(),
353-
)
354-
.map_err(BeaconChainError::from)?;
342+
// Include the proposer shuffling from the current epoch, which is likely to be useful
343+
// pre-Fulu, and probably redundant post-Fulu (it should already have been in the cache).
344+
let current_epoch_decision_root = state.proposer_shuffling_decision_root_at_epoch(
345+
state.current_epoch(),
346+
head_block_root,
347+
&beacon_chain.spec,
348+
)?;
349+
beacon_chain.with_proposer_cache(
350+
current_epoch_decision_root,
351+
state.current_epoch(),
352+
|_| Ok(()),
353+
|| {
354+
debug!(
355+
shuffling_decision_root = ?current_epoch_decision_root,
356+
epoch = %state.current_epoch(),
357+
"Computing current epoch proposer shuffling in state advance"
358+
);
359+
Ok::<_, Error>((advanced_state_root, state.clone()))
360+
},
361+
)?;
362+
363+
// For epochs *greater than* the Fulu fork epoch, we have also determined the proposer
364+
// shuffling for the next epoch.
365+
let next_epoch = state.next_epoch()?;
366+
let next_epoch_decision_root = state.proposer_shuffling_decision_root_at_epoch(
367+
next_epoch,
368+
head_block_root,
369+
&beacon_chain.spec,
370+
)?;
371+
beacon_chain.with_proposer_cache(
372+
next_epoch_decision_root,
373+
next_epoch,
374+
|_| Ok(()),
375+
|| {
376+
debug!(
377+
shuffling_decision_root = ?next_epoch_decision_root,
378+
epoch = %next_epoch,
379+
"Computing next epoch proposer shuffling in state advance"
380+
);
381+
Ok::<_, Error>((advanced_state_root, state.clone()))
382+
},
383+
)?;
355384

356385
// Update the attester cache.
357386
let shuffling_id =
@@ -406,7 +435,6 @@ fn advance_head<T: BeaconChainTypes>(beacon_chain: &Arc<BeaconChain<T>>) -> Resu
406435
// even if we race with the deletion of this state by the finalization pruning code, the worst
407436
// case is we end up with a finalized state stored, that will get pruned the next time pruning
408437
// runs.
409-
let advanced_state_root = state.update_tree_hash_cache()?;
410438
beacon_chain.store.put_state(&advanced_state_root, &state)?;
411439

412440
debug!(

0 commit comments

Comments
 (0)