-
Notifications
You must be signed in to change notification settings - Fork 995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add execution_optimistic
field to API responses
#10389
Changes from all commits
3a7943a
b63b1e8
28d8270
93ce171
45c97b2
7164fe3
ed0e66b
67be852
ce50ba4
25722c6
a663286
a49a623
1cbcd47
d842f39
2bfafc7
c3541fa
a77362b
a181dac
5ace770
6f82ab9
918dc99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,16 +148,21 @@ func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error { | |
"oldSlot": fmt.Sprintf("%d", headSlot), | ||
}).Debug("Chain reorg occurred") | ||
absoluteSlotDifference := slots.AbsoluteValueSlotDifference(newHeadSlot, headSlot) | ||
isOptimistic, err := s.IsOptimistic(ctx) | ||
if err != nil { | ||
return errors.Wrap(err, "could not check if node is optimistically synced") | ||
} | ||
s.cfg.StateNotifier.StateFeed().Send(&feed.Event{ | ||
Type: statefeed.Reorg, | ||
Data: ðpbv1.EventChainReorg{ | ||
Slot: newHeadSlot, | ||
Depth: absoluteSlotDifference, | ||
OldHeadBlock: oldHeadRoot[:], | ||
NewHeadBlock: headRoot[:], | ||
OldHeadState: oldStateRoot, | ||
NewHeadState: newStateRoot, | ||
Epoch: slots.ToEpoch(newHeadSlot), | ||
Slot: newHeadSlot, | ||
Depth: absoluteSlotDifference, | ||
OldHeadBlock: oldHeadRoot[:], | ||
NewHeadBlock: headRoot[:], | ||
OldHeadState: oldStateRoot, | ||
NewHeadState: newStateRoot, | ||
Epoch: slots.ToEpoch(newHeadSlot), | ||
ExecutionOptimistic: isOptimistic, | ||
}, | ||
}) | ||
|
||
|
@@ -179,7 +184,7 @@ func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error { | |
// Forward an event capturing a new chain head over a common event feed | ||
// done in a goroutine to avoid blocking the critical runtime main routine. | ||
go func() { | ||
if err := s.notifyNewHeadEvent(newHeadSlot, newHeadState, newStateRoot, headRoot[:]); err != nil { | ||
if err := s.notifyNewHeadEvent(ctx, newHeadSlot, newHeadState, newStateRoot, headRoot[:]); err != nil { | ||
log.WithError(err).Error("Could not notify event feed of new chain head") | ||
} | ||
}() | ||
|
@@ -299,6 +304,7 @@ func (s *Service) hasHeadState() bool { | |
// Notifies a common event feed of a new chain head event. Called right after a new | ||
// chain head is determined, set, and saved to disk. | ||
func (s *Service) notifyNewHeadEvent( | ||
ctx context.Context, | ||
newHeadSlot types.Slot, | ||
newHeadState state.BeaconState, | ||
newHeadStateRoot, | ||
|
@@ -332,6 +338,10 @@ func (s *Service) notifyNewHeadEvent( | |
return errors.Wrap(err, "could not get duty dependent root") | ||
} | ||
} | ||
isOptimistic, err := s.IsOptimistic(ctx) | ||
if err != nil { | ||
return errors.Wrap(err, "could not check if node is optimistically synced") | ||
} | ||
s.cfg.StateNotifier.StateFeed().Send(&feed.Event{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, is this function being called right after setting head or right after the return from the engine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a question for me? |
||
Type: statefeed.NewHead, | ||
Data: ðpbv1.EventHead{ | ||
|
@@ -341,6 +351,7 @@ func (s *Service) notifyNewHeadEvent( | |
EpochTransition: slots.IsEpochStart(newHeadSlot), | ||
PreviousDutyDependentRoot: previousDutyDependentRoot, | ||
CurrentDutyDependentRoot: currentDutyDependentRoot, | ||
ExecutionOptimistic: isOptimistic, | ||
}, | ||
}) | ||
return nil | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -232,14 +232,19 @@ func (s *Service) onBlock(ctx context.Context, signed block.SignedBeaconBlock, b | |
if err := s.cfg.ForkChoiceStore.Prune(ctx, fRoot); err != nil { | ||
return errors.Wrap(err, "could not prune proto array fork choice nodes") | ||
} | ||
isOptimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(ctx, fRoot) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this is better inside the goroutine, I don't think it makes a big difference but something worth considering There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The call above locks on |
||
if err != nil { | ||
return errors.Wrap(err, "could not check if node is optimistically synced") | ||
} | ||
go func() { | ||
// Send an event regarding the new finalized checkpoint over a common event feed. | ||
s.cfg.StateNotifier.StateFeed().Send(&feed.Event{ | ||
Type: statefeed.FinalizedCheckpoint, | ||
Data: ðpbv1.EventFinalizedCheckpoint{ | ||
Epoch: postState.FinalizedCheckpoint().Epoch, | ||
Block: postState.FinalizedCheckpoint().Root, | ||
State: signed.Block().StateRoot(), | ||
Epoch: postState.FinalizedCheckpoint().Epoch, | ||
Block: postState.FinalizedCheckpoint().Root, | ||
State: signed.Block().StateRoot(), | ||
ExecutionOptimistic: isOptimistic, | ||
}, | ||
}) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a short time lapse between we set the head and the optimistic status of the node is settled. How much of a race issue can there be here if we return optimistic when soon after the head will become VALID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't say how event subscribers will use this info. Is there an easy way to sync things? If not, I would just not worry about it at this point.