From 662d937ac8c2102baf80e990dce3bea01f14a460 Mon Sep 17 00:00:00 2001 From: Potuz Date: Wed, 27 Mar 2024 11:56:28 -0300 Subject: [PATCH 1/2] Remove check for duplicates in pending attestation queue The current queue will only save 1 unaggregated attestation for a pending block because we wrap the object into a SignedAggregatedAttestationAndProof with a zeroed aggregator. --- .../sync/pending_attestations_queue.go | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/beacon-chain/sync/pending_attestations_queue.go b/beacon-chain/sync/pending_attestations_queue.go index 82b80324a1e9..66d705db64d6 100644 --- a/beacon-chain/sync/pending_attestations_queue.go +++ b/beacon-chain/sync/pending_attestations_queue.go @@ -1,6 +1,7 @@ package sync import ( + "bytes" "context" "encoding/hex" "sync" @@ -157,7 +158,8 @@ func (s *Service) processAttestations(ctx context.Context, attestations []*ethpb // This defines how pending attestations is saved in the map. The key is the // root of the missing block. The value is the list of pending attestations -// that voted for that block root. +// that voted for that block root. The caller of this function is responsible +// for not sending repeated attestations to the pending queue. func (s *Service) savePendingAtt(att *ethpb.SignedAggregateAttestationAndProof) { root := bytesutil.ToBytes32(att.Message.Aggregate.Data.BeaconBlockRoot) @@ -178,17 +180,32 @@ func (s *Service) savePendingAtt(att *ethpb.SignedAggregateAttestationAndProof) s.blkRootToPendingAtts[root] = []*ethpb.SignedAggregateAttestationAndProof{att} return } - - // Skip if the attestation from the same aggregator already exists in the pending queue. + // Skip if the attestation from the same aggregator already exists in + // the pending queue. for _, a := range s.blkRootToPendingAtts[root] { - if a.Message.AggregatorIndex == att.Message.AggregatorIndex { + if attsAreEqual(att, a) { return } } - s.blkRootToPendingAtts[root] = append(s.blkRootToPendingAtts[root], att) } +func attsAreEqual(a, b *ethpb.SignedAggregateAttestationAndProof) bool { + if a.Signature != nil { + return b.Signature != nil && a.Message.AggregatorIndex == b.Message.AggregatorIndex + } + if b.Signature != nil { + return false + } + if a.Message.Aggregate.Data.Slot != b.Message.Aggregate.Data.Slot { + return false + } + if a.Message.Aggregate.Data.CommitteeIndex != b.Message.Aggregate.Data.CommitteeIndex { + return false + } + return bytes.Equal(a.Message.Aggregate.AggregationBits, b.Message.Aggregate.AggregationBits) +} + // This validates the pending attestations in the queue are still valid. // If not valid, a node will remove it in the queue in place. The validity // check specifies the pending attestation could not fall one epoch behind From 16f6cbd2ad1f0eaef62b80f69d1164271d68b9cd Mon Sep 17 00:00:00 2001 From: Potuz Date: Wed, 27 Mar 2024 13:33:38 -0300 Subject: [PATCH 2/2] fix tests --- beacon-chain/sync/pending_attestations_queue_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon-chain/sync/pending_attestations_queue_test.go b/beacon-chain/sync/pending_attestations_queue_test.go index 88288f310289..3f32b8fa8d51 100644 --- a/beacon-chain/sync/pending_attestations_queue_test.go +++ b/beacon-chain/sync/pending_attestations_queue_test.go @@ -399,7 +399,7 @@ func TestValidatePendingAtts_CanPruneOldAtts(t *testing.T) { assert.Equal(t, 0, len(s.blkRootToPendingAtts), "Did not delete block keys") } -func TestValidatePendingAtts_NoDuplicatingAggregatorIndex(t *testing.T) { +func TestValidatePendingAtts_NoDuplicatingAtts(t *testing.T) { s := &Service{ blkRootToPendingAtts: make(map[[32]byte][]*ethpb.SignedAggregateAttestationAndProof), } @@ -420,7 +420,7 @@ func TestValidatePendingAtts_NoDuplicatingAggregatorIndex(t *testing.T) { Message: ðpb.AggregateAttestationAndProof{ AggregatorIndex: 2, Aggregate: ðpb.Attestation{ - Data: ðpb.AttestationData{Slot: 3, BeaconBlockRoot: r2[:]}}}}) + Data: ðpb.AttestationData{Slot: 2, BeaconBlockRoot: r2[:]}}}}) assert.Equal(t, 1, len(s.blkRootToPendingAtts[r1]), "Did not save pending atts") assert.Equal(t, 1, len(s.blkRootToPendingAtts[r2]), "Did not save pending atts")