Remove disable-committee-aware-packing flag (#15162)

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
This commit is contained in:
Radosław Kapka
2025-04-10 21:48:16 +02:00
committed by GitHub
parent 774b9a7159
commit b0614fe137
5 changed files with 3 additions and 238 deletions

View File

@@ -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()
}

View File

@@ -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(&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

View File

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

View File

@@ -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

View File

@@ -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,