Skip to content

Conversation

Inspector-Butters
Copy link
Contributor

What type of PR is this?
Bug fix

What does this PR do? Why is it needed?
As per spec, light client p2p messages should be broadcasted with a delay of ~4 seconds after the slot start time. This PR adds that necessary delay.

It also adds the BASIS_POINTS values and logic to calculate component durations based on basis points.

Copy link
Contributor

@rkapka rkapka left a comment

Choose a reason for hiding this comment

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

There is also code in /beacon-chain/sync/validate_light_client.go that deals with timing. It's based on the old spec (

// [IGNORE] The optimistic_update is received after the block at signature_slot was given enough time
// to propagate through the network -- i.e. validate that one-third of optimistic_update.signature_slot
// has transpired (SECONDS_PER_SLOT / INTERVALS_PER_SLOT seconds after the start of the slot,
// with a MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance)
slotStart, err := slots.StartTime(s.cfg.clock.GenesisTime(), newUpdate.SignatureSlot())
if err != nil {
log.WithError(err).Debug("Peer sent a slot that would overflow slot start time")
return pubsub.ValidationReject, nil
}
earliestValidTime := slotStart.
Add(time.Second * time.Duration(params.BeaconConfig().SecondsPerSlot/params.BeaconConfig().IntervalsPerSlot)).
Add(-params.BeaconConfig().MaximumGossipClockDisparityDuration())
if s.cfg.clock.Now().Before(earliestValidTime) {
log.Debug("Newly received light client optimistic update ignored. not enough time passed for block to propagate")
return pubsub.ValidationIgnore, nil
}
and
// [IGNORE] The optimistic_update is received after the block at signature_slot was given enough time
// to propagate through the network -- i.e. validate that one-third of optimistic_update.signature_slot
// has transpired (SECONDS_PER_SLOT / INTERVALS_PER_SLOT seconds after the start of the slot,
// with a MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance)
slotStart, err := slots.StartTime(s.cfg.clock.GenesisTime(), newUpdate.SignatureSlot())
if err != nil {
log.WithError(err).Debug("Peer sent a slot that would overflow slot start time")
return pubsub.ValidationReject, nil
}
earliestValidTime := slotStart.
Add(time.Second * time.Duration(params.BeaconConfig().SecondsPerSlot/params.BeaconConfig().IntervalsPerSlot)).
Add(-params.BeaconConfig().MaximumGossipClockDisparityDuration())
if s.cfg.clock.Now().Before(earliestValidTime) {
log.Debug("Newly received light client finality update ignored. not enough time passed for block to propagate")
return pubsub.ValidationIgnore, nil
}
). Can you update these places to use the new BP construct too?

func TestService_BroadcastLightClientOptimisticUpdate(t *testing.T) {
params.SetupTestConfigCleanup(t)
config := params.BeaconConfig().Copy()
config.SyncMessageDueBPS = 833 // ~1 second
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we changed this to 1? Will the test still pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes the test will pass. but we want to be able to test that enough time has passed. if you make it 1, you might as well make it zero.

@Inspector-Butters Inspector-Butters added this pull request to the merge queue Oct 3, 2025
Merged via the queue into develop with commit 28eb1a4 Oct 3, 2025
17 checks passed
@Inspector-Butters Inspector-Butters deleted the delayed-lc-broadcast branch October 3, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants