Skip to content
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

Merged
merged 21 commits into from
Mar 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions beacon-chain/blockchain/head.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Type: statefeed.Reorg,
Data: &ethpbv1.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,
},
})

Expand All @@ -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")
}
}()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a question for me?

Type: statefeed.NewHead,
Data: &ethpbv1.EventHead{
Expand All @@ -341,6 +351,7 @@ func (s *Service) notifyNewHeadEvent(
EpochTransition: slots.IsEpochStart(newHeadSlot),
PreviousDutyDependentRoot: previousDutyDependentRoot,
CurrentDutyDependentRoot: currentDutyDependentRoot,
ExecutionOptimistic: isOptimistic,
},
})
return nil
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/blockchain/head_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func Test_notifyNewHeadEvent(t *testing.T) {
}
newHeadStateRoot := [32]byte{2}
newHeadRoot := [32]byte{3}
err := srv.notifyNewHeadEvent(1, bState, newHeadStateRoot[:], newHeadRoot[:])
err := srv.notifyNewHeadEvent(context.Background(), 1, bState, newHeadStateRoot[:], newHeadRoot[:])
require.NoError(t, err)
events := notifier.ReceivedEvents()
require.Equal(t, 1, len(events))
Expand Down Expand Up @@ -197,7 +197,7 @@ func Test_notifyNewHeadEvent(t *testing.T) {

newHeadStateRoot := [32]byte{2}
newHeadRoot := [32]byte{3}
err = srv.notifyNewHeadEvent(epoch2Start, bState, newHeadStateRoot[:], newHeadRoot[:])
err = srv.notifyNewHeadEvent(context.Background(), epoch2Start, bState, newHeadStateRoot[:], newHeadRoot[:])
require.NoError(t, err)
events := notifier.ReceivedEvents()
require.Equal(t, 1, len(events))
Expand Down
11 changes: 8 additions & 3 deletions beacon-chain/blockchain/process_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call above locks on notifyFockchoiceUpdate so this one is guaranteed to return immediately.

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: &ethpbv1.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,
},
})

Expand Down
3 changes: 2 additions & 1 deletion beacon-chain/core/feed/block/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ const (

// ReceivedBlockData is the data sent with ReceivedBlock events.
type ReceivedBlockData struct {
SignedBlock block.SignedBeaconBlock
SignedBlock block.SignedBeaconBlock
IsOptimistic bool
}
4 changes: 2 additions & 2 deletions beacon-chain/rpc/apimiddleware/custom_handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func TestReceiveEvents_TrailingSpace(t *testing.T) {
errJson := receiveEvents(ch, w, req)
assert.Equal(t, true, errJson == nil)
assert.Equal(t, `event: finalized_checkpoint
data: {"block":"0x666f6f","state":"0x666f6f","epoch":"1"}
data: {"block":"0x666f6f","state":"0x666f6f","epoch":"1","execution_optimistic":false}

`, w.Body.String())
}
Expand All @@ -273,5 +273,5 @@ func TestWriteEvent(t *testing.T) {
errJson := writeEvent(msg, w, &eventFinalizedCheckpointJson{})
require.Equal(t, true, errJson == nil)
written := w.Body.String()
assert.Equal(t, "event: test_event\ndata: {\"block\":\"0x666f6f\",\"state\":\"0x666f6f\",\"epoch\":\"1\"}\n\n", written)
assert.Equal(t, "event: test_event\ndata: {\"block\":\"0x666f6f\",\"state\":\"0x666f6f\",\"epoch\":\"1\",\"execution_optimistic\":false}\n\n", written)
}
87 changes: 54 additions & 33 deletions beacon-chain/rpc/apimiddleware/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ type feeRecipientsRequestJSON struct {

// stateRootResponseJson is used in /beacon/states/{state_id}/root API endpoint.
type stateRootResponseJson struct {
Data *stateRootResponse_StateRootJson `json:"data"`
Data *stateRootResponse_StateRootJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// stateRootResponse_StateRootJson is used in /beacon/states/{state_id}/root API endpoint.
Expand All @@ -45,12 +46,14 @@ type stateRootResponse_StateRootJson struct {

// stateForkResponseJson is used in /beacon/states/{state_id}/fork API endpoint.
type stateForkResponseJson struct {
Data *forkJson `json:"data"`
Data *forkJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// stateFinalityCheckpointResponseJson is used in /beacon/states/{state_id}/finality_checkpoints API endpoint.
type stateFinalityCheckpointResponseJson struct {
Data *stateFinalityCheckpointResponse_StateFinalityCheckpointJson `json:"data"`
Data *stateFinalityCheckpointResponse_StateFinalityCheckpointJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// stateFinalityCheckpointResponse_StateFinalityCheckpointJson is used in /beacon/states/{state_id}/finality_checkpoints API endpoint.
Expand All @@ -62,37 +65,44 @@ type stateFinalityCheckpointResponse_StateFinalityCheckpointJson struct {

// stateValidatorResponseJson is used in /beacon/states/{state_id}/validators API endpoint.
type stateValidatorsResponseJson struct {
Data []*validatorContainerJson `json:"data"`
Data []*validatorContainerJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// stateValidatorResponseJson is used in /beacon/states/{state_id}/validators/{validator_id} API endpoint.
type stateValidatorResponseJson struct {
Data *validatorContainerJson `json:"data"`
Data *validatorContainerJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// validatorBalancesResponseJson is used in /beacon/states/{state_id}/validator_balances API endpoint.
type validatorBalancesResponseJson struct {
Data []*validatorBalanceJson `json:"data"`
Data []*validatorBalanceJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// stateCommitteesResponseJson is used in /beacon/states/{state_id}/committees API endpoint.
type stateCommitteesResponseJson struct {
Data []*committeeJson `json:"data"`
Data []*committeeJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// syncCommitteesResponseJson is used in /beacon/states/{state_id}/sync_committees API endpoint.
type syncCommitteesResponseJson struct {
Data *syncCommitteeValidatorsJson `json:"data"`
Data *syncCommitteeValidatorsJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// blockHeadersResponseJson is used in /beacon/headers API endpoint.
type blockHeadersResponseJson struct {
Data []*blockHeaderContainerJson `json:"data"`
Data []*blockHeaderContainerJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// blockHeaderResponseJson is used in /beacon/headers/{block_id} API endpoint.
type blockHeaderResponseJson struct {
Data *blockHeaderContainerJson `json:"data"`
Data *blockHeaderContainerJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// blockResponseJson is used in /beacon/blocks/{block_id} API endpoint.
Expand All @@ -102,18 +112,21 @@ type blockResponseJson struct {

// blockV2ResponseJson is used in /v2/beacon/blocks/{block_id} API endpoint.
type blockV2ResponseJson struct {
Version string `json:"version" enum:"true"`
Data *signedBeaconBlockContainerV2Json `json:"data"`
Version string `json:"version" enum:"true"`
Data *signedBeaconBlockContainerV2Json `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// blockRootResponseJson is used in /beacon/blocks/{block_id}/root API endpoint.
type blockRootResponseJson struct {
Data *blockRootContainerJson `json:"data"`
Data *blockRootContainerJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// blockAttestationsResponseJson is used in /beacon/blocks/{block_id}/attestations API endpoint.
type blockAttestationsResponseJson struct {
Data []*attestationJson `json:"data"`
Data []*attestationJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// attestationsPoolResponseJson is used in /beacon/pool/attestations GET API endpoint.
Expand Down Expand Up @@ -191,8 +204,9 @@ type beaconStateResponseJson struct {

// beaconStateV2ResponseJson is used in /v2/debug/beacon/states/{state_id} API endpoint.
type beaconStateV2ResponseJson struct {
Version string `json:"version" enum:"true"`
Data *beaconStateContainerV2Json `json:"data"`
Version string `json:"version" enum:"true"`
Data *beaconStateContainerV2Json `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// forkChoiceHeadsResponseJson is used in /v1/debug/beacon/heads API endpoint.
Expand Down Expand Up @@ -227,19 +241,22 @@ type dutiesRequestJson struct {

// attesterDutiesResponseJson is used in /validator/duties/attester/{epoch} API endpoint.
type attesterDutiesResponseJson struct {
DependentRoot string `json:"dependent_root" hex:"true"`
Data []*attesterDutyJson `json:"data"`
DependentRoot string `json:"dependent_root" hex:"true"`
Data []*attesterDutyJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// proposerDutiesResponseJson is used in /validator/duties/proposer/{epoch} API endpoint.
type proposerDutiesResponseJson struct {
DependentRoot string `json:"dependent_root" hex:"true"`
Data []*proposerDutyJson `json:"data"`
DependentRoot string `json:"dependent_root" hex:"true"`
Data []*proposerDutyJson `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// syncCommitteeDutiesResponseJson is used in /validator/duties/sync/{epoch} API endpoint.
type syncCommitteeDutiesResponseJson struct {
Data []*syncCommitteeDuty `json:"data"`
Data []*syncCommitteeDuty `json:"data"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// produceBlockResponseJson is used in /validator/blocks/{slot} API endpoint.
Expand Down Expand Up @@ -847,33 +864,37 @@ type eventHeadJson struct {
Block string `json:"block" hex:"true"`
State string `json:"state" hex:"true"`
EpochTransition bool `json:"epoch_transition"`
ExecutionOptimistic bool `json:"execution_optimistic"`
PreviousDutyDependentRoot string `json:"previous_duty_dependent_root" hex:"true"`
CurrentDutyDependentRoot string `json:"current_duty_dependent_root" hex:"true"`
}

type receivedBlockDataJson struct {
Slot string `json:"slot"`
Block string `json:"block" hex:"true"`
Slot string `json:"slot"`
Block string `json:"block" hex:"true"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

type aggregatedAttReceivedDataJson struct {
Aggregate *attestationJson `json:"aggregate"`
}

type eventFinalizedCheckpointJson struct {
Block string `json:"block" hex:"true"`
State string `json:"state" hex:"true"`
Epoch string `json:"epoch"`
Block string `json:"block" hex:"true"`
State string `json:"state" hex:"true"`
Epoch string `json:"epoch"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

type eventChainReorgJson struct {
Slot string `json:"slot"`
Depth string `json:"depth"`
OldHeadBlock string `json:"old_head_block" hex:"true"`
NewHeadBlock string `json:"old_head_state" hex:"true"`
OldHeadState string `json:"new_head_block" hex:"true"`
NewHeadState string `json:"new_head_state" hex:"true"`
Epoch string `json:"epoch"`
Slot string `json:"slot"`
Depth string `json:"depth"`
OldHeadBlock string `json:"old_head_block" hex:"true"`
NewHeadBlock string `json:"old_head_state" hex:"true"`
OldHeadState string `json:"new_head_block" hex:"true"`
NewHeadState string `json:"new_head_state" hex:"true"`
Epoch string `json:"epoch"`
ExecutionOptimistic bool `json:"execution_optimistic"`
}

// ---------------
Expand Down
Loading