Remove error from signatures of UnaggregatedAttestations and pruneAttsFromPool (#15028)

rkapka · rkapka · commit b0d6329b7a64 · 2025-03-19T11:11:37.000+01:00
diff --git a/beacon-chain/blockchain/forkchoice_update_execution.go b/beacon-chain/blockchain/forkchoice_update_execution.go
@@ -105,9 +105,7 @@ func (s *Service) forkchoiceUpdateWithExecution(ctx context.Context, args *fcuCo
 	go firePayloadAttributesEvent(ctx, s.cfg.StateNotifier.StateFeed(), s.CurrentSlot()+1)
 
 	// Only need to prune attestations from pool if the head has changed.
-	if err := s.pruneAttsFromPool(s.ctx, args.headState, args.headBlock); err != nil {
-		log.WithError(err).Error("could not prune attestations from pool")
-	}
+	s.pruneAttsFromPool(s.ctx, args.headState, args.headBlock)
 	return nil
 }
 
diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go
@@ -423,13 +423,12 @@ func (s *Service) savePostStateInfo(ctx context.Context, r [32]byte, b interface
 
 // pruneAttsFromPool removes these attestations from the attestation pool
 // which are covered by attestations from the received block.
-func (s *Service) pruneAttsFromPool(ctx context.Context, headState state.BeaconState, headBlock interfaces.ReadOnlySignedBeaconBlock) error {
+func (s *Service) pruneAttsFromPool(ctx context.Context, headState state.BeaconState, headBlock interfaces.ReadOnlySignedBeaconBlock) {
 	for _, att := range headBlock.Block().Body().Attestations() {
 		if err := s.pruneCoveredAttsFromPool(ctx, headState, att); err != nil {
 			log.WithError(err).Warn("Could not prune attestations covered by a received block's attestation")
 		}
 	}
-	return nil
 }
 
 func (s *Service) pruneCoveredAttsFromPool(ctx context.Context, headState state.BeaconState, att ethpb.Att) error {
diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go
@@ -126,11 +126,10 @@ func Test_pruneAttsFromPool_Electra(t *testing.T) {
 	// into the correct number of aggregates.
 	require.Equal(t, 4, len(committees))
 
-	require.NoError(t, s.pruneAttsFromPool(ctx, st, rob))
+	s.pruneAttsFromPool(ctx, st, rob)
 	require.LogsDoNotContain(t, logHook, "Could not prune attestations")
 
-	attsInPool, err := s.cfg.AttPool.UnaggregatedAttestations()
-	require.NoError(t, err)
+	attsInPool := s.cfg.AttPool.UnaggregatedAttestations()
 	assert.Equal(t, 0, len(attsInPool))
 	attsInPool = s.cfg.AttPool.AggregatedAttestations()
 	require.Equal(t, 1, len(attsInPool))
@@ -934,7 +933,7 @@ func TestRemoveBlockAttestationsInPool(t *testing.T) {
 	require.NoError(t, service.cfg.AttPool.SaveAggregatedAttestations(atts))
 	wsb, err := consensusblocks.NewSignedBeaconBlock(b)
 	require.NoError(t, err)
-	require.NoError(t, service.pruneAttsFromPool(context.Background(), nil /* state not needed pre-Electra */, wsb))
+	service.pruneAttsFromPool(context.Background(), nil /* state not needed pre-Electra */, wsb)
 	require.LogsDoNotContain(t, logHook, "Could not prune attestations")
 	require.Equal(t, 0, service.cfg.AttPool.AggregatedAttestationCount())
 }
diff --git a/beacon-chain/operations/attestations/kv/aggregated.go b/beacon-chain/operations/attestations/kv/aggregated.go
@@ -23,10 +23,7 @@ import (
 func (c *AttCaches) AggregateUnaggregatedAttestations(ctx context.Context) error {
 	ctx, span := trace.StartSpan(ctx, "operations.attestations.kv.AggregateUnaggregatedAttestations")
 	defer span.End()
-	unaggregatedAtts, err := c.UnaggregatedAttestations()
-	if err != nil {
-		return err
-	}
+	unaggregatedAtts := c.UnaggregatedAttestations()
 	return c.aggregateUnaggregatedAtts(ctx, unaggregatedAtts)
 }
 
diff --git a/beacon-chain/operations/attestations/kv/unaggregated.go b/beacon-chain/operations/attestations/kv/unaggregated.go
@@ -53,7 +53,7 @@ func (c *AttCaches) SaveUnaggregatedAttestations(atts []ethpb.Att) error {
 }
 
 // UnaggregatedAttestations returns all the unaggregated attestations in cache.
-func (c *AttCaches) UnaggregatedAttestations() ([]ethpb.Att, error) {
+func (c *AttCaches) UnaggregatedAttestations() []ethpb.Att {
 	c.unAggregateAttLock.RLock()
 	defer c.unAggregateAttLock.RUnlock()
 	unAggregatedAtts := c.unAggregatedAtt
@@ -68,7 +68,7 @@ func (c *AttCaches) UnaggregatedAttestations() ([]ethpb.Att, error) {
 			atts = append(atts, att.Clone())
 		}
 	}
-	return atts, nil
+	return atts
 }
 
 // UnaggregatedAttestationsBySlotIndex returns the unaggregated attestations in cache,
diff --git a/beacon-chain/operations/attestations/kv/unaggregated_test.go b/beacon-chain/operations/attestations/kv/unaggregated_test.go
@@ -29,8 +29,7 @@ func TestKV_Unaggregated_UnaggregatedAttestations(t *testing.T) {
 		// cache a bitlist whose length is different from the attestation bitlist's length
 		cache.seenAtt.Set(id.String(), []bitfield.Bitlist{{0b1001}}, c.DefaultExpiration)
 
-		atts, err := cache.UnaggregatedAttestations()
-		require.NoError(t, err)
+		atts := cache.UnaggregatedAttestations()
 		assert.Equal(t, 0, len(atts))
 	})
 }
@@ -169,8 +168,7 @@ func TestKV_Unaggregated_DeleteUnaggregatedAttestation(t *testing.T) {
 		for _, att := range atts {
 			assert.NoError(t, cache.DeleteUnaggregatedAttestation(att))
 		}
-		returned, err := cache.UnaggregatedAttestations()
-		require.NoError(t, err)
+		returned := cache.UnaggregatedAttestations()
 		assert.DeepEqual(t, []ethpb.Att{}, returned)
 	})
 
@@ -234,11 +232,10 @@ func TestKV_Unaggregated_DeleteSeenUnaggregatedAttestations(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, 1, count)
 		assert.Equal(t, 2, cache.UnaggregatedAttestationCount())
-		returned, err := cache.UnaggregatedAttestations()
+		returned := cache.UnaggregatedAttestations()
 		sort.Slice(returned, func(i, j int) bool {
 			return bytes.Compare(returned[i].GetAggregationBits(), returned[j].GetAggregationBits()) < 0
 		})
-		require.NoError(t, err)
 		assert.DeepEqual(t, []ethpb.Att{atts[0], atts[2]}, returned)
 	})
 
@@ -261,8 +258,7 @@ func TestKV_Unaggregated_DeleteSeenUnaggregatedAttestations(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, 3, count)
 		assert.Equal(t, 0, cache.UnaggregatedAttestationCount())
-		returned, err := cache.UnaggregatedAttestations()
-		require.NoError(t, err)
+		returned := cache.UnaggregatedAttestations()
 		assert.DeepEqual(t, []ethpb.Att{}, returned)
 	})
 
diff --git a/beacon-chain/operations/attestations/mock/mock.go b/beacon-chain/operations/attestations/mock/mock.go
@@ -79,8 +79,8 @@ func (m *PoolMock) SaveUnaggregatedAttestations(atts []ethpb.Att) error {
 }
 
 // UnaggregatedAttestations --
-func (m *PoolMock) UnaggregatedAttestations() ([]ethpb.Att, error) {
-	return m.UnaggregatedAtts, nil
+func (m *PoolMock) UnaggregatedAttestations() []ethpb.Att {
+	return m.UnaggregatedAtts
 }
 
 // UnaggregatedAttestationsBySlotIndex --
diff --git a/beacon-chain/operations/attestations/pool.go b/beacon-chain/operations/attestations/pool.go
@@ -26,7 +26,7 @@ type Pool interface {
 	// For unaggregated attestations.
 	SaveUnaggregatedAttestation(att ethpb.Att) error
 	SaveUnaggregatedAttestations(atts []ethpb.Att) error
-	UnaggregatedAttestations() ([]ethpb.Att, error)
+	UnaggregatedAttestations() []ethpb.Att
 	UnaggregatedAttestationsBySlotIndex(ctx context.Context, slot primitives.Slot, committeeIndex primitives.CommitteeIndex) []*ethpb.Attestation
 	UnaggregatedAttestationsBySlotIndexElectra(ctx context.Context, slot primitives.Slot, committeeIndex primitives.CommitteeIndex) []*ethpb.AttestationElectra
 	DeleteUnaggregatedAttestation(att ethpb.Att) error
diff --git a/beacon-chain/operations/attestations/prune_expired.go b/beacon-chain/operations/attestations/prune_expired.go
@@ -61,12 +61,8 @@ func (s *Service) pruneExpiredAtts() {
 	if _, err := s.cfg.Pool.DeleteSeenUnaggregatedAttestations(); err != nil {
 		log.WithError(err).Error("Cannot delete seen attestations")
 	}
-	unAggregatedAtts, err := s.cfg.Pool.UnaggregatedAttestations()
-	if err != nil {
-		log.WithError(err).Error("Could not get unaggregated attestations")
-		return
-	}
-	for _, att := range unAggregatedAtts {
+
+	for _, att := range s.cfg.Pool.UnaggregatedAttestations() {
 		if s.expired(att.GetData().Slot) {
 			if err := s.cfg.Pool.DeleteUnaggregatedAttestation(att); err != nil {
 				log.WithError(err).Error("Could not delete expired unaggregated attestation")
diff --git a/beacon-chain/operations/attestations/prune_expired_test.go b/beacon-chain/operations/attestations/prune_expired_test.go
@@ -54,9 +54,7 @@ func TestPruneExpired_Ticker(t *testing.T) {
 
 	done := make(chan struct{}, 1)
 	async.RunEvery(ctx, 500*time.Millisecond, func() {
-		atts, err := s.cfg.Pool.UnaggregatedAttestations()
-		require.NoError(t, err)
-		for _, attestation := range atts {
+		for _, attestation := range s.cfg.Pool.UnaggregatedAttestations() {
 			if attestation.GetData().Slot == 0 {
 				return
 			}
diff --git a/beacon-chain/rpc/eth/beacon/handlers_pool.go b/beacon-chain/rpc/eth/beacon/handlers_pool.go
@@ -55,11 +55,7 @@ func (s *Server) ListAttestations(w http.ResponseWriter, r *http.Request) {
 		attestations = s.AttestationCache.GetAll()
 	} else {
 		attestations = s.AttestationsPool.AggregatedAttestations()
-		unaggAtts, err := s.AttestationsPool.UnaggregatedAttestations()
-		if err != nil {
-			httputil.HandleError(w, "Could not get unaggregated attestations: "+err.Error(), http.StatusInternalServerError)
-			return
-		}
+		unaggAtts := s.AttestationsPool.UnaggregatedAttestations()
 		attestations = append(attestations, unaggAtts...)
 	}
 
@@ -114,11 +110,7 @@ func (s *Server) ListAttestationsV2(w http.ResponseWriter, r *http.Request) {
 		attestations = s.AttestationCache.GetAll()
 	} else {
 		attestations = s.AttestationsPool.AggregatedAttestations()
-		unaggAtts, err := s.AttestationsPool.UnaggregatedAttestations()
-		if err != nil {
-			httputil.HandleError(w, "Could not get unaggregated attestations: "+err.Error(), http.StatusInternalServerError)
-			return
-		}
+		unaggAtts := s.AttestationsPool.UnaggregatedAttestations()
 		attestations = append(attestations, unaggAtts...)
 	}
 
diff --git a/beacon-chain/rpc/eth/validator/handlers.go b/beacon-chain/rpc/eth/validator/handlers.go
@@ -162,11 +162,7 @@ func (s *Server) aggregatedAttestation(w http.ResponseWriter, slot primitives.Sl
 		return nil
 	}
 
-	atts, err := s.AttestationsPool.UnaggregatedAttestations()
-	if err != nil {
-		httputil.HandleError(w, "Could not get unaggregated attestations: "+err.Error(), http.StatusInternalServerError)
-		return nil
-	}
+	atts := s.AttestationsPool.UnaggregatedAttestations()
 	match, err = matchingAtts(atts, slot, attDataRoot, index)
 	if err != nil {
 		httputil.HandleError(w, "Could not get matching attestations: "+err.Error(), http.StatusInternalServerError)
diff --git a/beacon-chain/rpc/eth/validator/handlers_test.go b/beacon-chain/rpc/eth/validator/handlers_test.go
@@ -118,8 +118,7 @@ func TestGetAggregateAttestation(t *testing.T) {
 
 		pool := attestations.NewPool()
 		require.NoError(t, pool.SaveUnaggregatedAttestations([]ethpbalpha.Att{unaggSlot3_Root1_1, unaggSlot3_Root1_2, unaggSlot3_Root2, unaggSlot4}), "Failed to save unaggregated attestations")
-		unagg, err := pool.UnaggregatedAttestations()
-		require.NoError(t, err)
+		unagg := pool.UnaggregatedAttestations()
 		require.Equal(t, 4, len(unagg), "Expected 4 unaggregated attestations")
 		require.NoError(t, pool.SaveAggregatedAttestations([]ethpbalpha.Att{aggSlot1_Root1_1, aggSlot1_Root1_2, aggSlot1_Root2, aggSlot2}), "Failed to save aggregated attestations")
 		agg := pool.AggregatedAttestations()
@@ -268,8 +267,7 @@ func TestGetAggregateAttestation(t *testing.T) {
 
 			pool := attestations.NewPool()
 			require.NoError(t, pool.SaveUnaggregatedAttestations([]ethpbalpha.Att{unaggSlot3_Root1_1, unaggSlot3_Root1_2, unaggSlot3_Root2, unaggSlot4}), "Failed to save unaggregated attestations")
-			unagg, err := pool.UnaggregatedAttestations()
-			require.NoError(t, err)
+			unagg := pool.UnaggregatedAttestations()
 			require.Equal(t, 4, len(unagg), "Expected 4 unaggregated attestations")
 			require.NoError(t, pool.SaveAggregatedAttestations([]ethpbalpha.Att{aggSlot1_Root1_1, aggSlot1_Root1_2, aggSlot1_Root2, aggSlot2, postElectraAtt}), "Failed to save aggregated attestations")
 			agg := pool.AggregatedAttestations()
@@ -373,8 +371,7 @@ func TestGetAggregateAttestation(t *testing.T) {
 
 			pool := attestations.NewPool()
 			require.NoError(t, pool.SaveUnaggregatedAttestations([]ethpbalpha.Att{unaggSlot3_Root1_1, unaggSlot3_Root1_2, unaggSlot3_Root2, unaggSlot4}), "Failed to save unaggregated attestations")
-			unagg, err := pool.UnaggregatedAttestations()
-			require.NoError(t, err)
+			unagg := pool.UnaggregatedAttestations()
 			require.Equal(t, 4, len(unagg), "Expected 4 unaggregated attestations")
 			require.NoError(t, pool.SaveAggregatedAttestations([]ethpbalpha.Att{aggSlot1_Root1_1, aggSlot1_Root1_2, aggSlot1_Root2, aggSlot2, preElectraAtt}), "Failed to save aggregated attestations")
 			agg := pool.AggregatedAttestations()
diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go
@@ -40,10 +40,7 @@ func (vs *Server) packAttestations(ctx context.Context, latestState state.Beacon
 		atts = vs.AttPool.AggregatedAttestations()
 		atts = vs.validateAndDeleteAttsInPool(ctx, latestState, atts)
 
-		uAtts, err := vs.AttPool.UnaggregatedAttestations()
-		if err != nil {
-			return nil, errors.Wrap(err, "could not get unaggregated attestations")
-		}
+		uAtts := vs.AttPool.UnaggregatedAttestations()
 		uAtts = vs.validateAndDeleteAttsInPool(ctx, latestState, uAtts)
 		atts = append(atts, uAtts...)
 	}
diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go
@@ -2949,8 +2949,7 @@ func TestProposer_DeleteAttsInPool_Aggregated(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, s.deleteAttsInPool(context.Background(), append(aa, unaggregatedAtts...)))
 	assert.Equal(t, 0, len(s.AttPool.AggregatedAttestations()), "Did not delete aggregated attestation")
-	atts, err := s.AttPool.UnaggregatedAttestations()
-	require.NoError(t, err)
+	atts := s.AttPool.UnaggregatedAttestations()
 	assert.Equal(t, 0, len(atts), "Did not delete unaggregated attestation")
 }
 
diff --git a/beacon-chain/sync/pending_attestations_queue_test.go b/beacon-chain/sync/pending_attestations_queue_test.go
@@ -153,8 +153,7 @@ func TestProcessPendingAtts_HasBlockSaveUnAggregatedAtt(t *testing.T) {
 			}
 		}
 	}()
-	atts, err := r.cfg.attPool.UnaggregatedAttestations()
-	require.NoError(t, err)
+	atts := r.cfg.attPool.UnaggregatedAttestations()
 	assert.Equal(t, 1, len(atts), "Did not save unaggregated att")
 	assert.DeepEqual(t, att, atts[0], "Incorrect saved att")
 	assert.Equal(t, 0, len(r.cfg.attPool.AggregatedAttestations()), "Did save aggregated att")
@@ -248,8 +247,7 @@ func TestProcessPendingAtts_HasBlockSaveUnAggregatedAttElectra(t *testing.T) {
 			}
 		}
 	}()
-	atts, err := r.cfg.attPool.UnaggregatedAttestations()
-	require.NoError(t, err)
+	atts := r.cfg.attPool.UnaggregatedAttestations()
 	require.Equal(t, 1, len(atts), "Did not save unaggregated att")
 	assert.DeepEqual(t, att.ToAttestationElectra(committee), atts[0], "Incorrect saved att")
 	assert.Equal(t, 0, len(r.cfg.attPool.AggregatedAttestations()), "Did save aggregated att")
@@ -457,8 +455,7 @@ func TestProcessPendingAtts_HasBlockSaveAggregatedAtt(t *testing.T) {
 
 	assert.Equal(t, 1, len(r.cfg.attPool.AggregatedAttestations()), "Did not save aggregated att")
 	assert.DeepEqual(t, att, r.cfg.attPool.AggregatedAttestations()[0], "Incorrect saved att")
-	atts, err := r.cfg.attPool.UnaggregatedAttestations()
-	require.NoError(t, err)
+	atts := r.cfg.attPool.UnaggregatedAttestations()
 	assert.Equal(t, 0, len(atts), "Did save aggregated att")
 	require.LogsContain(t, hook, "Verified and saved pending attestations to pool")
 	cancel()
diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof_test.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof_test.go
@@ -57,7 +57,6 @@ func TestBeaconAggregateProofSubscriber_CanSaveUnaggregatedAttestation(t *testin
 	}
 	require.NoError(t, r.beaconAggregateProofSubscriber(context.Background(), a))
 
-	atts, err := r.cfg.attPool.UnaggregatedAttestations()
-	require.NoError(t, err)
+	atts := r.cfg.attPool.UnaggregatedAttestations()
 	assert.DeepEqual(t, []ethpb.Att{a.Message.Aggregate}, atts, "Did not save unaggregated attestation")
 }

Original file line number	Diff line number	Diff line change
`@@ -105,9 +105,7 @@ func (s Service) forkchoiceUpdateWithExecution(ctx context.Context, args fcuCo`
`105`	`105`	`go firePayloadAttributesEvent(ctx, s.cfg.StateNotifier.StateFeed(), s.CurrentSlot()+1)`
`106`	`106`
`107`	`107`	`// Only need to prune attestations from pool if the head has changed.`
`108`		`- if err := s.pruneAttsFromPool(s.ctx, args.headState, args.headBlock); err != nil {`
`109`		`- log.WithError(err).Error("could not prune attestations from pool")`
`110`		`- }`
	`108`	`+ s.pruneAttsFromPool(s.ctx, args.headState, args.headBlock)`
`111`	`109`	`return nil`
`112`	`110`	`}`
`113`	`111`
Original file line number	Diff line number	Diff line change
`@@ -423,13 +423,12 @@ func (s *Service) savePostStateInfo(ctx context.Context, r [32]byte, b interface`
`423`	`423`
`424`	`424`	`// pruneAttsFromPool removes these attestations from the attestation pool`
`425`	`425`	`// which are covered by attestations from the received block.`
`426`		`-func (s *Service) pruneAttsFromPool(ctx context.Context, headState state.BeaconState, headBlock interfaces.ReadOnlySignedBeaconBlock) error {`
	`426`	`+func (s *Service) pruneAttsFromPool(ctx context.Context, headState state.BeaconState, headBlock interfaces.ReadOnlySignedBeaconBlock) {`
`427`	`427`	`for _, att := range headBlock.Block().Body().Attestations() {`
`428`	`428`	`if err := s.pruneCoveredAttsFromPool(ctx, headState, att); err != nil {`
`429`	`429`	`log.WithError(err).Warn("Could not prune attestations covered by a received block's attestation")`
`430`	`430`	`}`
`431`	`431`	`}`
`432`		`- return nil`
`433`	`432`	`}`
`434`	`433`
`435`	`434`	`func (s *Service) pruneCoveredAttsFromPool(ctx context.Context, headState state.BeaconState, att ethpb.Att) error {`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ func (c *AttCaches) SaveUnaggregatedAttestations(atts []ethpb.Att) error {`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`// UnaggregatedAttestations returns all the unaggregated attestations in cache.`
`56`		`-func (c *AttCaches) UnaggregatedAttestations() ([]ethpb.Att, error) {`
	`56`	`+func (c *AttCaches) UnaggregatedAttestations() []ethpb.Att {`
`57`	`57`	`c.unAggregateAttLock.RLock()`
`58`	`58`	`defer c.unAggregateAttLock.RUnlock()`
`59`	`59`	`unAggregatedAtts := c.unAggregatedAtt`
`@@ -68,7 +68,7 @@ func (c *AttCaches) UnaggregatedAttestations() ([]ethpb.Att, error) {`
`68`	`68`	`atts = append(atts, att.Clone())`
`69`	`69`	`}`
`70`	`70`	`}`
`71`		`- return atts, nil`
	`71`	`+ return atts`
`72`	`72`	`}`
`73`	`73`
`74`	`74`	`// UnaggregatedAttestationsBySlotIndex returns the unaggregated attestations in cache,`
Original file line number	Diff line number	Diff line change
`@@ -79,8 +79,8 @@ func (m *PoolMock) SaveUnaggregatedAttestations(atts []ethpb.Att) error {`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`// UnaggregatedAttestations --`
`82`		`-func (m *PoolMock) UnaggregatedAttestations() ([]ethpb.Att, error) {`
`83`		`- return m.UnaggregatedAtts, nil`
	`82`	`+func (m *PoolMock) UnaggregatedAttestations() []ethpb.Att {`
	`83`	`+ return m.UnaggregatedAtts`
`84`	`84`	`}`
`85`	`85`
`86`	`86`	`// UnaggregatedAttestationsBySlotIndex --`