From ae4b982a6c7bafe5f5d4b64247856ffc2d8e5fa1 Mon Sep 17 00:00:00 2001 From: Bastin <43618253+Inspector-Butters@users.noreply.github.com> Date: Fri, 1 Aug 2025 14:35:21 +0200 Subject: [PATCH] Fix finality update bugs & Move broadcast logic to LC Store (#15540) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix IsBetterFinalityUpdate and add tests fix finality update bugs * Update lightclient.go Co-authored-by: RadosÅ‚aw Kapka * Update beacon-chain/core/light-client/lightclient.go --------- Co-authored-by: RadosÅ‚aw Kapka --- beacon-chain/blockchain/options.go | 2 +- .../blockchain/process_block_helpers.go | 28 +---- beacon-chain/blockchain/process_block_test.go | 36 +++--- beacon-chain/core/light-client/BUILD.bazel | 7 ++ beacon-chain/core/light-client/lightclient.go | 33 ++++- beacon-chain/core/light-client/store.go | 46 ++++++- beacon-chain/core/light-client/store_test.go | 119 ++++++++++++++++-- beacon-chain/node/node.go | 2 +- beacon-chain/p2p/BUILD.bazel | 2 - beacon-chain/p2p/broadcaster_test.go | 8 +- beacon-chain/rpc/eth/light-client/BUILD.bazel | 2 + .../rpc/eth/light-client/handlers_test.go | 40 +++--- beacon-chain/sync/BUILD.bazel | 1 + beacon-chain/sync/rpc_light_client_test.go | 13 +- beacon-chain/sync/subscriber_light_client.go | 4 +- beacon-chain/sync/subscriber_test.go | 5 +- beacon-chain/sync/validate_light_client.go | 2 +- .../sync/validate_light_client_test.go | 9 +- changelog/bastin_move_broadcast_to_store.md | 5 + testing/util/BUILD.bazel | 1 + testing/util/lightclient.go | 76 +++++++++++ 21 files changed, 336 insertions(+), 105 deletions(-) create mode 100644 changelog/bastin_move_broadcast_to_store.md diff --git a/beacon-chain/blockchain/options.go b/beacon-chain/blockchain/options.go index 10d885c4bf..2d4addaa22 100644 --- a/beacon-chain/blockchain/options.go +++ b/beacon-chain/blockchain/options.go @@ -36,7 +36,7 @@ func WithMaxGoroutines(x int) Option { // WithLCStore for light client store access. func WithLCStore() Option { return func(s *Service) error { - s.lcStore = lightclient.NewLightClientStore(s.cfg.BeaconDB) + s.lcStore = lightclient.NewLightClientStore(s.cfg.BeaconDB, s.cfg.P2P, s.cfg.StateNotifier.StateFeed()) return nil } } diff --git a/beacon-chain/blockchain/process_block_helpers.go b/beacon-chain/blockchain/process_block_helpers.go index 39b3c84cad..826a465d87 100644 --- a/beacon-chain/blockchain/process_block_helpers.go +++ b/beacon-chain/blockchain/process_block_helpers.go @@ -1,7 +1,6 @@ package blockchain import ( - "bytes" "context" "fmt" "strings" @@ -198,8 +197,7 @@ func (s *Service) processLightClientFinalityUpdate( finalizedCheckpoint := attestedState.FinalizedCheckpoint() - // Check if the finalized checkpoint has changed - if finalizedCheckpoint == nil || bytes.Equal(finalizedCheckpoint.GetRoot(), postState.FinalizedCheckpoint().Root) { + if finalizedCheckpoint == nil { return nil } @@ -224,17 +222,7 @@ func (s *Service) processLightClientFinalityUpdate( return nil } - log.Debug("Saving new light client finality update") - s.lcStore.SetLastFinalityUpdate(newUpdate) - - s.cfg.StateNotifier.StateFeed().Send(&feed.Event{ - Type: statefeed.LightClientFinalityUpdate, - Data: newUpdate, - }) - - if err = s.cfg.P2P.BroadcastLightClientFinalityUpdate(ctx, newUpdate); err != nil { - return errors.Wrap(err, "could not broadcast light client finality update") - } + s.lcStore.SetLastFinalityUpdate(newUpdate, true) return nil } @@ -266,17 +254,7 @@ func (s *Service) processLightClientOptimisticUpdate(ctx context.Context, signed return nil } - log.Debug("Saving new light client optimistic update") - s.lcStore.SetLastOptimisticUpdate(newUpdate) - - s.cfg.StateNotifier.StateFeed().Send(&feed.Event{ - Type: statefeed.LightClientOptimisticUpdate, - Data: newUpdate, - }) - - if err = s.cfg.P2P.BroadcastLightClientOptimisticUpdate(ctx, newUpdate); err != nil { - return errors.Wrap(err, "could not broadcast light client optimistic update") - } + s.lcStore.SetLastOptimisticUpdate(newUpdate, true) return nil } diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index a027c8ab3c..2cac589652 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -3170,7 +3170,7 @@ func TestProcessLightClientOptimisticUpdate(t *testing.T) { t.Run(version.String(testVersion)+"_"+tc.name, func(t *testing.T) { s.genesisTime = time.Unix(time.Now().Unix()-(int64(forkEpoch)*int64(params.BeaconConfig().SlotsPerEpoch)*int64(params.BeaconConfig().SecondsPerSlot)), 0) - s.lcStore = &lightClient.Store{} + s.lcStore = lightClient.NewLightClientStore(s.cfg.BeaconDB, s.cfg.P2P, s.cfg.StateNotifier.StateFeed()) var oldActualUpdate interfaces.LightClientOptimisticUpdate var err error @@ -3246,39 +3246,39 @@ func TestProcessLightClientFinalityUpdate(t *testing.T) { expectReplace: true, }, { - name: "Old update is better - age - no supermajority", + name: "Old update is better - finalized slot is higher", oldOptions: []util.LightClientOption{util.WithIncreasedFinalizedSlot(1)}, newOptions: []util.LightClientOption{}, expectReplace: false, }, { - name: "Old update is better - age - both supermajority", - oldOptions: []util.LightClientOption{util.WithIncreasedFinalizedSlot(1), util.WithSupermajority()}, - newOptions: []util.LightClientOption{util.WithSupermajority()}, - expectReplace: false, - }, - { - name: "Old update is better - supermajority", - oldOptions: []util.LightClientOption{util.WithSupermajority()}, + name: "Old update is better - attested slot is higher", + oldOptions: []util.LightClientOption{util.WithIncreasedAttestedSlot(1)}, newOptions: []util.LightClientOption{}, expectReplace: false, }, { - name: "New update is better - age - both supermajority", - oldOptions: []util.LightClientOption{util.WithSupermajority()}, - newOptions: []util.LightClientOption{util.WithIncreasedFinalizedSlot(1), util.WithSupermajority()}, + name: "Old update is better - signature slot is higher", + oldOptions: []util.LightClientOption{util.WithIncreasedSignatureSlot(1)}, + newOptions: []util.LightClientOption{}, + expectReplace: false, + }, + { + name: "New update is better - finalized slot is higher", + oldOptions: []util.LightClientOption{}, + newOptions: []util.LightClientOption{util.WithIncreasedAttestedSlot(1)}, expectReplace: true, }, { - name: "New update is better - age - no supermajority", + name: "New update is better - attested slot is higher", oldOptions: []util.LightClientOption{}, - newOptions: []util.LightClientOption{util.WithIncreasedFinalizedSlot(1)}, + newOptions: []util.LightClientOption{util.WithIncreasedAttestedSlot(1)}, expectReplace: true, }, { - name: "New update is better - supermajority", + name: "New update is better - signature slot is higher", oldOptions: []util.LightClientOption{}, - newOptions: []util.LightClientOption{util.WithSupermajority()}, + newOptions: []util.LightClientOption{util.WithIncreasedSignatureSlot(1)}, expectReplace: true, }, } @@ -3310,7 +3310,7 @@ func TestProcessLightClientFinalityUpdate(t *testing.T) { t.Run(version.String(testVersion)+"_"+tc.name, func(t *testing.T) { s.genesisTime = time.Unix(time.Now().Unix()-(int64(forkEpoch)*int64(params.BeaconConfig().SlotsPerEpoch)*int64(params.BeaconConfig().SecondsPerSlot)), 0) - s.lcStore = &lightClient.Store{} + s.lcStore = lightClient.NewLightClientStore(s.cfg.BeaconDB, s.cfg.P2P, s.cfg.StateNotifier.StateFeed()) var actualOldUpdate, actualNewUpdate interfaces.LightClientFinalityUpdate var err error diff --git a/beacon-chain/core/light-client/BUILD.bazel b/beacon-chain/core/light-client/BUILD.bazel index 6d3d664d87..1b45a61efb 100644 --- a/beacon-chain/core/light-client/BUILD.bazel +++ b/beacon-chain/core/light-client/BUILD.bazel @@ -10,8 +10,12 @@ go_library( importpath = "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client", visibility = ["//visibility:public"], deps = [ + "//async/event:go_default_library", + "//beacon-chain/core/feed:go_default_library", + "//beacon-chain/core/feed/state:go_default_library", "//beacon-chain/db/iface:go_default_library", "//beacon-chain/execution:go_default_library", + "//beacon-chain/p2p:go_default_library", "//beacon-chain/state:go_default_library", "//config/fieldparams:go_default_library", "//config/params:go_default_library", @@ -39,6 +43,9 @@ go_test( ], deps = [ ":go_default_library", + "//async/event:go_default_library", + "//beacon-chain/db/testing:go_default_library", + "//beacon-chain/p2p/testing:go_default_library", "//config/fieldparams:go_default_library", "//config/params:go_default_library", "//consensus-types:go_default_library", diff --git a/beacon-chain/core/light-client/lightclient.go b/beacon-chain/core/light-client/lightclient.go index e8d9b1b528..cb329827aa 100644 --- a/beacon-chain/core/light-client/lightclient.go +++ b/beacon-chain/core/light-client/lightclient.go @@ -750,7 +750,9 @@ func UpdateHasSupermajority(syncAggregate *pb.SyncAggregate) bool { return numActiveParticipants*3 >= maxActiveParticipants*2 } -func IsBetterFinalityUpdate(newUpdate, oldUpdate interfaces.LightClientFinalityUpdate) bool { +// IsFinalityUpdateValidForBroadcast checks if a finality update needs to be broadcasted. +// It is also used to check if an incoming gossiped finality update is valid for forwarding and saving. +func IsFinalityUpdateValidForBroadcast(newUpdate, oldUpdate interfaces.LightClientFinalityUpdate) bool { if oldUpdate == nil { return true } @@ -772,6 +774,35 @@ func IsBetterFinalityUpdate(newUpdate, oldUpdate interfaces.LightClientFinalityU return true } +// IsBetterFinalityUpdate checks if the new finality update is better than the old one for saving. +// This does not concern broadcasting, but rather the decision of whether to save the new update. +// For broadcasting checks, use IsFinalityUpdateValidForBroadcast. +func IsBetterFinalityUpdate(newUpdate, oldUpdate interfaces.LightClientFinalityUpdate) bool { + if oldUpdate == nil { + return true + } + + // Full nodes SHOULD provide the LightClientFinalityUpdate with the highest attested_header.beacon.slot (if multiple, highest signature_slot) + newFinalizedSlot := newUpdate.FinalizedHeader().Beacon().Slot + newAttestedSlot := newUpdate.AttestedHeader().Beacon().Slot + + oldFinalizedSlot := oldUpdate.FinalizedHeader().Beacon().Slot + oldAttestedSlot := oldUpdate.AttestedHeader().Beacon().Slot + + if newFinalizedSlot < oldFinalizedSlot { + return false + } + if newFinalizedSlot == oldFinalizedSlot { + if newAttestedSlot < oldAttestedSlot { + return false + } + if newAttestedSlot == oldAttestedSlot && newUpdate.SignatureSlot() <= oldUpdate.SignatureSlot() { + return false + } + } + return true +} + func IsBetterOptimisticUpdate(newUpdate, oldUpdate interfaces.LightClientOptimisticUpdate) bool { if oldUpdate == nil { return true diff --git a/beacon-chain/core/light-client/store.go b/beacon-chain/core/light-client/store.go index 8271429131..047aa793fc 100644 --- a/beacon-chain/core/light-client/store.go +++ b/beacon-chain/core/light-client/store.go @@ -4,7 +4,11 @@ import ( "context" "sync" + "github.com/OffchainLabs/prysm/v6/async/event" + "github.com/OffchainLabs/prysm/v6/beacon-chain/core/feed" + statefeed "github.com/OffchainLabs/prysm/v6/beacon-chain/core/feed/state" "github.com/OffchainLabs/prysm/v6/beacon-chain/db/iface" + "github.com/OffchainLabs/prysm/v6/beacon-chain/p2p" "github.com/OffchainLabs/prysm/v6/consensus-types/interfaces" "github.com/pkg/errors" log "github.com/sirupsen/logrus" @@ -16,13 +20,17 @@ type Store struct { mu sync.RWMutex beaconDB iface.HeadAccessDatabase - lastFinalityUpdate interfaces.LightClientFinalityUpdate - lastOptimisticUpdate interfaces.LightClientOptimisticUpdate + lastFinalityUpdate interfaces.LightClientFinalityUpdate // tracks the best finality update seen so far + lastOptimisticUpdate interfaces.LightClientOptimisticUpdate // tracks the best optimistic update seen so far + p2p p2p.Accessor + stateFeed event.SubscriberSender } -func NewLightClientStore(db iface.HeadAccessDatabase) *Store { +func NewLightClientStore(db iface.HeadAccessDatabase, p p2p.Accessor, e event.SubscriberSender) *Store { return &Store{ - beaconDB: db, + beaconDB: db, + p2p: p, + stateFeed: e, } } @@ -143,10 +151,23 @@ func (s *Store) SaveLightClientUpdate(ctx context.Context, period uint64, update return nil } -func (s *Store) SetLastFinalityUpdate(update interfaces.LightClientFinalityUpdate) { +func (s *Store) SetLastFinalityUpdate(update interfaces.LightClientFinalityUpdate, broadcast bool) { s.mu.Lock() defer s.mu.Unlock() + + if broadcast && IsFinalityUpdateValidForBroadcast(update, s.lastFinalityUpdate) { + if err := s.p2p.BroadcastLightClientFinalityUpdate(context.Background(), update); err != nil { + log.WithError(err).Error("Could not broadcast light client finality update") + } + } + s.lastFinalityUpdate = update + log.Debug("Saved new light client finality update") + + s.stateFeed.Send(&feed.Event{ + Type: statefeed.LightClientFinalityUpdate, + Data: update, + }) } func (s *Store) LastFinalityUpdate() interfaces.LightClientFinalityUpdate { @@ -155,10 +176,23 @@ func (s *Store) LastFinalityUpdate() interfaces.LightClientFinalityUpdate { return s.lastFinalityUpdate } -func (s *Store) SetLastOptimisticUpdate(update interfaces.LightClientOptimisticUpdate) { +func (s *Store) SetLastOptimisticUpdate(update interfaces.LightClientOptimisticUpdate, broadcast bool) { s.mu.Lock() defer s.mu.Unlock() + + if broadcast { + if err := s.p2p.BroadcastLightClientOptimisticUpdate(context.Background(), update); err != nil { + log.WithError(err).Error("Could not broadcast light client optimistic update") + } + } + s.lastOptimisticUpdate = update + log.Debug("Saved new light client optimistic update") + + s.stateFeed.Send(&feed.Event{ + Type: statefeed.LightClientOptimisticUpdate, + Data: update, + }) } func (s *Store) LastOptimisticUpdate() interfaces.LightClientOptimisticUpdate { diff --git a/beacon-chain/core/light-client/store_test.go b/beacon-chain/core/light-client/store_test.go index eda07f2a30..abe2440f1e 100644 --- a/beacon-chain/core/light-client/store_test.go +++ b/beacon-chain/core/light-client/store_test.go @@ -3,7 +3,10 @@ package light_client_test import ( "testing" + "github.com/OffchainLabs/prysm/v6/async/event" lightClient "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client" + testDB "github.com/OffchainLabs/prysm/v6/beacon-chain/db/testing" + p2pTesting "github.com/OffchainLabs/prysm/v6/beacon-chain/p2p/testing" "github.com/OffchainLabs/prysm/v6/config/params" "github.com/OffchainLabs/prysm/v6/runtime/version" "github.com/OffchainLabs/prysm/v6/testing/require" @@ -21,7 +24,7 @@ func TestLightClientStore(t *testing.T) { params.OverrideBeaconConfig(cfg) // Initialize the light client store - lcStore := &lightClient.Store{} + lcStore := lightClient.NewLightClientStore(testDB.SetupDB(t), &p2pTesting.FakeP2P{}, new(event.Feed)) // Create test light client updates for Capella and Deneb lCapella := util.NewTestLightClient(t, version.Capella) @@ -45,24 +48,118 @@ func TestLightClientStore(t *testing.T) { require.IsNil(t, lcStore.LastOptimisticUpdate(), "lastOptimisticUpdate should be nil") // Set and get finality with Capella update. Optimistic update should be nil - lcStore.SetLastFinalityUpdate(finUpdateCapella) + lcStore.SetLastFinalityUpdate(finUpdateCapella, false) require.Equal(t, finUpdateCapella, lcStore.LastFinalityUpdate(), "lastFinalityUpdate is wrong") require.IsNil(t, lcStore.LastOptimisticUpdate(), "lastOptimisticUpdate should be nil") // Set and get optimistic with Capella update. Finality update should be Capella - lcStore.SetLastOptimisticUpdate(opUpdateCapella) + lcStore.SetLastOptimisticUpdate(opUpdateCapella, false) require.Equal(t, opUpdateCapella, lcStore.LastOptimisticUpdate(), "lastOptimisticUpdate is wrong") require.Equal(t, finUpdateCapella, lcStore.LastFinalityUpdate(), "lastFinalityUpdate is wrong") // Set and get finality and optimistic with Deneb update - lcStore.SetLastFinalityUpdate(finUpdateDeneb) - lcStore.SetLastOptimisticUpdate(opUpdateDeneb) + lcStore.SetLastFinalityUpdate(finUpdateDeneb, false) + lcStore.SetLastOptimisticUpdate(opUpdateDeneb, false) require.Equal(t, finUpdateDeneb, lcStore.LastFinalityUpdate(), "lastFinalityUpdate is wrong") require.Equal(t, opUpdateDeneb, lcStore.LastOptimisticUpdate(), "lastOptimisticUpdate is wrong") - - // Set and get finality and optimistic with nil update - lcStore.SetLastFinalityUpdate(nil) - lcStore.SetLastOptimisticUpdate(nil) - require.IsNil(t, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should be nil") - require.IsNil(t, lcStore.LastOptimisticUpdate(), "lastOptimisticUpdate should be nil") +} + +func TestLightClientStore_SetLastFinalityUpdate(t *testing.T) { + p2p := p2pTesting.NewTestP2P(t) + lcStore := lightClient.NewLightClientStore(testDB.SetupDB(t), p2p, new(event.Feed)) + + // update 0 with basic data and no supermajority following an empty lastFinalityUpdate - should save and broadcast + l0 := util.NewTestLightClient(t, version.Altair) + update0, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l0.Ctx, l0.State, l0.Block, l0.AttestedState, l0.AttestedBlock, l0.FinalizedBlock) + require.NoError(t, err, "Failed to create light client finality update") + + require.Equal(t, true, lightClient.IsBetterFinalityUpdate(update0, lcStore.LastFinalityUpdate()), "update0 should be better than nil") + // update0 should be valid for broadcast - meaning it should be broadcasted + require.Equal(t, true, lightClient.IsFinalityUpdateValidForBroadcast(update0, lcStore.LastFinalityUpdate()), "update0 should be valid for broadcast") + + lcStore.SetLastFinalityUpdate(update0, true) + require.Equal(t, update0, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should match the set value") + require.Equal(t, true, p2p.BroadcastCalled.Load(), "Broadcast should have been called after setting a new last finality update when previous is nil") + p2p.BroadcastCalled.Store(false) // Reset for next test + + // update 1 with same finality slot, increased attested slot, and no supermajority - should save but not broadcast + l1 := util.NewTestLightClient(t, version.Altair, util.WithIncreasedAttestedSlot(1)) + update1, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l1.Ctx, l1.State, l1.Block, l1.AttestedState, l1.AttestedBlock, l1.FinalizedBlock) + require.NoError(t, err, "Failed to create light client finality update") + + require.Equal(t, true, lightClient.IsBetterFinalityUpdate(update1, update0), "update1 should be better than update0") + // update1 should not be valid for broadcast - meaning it should not be broadcasted + require.Equal(t, false, lightClient.IsFinalityUpdateValidForBroadcast(update1, lcStore.LastFinalityUpdate()), "update1 should not be valid for broadcast") + + lcStore.SetLastFinalityUpdate(update1, true) + require.Equal(t, update1, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should match the set value") + require.Equal(t, false, p2p.BroadcastCalled.Load(), "Broadcast should not have been called after setting a new last finality update without supermajority") + p2p.BroadcastCalled.Store(false) // Reset for next test + + // update 2 with same finality slot, increased attested slot, and supermajority - should save and broadcast + l2 := util.NewTestLightClient(t, version.Altair, util.WithIncreasedAttestedSlot(2), util.WithSupermajority()) + update2, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l2.Ctx, l2.State, l2.Block, l2.AttestedState, l2.AttestedBlock, l2.FinalizedBlock) + require.NoError(t, err, "Failed to create light client finality update") + + require.Equal(t, true, lightClient.IsBetterFinalityUpdate(update2, update1), "update2 should be better than update1") + // update2 should be valid for broadcast - meaning it should be broadcasted + require.Equal(t, true, lightClient.IsFinalityUpdateValidForBroadcast(update2, lcStore.LastFinalityUpdate()), "update2 should be valid for broadcast") + + lcStore.SetLastFinalityUpdate(update2, true) + require.Equal(t, update2, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should match the set value") + require.Equal(t, true, p2p.BroadcastCalled.Load(), "Broadcast should have been called after setting a new last finality update with supermajority") + p2p.BroadcastCalled.Store(false) // Reset for next test + + // update 3 with same finality slot, increased attested slot, and supermajority - should save but not broadcast + l3 := util.NewTestLightClient(t, version.Altair, util.WithIncreasedAttestedSlot(3), util.WithSupermajority()) + update3, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l3.Ctx, l3.State, l3.Block, l3.AttestedState, l3.AttestedBlock, l3.FinalizedBlock) + require.NoError(t, err, "Failed to create light client finality update") + + require.Equal(t, true, lightClient.IsBetterFinalityUpdate(update3, update2), "update3 should be better than update2") + // update3 should not be valid for broadcast - meaning it should not be broadcasted + require.Equal(t, false, lightClient.IsFinalityUpdateValidForBroadcast(update3, lcStore.LastFinalityUpdate()), "update3 should not be valid for broadcast") + + lcStore.SetLastFinalityUpdate(update3, true) + require.Equal(t, update3, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should match the set value") + require.Equal(t, false, p2p.BroadcastCalled.Load(), "Broadcast should not have been when previous was already broadcast") + + // update 4 with increased finality slot, increased attested slot, and supermajority - should save and broadcast + l4 := util.NewTestLightClient(t, version.Altair, util.WithIncreasedFinalizedSlot(1), util.WithIncreasedAttestedSlot(1), util.WithSupermajority()) + update4, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l4.Ctx, l4.State, l4.Block, l4.AttestedState, l4.AttestedBlock, l4.FinalizedBlock) + require.NoError(t, err, "Failed to create light client finality update") + + require.Equal(t, true, lightClient.IsBetterFinalityUpdate(update4, update3), "update4 should be better than update3") + // update4 should be valid for broadcast - meaning it should be broadcasted + require.Equal(t, true, lightClient.IsFinalityUpdateValidForBroadcast(update4, lcStore.LastFinalityUpdate()), "update4 should be valid for broadcast") + + lcStore.SetLastFinalityUpdate(update4, true) + require.Equal(t, update4, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should match the set value") + require.Equal(t, true, p2p.BroadcastCalled.Load(), "Broadcast should have been called after a new finality update with increased finality slot") + p2p.BroadcastCalled.Store(false) // Reset for next test + + // update 5 with the same new finality slot, increased attested slot, and supermajority - should save but not broadcast + l5 := util.NewTestLightClient(t, version.Altair, util.WithIncreasedFinalizedSlot(1), util.WithIncreasedAttestedSlot(2), util.WithSupermajority()) + update5, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l5.Ctx, l5.State, l5.Block, l5.AttestedState, l5.AttestedBlock, l5.FinalizedBlock) + require.NoError(t, err, "Failed to create light client finality update") + + require.Equal(t, true, lightClient.IsBetterFinalityUpdate(update5, update4), "update5 should be better than update4") + // update5 should not be valid for broadcast - meaning it should not be broadcasted + require.Equal(t, false, lightClient.IsFinalityUpdateValidForBroadcast(update5, lcStore.LastFinalityUpdate()), "update5 should not be valid for broadcast") + + lcStore.SetLastFinalityUpdate(update5, true) + require.Equal(t, update5, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should match the set value") + require.Equal(t, false, p2p.BroadcastCalled.Load(), "Broadcast should not have been called when previous was already broadcast with supermajority") + + // update 6 with the same new finality slot, increased attested slot, and no supermajority - should save but not broadcast + l6 := util.NewTestLightClient(t, version.Altair, util.WithIncreasedFinalizedSlot(1), util.WithIncreasedAttestedSlot(3)) + update6, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l6.Ctx, l6.State, l6.Block, l6.AttestedState, l6.AttestedBlock, l6.FinalizedBlock) + require.NoError(t, err, "Failed to create light client finality update") + + require.Equal(t, true, lightClient.IsBetterFinalityUpdate(update6, update5), "update6 should be better than update5") + // update6 should not be valid for broadcast - meaning it should not be broadcasted + require.Equal(t, false, lightClient.IsFinalityUpdateValidForBroadcast(update6, lcStore.LastFinalityUpdate()), "update6 should not be valid for broadcast") + + lcStore.SetLastFinalityUpdate(update6, true) + require.Equal(t, update6, lcStore.LastFinalityUpdate(), "lastFinalityUpdate should match the set value") + require.Equal(t, false, p2p.BroadcastCalled.Load(), "Broadcast should not have been called when previous was already broadcast with supermajority") } diff --git a/beacon-chain/node/node.go b/beacon-chain/node/node.go index 12aedbe3c1..8e979a3ad3 100644 --- a/beacon-chain/node/node.go +++ b/beacon-chain/node/node.go @@ -236,7 +236,7 @@ func New(cliCtx *cli.Context, cancel context.CancelFunc, opts ...Option) (*Beaco beacon.finalizedStateAtStartUp = nil if features.Get().EnableLightClient { - beacon.lcStore = lightclient.NewLightClientStore(beacon.db) + beacon.lcStore = lightclient.NewLightClientStore(beacon.db, beacon.fetchP2P(), beacon.StateFeed()) } return beacon, nil diff --git a/beacon-chain/p2p/BUILD.bazel b/beacon-chain/p2p/BUILD.bazel index 4d93ee76df..9650417b28 100644 --- a/beacon-chain/p2p/BUILD.bazel +++ b/beacon-chain/p2p/BUILD.bazel @@ -147,7 +147,6 @@ go_test( "//beacon-chain/blockchain/testing:go_default_library", "//beacon-chain/cache:go_default_library", "//beacon-chain/core/helpers:go_default_library", - "//beacon-chain/core/light-client:go_default_library", "//beacon-chain/core/peerdas:go_default_library", "//beacon-chain/core/signing:go_default_library", "//beacon-chain/db/testing:go_default_library", @@ -174,7 +173,6 @@ go_test( "//proto/prysm/v1alpha1:go_default_library", "//proto/prysm/v1alpha1/metadata:go_default_library", "//proto/testing:go_default_library", - "//runtime/version:go_default_library", "//testing/assert:go_default_library", "//testing/require:go_default_library", "//testing/util:go_default_library", diff --git a/beacon-chain/p2p/broadcaster_test.go b/beacon-chain/p2p/broadcaster_test.go index 31e2f68bba..e2ff040ed5 100644 --- a/beacon-chain/p2p/broadcaster_test.go +++ b/beacon-chain/p2p/broadcaster_test.go @@ -11,7 +11,6 @@ import ( "github.com/OffchainLabs/prysm/v6/beacon-chain/blockchain/kzg" "github.com/OffchainLabs/prysm/v6/beacon-chain/core/helpers" - lightClient "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client" "github.com/OffchainLabs/prysm/v6/beacon-chain/core/peerdas" "github.com/OffchainLabs/prysm/v6/beacon-chain/p2p/peers" "github.com/OffchainLabs/prysm/v6/beacon-chain/p2p/peers/scorers" @@ -24,7 +23,6 @@ import ( "github.com/OffchainLabs/prysm/v6/network/forks" ethpb "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1" testpb "github.com/OffchainLabs/prysm/v6/proto/testing" - "github.com/OffchainLabs/prysm/v6/runtime/version" "github.com/OffchainLabs/prysm/v6/testing/assert" "github.com/OffchainLabs/prysm/v6/testing/require" "github.com/OffchainLabs/prysm/v6/testing/util" @@ -546,8 +544,7 @@ func TestService_BroadcastLightClientOptimisticUpdate(t *testing.T) { }), } - l := util.NewTestLightClient(t, version.Altair) - msg, err := lightClient.NewLightClientOptimisticUpdateFromBeaconState(l.Ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock) + msg, err := util.MockOptimisticUpdate() require.NoError(t, err) GossipTypeMapping[reflect.TypeOf(msg)] = LightClientOptimisticUpdateTopicFormat @@ -613,8 +610,7 @@ func TestService_BroadcastLightClientFinalityUpdate(t *testing.T) { }), } - l := util.NewTestLightClient(t, version.Altair) - msg, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(l.Ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock, l.FinalizedBlock) + msg, err := util.MockFinalityUpdate() require.NoError(t, err) GossipTypeMapping[reflect.TypeOf(msg)] = LightClientFinalityUpdateTopicFormat diff --git a/beacon-chain/rpc/eth/light-client/BUILD.bazel b/beacon-chain/rpc/eth/light-client/BUILD.bazel index 6077182660..016ac4c995 100644 --- a/beacon-chain/rpc/eth/light-client/BUILD.bazel +++ b/beacon-chain/rpc/eth/light-client/BUILD.bazel @@ -33,9 +33,11 @@ go_test( embed = [":go_default_library"], deps = [ "//api/server/structs:go_default_library", + "//async/event:go_default_library", "//beacon-chain/core/helpers:go_default_library", "//beacon-chain/core/light-client:go_default_library", "//beacon-chain/db/testing:go_default_library", + "//beacon-chain/p2p/testing:go_default_library", "//config/fieldparams:go_default_library", "//config/params:go_default_library", "//consensus-types/blocks:go_default_library", diff --git a/beacon-chain/rpc/eth/light-client/handlers_test.go b/beacon-chain/rpc/eth/light-client/handlers_test.go index 233549e801..2d7c5cd5bf 100644 --- a/beacon-chain/rpc/eth/light-client/handlers_test.go +++ b/beacon-chain/rpc/eth/light-client/handlers_test.go @@ -11,9 +11,11 @@ import ( "testing" "github.com/OffchainLabs/prysm/v6/api/server/structs" + "github.com/OffchainLabs/prysm/v6/async/event" "github.com/OffchainLabs/prysm/v6/beacon-chain/core/helpers" lightclient "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client" dbtesting "github.com/OffchainLabs/prysm/v6/beacon-chain/db/testing" + p2ptesting "github.com/OffchainLabs/prysm/v6/beacon-chain/p2p/testing" fieldparams "github.com/OffchainLabs/prysm/v6/config/fieldparams" "github.com/OffchainLabs/prysm/v6/config/params" "github.com/OffchainLabs/prysm/v6/consensus-types/blocks" @@ -53,7 +55,7 @@ func TestLightClientHandler_GetLightClientBootstrap(t *testing.T) { require.NoError(t, err) db := dbtesting.SetupDB(t) - lcStore := lightclient.NewLightClientStore(db) + lcStore := lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)) err = db.SaveLightClientBootstrap(l.Ctx, blockRoot[:], bootstrap) require.NoError(t, err) @@ -97,7 +99,7 @@ func TestLightClientHandler_GetLightClientBootstrap(t *testing.T) { require.NoError(t, err) db := dbtesting.SetupDB(t) - lcStore := lightclient.NewLightClientStore(db) + lcStore := lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)) err = db.SaveLightClientBootstrap(l.Ctx, blockRoot[:], bootstrap) require.NoError(t, err) @@ -141,7 +143,7 @@ func TestLightClientHandler_GetLightClientBootstrap(t *testing.T) { t.Run("no bootstrap found", func(t *testing.T) { s := &Server{ - LCStore: lightclient.NewLightClientStore(dbtesting.SetupDB(t)), + LCStore: lightclient.NewLightClientStore(dbtesting.SetupDB(t), &p2ptesting.FakeP2P{}, new(event.Feed)), } request := httptest.NewRequest("GET", "http://foo.com/", nil) request.SetPathValue("block_root", hexutil.Encode([]byte{0x00, 0x01, 0x02})) @@ -184,7 +186,7 @@ func TestLightClientHandler_GetLightClientByRange(t *testing.T) { } s := &Server{ - LCStore: lightclient.NewLightClientStore(db), + LCStore: lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)), } updatePeriod := startPeriod @@ -325,7 +327,7 @@ func TestLightClientHandler_GetLightClientByRange(t *testing.T) { db := dbtesting.SetupDB(t) s := &Server{ - LCStore: lightclient.NewLightClientStore(db), + LCStore: lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)), } updates := make([]interfaces.LightClientUpdate, 2) @@ -445,7 +447,7 @@ func TestLightClientHandler_GetLightClientByRange(t *testing.T) { db := dbtesting.SetupDB(t) s := &Server{ - LCStore: lightclient.NewLightClientStore(db), + LCStore: lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)), } updates := make([]interfaces.LightClientUpdate, 3) @@ -492,7 +494,7 @@ func TestLightClientHandler_GetLightClientByRange(t *testing.T) { db := dbtesting.SetupDB(t) s := &Server{ - LCStore: lightclient.NewLightClientStore(db), + LCStore: lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)), } updates := make([]interfaces.LightClientUpdate, 3) @@ -536,7 +538,7 @@ func TestLightClientHandler_GetLightClientByRange(t *testing.T) { t.Run("start period before altair", func(t *testing.T) { db := dbtesting.SetupDB(t) s := &Server{ - LCStore: lightclient.NewLightClientStore(db), + LCStore: lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)), } startPeriod := 0 url := fmt.Sprintf("http://foo.com/?count=128&start_period=%d", startPeriod) @@ -559,7 +561,7 @@ func TestLightClientHandler_GetLightClientByRange(t *testing.T) { t.Run("missing update in the middle", func(t *testing.T) { db := dbtesting.SetupDB(t) s := &Server{ - LCStore: lightclient.NewLightClientStore(db), + LCStore: lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)), } updates := make([]interfaces.LightClientUpdate, 3) @@ -603,7 +605,7 @@ func TestLightClientHandler_GetLightClientByRange(t *testing.T) { t.Run("missing update at the beginning", func(t *testing.T) { db := dbtesting.SetupDB(t) s := &Server{ - LCStore: lightclient.NewLightClientStore(db), + LCStore: lightclient.NewLightClientStore(db, &p2ptesting.FakeP2P{}, new(event.Feed)), } updates := make([]interfaces.LightClientUpdate, 3) @@ -663,8 +665,8 @@ func TestLightClientHandler_GetLightClientFinalityUpdate(t *testing.T) { update, err := lightclient.NewLightClientFinalityUpdateFromBeaconState(ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock, l.FinalizedBlock) require.NoError(t, err) - s := &Server{LCStore: &lightclient.Store{}} - s.LCStore.SetLastFinalityUpdate(update) + s := &Server{LCStore: lightclient.NewLightClientStore(dbtesting.SetupDB(t), &p2ptesting.FakeP2P{}, new(event.Feed))} + s.LCStore.SetLastFinalityUpdate(update, false) request := httptest.NewRequest("GET", "http://foo.com", nil) writer := httptest.NewRecorder() @@ -688,8 +690,8 @@ func TestLightClientHandler_GetLightClientFinalityUpdate(t *testing.T) { update, err := lightclient.NewLightClientFinalityUpdateFromBeaconState(ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock, l.FinalizedBlock) require.NoError(t, err) - s := &Server{LCStore: &lightclient.Store{}} - s.LCStore.SetLastFinalityUpdate(update) + s := &Server{LCStore: lightclient.NewLightClientStore(dbtesting.SetupDB(t), &p2ptesting.FakeP2P{}, new(event.Feed))} + s.LCStore.SetLastFinalityUpdate(update, false) request := httptest.NewRequest("GET", "http://foo.com", nil) request.Header.Add("Accept", "application/octet-stream") @@ -727,7 +729,7 @@ func TestLightClientHandler_GetLightClientOptimisticUpdate(t *testing.T) { helpers.ClearCache() t.Run("no update", func(t *testing.T) { - s := &Server{LCStore: &lightclient.Store{}} + s := &Server{LCStore: lightclient.NewLightClientStore(dbtesting.SetupDB(t), &p2ptesting.FakeP2P{}, new(event.Feed))} request := httptest.NewRequest("GET", "http://foo.com", nil) writer := httptest.NewRecorder() @@ -743,8 +745,8 @@ func TestLightClientHandler_GetLightClientOptimisticUpdate(t *testing.T) { update, err := lightclient.NewLightClientOptimisticUpdateFromBeaconState(ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock) require.NoError(t, err) - s := &Server{LCStore: &lightclient.Store{}} - s.LCStore.SetLastOptimisticUpdate(update) + s := &Server{LCStore: lightclient.NewLightClientStore(dbtesting.SetupDB(t), &p2ptesting.FakeP2P{}, new(event.Feed))} + s.LCStore.SetLastOptimisticUpdate(update, false) request := httptest.NewRequest("GET", "http://foo.com", nil) writer := httptest.NewRecorder() @@ -767,8 +769,8 @@ func TestLightClientHandler_GetLightClientOptimisticUpdate(t *testing.T) { update, err := lightclient.NewLightClientOptimisticUpdateFromBeaconState(ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock) require.NoError(t, err) - s := &Server{LCStore: &lightclient.Store{}} - s.LCStore.SetLastOptimisticUpdate(update) + s := &Server{LCStore: lightclient.NewLightClientStore(dbtesting.SetupDB(t), &p2ptesting.FakeP2P{}, new(event.Feed))} + s.LCStore.SetLastOptimisticUpdate(update, false) request := httptest.NewRequest("GET", "http://foo.com", nil) request.Header.Add("Accept", "application/octet-stream") diff --git a/beacon-chain/sync/BUILD.bazel b/beacon-chain/sync/BUILD.bazel index 5c9383c97a..af21ff7a85 100644 --- a/beacon-chain/sync/BUILD.bazel +++ b/beacon-chain/sync/BUILD.bazel @@ -212,6 +212,7 @@ go_test( shard_count = 4, deps = [ "//async/abool:go_default_library", + "//async/event:go_default_library", "//beacon-chain/blockchain:go_default_library", "//beacon-chain/blockchain/kzg:go_default_library", "//beacon-chain/blockchain/testing:go_default_library", diff --git a/beacon-chain/sync/rpc_light_client_test.go b/beacon-chain/sync/rpc_light_client_test.go index 5154e99d0f..07ad7b7b44 100644 --- a/beacon-chain/sync/rpc_light_client_test.go +++ b/beacon-chain/sync/rpc_light_client_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/OffchainLabs/prysm/v6/async/abool" + "github.com/OffchainLabs/prysm/v6/async/event" mockChain "github.com/OffchainLabs/prysm/v6/beacon-chain/blockchain/testing" lightClient "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client" db "github.com/OffchainLabs/prysm/v6/beacon-chain/db/testing" @@ -54,7 +55,7 @@ func TestRPC_LightClientBootstrap(t *testing.T) { stateNotifier: &mockChain.MockStateNotifier{}, }, chainStarted: abool.New(), - lcStore: lightClient.NewLightClientStore(d), + lcStore: lightClient.NewLightClientStore(d, &p2ptest.FakeP2P{}, new(event.Feed)), subHandler: newSubTopicHandler(), rateLimiter: newRateLimiter(p1), } @@ -176,7 +177,7 @@ func TestRPC_LightClientOptimisticUpdate(t *testing.T) { stateNotifier: &mockChain.MockStateNotifier{}, }, chainStarted: abool.New(), - lcStore: &lightClient.Store{}, + lcStore: lightClient.NewLightClientStore(d, &p2ptest.FakeP2P{}, new(event.Feed)), subHandler: newSubTopicHandler(), rateLimiter: newRateLimiter(p1), } @@ -202,7 +203,7 @@ func TestRPC_LightClientOptimisticUpdate(t *testing.T) { update, err := lightClient.NewLightClientOptimisticUpdateFromBeaconState(ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock) require.NoError(t, err) - r.lcStore.SetLastOptimisticUpdate(update) + r.lcStore.SetLastOptimisticUpdate(update, false) var wg sync.WaitGroup wg.Add(1) @@ -296,7 +297,7 @@ func TestRPC_LightClientFinalityUpdate(t *testing.T) { stateNotifier: &mockChain.MockStateNotifier{}, }, chainStarted: abool.New(), - lcStore: &lightClient.Store{}, + lcStore: lightClient.NewLightClientStore(d, &p2ptest.FakeP2P{}, new(event.Feed)), subHandler: newSubTopicHandler(), rateLimiter: newRateLimiter(p1), } @@ -322,7 +323,7 @@ func TestRPC_LightClientFinalityUpdate(t *testing.T) { update, err := lightClient.NewLightClientFinalityUpdateFromBeaconState(ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock, l.FinalizedBlock) require.NoError(t, err) - r.lcStore.SetLastFinalityUpdate(update) + r.lcStore.SetLastFinalityUpdate(update, false) var wg sync.WaitGroup wg.Add(1) @@ -416,7 +417,7 @@ func TestRPC_LightClientUpdatesByRange(t *testing.T) { stateNotifier: &mockChain.MockStateNotifier{}, }, chainStarted: abool.New(), - lcStore: lightClient.NewLightClientStore(d), + lcStore: lightClient.NewLightClientStore(d, &p2ptest.FakeP2P{}, new(event.Feed)), subHandler: newSubTopicHandler(), rateLimiter: newRateLimiter(p1), } diff --git a/beacon-chain/sync/subscriber_light_client.go b/beacon-chain/sync/subscriber_light_client.go index 2c12ff2216..0c277c0a5f 100644 --- a/beacon-chain/sync/subscriber_light_client.go +++ b/beacon-chain/sync/subscriber_light_client.go @@ -28,7 +28,7 @@ func (s *Service) lightClientOptimisticUpdateSubscriber(_ context.Context, msg p "attestedHeaderRoot": fmt.Sprintf("%x", attestedHeaderRoot), }).Debug("Saving newly received light client optimistic update.") - s.lcStore.SetLastOptimisticUpdate(update) + s.lcStore.SetLastOptimisticUpdate(update, false) s.cfg.stateNotifier.StateFeed().Send(&feed.Event{ Type: statefeed.LightClientOptimisticUpdate, @@ -55,7 +55,7 @@ func (s *Service) lightClientFinalityUpdateSubscriber(_ context.Context, msg pro "attestedHeaderRoot": fmt.Sprintf("%x", attestedHeaderRoot), }).Debug("Saving newly received light client finality update.") - s.lcStore.SetLastFinalityUpdate(update) + s.lcStore.SetLastFinalityUpdate(update, false) s.cfg.stateNotifier.StateFeed().Send(&feed.Event{ Type: statefeed.LightClientFinalityUpdate, diff --git a/beacon-chain/sync/subscriber_test.go b/beacon-chain/sync/subscriber_test.go index b0e04a5800..1443847a1b 100644 --- a/beacon-chain/sync/subscriber_test.go +++ b/beacon-chain/sync/subscriber_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/OffchainLabs/prysm/v6/async/abool" + "github.com/OffchainLabs/prysm/v6/async/event" mockChain "github.com/OffchainLabs/prysm/v6/beacon-chain/blockchain/testing" "github.com/OffchainLabs/prysm/v6/beacon-chain/cache" lightClient "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client" @@ -684,7 +685,7 @@ func TestSubscribe_ReceivesLCOptimisticUpdate(t *testing.T) { stateNotifier: &mockChain.MockStateNotifier{}, }, chainStarted: abool.New(), - lcStore: &lightClient.Store{}, + lcStore: lightClient.NewLightClientStore(d, &p2ptest.FakeP2P{}, new(event.Feed)), subHandler: newSubTopicHandler(), } topic := p2p.LightClientOptimisticUpdateTopicFormat @@ -751,7 +752,7 @@ func TestSubscribe_ReceivesLCFinalityUpdate(t *testing.T) { stateNotifier: &mockChain.MockStateNotifier{}, }, chainStarted: abool.New(), - lcStore: &lightClient.Store{}, + lcStore: lightClient.NewLightClientStore(d, &p2ptest.FakeP2P{}, new(event.Feed)), subHandler: newSubTopicHandler(), } topic := p2p.LightClientFinalityUpdateTopicFormat diff --git a/beacon-chain/sync/validate_light_client.go b/beacon-chain/sync/validate_light_client.go index e96bbf3d00..1ca629b898 100644 --- a/beacon-chain/sync/validate_light_client.go +++ b/beacon-chain/sync/validate_light_client.go @@ -131,7 +131,7 @@ func (s *Service) validateLightClientFinalityUpdate(ctx context.Context, pid pee return pubsub.ValidationIgnore, nil } - if !lightclient.IsBetterFinalityUpdate(newUpdate, s.lcStore.LastFinalityUpdate()) { + if !lightclient.IsFinalityUpdateValidForBroadcast(newUpdate, s.lcStore.LastFinalityUpdate()) { log.WithFields(logrus.Fields{ "attestedSlot": fmt.Sprintf("%d", newUpdate.AttestedHeader().Beacon().Slot), "signatureSlot": fmt.Sprintf("%d", newUpdate.SignatureSlot()), diff --git a/beacon-chain/sync/validate_light_client_test.go b/beacon-chain/sync/validate_light_client_test.go index f1ac8ac1e3..c37e8211c3 100644 --- a/beacon-chain/sync/validate_light_client_test.go +++ b/beacon-chain/sync/validate_light_client_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/OffchainLabs/prysm/v6/async/event" mock "github.com/OffchainLabs/prysm/v6/beacon-chain/blockchain/testing" lightClient "github.com/OffchainLabs/prysm/v6/beacon-chain/core/light-client" "github.com/OffchainLabs/prysm/v6/beacon-chain/p2p" @@ -102,7 +103,7 @@ func TestValidateLightClientOptimisticUpdate(t *testing.T) { // drift back appropriate number of epochs based on fork + 2 slots for signature slot + time for gossip propagation + any extra drift genesisDrift := v*slotsPerEpoch*secondsPerSlot + 2*secondsPerSlot + secondsPerSlot/slotIntervals + test.genesisDrift chainService := &mock.ChainService{Genesis: time.Unix(time.Now().Unix()-int64(genesisDrift), 0)} - s := &Service{cfg: &config{p2p: p, initialSync: &mockSync.Sync{}, clock: startup.NewClock(chainService.Genesis, chainService.ValidatorsRoot)}, lcStore: &lightClient.Store{}} + s := &Service{cfg: &config{p2p: p, initialSync: &mockSync.Sync{}, clock: startup.NewClock(chainService.Genesis, chainService.ValidatorsRoot)}, lcStore: lightClient.NewLightClientStore(nil, &p2ptest.FakeP2P{}, new(event.Feed))} var oldUpdate interfaces.LightClientOptimisticUpdate var err error @@ -111,7 +112,7 @@ func TestValidateLightClientOptimisticUpdate(t *testing.T) { oldUpdate, err = lightClient.NewLightClientOptimisticUpdateFromBeaconState(l.Ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock) require.NoError(t, err) - s.lcStore.SetLastOptimisticUpdate(oldUpdate) + s.lcStore.SetLastOptimisticUpdate(oldUpdate, false) } l := util.NewTestLightClient(t, v, test.newUpdateOptions...) @@ -242,7 +243,7 @@ func TestValidateLightClientFinalityUpdate(t *testing.T) { // drift back appropriate number of epochs based on fork + 2 slots for signature slot + time for gossip propagation + any extra drift genesisDrift := v*slotsPerEpoch*secondsPerSlot + 2*secondsPerSlot + secondsPerSlot/slotIntervals + test.genesisDrift chainService := &mock.ChainService{Genesis: time.Unix(time.Now().Unix()-int64(genesisDrift), 0)} - s := &Service{cfg: &config{p2p: p, initialSync: &mockSync.Sync{}, clock: startup.NewClock(chainService.Genesis, chainService.ValidatorsRoot)}, lcStore: &lightClient.Store{}} + s := &Service{cfg: &config{p2p: p, initialSync: &mockSync.Sync{}, clock: startup.NewClock(chainService.Genesis, chainService.ValidatorsRoot)}, lcStore: lightClient.NewLightClientStore(nil, &p2ptest.FakeP2P{}, new(event.Feed))} var oldUpdate interfaces.LightClientFinalityUpdate var err error @@ -251,7 +252,7 @@ func TestValidateLightClientFinalityUpdate(t *testing.T) { oldUpdate, err = lightClient.NewLightClientFinalityUpdateFromBeaconState(l.Ctx, l.State, l.Block, l.AttestedState, l.AttestedBlock, l.FinalizedBlock) require.NoError(t, err) - s.lcStore.SetLastFinalityUpdate(oldUpdate) + s.lcStore.SetLastFinalityUpdate(oldUpdate, false) } l := util.NewTestLightClient(t, v, test.newUpdateOptions...) diff --git a/changelog/bastin_move_broadcast_to_store.md b/changelog/bastin_move_broadcast_to_store.md new file mode 100644 index 0000000000..5bae68b2a9 --- /dev/null +++ b/changelog/bastin_move_broadcast_to_store.md @@ -0,0 +1,5 @@ +### Changed + +- Moved the broadcast and event notifier logic for saving LC updates to the store function. +- Fixed the issue with broadcasting more than twice per LC Finality update, and the if-case bug. +- Separated the finality update validation rules for saving and broadcasting. \ No newline at end of file diff --git a/testing/util/BUILD.bazel b/testing/util/BUILD.bazel index fa92f19f7c..e71ea7b12f 100644 --- a/testing/util/BUILD.bazel +++ b/testing/util/BUILD.bazel @@ -53,6 +53,7 @@ go_library( "//consensus-types:go_default_library", "//consensus-types/blocks:go_default_library", "//consensus-types/interfaces:go_default_library", + "//consensus-types/light-client:go_default_library", "//consensus-types/primitives:go_default_library", "//container/trie:go_default_library", "//crypto/bls:go_default_library", diff --git a/testing/util/lightclient.go b/testing/util/lightclient.go index c30c06d377..f8cd15f96d 100644 --- a/testing/util/lightclient.go +++ b/testing/util/lightclient.go @@ -10,6 +10,7 @@ import ( consensus_types "github.com/OffchainLabs/prysm/v6/consensus-types" "github.com/OffchainLabs/prysm/v6/consensus-types/blocks" "github.com/OffchainLabs/prysm/v6/consensus-types/interfaces" + lightclienttypes "github.com/OffchainLabs/prysm/v6/consensus-types/light-client" "github.com/OffchainLabs/prysm/v6/consensus-types/primitives" "github.com/OffchainLabs/prysm/v6/encoding/ssz" v11 "github.com/OffchainLabs/prysm/v6/proto/engine/v1" @@ -28,6 +29,7 @@ type TestLightClient struct { version int increaseAttestedSlotBy uint64 increaseFinalizedSlotBy uint64 + increaseSignatureSlotBy uint64 T *testing.T Ctx context.Context @@ -112,6 +114,13 @@ func WithIncreasedFinalizedSlot(increaseBy uint64) LightClientOption { } } +// WithIncreasedSignatureSlot specifies the number of slots to increase the signature slot by. This does not affect the attested/finalized block's slot. +func WithIncreasedSignatureSlot(increaseBy uint64) LightClientOption { + return func(l *TestLightClient) { + l.increaseSignatureSlotBy = increaseBy + } +} + func (l *TestLightClient) setupTestAltair() *TestLightClient { ctx := context.Background() @@ -121,6 +130,9 @@ func (l *TestLightClient) setupTestAltair() *TestLightClient { } signatureSlot := attestedSlot.Add(1) + if l.increaseSignatureSlotBy > 0 { + signatureSlot = signatureSlot.Add(l.increaseSignatureSlotBy) + } // Attested State attestedState, err := NewBeaconStateAltair() @@ -232,6 +244,9 @@ func (l *TestLightClient) setupTestBellatrix() *TestLightClient { } signatureSlot := attestedSlot.Add(1) + if l.increaseSignatureSlotBy > 0 { + signatureSlot = signatureSlot.Add(l.increaseSignatureSlotBy) + } // Attested State & Block attestedState, err := NewBeaconStateBellatrix() @@ -404,6 +419,9 @@ func (l *TestLightClient) setupTestCapella() *TestLightClient { } signatureSlot := attestedSlot.Add(1) + if l.increaseSignatureSlotBy > 0 { + signatureSlot = signatureSlot.Add(l.increaseSignatureSlotBy) + } // Attested State attestedState, err := NewBeaconStateCapella() @@ -577,6 +595,9 @@ func (l *TestLightClient) setupTestDeneb() *TestLightClient { } signatureSlot := attestedSlot.Add(1) + if l.increaseSignatureSlotBy > 0 { + signatureSlot = signatureSlot.Add(l.increaseSignatureSlotBy) + } // Attested State attestedState, err := NewBeaconStateDeneb() @@ -751,6 +772,9 @@ func (l *TestLightClient) setupTestElectra() *TestLightClient { } signatureSlot := attestedSlot.Add(1) + if l.increaseSignatureSlotBy > 0 { + signatureSlot = signatureSlot.Add(l.increaseSignatureSlotBy) + } // Attested State & Block attestedState, err := NewBeaconStateElectra() @@ -1044,3 +1068,55 @@ func (l *TestLightClient) CheckSyncAggregate(sa *ethpb.SyncAggregate) { require.DeepSSZEqual(l.T, syncAggregate.SyncCommitteeBits, sa.SyncCommitteeBits, "SyncAggregate bits is not equal") require.DeepSSZEqual(l.T, syncAggregate.SyncCommitteeSignature, sa.SyncCommitteeSignature, "SyncAggregate signature is not equal") } + +func MockOptimisticUpdate() (interfaces.LightClientOptimisticUpdate, error) { + pbUpdate := ðpb.LightClientOptimisticUpdateAltair{ + AttestedHeader: ðpb.LightClientHeaderAltair{ + Beacon: ðpb.BeaconBlockHeader{ + Slot: primitives.Slot(32), + ParentRoot: make([]byte, 32), + StateRoot: make([]byte, 32), + BodyRoot: make([]byte, 32), + }, + }, + SyncAggregate: ðpb.SyncAggregate{ + SyncCommitteeBits: make([]byte, 64), + SyncCommitteeSignature: make([]byte, 96), + }, + SignatureSlot: primitives.Slot(33), + } + return lightclienttypes.NewWrappedOptimisticUpdateAltair(pbUpdate) +} + +func MockFinalityUpdate() (interfaces.LightClientFinalityUpdate, error) { + finalityBranch := make([][]byte, fieldparams.FinalityBranchDepth) + for i := 0; i < len(finalityBranch); i++ { + finalityBranch[i] = make([]byte, 32) + } + + pbUpdate := ðpb.LightClientFinalityUpdateAltair{ + FinalizedHeader: ðpb.LightClientHeaderAltair{ + Beacon: ðpb.BeaconBlockHeader{ + Slot: primitives.Slot(31), + ParentRoot: make([]byte, 32), + StateRoot: make([]byte, 32), + BodyRoot: make([]byte, 32), + }, + }, + FinalityBranch: finalityBranch, + AttestedHeader: ðpb.LightClientHeaderAltair{ + Beacon: ðpb.BeaconBlockHeader{ + Slot: primitives.Slot(32), + ParentRoot: make([]byte, 32), + StateRoot: make([]byte, 32), + BodyRoot: make([]byte, 32), + }, + }, + SyncAggregate: ðpb.SyncAggregate{ + SyncCommitteeBits: make([]byte, 64), + SyncCommitteeSignature: make([]byte, 96), + }, + SignatureSlot: primitives.Slot(33), + } + return lightclienttypes.NewWrappedFinalityUpdateAltair(pbUpdate) +}