Remove disable-committee-aware-packing flag

Author: rkapka

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go

@@ -188,73 +188,6 @@ func (a proposerAtts) filter(ctx context.Context, st state.BeaconState) (propose
 	return validAtts, invalidAtts
 }
 
-// sortByProfitabilityUsingMaxCover orders attestations by highest slot and by highest aggregation bit count.
-// Duplicate bits are counted only once, using max-cover algorithm.
-func (a proposerAtts) sortByProfitabilityUsingMaxCover() (proposerAtts, error) {
-	// Separate attestations by slot, as slot number takes higher precedence when sorting.
-	var slots []primitives.Slot
-	attsBySlot := map[primitives.Slot]proposerAtts{}
-	for _, att := range a {
-		if _, ok := attsBySlot[att.GetData().Slot]; !ok {
-			slots = append(slots, att.GetData().Slot)
-		}
-		attsBySlot[att.GetData().Slot] = append(attsBySlot[att.GetData().Slot], att)
-	}
-
-	selectAtts := func(atts proposerAtts) (proposerAtts, error) {
-		if len(atts) < 2 {
-			return atts, nil
-		}
-		candidates := make([]*bitfield.Bitlist64, len(atts))
-		for i := 0; i < len(atts); i++ {
-			var err error
-			candidates[i], err = atts[i].GetAggregationBits().ToBitlist64()
-			if err != nil {
-				return nil, err
-			}
-		}
-		// Add selected candidates on top, those that are not selected - append at bottom.
-		selectedKeys, _, err := aggregation.MaxCover(candidates, len(candidates), true /* allowOverlaps */)
-		if err == nil {
-			// Pick selected attestations first, leftover attestations will be appended at the end.
-			// Both lists will be sorted by number of bits set.
-			selectedAtts := make(proposerAtts, selectedKeys.Count())
-			leftoverAtts := make(proposerAtts, selectedKeys.Not().Count())
-			for i, key := range selectedKeys.BitIndices() {
-				selectedAtts[i] = atts[key]
-			}
-			for i, key := range selectedKeys.Not().BitIndices() {
-				leftoverAtts[i] = atts[key]
-			}
-			sort.Slice(selectedAtts, func(i, j int) bool {
-				return selectedAtts[i].GetAggregationBits().Count() > selectedAtts[j].GetAggregationBits().Count()
-			})
-			sort.Slice(leftoverAtts, func(i, j int) bool {
-				return leftoverAtts[i].GetAggregationBits().Count() > leftoverAtts[j].GetAggregationBits().Count()
-			})
-			return append(selectedAtts, leftoverAtts...), nil
-		}
-		return atts, nil
-	}
-
-	// Select attestations. Slots are sorted from higher to lower at this point. Within slots attestations
-	// are sorted to maximize profitability (greedily selected, with previous attestations' bits
-	// evaluated before including any new attestation).
-	var sortedAtts proposerAtts
-	sort.Slice(slots, func(i, j int) bool {
-		return slots[i] > slots[j]
-	})
-	for _, slot := range slots {
-		selected, err := selectAtts(attsBySlot[slot])
-		if err != nil {
-			return nil, err
-		}
-		sortedAtts = append(sortedAtts, selected...)
-	}
-
-	return sortedAtts, nil
-}
-
 // sort attestations as follows:
 //
 //   - all attestations selected by max-cover are taken, leftover attestations are discarded
@@ -266,10 +199,6 @@ func (a proposerAtts) sort() (proposerAtts, error) {
 	if len(a) < 2 {
 		return a, nil
 	}
-
-	if features.Get().DisableCommitteeAwarePacking {
-		return a.sortByProfitabilityUsingMaxCover()
-	}
 	return a.sortBySlotAndCommittee()
 }
 

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/v5/beacon-chain/operations/attestations"
 	"github.com/prysmaticlabs/prysm/v5/beacon-chain/operations/attestations/mock"
-	"github.com/prysmaticlabs/prysm/v5/config/features"
 	"github.com/prysmaticlabs/prysm/v5/config/params"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
 	"github.com/prysmaticlabs/prysm/v5/crypto/bls/blst"
@@ -26,162 +25,6 @@ import (
 	"github.com/prysmaticlabs/prysm/v5/time/slots"
 )
 
-func TestProposer_ProposerAtts_sort(t *testing.T) {
-	feat := features.Get()
-	feat.DisableCommitteeAwarePacking = true
-	reset := features.InitWithReset(feat)
-	defer reset()
-
-	type testData struct {
-		slot primitives.Slot
-		bits bitfield.Bitlist
-	}
-	getAtts := func(data []testData) proposerAtts {
-		var atts proposerAtts
-		for _, att := range data {
-			atts = append(atts, util.HydrateAttestation(&ethpb.Attestation{
-				Data: &ethpb.AttestationData{Slot: att.slot}, AggregationBits: att.bits}))
-		}
-		return atts
-	}
-
-	t.Run("no atts", func(t *testing.T) {
-		atts := getAtts([]testData{})
-		want := getAtts([]testData{})
-		atts, err := atts.sort()
-		if err != nil {
-			t.Error(err)
-		}
-		require.DeepEqual(t, want, atts)
-	})
-
-	t.Run("single att", func(t *testing.T) {
-		atts := getAtts([]testData{
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-		})
-		want := getAtts([]testData{
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-		})
-		atts, err := atts.sort()
-		if err != nil {
-			t.Error(err)
-		}
-		require.DeepEqual(t, want, atts)
-	})
-
-	t.Run("single att per slot", func(t *testing.T) {
-		atts := getAtts([]testData{
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-		})
-		want := getAtts([]testData{
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-		})
-		atts, err := atts.sort()
-		if err != nil {
-			t.Error(err)
-		}
-		require.DeepEqual(t, want, atts)
-	})
-
-	t.Run("two atts on one of the slots", func(t *testing.T) {
-		atts := getAtts([]testData{
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-			{4, bitfield.Bitlist{0b11110000, 0b1}},
-		})
-		want := getAtts([]testData{
-			{4, bitfield.Bitlist{0b11110000, 0b1}},
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-		})
-		atts, err := atts.sort()
-		if err != nil {
-			t.Error(err)
-		}
-		require.DeepEqual(t, want, atts)
-	})
-
-	t.Run("compare to native sort", func(t *testing.T) {
-		// The max-cover based approach will select 0b00001100 instead, despite lower bit count
-		// (since it has two new/unknown bits).
-		t.Run("max-cover", func(t *testing.T) {
-			atts := getAtts([]testData{
-				{1, bitfield.Bitlist{0b11000011, 0b1}},
-				{1, bitfield.Bitlist{0b11001000, 0b1}},
-				{1, bitfield.Bitlist{0b00001100, 0b1}},
-			})
-			want := getAtts([]testData{
-				{1, bitfield.Bitlist{0b11000011, 0b1}},
-				{1, bitfield.Bitlist{0b00001100, 0b1}},
-				{1, bitfield.Bitlist{0b11001000, 0b1}},
-			})
-			atts, err := atts.sort()
-			if err != nil {
-				t.Error(err)
-			}
-			require.DeepEqual(t, want, atts)
-		})
-	})
-
-	t.Run("multiple slots", func(t *testing.T) {
-		atts := getAtts([]testData{
-			{2, bitfield.Bitlist{0b11100000, 0b1}},
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-			{4, bitfield.Bitlist{0b11110000, 0b1}},
-			{1, bitfield.Bitlist{0b11100000, 0b1}},
-			{3, bitfield.Bitlist{0b11000000, 0b1}},
-		})
-		want := getAtts([]testData{
-			{4, bitfield.Bitlist{0b11110000, 0b1}},
-			{4, bitfield.Bitlist{0b11100000, 0b1}},
-			{3, bitfield.Bitlist{0b11000000, 0b1}},
-			{2, bitfield.Bitlist{0b11100000, 0b1}},
-			{1, bitfield.Bitlist{0b11100000, 0b1}},
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-		})
-		atts, err := atts.sort()
-		if err != nil {
-			t.Error(err)
-		}
-		require.DeepEqual(t, want, atts)
-	})
-
-	t.Run("follows max-cover", func(t *testing.T) {
-		// Items at slot 4, must be first split into two lists by max-cover, with
-		// 0b10000011 scoring higher (as it provides more info in addition to already selected
-		// attestations) than 0b11100001 (despite naive bit count suggesting otherwise). Then,
-		// both selected and non-selected attestations must be additionally sorted by bit count.
-		atts := getAtts([]testData{
-			{4, bitfield.Bitlist{0b00000001, 0b1}},
-			{4, bitfield.Bitlist{0b11100001, 0b1}},
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-			{2, bitfield.Bitlist{0b11100000, 0b1}},
-			{4, bitfield.Bitlist{0b10000011, 0b1}},
-			{4, bitfield.Bitlist{0b11111000, 0b1}},
-			{1, bitfield.Bitlist{0b11100000, 0b1}},
-			{3, bitfield.Bitlist{0b11000000, 0b1}},
-		})
-		want := getAtts([]testData{
-			{4, bitfield.Bitlist{0b11111000, 0b1}},
-			{4, bitfield.Bitlist{0b10000011, 0b1}},
-			{4, bitfield.Bitlist{0b11100001, 0b1}},
-			{4, bitfield.Bitlist{0b00000001, 0b1}},
-			{3, bitfield.Bitlist{0b11000000, 0b1}},
-			{2, bitfield.Bitlist{0b11100000, 0b1}},
-			{1, bitfield.Bitlist{0b11100000, 0b1}},
-			{1, bitfield.Bitlist{0b11000000, 0b1}},
-		})
-		atts, err := atts.sort()
-		if err != nil {
-			t.Error(err)
-		}
-		require.DeepEqual(t, want, atts)
-	})
-}
-
 func TestProposer_ProposerAtts_committeeAwareSort(t *testing.T) {
 	type testData struct {
 		slot primitives.Slot

changelog/radek_remove-committee-aware-packing-flag.md

@@ -0,0 +1,3 @@
+### Removed
+
+- Remove `disable-committee-aware-packing` flag.

config/features/config.go

@@ -49,7 +49,6 @@ type Flags struct {
 	EnableDoppelGanger                  bool // EnableDoppelGanger enables doppelganger protection on startup for the validator.
 	EnableHistoricalSpaceRepresentation bool // EnableHistoricalSpaceRepresentation enables the saving of registry validators in separate buckets to save space
 	EnableBeaconRESTApi                 bool // EnableBeaconRESTApi enables experimental usage of the beacon REST API by the validator when querying a beacon node
-	DisableCommitteeAwarePacking        bool // DisableCommitteeAwarePacking changes the attestation packing algorithm to one that is not aware of attesting committees.
 	EnableExperimentalAttestationPool   bool // EnableExperimentalAttestationPool enables an experimental attestation pool design.
 	// Logging related toggles.
 	DisableGRPCConnectionLogs bool // Disables logging when a new grpc client has connected.
@@ -267,10 +266,6 @@ func ConfigureBeaconChain(ctx *cli.Context) error {
 		logDisabled(DisableQUIC)
 		cfg.EnableQUIC = false
 	}
-	if ctx.IsSet(DisableCommitteeAwarePacking.Name) {
-		logEnabled(DisableCommitteeAwarePacking)
-		cfg.DisableCommitteeAwarePacking = true
-	}
 	if ctx.IsSet(EnableDiscoveryReboot.Name) {
 		logEnabled(EnableDiscoveryReboot)
 		cfg.EnableDiscoveryReboot = true

config/features/flags.go

@@ -167,10 +167,6 @@ var (
 		Name:  "disable-quic",
 		Usage: "Disables connecting using the QUIC protocol with peers.",
 	}
-	DisableCommitteeAwarePacking = &cli.BoolFlag{
-		Name:  "disable-committee-aware-packing",
-		Usage: "Changes the attestation packing algorithm to one that is not aware of attesting committees.",
-	}
 	EnableDiscoveryReboot = &cli.BoolFlag{
 		Name:  "enable-discovery-reboot",
 		Usage: "Experimental: Enables the discovery listener to rebooted in the event of connectivity issues.",
@@ -246,7 +242,6 @@ var BeaconChainFlags = combinedFlags([]cli.Flag{
 	EnableLightClient,
 	BlobSaveFsync,
 	DisableQUIC,
-	DisableCommitteeAwarePacking,
 	EnableDiscoveryReboot,
 	enableExperimentalAttestationPool,
 	forceHeadFlag,