Skip to content

Commit d41c236

Browse files
michaelsproulmichaelsproul
committed
Use new proposer cache method in block verif
1 parent b34af93 commit d41c236

File tree

3 files changed

+82
-54
lines changed

3 files changed

+82
-54
lines changed

‎beacon_node/beacon_chain/src/beacon_chain.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6549,18 +6549,23 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
65496549
}
65506550
}
65516551

6552-
pub fn with_proposer_cache<V>(
6552+
pub fn with_proposer_cache<V, E: From<BeaconChainError> + From<BeaconStateError>>(
65536553
&self,
65546554
shuffling_decision_block: Hash256,
65556555
proposal_epoch: Epoch,
65566556
accessor: impl Fn(&mut BeaconProposerCache) -> Option<V>,
6557-
state_provider: impl FnOnce() -> Result<Option<(Hash256, BeaconState<T::EthSpec>)>, Error>,
6558-
) -> Result<Option<V>, Error> {
6557+
state_provider: impl FnOnce() -> Result<Option<(Hash256, BeaconState<T::EthSpec>)>, E>,
6558+
) -> Result<Option<V>, E> {
65596559
let mut cache = self.beacon_proposer_cache.lock();
65606560
if let Some(value) = accessor(&mut cache) {
65616561
return Ok(Some(value));
65626562
}
65636563
drop(cache);
6564+
debug!(
6565+
?shuffling_decision_block,
6566+
%proposal_epoch,
6567+
"Proposer shuffling cache miss"
6568+
);
65646569

65656570
// Fetch the state on-demand if the required epoch was missing from the cache.
65666571
let Some((state_root, mut state)) = state_provider()? else {
@@ -6587,7 +6592,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
65876592
return Err(Error::ProposerCacheIncorrectState {
65886593
state_decision_block_root,
65896594
requested_decision_block_root: shuffling_decision_block,
6590-
});
6595+
}
6596+
.into());
65916597
}
65926598

65936599
// Add the proposers to the cache, and re-run the accessor function which is now very likely

‎beacon_node/beacon_chain/src/block_verification.rs‎

Lines changed: 68 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -947,62 +947,80 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
947947
});
948948
}
949949

950+
let parent_epoch = parent_block.slot.epoch(T::EthSpec::slots_per_epoch());
950951
let proposer_shuffling_decision_block =
951-
if parent_block.slot.epoch(T::EthSpec::slots_per_epoch()) == block_epoch {
952-
parent_block
953-
.next_epoch_shuffling_id
954-
.shuffling_decision_block
952+
if !chain.spec.fork_name_at_epoch(block_epoch).fulu_enabled() {
953+
// Prior to Fulu the proposer shuffling decision root for the current epoch is the same
954+
// as the attestation shuffling for the *next* epoch, i.e. it is determined at the start
955+
// of the epoch.
956+
if block_epoch == parent_epoch {
957+
parent_block
958+
.next_epoch_shuffling_id
959+
.shuffling_decision_block
960+
} else {
961+
// Otherwise, the block epoch is greater, so its decision root is the parent
962+
// root itself.
963+
parent_block.root
964+
}
955965
} else {
956-
parent_block.root
966+
// After Fulu the proposer shuffling is determined with lookahead, so if the block
967+
// lies in the same epoch as its parent, its decision root is the same as the
968+
// parent's current epoch attester shuffling
969+
//
970+
// i.e. the block from the end of epoch N - 2.
971+
if block_epoch == parent_epoch {
972+
parent_block
973+
.current_epoch_shuffling_id
974+
.shuffling_decision_block
975+
} else {
976+
// If the block is in a new epoch, then it instead shares its decision root with
977+
// the parent's *next epoch* shuffling.
978+
parent_block
979+
.next_epoch_shuffling_id
980+
.shuffling_decision_block
981+
}
957982
};
958983

959984
// We assign to a variable instead of using `if let Some` directly to ensure we drop the
960985
// write lock before trying to acquire it again in the `else` clause.
961-
let proposer_opt = chain
962-
.beacon_proposer_cache
963-
.lock()
964-
.get_slot::<T::EthSpec>(proposer_shuffling_decision_block, block.slot());
965-
let (expected_proposer, fork, parent, block) = if let Some(proposer) = proposer_opt {
966-
// The proposer index was cached and we can return it without needing to load the
967-
// parent.
968-
(proposer.index, proposer.fork, None, block)
969-
} else {
970-
// The proposer index was *not* cached and we must load the parent in order to determine
971-
// the proposer index.
972-
let (mut parent, block) = load_parent(block, chain)?;
973-
974-
debug!(
975-
parent_root = ?parent.beacon_block_root,
976-
parent_slot = %parent.beacon_block.slot(),
977-
?block_root,
978-
block_slot = %block.slot(),
979-
"Proposer shuffling cache miss"
980-
);
981-
982-
// The state produced is only valid for determining proposer/attester shuffling indices.
983-
let state = cheap_state_advance_to_obtain_committees::<_, BlockError>(
984-
&mut parent.pre_state,
985-
parent.beacon_state_root,
986-
block.slot(),
987-
&chain.spec,
988-
)?;
989-
990-
let epoch = state.current_epoch();
991-
let proposers = state.get_beacon_proposer_indices(epoch, &chain.spec)?;
992-
let proposer_index = *proposers
993-
.get(block.slot().as_usize() % T::EthSpec::slots_per_epoch() as usize)
994-
.ok_or_else(|| BeaconChainError::NoProposerForSlot(block.slot()))?;
995-
996-
// Prime the proposer shuffling cache with the newly-learned value.
997-
chain.beacon_proposer_cache.lock().insert(
998-
block_epoch,
999-
proposer_shuffling_decision_block,
1000-
proposers,
1001-
state.fork(),
1002-
)?;
1003-
1004-
(proposer_index, state.fork(), Some(parent), block)
986+
let block_slot = block.slot();
987+
let mut opt_parent = None;
988+
let mut opt_block = Some(block);
989+
let Some(proposer) = chain.with_proposer_cache::<_, BlockError>(
990+
proposer_shuffling_decision_block,
991+
block_epoch,
992+
|cache| cache.get_slot::<T::EthSpec>(proposer_shuffling_decision_block, block_slot),
993+
|| {
994+
// The proposer index was *not* cached and we must load the parent in order to
995+
// determine the proposer index.
996+
let Some(block) = opt_block.take() else {
997+
return Ok(None);
998+
};
999+
let (mut parent, block) = load_parent(block, chain)?;
1000+
let parent_state_root = if let Some(state_root) = parent.beacon_state_root {
1001+
state_root
1002+
} else {
1003+
// FIXME(sproul): a little inefficient?
1004+
parent.pre_state.canonical_root()?
1005+
};
1006+
let result = Ok(Some((parent_state_root, parent.pre_state.clone())));
1007+
opt_parent = Some(parent);
1008+
opt_block = Some(block);
1009+
result
1010+
},
1011+
)?
1012+
else {
1013+
return Err(BlockError::InternalError(format!(
1014+
"Unable to determine proposer for slot {block_slot}",
1015+
)));
10051016
};
1017+
let expected_proposer = proposer.index;
1018+
let fork = proposer.fork;
1019+
let block = opt_block.ok_or_else(|| {
1020+
BlockError::InternalError(format!(
1021+
"Block removed and not put back for slot {block_slot}",
1022+
))
1023+
})?;
10061024

10071025
let signature_is_valid = {
10081026
let pubkey_cache = get_validator_pubkey_cache(chain)?;
@@ -1077,7 +1095,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
10771095
Ok(Self {
10781096
block,
10791097
block_root,
1080-
parent,
1098+
parent: opt_parent,
10811099
consensus_context,
10821100
})
10831101
}

‎beacon_node/beacon_chain/src/errors.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,10 @@ pub enum BeaconChainError {
234234
state_decision_block_root: Hash256,
235235
requested_decision_block_root: Hash256,
236236
},
237+
ProposerCacheAccessorFailure {
238+
decision_block_root: Hash256,
239+
proposal_epoch: Epoch,
240+
},
237241
}
238242

239243
easy_from_to!(SlotProcessingError, BeaconChainError);

0 commit comments

Comments
 (0)