Pack Electra attestations by marginal proposer reward

[potuz]

Replace attestation packing by choosing the most profitable attestations instead of the ones with most bits.

There's a reason why Prysm was packing 8 attestations on devnets instead of just 1 or 2 like other clients, we put many attestations that are worthless since the intersection with others are high.

[james-prysm]

before these kinds of errors just meant 0, but i think now it bubbles them up
old

r, err := electra.GetProposerRewardNumerator(ctx, st, att, totalBalance)
		if err != nil {
			log.WithError(err).Debug("Failed to get proposer reward numerator")
			return 0
		}

[potuz]

Fixed in d9e005e

[james-prysm]

because we are looping here
for round := 1; uint64(len(selected)) < limit && h.Len() > 0; round++ {
var best *attCandidate
for h.Len() > 0 {
c := heap.Pop(&h).(*attCandidate)
if c.scoredRound != round {
c.score, err = c.marginalReward(st, participationFor(c), totalBalance)

i think this is getting calculated more times than necessary. not sure how much of an issue this is

[potuz]

We called BaseRewardWithTotalBalance per attesting index on every re-score, reworked it by getting all effective balances and total active balance outside of the loop

[james-prysm]

nit since idx is declared outside why not just do idx < maxLayers and do idx ++ at the end

[james-prysm]

Could we link the consensus-spec process_attestation here? This mirrors proposer_reward_numerator; renaming it to marginalRewardNumerator might also clarify that the denominator is intentionally omitted.

[james-prysm]

ai keeps flagging this as a bug to remove this and the cap above on max layers.

here is an associated test

// Layer generation must feed every pool aggregate to reward-aware selection; capping layers at
// the block limit would let max-cover's bit-count order discard the only profitable aggregate.
func TestOnChainAggregates_KeepsLayersForMarginalScoring(t *testing.T) {
	ctx := t.Context()

	params.SetupTestConfigCleanup(t)
	cfg := params.BeaconConfig().Copy()
	cfg.ElectraForkEpoch = 1
	params.OverrideBeaconConfig(cfg)

	key, err := blst.RandKey()
	require.NoError(t, err)
	sig := key.Sign([]byte{'X'})

	st, _ := util.DeterministicGenesisStateElectra(t, 1024)
	require.NoError(t, st.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))

	committees, err := helpers.BeaconCommittees(ctx, st, 0)
	require.NoError(t, err)
	committee := committees[0]
	require.Equal(t, true, len(committee) >= 18) // The largest aggregate below sets bit 17.

	// The state is one epoch past these slot-0 attestations, so they target the previous epoch.
	// Mark every committee vote as already counted except position 9.
	participation, err := st.PreviousEpochParticipation()
	require.NoError(t, err)
	for position, validatorIndex := range committee {
		if position != 9 {
			participation[validatorIndex] = 0b111
		}
	}
	require.NoError(t, st.SetPreviousParticipationBits(participation))

	committeeBits := primitives.NewAttestationCommitteeBits()
	committeeBits.SetBitAt(0, true)
	data := util.HydrateAttestationData(&ethpb.AttestationData{})

	newAggregate := func(positions ...uint64) *ethpb.AttestationElectra {
		bits := bitfield.NewBitlist(uint64(len(committee)))
		for _, position := range positions {
			bits.SetBitAt(position, true)
		}
		return &ethpb.AttestationElectra{
			AggregationBits: bits,
			CommitteeBits:   committeeBits,
			Data:            data,
			Signature:       sig.Marshal(),
		}
	}

	// Every aggregate shares bit 0 and owns bits no other has, so none is a subset of another
	// and max-cover keeps all nine, sorting these eight 3-bit aggregates ahead of the 2-bit one.
	networkAggregates := make([]ethpb.Att, 0, 9)
	for i := range uint64(8) {
		networkAggregates = append(networkAggregates, newAggregate(0, i+1, i+10))
	}
	// The ninth layer is smaller, but validator position 9 is the only uncovered vote.
	networkAggregates = append(networkAggregates, newAggregate(0, 9))

	candidates, err := onChainAggregates(map[ethpbattestation.Id][]ethpb.Att{{}: networkAggregates})
	require.NoError(t, err)
	// All nine layers must survive generation; enforcing the block limit is selection's job.
	require.Equal(t, 9, len(candidates))

	// The eight covered aggregates score zero, so only the fresh-vote aggregate is packed.
	selected, err := candidates.selectByMarginalReward(ctx, st, params.BeaconConfig().MaxAttestationsElectra)
	require.NoError(t, err)
	require.Equal(t, 1, len(selected))
	require.DeepEqual(t, networkAggregates[8].GetAggregationBits(), selected[0].GetAggregationBits())
}