Ensure dependent root consistency in head events (#2753)

## Issue Addressed

@paulhauner noticed that when we send head events, we use the block root from `new_head` in `fork_choice_internal`, but calculate `dependent_root` and `previous_dependent_root` using the `canonical_head`. This is normally fine because `new_head` updates the `canonical_head` in `fork_choice_internal`, but it's possible we have a reorg updating `canonical_head` before our head events are sent. So this PR ensures `dependent_root` and `previous_dependent_root` are always derived from the state associated with `new_head`.



Co-authored-by: realbigsean <seananderson33@gmail.com>
This commit is contained in:
realbigsean 2021-11-02 02:26:32 +00:00
parent 80627b428b
commit c4ad0e3fb3

View File

@ -3050,14 +3050,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// These fields are used for server-sent events. // These fields are used for server-sent events.
let state_root = new_head.beacon_state_root(); let state_root = new_head.beacon_state_root();
let head_slot = new_head.beacon_state.slot(); let head_slot = new_head.beacon_state.slot();
let target_epoch_start_slot = new_head
.beacon_state
.current_epoch()
.start_slot(T::EthSpec::slots_per_epoch());
let prev_target_epoch_start_slot = new_head
.beacon_state
.previous_epoch()
.start_slot(T::EthSpec::slots_per_epoch());
let head_proposer_index = new_head.beacon_block.message().proposer_index(); let head_proposer_index = new_head.beacon_block.message().proposer_index();
let proposer_graffiti = new_head let proposer_graffiti = new_head
.beacon_block .beacon_block
@ -3066,6 +3058,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
.graffiti() .graffiti()
.as_utf8_lossy(); .as_utf8_lossy();
// Find the dependent roots associated with this head before updating the snapshot. This
// is to ensure consistency when sending server sent events later in this method.
let dependent_root = new_head
.beacon_state
.proposer_shuffling_decision_root(self.genesis_block_root);
let prev_dependent_root = new_head
.beacon_state
.attester_shuffling_decision_root(self.genesis_block_root, RelativeEpoch::Current);
drop(lag_timer); drop(lag_timer);
// Update the snapshot that stores the head of the chain at the time it received the // Update the snapshot that stores the head of the chain at the time it received the
@ -3210,12 +3211,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// Register a server-sent event if necessary // Register a server-sent event if necessary
if let Some(event_handler) = self.event_handler.as_ref() { if let Some(event_handler) = self.event_handler.as_ref() {
if event_handler.has_head_subscribers() { if event_handler.has_head_subscribers() {
if let Ok(Some(current_duty_dependent_root)) = match (dependent_root, prev_dependent_root) {
self.block_root_at_slot(target_epoch_start_slot - 1, WhenSlotSkipped::Prev) (Ok(current_duty_dependent_root), Ok(previous_duty_dependent_root)) => {
{
if let Ok(Some(previous_duty_dependent_root)) = self
.block_root_at_slot(prev_target_epoch_start_slot - 1, WhenSlotSkipped::Prev)
{
event_handler.register(EventKind::Head(SseHead { event_handler.register(EventKind::Head(SseHead {
slot: head_slot, slot: head_slot,
block: beacon_block_root, block: beacon_block_root,
@ -3224,17 +3221,14 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
previous_duty_dependent_root, previous_duty_dependent_root,
epoch_transition: is_epoch_transition, epoch_transition: is_epoch_transition,
})); }));
} else { }
(Err(e), _) | (_, Err(e)) => {
warn!( warn!(
self.log, self.log,
"Unable to find previous target root, cannot register head event" "Unable to find dependent roots, cannot register head event";
"error" => ?e
); );
} }
} else {
warn!(
self.log,
"Unable to find current target root, cannot register head event"
);
} }
} }