diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go index 24d22e92c2..642e3b81e9 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations.go +++ b/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() } diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go index 2583d620f2..e9096bc991 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go @@ -13,7 +13,6 @@ import ( "github.com/OffchainLabs/prysm/v6/beacon-chain/core/helpers" "github.com/OffchainLabs/prysm/v6/beacon-chain/operations/attestations" "github.com/OffchainLabs/prysm/v6/beacon-chain/operations/attestations/mock" - "github.com/OffchainLabs/prysm/v6/config/features" "github.com/OffchainLabs/prysm/v6/config/params" "github.com/OffchainLabs/prysm/v6/consensus-types/primitives" "github.com/OffchainLabs/prysm/v6/crypto/bls/blst" @@ -26,162 +25,6 @@ import ( "github.com/prysmaticlabs/go-bitfield" ) -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(ðpb.Attestation{ - Data: ðpb.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 diff --git a/changelog/radek_remove-committee-aware-packing-flag.md b/changelog/radek_remove-committee-aware-packing-flag.md new file mode 100644 index 0000000000..1c11763098 --- /dev/null +++ b/changelog/radek_remove-committee-aware-packing-flag.md @@ -0,0 +1,3 @@ +### Removed + +- Remove `disable-committee-aware-packing` flag. \ No newline at end of file diff --git a/config/features/config.go b/config/features/config.go index ab1519948f..60a9fc7a7f 100644 --- a/config/features/config.go +++ b/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 diff --git a/config/features/flags.go b/config/features/flags.go index b205a6c0d7..8e1d9edbcf 100644 --- a/config/features/flags.go +++ b/config/features/flags.go @@ -168,10 +168,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.", @@ -247,7 +243,6 @@ var BeaconChainFlags = combinedFlags([]cli.Flag{ EnableLightClient, BlobSaveFsync, DisableQUIC, - DisableCommitteeAwarePacking, EnableDiscoveryReboot, enableExperimentalAttestationPool, forceHeadFlag,