Skip to content

Commit

Permalink
Do not check parent weight on early FCU (#13683)
Browse files Browse the repository at this point in the history
When a late block arrives and the beacon is proposing the next block, we
perform several checks to allow for the next block to reorg the incoming
late block.

Among those checks, we check that the parent block has been heavily
attested (currently 160% of the committee size).

We perform this check in these circumstances:
- When the late block arrives
- At 10 seconds into the slot
- At 0 seconds into the next slot (at proposing time)

The problem is that for blocks that arrive between 4 seconds and 10
seconds, the parent block will not have yet this expected weight since
attestations from the current committee were not imported yet, and thus
Prysm will send an FCU with payload attributes anyway at this time.

What happens is that Prysm keeps the EL building different blocks based
on different parents at the same time, when later in the next slot it
calls to propose, it will reorg the late block anyway and the EL would
have been computing a second payload uselessly.

This PR enables this check only when calling `ShouldOverrideFCU` after
10 seconds into the slot which we do only after having imported the
current attestations. We may want to actually remove this check entirely
from `ShouldOverrideFCU` and only keep it in `ProposerHead`.

Shout out to Anthithesis for reporting an issue that led to this
discoverly.

(cherry picked from commit b6ce6c2)
  • Loading branch information
potuz authored and prestonvanloon committed Mar 5, 2024
1 parent 147f48c commit 04a5b2e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ func (f *ForkChoice) ShouldOverrideFCU() (override bool) {
return
}

// Return early if we are checking before 10 seconds into the slot
secs, err := slots.SecondsSinceSlotStart(head.slot, f.store.genesisTime, uint64(time.Now().Unix()))
if err != nil {
log.WithError(err).Error("could not check current slot")
return true
}
if secs < ProcessAttestationsThreshold {
return true
}
// Only orphan a block if the parent LMD vote is strong
if parent.weight*100 < f.store.committeeWeight*params.BeaconConfig().ReorgParentWeightThreshold {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,19 @@ func TestForkChoice_ShouldOverrideFCU(t *testing.T) {
require.Equal(t, false, f.ShouldOverrideFCU())
f.store.headNode.parent = saved
})
t.Run("parent is weak", func(t *testing.T) {
t.Run("parent is weak early call", func(t *testing.T) {
saved := f.store.headNode.parent.weight
f.store.headNode.parent.weight = 0
require.Equal(t, true, f.ShouldOverrideFCU())
f.store.headNode.parent.weight = saved
})
t.Run("parent is weak late call", func(t *testing.T) {
saved := f.store.headNode.parent.weight
driftGenesisTime(f, 2, 11)
f.store.headNode.parent.weight = 0
require.Equal(t, false, f.ShouldOverrideFCU())
f.store.headNode.parent.weight = saved
driftGenesisTime(f, 2, orphanLateBlockFirstThreshold+1)
})
t.Run("Head is strong", func(t *testing.T) {
f.store.headNode.weight = f.store.committeeWeight
Expand Down

0 comments on commit 04a5b2e

Please sign in to comment.