Don't use MaxCover for Electra on-chain aggregates (#14925)

* Don't use MaxCover for Electra on-chain aggregates * changelog <3
prysmaticlabs · Feb 18, 2025 · 961d8e1 · 961d8e1
1 parent d396a99
commit 961d8e1
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 13 deletions.
diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go
@@ -2,8 +2,10 @@ package validator
 
 import (
 	"bytes"
+	"cmp"
 	"context"
 	"fmt"
+	"slices"
 	"sort"
 
 	"github.com/pkg/errors"
@@ -277,7 +279,14 @@ func (a proposerAtts) sortOnChainAggregates() (proposerAtts, error) {
 		return a, nil
 	}
 
-	return a.sortByProfitabilityUsingMaxCover()
+	// Sort by slot first, then by bit count.
+	slices.SortFunc(a, func(a, b ethpb.Att) int {
+		return cmp.Or(
+			-cmp.Compare(a.GetData().Slot, b.GetData().Slot),
+			-cmp.Compare(a.GetAggregationBits().Count(), b.GetAggregationBits().Count()))
+	})
+
+	return a, nil
 }
 
 // Separate attestations by slot, as slot number takes higher precedence when sorting.

diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go
@@ -794,17 +794,19 @@ func Test_packAttestations_ElectraOnChainAggregates(t *testing.T) {
 
 	require.NoError(t, st.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))
 
-	atts, err := s.packAttestations(ctx, st, params.BeaconConfig().SlotsPerEpoch)
-	require.NoError(t, err)
-	require.Equal(t, 6, len(atts))
-	assert.Equal(t, true,
-		atts[0].GetAggregationBits().Count() >= atts[1].GetAggregationBits().Count() &&
-			atts[1].GetAggregationBits().Count() >= atts[2].GetAggregationBits().Count() &&
-			atts[2].GetAggregationBits().Count() >= atts[3].GetAggregationBits().Count() &&
-			atts[3].GetAggregationBits().Count() >= atts[4].GetAggregationBits().Count() &&
-			atts[4].GetAggregationBits().Count() >= atts[5].GetAggregationBits().Count(),
-		"on-chain aggregates are not sorted by aggregation bit count",
-	)
+	t.Run("ok", func(t *testing.T) {
+		atts, err := s.packAttestations(ctx, st, params.BeaconConfig().SlotsPerEpoch)
+		require.NoError(t, err)
+		require.Equal(t, 6, len(atts))
+		assert.Equal(t, true,
+			atts[0].GetAggregationBits().Count() >= atts[1].GetAggregationBits().Count() &&
+				atts[1].GetAggregationBits().Count() >= atts[2].GetAggregationBits().Count() &&
+				atts[2].GetAggregationBits().Count() >= atts[3].GetAggregationBits().Count() &&
+				atts[3].GetAggregationBits().Count() >= atts[4].GetAggregationBits().Count() &&
+				atts[4].GetAggregationBits().Count() >= atts[5].GetAggregationBits().Count(),
+			"on-chain aggregates are not sorted by aggregation bit count",
+		)
+	})
 
 	t.Run("slot takes precedence", func(t *testing.T) {
 		moreRecentAtt := &ethpb.AttestationElectra{
@@ -814,7 +816,7 @@ func Test_packAttestations_ElectraOnChainAggregates(t *testing.T) {
 			Signature:       sig.Marshal(),
 		}
 		require.NoError(t, pool.SaveUnaggregatedAttestations([]ethpb.Att{moreRecentAtt}))
-		atts, err = s.packAttestations(ctx, st, params.BeaconConfig().SlotsPerEpoch)
+		atts, err := s.packAttestations(ctx, st, params.BeaconConfig().SlotsPerEpoch)
 		require.NoError(t, err)
 		require.Equal(t, 7, len(atts))
 		assert.Equal(t, true, atts[0].GetData().Slot == 1)

diff --git a/changelog/radek_no-maxcover-for-electra.md b/changelog/radek_no-maxcover-for-electra.md
@@ -0,0 +1,3 @@
+### Changed
+
+- Don't use MaxCover for Electra on-chain attestations.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		### Changed

		- Don't use MaxCover for Electra on-chain attestations.