diff --git a/beacon-chain/blockchain/BUILD.bazel b/beacon-chain/blockchain/BUILD.bazel index 317a731ec4..c5d3ad306b 100644 --- a/beacon-chain/blockchain/BUILD.bazel +++ b/beacon-chain/blockchain/BUILD.bazel @@ -97,6 +97,7 @@ go_library( "@com_github_pkg_errors//:go_default_library", "@com_github_prometheus_client_golang//prometheus:go_default_library", "@com_github_prometheus_client_golang//prometheus/promauto:go_default_library", + "@com_github_prysmaticlabs_go_bitfield//:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", "@org_golang_x_sync//errgroup:go_default_library", ], @@ -155,6 +156,7 @@ go_test( "//beacon-chain/forkchoice/doubly-linked-tree:go_default_library", "//beacon-chain/forkchoice/types:go_default_library", "//beacon-chain/operations/attestations:go_default_library", + "//beacon-chain/operations/attestations/kv:go_default_library", "//beacon-chain/operations/blstoexec:go_default_library", "//beacon-chain/operations/slashings:go_default_library", "//beacon-chain/operations/voluntaryexits:go_default_library", @@ -186,6 +188,7 @@ go_test( "@com_github_ethereum_go_ethereum//core/types:go_default_library", "@com_github_holiman_uint256//:go_default_library", "@com_github_pkg_errors//:go_default_library", + "@com_github_prysmaticlabs_go_bitfield//:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", "@com_github_sirupsen_logrus//hooks/test:go_default_library", "@org_golang_google_protobuf//proto:go_default_library", diff --git a/beacon-chain/blockchain/forkchoice_update_execution.go b/beacon-chain/blockchain/forkchoice_update_execution.go index 5739f5d2f1..ed749d52c0 100644 --- a/beacon-chain/blockchain/forkchoice_update_execution.go +++ b/beacon-chain/blockchain/forkchoice_update_execution.go @@ -103,7 +103,7 @@ func (s *Service) forkchoiceUpdateWithExecution(ctx context.Context, args *fcuCo } // Only need to prune attestations from pool if the head has changed. - if err := s.pruneAttsFromPool(args.headBlock); err != nil { + if err := s.pruneAttsFromPool(s.ctx, args.headState, args.headBlock); err != nil { log.WithError(err).Error("could not prune attestations from pool") } return nil diff --git a/beacon-chain/blockchain/process_attestation.go b/beacon-chain/blockchain/process_attestation.go index 540a4ba400..36fbb2dd3f 100644 --- a/beacon-chain/blockchain/process_attestation.go +++ b/beacon-chain/blockchain/process_attestation.go @@ -80,7 +80,7 @@ func (s *Service) OnAttestation(ctx context.Context, a ethpb.Att, disparity time } // Use the target state to verify attesting indices are valid. - committees, err := helpers.AttestationCommittees(ctx, baseState, a) + committees, err := helpers.AttestationCommitteesFromState(ctx, baseState, a) if err != nil { return err } diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index a599351fcf..07a49b6e69 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -6,6 +6,7 @@ import ( "time" "github.com/pkg/errors" + "github.com/prysmaticlabs/go-bitfield" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers" coreTime "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/time" @@ -15,6 +16,7 @@ import ( forkchoicetypes "github.com/prysmaticlabs/prysm/v5/beacon-chain/forkchoice/types" "github.com/prysmaticlabs/prysm/v5/beacon-chain/state" "github.com/prysmaticlabs/prysm/v5/config/features" + fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" "github.com/prysmaticlabs/prysm/v5/config/params" consensusblocks "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" @@ -368,7 +370,7 @@ func (s *Service) handleEpochBoundary(ctx context.Context, slot primitives.Slot, func (s *Service) handleBlockAttestations(ctx context.Context, blk interfaces.ReadOnlyBeaconBlock, st state.BeaconState) error { // Feed in block's attestations to fork choice store. for _, a := range blk.Body().Attestations() { - committees, err := helpers.AttestationCommittees(ctx, st, a) + committees, err := helpers.AttestationCommitteesFromState(ctx, st, a) if err != nil { return err } @@ -419,27 +421,98 @@ func (s *Service) savePostStateInfo(ctx context.Context, r [32]byte, b interface return nil } -// This removes the attestations in block `b` from the attestation mem pool. -func (s *Service) pruneAttsFromPool(headBlock interfaces.ReadOnlySignedBeaconBlock) error { - atts := headBlock.Block().Body().Attestations() - for _, att := range atts { - if features.Get().EnableExperimentalAttestationPool { - if err := s.cfg.AttestationCache.DeleteCovered(att); err != nil { - return errors.Wrap(err, "could not delete attestation") - } - } else if att.IsAggregated() { - if err := s.cfg.AttPool.DeleteAggregatedAttestation(att); err != nil { - return err - } - } else { - if err := s.cfg.AttPool.DeleteUnaggregatedAttestation(att); err != nil { - return err - } +// pruneAttsFromPool removes these attestations from the attestation pool +// which are covered by attestations from the received block. +func (s *Service) pruneAttsFromPool(ctx context.Context, headState state.BeaconState, headBlock interfaces.ReadOnlySignedBeaconBlock) error { + for _, att := range headBlock.Block().Body().Attestations() { + if err := s.pruneCoveredAttsFromPool(ctx, headState, att); err != nil { + log.WithError(err).Warn("Could not prune attestations covered by a received block's attestation") } } return nil } +func (s *Service) pruneCoveredAttsFromPool(ctx context.Context, headState state.BeaconState, att ethpb.Att) error { + switch { + case !att.IsAggregated(): + return s.cfg.AttPool.DeleteUnaggregatedAttestation(att) + case att.Version() == version.Phase0: + if features.Get().EnableExperimentalAttestationPool { + return errors.Wrap(s.cfg.AttestationCache.DeleteCovered(att), "could not delete covered attestation") + } + return errors.Wrap(s.cfg.AttPool.DeleteAggregatedAttestation(att), "could not delete aggregated attestation") + default: + return s.pruneCoveredElectraAttsFromPool(ctx, headState, att) + } +} + +// pruneCoveredElectraAttsFromPool handles removing aggregated Electra attestations from the pool after receiving a block. +// Because in Electra block attestations can combine aggregates for multiple committees, comparing attestation bits +// of a block attestation with attestations bits of an aggregate can cause unexpected results, leading to covered +// aggregates not being removed from the pool. +// +// To make sure aggregates are removed, we decompose the block attestation into dummy aggregates, with each +// aggregate accounting for one committee. This allows us to compare aggregates in the same way it's done for +// Phase0. Even though we can't provide a valid signature for the dummy aggregate, it does not matter because +// signatures play no part in pruning attestations. +func (s *Service) pruneCoveredElectraAttsFromPool(ctx context.Context, headState state.BeaconState, att ethpb.Att) error { + if att.Version() == version.Phase0 { + log.Error("Called pruneCoveredElectraAttsFromPool with a Phase0 attestation") + return nil + } + + // We don't want to recompute committees. If they are not cached already, + // we allow attestations to stay in the pool. If these attestations are + // included in a later block, they will be redundant. But given that + // they were not cached in the first place, it's unlikely that they + // will be chosen into a block. + ok, committees, err := helpers.AttestationCommitteesFromCache(ctx, headState, att) + if err != nil { + return errors.Wrap(err, "could not get attestation committees") + } + if !ok { + log.Debug("Attestation committees are not cached. Skipping attestation pruning.") + return nil + } + + committeeIndices := att.CommitteeBitsVal().BitIndices() + offset := uint64(0) + + // Sanity check as this should never happen + if len(committeeIndices) != len(committees) { + return errors.New("committee indices and committees have different lengths") + } + + for i, c := range committees { + ab := bitfield.NewBitlist(uint64(len(c))) + for j := uint64(0); j < uint64(len(c)); j++ { + ab.SetBitAt(j, att.GetAggregationBits().BitAt(j+offset)) + } + + cb := primitives.NewAttestationCommitteeBits() + cb.SetBitAt(uint64(committeeIndices[i]), true) + + a := ðpb.AttestationElectra{ + AggregationBits: ab, + Data: att.GetData(), + CommitteeBits: cb, + Signature: make([]byte, fieldparams.BLSSignatureLength), + } + + if features.Get().EnableExperimentalAttestationPool { + if err = s.cfg.AttestationCache.DeleteCovered(a); err != nil { + return errors.Wrap(err, "could not delete covered attestation") + } + } else if err = s.cfg.AttPool.DeleteAggregatedAttestation(a); err != nil { + return errors.Wrap(err, "could not delete aggregated attestation") + } + + offset += uint64(len(c)) + } + + return nil +} + // validateMergeTransitionBlock validates the merge transition block. func (s *Service) validateMergeTransitionBlock(ctx context.Context, stateVersion int, stateHeader interfaces.ExecutionData, blk interfaces.ReadOnlySignedBeaconBlock) error { // Skip validation if block is older than Bellatrix. diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index b7f4a39e21..fc0426eb1b 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -12,8 +12,10 @@ import ( "github.com/ethereum/go-ethereum/common" gethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/pkg/errors" + "github.com/prysmaticlabs/go-bitfield" "github.com/prysmaticlabs/prysm/v5/beacon-chain/cache" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/blocks" + "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers" lightClient "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/light-client" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/signing" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/transition" @@ -25,6 +27,7 @@ import ( mockExecution "github.com/prysmaticlabs/prysm/v5/beacon-chain/execution/testing" doublylinkedtree "github.com/prysmaticlabs/prysm/v5/beacon-chain/forkchoice/doubly-linked-tree" forkchoicetypes "github.com/prysmaticlabs/prysm/v5/beacon-chain/forkchoice/types" + "github.com/prysmaticlabs/prysm/v5/beacon-chain/operations/attestations/kv" "github.com/prysmaticlabs/prysm/v5/beacon-chain/state" "github.com/prysmaticlabs/prysm/v5/config/features" fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" @@ -45,6 +48,90 @@ import ( logTest "github.com/sirupsen/logrus/hooks/test" ) +func Test_pruneAttsFromPool_Electra(t *testing.T) { + ctx := context.Background() + + params.SetupTestConfigCleanup(t) + cfg := params.BeaconConfig().Copy() + cfg.TargetCommitteeSize = 8 + params.OverrideBeaconConfig(cfg) + + s := Service{ + cfg: &config{ + AttPool: kv.NewAttCaches(), + }, + } + + data := ðpb.AttestationData{ + BeaconBlockRoot: make([]byte, 32), + Source: ðpb.Checkpoint{Root: make([]byte, 32)}, + Target: ðpb.Checkpoint{Root: make([]byte, 32)}, + } + + cb := primitives.NewAttestationCommitteeBits() + cb.SetBitAt(0, true) + att1 := ðpb.AttestationElectra{ + AggregationBits: bitfield.Bitlist{0b11110111, 0b00000001}, + Data: data, + Signature: make([]byte, 96), + CommitteeBits: cb, + } + + cb = primitives.NewAttestationCommitteeBits() + cb.SetBitAt(1, true) + att2 := ðpb.AttestationElectra{ + AggregationBits: bitfield.Bitlist{0b11110111, 0b00000001}, + Data: data, + Signature: make([]byte, 96), + CommitteeBits: cb, + } + + cb = primitives.NewAttestationCommitteeBits() + cb.SetBitAt(3, true) + att3 := ðpb.AttestationElectra{ + AggregationBits: bitfield.Bitlist{0b11110111, 0b00000001}, + Data: data, + Signature: make([]byte, 96), + CommitteeBits: cb, + } + + require.NoError(t, s.cfg.AttPool.SaveAggregatedAttestation(att1)) + require.NoError(t, s.cfg.AttPool.SaveAggregatedAttestation(att2)) + require.NoError(t, s.cfg.AttPool.SaveAggregatedAttestation(att3)) + require.Equal(t, 3, len(s.cfg.AttPool.AggregatedAttestations())) + + cb = primitives.NewAttestationCommitteeBits() + cb.SetBitAt(0, true) + cb.SetBitAt(1, true) + onChainAtt := ðpb.AttestationElectra{ + AggregationBits: bitfield.Bitlist{0b11110111, 0b11110111, 0b00000001}, + Data: data, + Signature: make([]byte, 96), + CommitteeBits: cb, + } + bl := ðpb.SignedBeaconBlockElectra{ + Block: ðpb.BeaconBlockElectra{ + Body: ðpb.BeaconBlockBodyElectra{ + Attestations: []*ethpb.AttestationElectra{onChainAtt}, + }, + }, + Signature: make([]byte, 96), + } + rob, err := consensusblocks.NewSignedBeaconBlock(bl) + require.NoError(t, err) + st, _ := util.DeterministicGenesisStateElectra(t, 1024) + committees, err := helpers.BeaconCommittees(ctx, st, 0) + require.NoError(t, err) + // Sanity check to make sure the on-chain att will be decomposed + // into the correct number of aggregates. + require.Equal(t, 4, len(committees)) + + require.NoError(t, s.pruneAttsFromPool(ctx, st, rob)) + attsInPool := s.cfg.AttPool.AggregatedAttestations() + require.Equal(t, 1, len(attsInPool)) + assert.DeepEqual(t, att3, attsInPool[0]) +} + func TestStore_OnBlockBatch(t *testing.T) { service, tr := minimalTestService(t) ctx := tr.ctx @@ -840,7 +927,7 @@ func TestRemoveBlockAttestationsInPool(t *testing.T) { require.NoError(t, service.cfg.AttPool.SaveAggregatedAttestations(atts)) wsb, err := consensusblocks.NewSignedBeaconBlock(b) require.NoError(t, err) - require.NoError(t, service.pruneAttsFromPool(wsb)) + require.NoError(t, service.pruneAttsFromPool(context.Background(), nil /* state not needed pre-Electra */, wsb)) require.Equal(t, 0, service.cfg.AttPool.AggregatedAttestationCount()) } diff --git a/beacon-chain/blockchain/receive_block.go b/beacon-chain/blockchain/receive_block.go index d675aeddde..8bd12705ec 100644 --- a/beacon-chain/blockchain/receive_block.go +++ b/beacon-chain/blockchain/receive_block.go @@ -547,7 +547,7 @@ func (s *Service) sendBlockAttestationsToSlasher(signed interfaces.ReadOnlySigne // is done in the background to avoid adding more load to this critical code path. ctx := context.TODO() for _, att := range signed.Block().Body().Attestations() { - committees, err := helpers.AttestationCommittees(ctx, preState, att) + committees, err := helpers.AttestationCommitteesFromState(ctx, preState, att) if err != nil { log.WithError(err).Error("Could not get attestation committees") return diff --git a/beacon-chain/core/altair/attestation.go b/beacon-chain/core/altair/attestation.go index 7c11d68b75..e15ef74b6b 100644 --- a/beacon-chain/core/altair/attestation.go +++ b/beacon-chain/core/altair/attestation.go @@ -66,7 +66,7 @@ func ProcessAttestationNoVerifySignature( if err != nil { return nil, err } - committees, err := helpers.AttestationCommittees(ctx, beaconState, att) + committees, err := helpers.AttestationCommitteesFromState(ctx, beaconState, att) if err != nil { return nil, err } diff --git a/beacon-chain/core/blocks/signature.go b/beacon-chain/core/blocks/signature.go index dedd5856ec..dc2428e2a6 100644 --- a/beacon-chain/core/blocks/signature.go +++ b/beacon-chain/core/blocks/signature.go @@ -192,7 +192,7 @@ func createAttestationSignatureBatch( descs := make([]string, len(atts)) for i, a := range atts { sigs[i] = a.GetSignature() - committees, err := helpers.AttestationCommittees(ctx, beaconState, a) + committees, err := helpers.AttestationCommitteesFromState(ctx, beaconState, a) if err != nil { return nil, err } diff --git a/beacon-chain/core/helpers/beacon_committee.go b/beacon-chain/core/helpers/beacon_committee.go index 2e70cf5f0f..5882109535 100644 --- a/beacon-chain/core/helpers/beacon_committee.go +++ b/beacon-chain/core/helpers/beacon_committee.go @@ -30,6 +30,13 @@ var ( proposerIndicesCache = cache.NewProposerIndicesCache() ) +type beaconCommitteeFunc = func( + ctx context.Context, + state state.ReadOnlyBeaconState, + slot primitives.Slot, + committeeIndex primitives.CommitteeIndex, +) ([]primitives.ValidatorIndex, error) + // SlotCommitteeCount returns the number of beacon committees of a slot. The // active validator count is provided as an argument rather than an imported implementation // from the spec definition. Having the active validator count as an argument allows for @@ -59,21 +66,48 @@ func SlotCommitteeCount(activeValidatorCount uint64) uint64 { return committeesPerSlot } -// AttestationCommittees returns beacon state committees that reflect attestation's committee indices. -func AttestationCommittees(ctx context.Context, st state.ReadOnlyBeaconState, att ethpb.Att) ([][]primitives.ValidatorIndex, error) { +// AttestationCommitteesFromState returns beacon state committees that reflect attestation's committee indices. +func AttestationCommitteesFromState(ctx context.Context, st state.ReadOnlyBeaconState, att ethpb.Att) ([][]primitives.ValidatorIndex, error) { + return attestationCommittees(ctx, st, att, BeaconCommitteeFromState) +} + +// AttestationCommitteesFromCache has the same functionality as AttestationCommitteesFromState, but only returns a value +// when all attestation committees are already cached. +func AttestationCommitteesFromCache(ctx context.Context, st state.ReadOnlyBeaconState, att ethpb.Att) (bool, [][]primitives.ValidatorIndex, error) { + committees, err := attestationCommittees(ctx, st, att, BeaconCommitteeFromCache) + if err != nil { + return false, nil, err + } + if len(committees) == 0 { + return false, nil, nil + } + for _, c := range committees { + if len(c) == 0 { + return false, nil, nil + } + } + return true, committees, nil +} + +func attestationCommittees( + ctx context.Context, + st state.ReadOnlyBeaconState, + att ethpb.Att, + committeeFunc beaconCommitteeFunc, +) ([][]primitives.ValidatorIndex, error) { var committees [][]primitives.ValidatorIndex if att.Version() >= version.Electra { committeeIndices := att.CommitteeBitsVal().BitIndices() committees = make([][]primitives.ValidatorIndex, len(committeeIndices)) for i, ci := range committeeIndices { - committee, err := BeaconCommitteeFromState(ctx, st, att.GetData().Slot, primitives.CommitteeIndex(ci)) + committee, err := committeeFunc(ctx, st, att.GetData().Slot, primitives.CommitteeIndex(ci)) if err != nil { return nil, err } committees[i] = committee } } else { - committee, err := BeaconCommitteeFromState(ctx, st, att.GetData().Slot, att.GetData().CommitteeIndex) + committee, err := committeeFunc(ctx, st, att.GetData().Slot, att.GetData().CommitteeIndex) if err != nil { return nil, err } @@ -164,6 +198,27 @@ func BeaconCommitteeFromState(ctx context.Context, state state.ReadOnlyBeaconSta return BeaconCommittee(ctx, activeIndices, seed, slot, committeeIndex) } +// BeaconCommitteeFromCache has the same functionality as BeaconCommitteeFromState, but only returns a value +// when the committee is already cached. +func BeaconCommitteeFromCache( + ctx context.Context, + state state.ReadOnlyBeaconState, + slot primitives.Slot, + committeeIndex primitives.CommitteeIndex, +) ([]primitives.ValidatorIndex, error) { + epoch := slots.ToEpoch(slot) + seed, err := Seed(state, epoch, params.BeaconConfig().DomainBeaconAttester) + if err != nil { + return nil, errors.Wrap(err, "could not get seed") + } + + committee, err := committeeCache.Committee(ctx, slot, seed, committeeIndex) + if err != nil { + return nil, errors.Wrap(err, "could not interface with committee cache") + } + return committee, nil +} + // BeaconCommittee returns the beacon committee of a given slot and committee index. The // validator indices and seed are provided as an argument rather than an imported implementation // from the spec definition. Having them as an argument allows for cheaper computation run time. diff --git a/beacon-chain/core/helpers/beacon_committee_test.go b/beacon-chain/core/helpers/beacon_committee_test.go index a9a95facc4..bf21e3cd37 100644 --- a/beacon-chain/core/helpers/beacon_committee_test.go +++ b/beacon-chain/core/helpers/beacon_committee_test.go @@ -729,7 +729,9 @@ func TestCommitteeIndices(t *testing.T) { assert.DeepEqual(t, []primitives.CommitteeIndex{0, 1, 3}, indices) } -func TestAttestationCommittees(t *testing.T) { +func TestAttestationCommitteesFromState(t *testing.T) { + ctx := context.Background() + validators := make([]*ethpb.Validator, params.BeaconConfig().SlotsPerEpoch.Mul(params.BeaconConfig().TargetCommitteeSize)) for i := 0; i < len(validators); i++ { validators[i] = ðpb.Validator{ @@ -745,7 +747,7 @@ func TestAttestationCommittees(t *testing.T) { t.Run("pre-Electra", func(t *testing.T) { att := ðpb.Attestation{Data: ðpb.AttestationData{CommitteeIndex: 0}} - committees, err := helpers.AttestationCommittees(context.Background(), state, att) + committees, err := helpers.AttestationCommitteesFromState(ctx, state, att) require.NoError(t, err) require.Equal(t, 1, len(committees)) assert.Equal(t, params.BeaconConfig().TargetCommitteeSize, uint64(len(committees[0]))) @@ -755,7 +757,7 @@ func TestAttestationCommittees(t *testing.T) { bits.SetBitAt(0, true) bits.SetBitAt(1, true) att := ðpb.AttestationElectra{CommitteeBits: bits, Data: ðpb.AttestationData{}} - committees, err := helpers.AttestationCommittees(context.Background(), state, att) + committees, err := helpers.AttestationCommitteesFromState(ctx, state, att) require.NoError(t, err) require.Equal(t, 2, len(committees)) assert.Equal(t, params.BeaconConfig().TargetCommitteeSize, uint64(len(committees[0]))) @@ -763,9 +765,58 @@ func TestAttestationCommittees(t *testing.T) { }) } -func TestBeaconCommittees(t *testing.T) { - prevConfig := params.BeaconConfig().Copy() - defer params.OverrideBeaconConfig(prevConfig) +func TestAttestationCommitteesFromCache(t *testing.T) { + ctx := context.Background() + + validators := make([]*ethpb.Validator, params.BeaconConfig().SlotsPerEpoch.Mul(params.BeaconConfig().TargetCommitteeSize)) + for i := 0; i < len(validators); i++ { + validators[i] = ðpb.Validator{ + ExitEpoch: params.BeaconConfig().FarFutureEpoch, + } + } + + state, err := state_native.InitializeFromProtoPhase0(ðpb.BeaconState{ + Validators: validators, + RandaoMixes: make([][]byte, params.BeaconConfig().EpochsPerHistoricalVector), + }) + require.NoError(t, err) + + t.Run("pre-Electra", func(t *testing.T) { + helpers.ClearCache() + att := ðpb.Attestation{Data: ðpb.AttestationData{CommitteeIndex: 0}} + ok, _, err := helpers.AttestationCommitteesFromCache(ctx, state, att) + require.NoError(t, err) + require.Equal(t, false, ok) + require.NoError(t, helpers.UpdateCommitteeCache(ctx, state, 0)) + ok, committees, err := helpers.AttestationCommitteesFromCache(ctx, state, att) + require.NoError(t, err) + require.Equal(t, true, ok) + require.Equal(t, 1, len(committees)) + assert.Equal(t, params.BeaconConfig().TargetCommitteeSize, uint64(len(committees[0]))) + }) + t.Run("post-Electra", func(t *testing.T) { + helpers.ClearCache() + bits := primitives.NewAttestationCommitteeBits() + bits.SetBitAt(0, true) + bits.SetBitAt(1, true) + att := ðpb.AttestationElectra{CommitteeBits: bits, Data: ðpb.AttestationData{}} + ok, _, err := helpers.AttestationCommitteesFromCache(ctx, state, att) + require.NoError(t, err) + require.Equal(t, false, ok) + require.NoError(t, helpers.UpdateCommitteeCache(ctx, state, 0)) + ok, committees, err := helpers.AttestationCommitteesFromCache(ctx, state, att) + require.NoError(t, err) + require.Equal(t, true, ok) + require.Equal(t, 2, len(committees)) + assert.Equal(t, params.BeaconConfig().TargetCommitteeSize, uint64(len(committees[0]))) + assert.Equal(t, params.BeaconConfig().TargetCommitteeSize, uint64(len(committees[1]))) + }) +} + +func TestBeaconCommitteesFromState(t *testing.T) { + ctx := context.Background() + + params.SetupTestConfigCleanup(t) c := params.BeaconConfig().Copy() c.MinGenesisActiveValidatorCount = 128 c.SlotsPerEpoch = 4 @@ -774,15 +825,49 @@ func TestBeaconCommittees(t *testing.T) { state, _ := util.DeterministicGenesisState(t, 256) - activeCount, err := helpers.ActiveValidatorCount(context.Background(), state, 0) + activeCount, err := helpers.ActiveValidatorCount(ctx, state, 0) require.NoError(t, err) committeesPerSlot := helpers.SlotCommitteeCount(activeCount) - committees, err := helpers.BeaconCommittees(context.Background(), state, 0) + committees, err := helpers.BeaconCommittees(ctx, state, 0) require.NoError(t, err) require.Equal(t, committeesPerSlot, uint64(len(committees))) for idx := primitives.CommitteeIndex(0); idx < primitives.CommitteeIndex(len(committees)); idx++ { - committee, err := helpers.BeaconCommitteeFromState(context.Background(), state, 0, idx) + committee, err := helpers.BeaconCommitteeFromState(ctx, state, 0, idx) require.NoError(t, err) - require.DeepEqual(t, committees[idx], committee) + assert.DeepEqual(t, committees[idx], committee) + } +} + +func TestBeaconCommitteesFromCache(t *testing.T) { + ctx := context.Background() + + params.SetupTestConfigCleanup(t) + c := params.BeaconConfig().Copy() + c.MinGenesisActiveValidatorCount = 128 + c.SlotsPerEpoch = 4 + c.TargetCommitteeSize = 16 + params.OverrideBeaconConfig(c) + + state, _ := util.DeterministicGenesisState(t, 256) + + activeCount, err := helpers.ActiveValidatorCount(ctx, state, 0) + require.NoError(t, err) + committeesPerSlot := helpers.SlotCommitteeCount(activeCount) + committees, err := helpers.BeaconCommittees(ctx, state, 0) + require.NoError(t, err) + require.Equal(t, committeesPerSlot, uint64(len(committees))) + + helpers.ClearCache() + for idx := primitives.CommitteeIndex(0); idx < primitives.CommitteeIndex(len(committees)); idx++ { + committee, err := helpers.BeaconCommitteeFromCache(ctx, state, 0, idx) + require.NoError(t, err) + assert.Equal(t, 0, len(committee)) + } + + require.NoError(t, helpers.UpdateCommitteeCache(ctx, state, 0)) + for idx := primitives.CommitteeIndex(0); idx < primitives.CommitteeIndex(len(committees)); idx++ { + committee, err := helpers.BeaconCommitteeFromCache(ctx, state, 0, idx) + require.NoError(t, err) + assert.DeepEqual(t, committees[idx], committee) } } diff --git a/changelog/radek_electra-packing-fix.md b/changelog/radek_electra-packing-fix.md new file mode 100644 index 0000000000..c2985f8fac --- /dev/null +++ b/changelog/radek_electra-packing-fix.md @@ -0,0 +1,3 @@ +### Fixed + +- Decompose Electra block attestations to prevent redundant packing. \ No newline at end of file diff --git a/proto/prysm/v1alpha1/attestation/attestation_utils.go b/proto/prysm/v1alpha1/attestation/attestation_utils.go index 863bd380df..3fcd57cc9f 100644 --- a/proto/prysm/v1alpha1/attestation/attestation_utils.go +++ b/proto/prysm/v1alpha1/attestation/attestation_utils.go @@ -6,6 +6,7 @@ import ( "bytes" "context" "fmt" + "runtime/debug" "slices" "sort" @@ -100,6 +101,10 @@ func AttestingIndices(att ethpb.Att, committees ...[]primitives.ValidatorIndex) for _, c := range committees { committeesLen += len(c) } + if aggBits.Len() == 0 { + fmt.Printf("committee_bits: %v, aggregation_bits: %v, slot: %d", att.CommitteeBitsVal(), att.GetAggregationBits(), att.GetData().Slot) + debug.PrintStack() + } if aggBits.Len() != uint64(committeesLen) { return nil, fmt.Errorf("bitfield length %d is not equal to committee length %d", aggBits.Len(), committeesLen) }