Skip to content
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
54ee83a
Ignore subset aggregates
terencechain May 10, 2022
9d8f6d6
Add test
terencechain May 17, 2022
9735917
Merge branch 'develop' into ignore-subset-aggregate
terencechain May 17, 2022
6fa31aa
Update BUILD.bazel
terencechain May 17, 2022
0e69037
Merge branch 'ignore-subset-aggregate' of github.com:prysmaticlabs/pr…
terencechain May 17, 2022
e1368e9
Update validate_sync_contribution_proof_test.go
terencechain May 17, 2022
677b2e1
Update validate_sync_contribution_proof_test.go
terencechain May 17, 2022
7271561
Merge branch 'ignore-subset-aggregate' of github.com:prysmaticlabs/pr…
terencechain May 17, 2022
b1f7eeb
Don't utilize pooled objects. Use direct caches
terencechain May 18, 2022
dcb6ce0
Merge branch 'develop' into ignore-subset-aggregate
terencechain May 18, 2022
417187f
Handle mainnet/minimal better
terencechain May 18, 2022
31bba12
Merge branch 'develop' into ignore-subset-aggregate
terencechain May 18, 2022
1613754
Revert att changes
terencechain May 19, 2022
ee2c910
Check overlaps before set
terencechain May 20, 2022
18c2fcb
Merge branch 'develop' into ignore-subset-aggregate
terencechain May 20, 2022
2e98bd5
Merge branch 'develop' of github.com:prysmaticlabs/prysm into ignore-…
terencechain May 24, 2022
d18079a
Feedbacks
terencechain May 24, 2022
592db2b
Fixed a copy bug
terencechain May 24, 2022
ae6568e
Merge branch 'develop' of github.com:prysmaticlabs/prysm into ignore-…
terencechain May 24, 2022
78a4a5b
Fixed the same copy bug in committee indices cache
terencechain May 24, 2022
ba75634
Use SafeCopyBytes
terencechain May 24, 2022
58a86fb
Merge branch 'develop' into ignore-subset-aggregate
terencechain May 25, 2022
1307233
Merge refs/heads/develop into ignore-subset-aggregate
prylabs-bulldozer[bot] May 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions beacon-chain/sync/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ type Service struct {
seenSyncContributionCache *lru.Cache
badBlockCache *lru.Cache
badBlockLock sync.RWMutex
syncContributionBitsOverlapLock sync.RWMutex
syncContributionBitsOverlapCache *lru.Cache
signatureChan chan *signatureVerifier
}

Expand Down Expand Up @@ -221,6 +223,7 @@ func (s *Service) initCaches() {
s.seenUnAggregatedAttestationCache = lruwrpr.New(seenUnaggregatedAttSize)
s.seenSyncMessageCache = lruwrpr.New(seenSyncMsgSize)
s.seenSyncContributionCache = lruwrpr.New(seenSyncContributionSize)
s.syncContributionBitsOverlapCache = lruwrpr.New(seenSyncContributionSize)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simplicity, I opted in same cache size as seenSyncContributionSize, open for other suggestions

s.seenExitCache = lruwrpr.New(seenExitSize)
s.seenAttesterSlashingCache = make(map[uint64]bool)
s.seenProposerSlashingCache = lruwrpr.New(seenProposerSlashingSize)
Expand Down
4 changes: 1 addition & 3 deletions beacon-chain/sync/validate_beacon_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,7 @@ func (s *Service) setSeenCommitteeIndicesSlot(slot types.Slot, committeeID types
s.seenUnAggregatedAttestationLock.Lock()
defer s.seenUnAggregatedAttestationLock.Unlock()
b := append(bytesutil.Bytes32(uint64(slot)), bytesutil.Bytes32(uint64(committeeID))...)
var bits []byte
copy(bits, aggregateBits)
b = append(b, bits...)
b = append(b, bytesutil.SafeCopyBytes(aggregateBits)...)
s.seenUnAggregatedAttestationCache.Add(string(b), true)
}

Expand Down
27 changes: 27 additions & 0 deletions beacon-chain/sync/validate_beacon_attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,30 @@ func TestServiceValidateCommitteeIndexBeaconAttestation_Optimistic(t *testing.T)
valid := res == pubsub.ValidationIgnore
assert.Equal(t, true, valid, "Should have ignore this message")
}

func TestService_setSeenCommitteeIndicesSlot(t *testing.T) {
chainService := &mockChain.ChainService{
Genesis: time.Now(),
ValidatorsRoot: [32]byte{'A'},
}
s := NewService(context.Background(), WithP2P(p2ptest.NewTestP2P(t)), WithStateNotifier(chainService.StateNotifier()))
s.initCaches()

// Empty cache
b0 := []byte{9} // 1001
require.Equal(t, false, s.hasSeenCommitteeIndicesSlot(0, 0, b0))

// Cache some entries but same key
s.setSeenCommitteeIndicesSlot(0, 0, b0)
require.Equal(t, true, s.hasSeenCommitteeIndicesSlot(0, 0, b0))
b1 := []byte{14} // 1110
s.setSeenCommitteeIndicesSlot(0, 0, b1)
require.Equal(t, true, s.hasSeenCommitteeIndicesSlot(0, 0, b0))
require.Equal(t, true, s.hasSeenCommitteeIndicesSlot(0, 0, b1))

// Cache some entries with diff keys
s.setSeenCommitteeIndicesSlot(1, 2, b1)
require.Equal(t, false, s.hasSeenCommitteeIndicesSlot(1, 0, b1))
require.Equal(t, false, s.hasSeenCommitteeIndicesSlot(0, 2, b1))
require.Equal(t, true, s.hasSeenCommitteeIndicesSlot(1, 2, b1))
}
78 changes: 76 additions & 2 deletions beacon-chain/sync/validate_sync_contribution_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ func (s *Service) validateSyncContributionAndProof(ctx context.Context, pid peer
return result, err
}

s.setSyncContributionIndexSlotSeen(m.Message.Contribution.Slot, m.Message.AggregatorIndex, types.CommitteeIndex(m.Message.Contribution.SubcommitteeIndex))
con := m.Message.Contribution
if err := s.setSyncContributionBits(con); err != nil {
return pubsub.ValidationIgnore, err
}
s.setSyncContributionIndexSlotSeen(con.Slot, m.Message.AggregatorIndex, types.CommitteeIndex(con.SubcommitteeIndex))

msg.ValidatorData = m

Expand Down Expand Up @@ -149,7 +153,15 @@ func rejectEmptyContribution(m *ethpb.SignedContributionAndProof) validationFn {

func (s *Service) ignoreSeenSyncContribution(m *ethpb.SignedContributionAndProof) validationFn {
return func(ctx context.Context) (pubsub.ValidationResult, error) {
seen := s.hasSeenSyncContributionIndexSlot(m.Message.Contribution.Slot, m.Message.AggregatorIndex, types.CommitteeIndex(m.Message.Contribution.SubcommitteeIndex))
c := m.Message.Contribution
seen, err := s.hasSeenSyncContributionBits(c)
if err != nil {
return pubsub.ValidationIgnore, err
}
if seen {
return pubsub.ValidationIgnore, nil
}
seen = s.hasSeenSyncContributionIndexSlot(c.Slot, m.Message.AggregatorIndex, types.CommitteeIndex(c.SubcommitteeIndex))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is copying ignoreCached

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I removed ignoreCached. I could also use ignoreCached here too if you prefer that

if seen {
return pubsub.ValidationIgnore, nil
}
Expand Down Expand Up @@ -318,6 +330,68 @@ func (s *Service) setSyncContributionIndexSlotSeen(slot types.Slot, aggregatorIn
s.seenSyncContributionCache.Add(string(b), true)
}

// Set sync contribution's slot, root, committee index and bits.
func (s *Service) setSyncContributionBits(c *ethpb.SyncCommitteeContribution) error {
s.syncContributionBitsOverlapLock.Lock()
defer s.syncContributionBitsOverlapLock.Unlock()
// Copying due to how pb unmarshalling is carried out, prevent mutation.
b := append(bytesutil.SafeCopyBytes(c.BlockRoot), bytesutil.Bytes32(uint64(c.Slot))...)
b = append(b, bytesutil.Bytes32(c.SubcommitteeIndex)...)
v, ok := s.syncContributionBitsOverlapCache.Get(string(b))
if !ok {
s.syncContributionBitsOverlapCache.Add(string(b), [][]byte{c.AggregationBits.Bytes()})
return nil
}
bitsList, ok := v.([][]byte)
if !ok {
return errors.New("could not covert cached value to []bitfield.Bitvector")
}
has, err := bitListOverlaps(bitsList, c.AggregationBits)
if err != nil {
return err
}
if has {
return nil
}
s.syncContributionBitsOverlapCache.Add(string(b), append(bitsList, c.AggregationBits.Bytes()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to check whether it contains a bitlist that already contains over what the contribution has.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks!

return nil
}

// Check sync contribution bits don't have an overlap with one's in cache.
func (s *Service) hasSeenSyncContributionBits(c *ethpb.SyncCommitteeContribution) (bool, error) {
s.syncContributionBitsOverlapLock.RLock()
defer s.syncContributionBitsOverlapLock.RUnlock()
b := append(c.BlockRoot, bytesutil.Bytes32(uint64(c.Slot))...)
b = append(b, bytesutil.Bytes32(c.SubcommitteeIndex)...)
v, ok := s.syncContributionBitsOverlapCache.Get(string(b))
if !ok {
return false, nil
}
bitsList, ok := v.([][]byte)
if !ok {
return false, errors.New("could not covert cached value to []bitfield.Bitvector128")
}
return bitListOverlaps(bitsList, c.AggregationBits.Bytes())
}

// bitListOverlaps returns true if there's an overlap between two bitlists.
func bitListOverlaps(bitLists [][]byte, b []byte) (bool, error) {
for _, bitList := range bitLists {
if bitList == nil {
return false, errors.New("nil bitfield")
}
bl := ethpb.ConvertToSyncContributionBitVector(bitList)
overlaps, err := bl.Overlaps(ethpb.ConvertToSyncContributionBitVector(b))
if err != nil {
return false, err
}
if overlaps {
return true, nil
}
}
return false, nil
}

// verifySyncSelectionData verifies that the provided sync contribution has a valid
// selection proof.
func (s *Service) verifySyncSelectionData(ctx context.Context, m *ethpb.ContributionAndProof) error {
Expand Down
118 changes: 118 additions & 0 deletions beacon-chain/sync/validate_sync_contribution_proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,3 +1095,121 @@ func syncSelectionProofSigningRoot(st state.BeaconState, slot types.Slot, comIdx
selectionData := &ethpb.SyncAggregatorSelectionData{Slot: slot, SubcommitteeIndex: uint64(comIdx)}
return signing.ComputeSigningRoot(selectionData, dom)
}

func TestService_setSyncContributionIndexSlotSeen(t *testing.T) {
chainService := &mockChain.ChainService{
Genesis: time.Now(),
ValidatorsRoot: [32]byte{'A'},
}
s := NewService(context.Background(), WithP2P(mockp2p.NewTestP2P(t)), WithStateNotifier(chainService.StateNotifier()))
s.initCaches()

// Empty cache
b0 := bitfield.NewBitvector128()
b0.SetBitAt(0, true)
has, err := s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
AggregationBits: b0,
})
require.NoError(t, err)
require.Equal(t, false, has)

// Cache with entries but same key
require.NoError(t, s.setSyncContributionBits(&ethpb.SyncCommitteeContribution{
AggregationBits: b0,
}))
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
AggregationBits: b0,
})
require.NoError(t, err)
require.Equal(t, true, has)
b1 := bitfield.NewBitvector128()
b1.SetBitAt(1, true)
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
AggregationBits: b1,
})
require.NoError(t, err)
require.Equal(t, false, has)
b2 := bitfield.NewBitvector128()
b2.SetBitAt(1, true)
b2.SetBitAt(2, true)
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
AggregationBits: b2,
})
require.NoError(t, err)
require.Equal(t, false, has)
b2.SetBitAt(0, true)
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
AggregationBits: b2,
})
require.NoError(t, err)
require.Equal(t, true, has)

// Make sure set doesn't contain existing overlaps
require.Equal(t, 1, s.syncContributionBitsOverlapCache.Len())

// Cache with entries but different key
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 1,
SubcommitteeIndex: 2,
BlockRoot: []byte{'A'},
AggregationBits: b0,
})
require.NoError(t, err)
require.Equal(t, false, has)
require.NoError(t, s.setSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 1,
SubcommitteeIndex: 2,
BlockRoot: []byte{'A'},
AggregationBits: b2,
}))
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 1,
SubcommitteeIndex: 2,
BlockRoot: []byte{'A'},
AggregationBits: b0,
})
require.NoError(t, err)
require.Equal(t, true, has)
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 1,
SubcommitteeIndex: 2,
BlockRoot: []byte{'A'},
AggregationBits: b1,
})
require.NoError(t, err)
require.Equal(t, true, has)
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 1,
SubcommitteeIndex: 2,
BlockRoot: []byte{'A'},
AggregationBits: b2,
})
require.NoError(t, err)
require.Equal(t, true, has)

// Check invariant with the keys
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 2,
SubcommitteeIndex: 2,
BlockRoot: []byte{'A'},
AggregationBits: b0,
})
require.NoError(t, err)
require.Equal(t, false, has)
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 1,
SubcommitteeIndex: 2,
BlockRoot: []byte{'B'},
AggregationBits: b0,
})
require.NoError(t, err)
require.Equal(t, false, has)
has, err = s.hasSeenSyncContributionBits(&ethpb.SyncCommitteeContribution{
Slot: 1,
SubcommitteeIndex: 3,
BlockRoot: []byte{'A'},
AggregationBits: b0,
})
require.NoError(t, err)
require.Equal(t, false, has)
}
4 changes: 4 additions & 0 deletions proto/prysm/v1alpha1/sync_committee_mainnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ import (
func NewSyncCommitteeAggregationBits() bitfield.Bitvector128 {
return bitfield.NewBitvector128()
}

func ConvertToSyncContributionBitVector(b []byte) bitfield.Bitvector128 {
return b
}
4 changes: 4 additions & 0 deletions proto/prysm/v1alpha1/sync_committee_minimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ import (
func NewSyncCommitteeAggregationBits() bitfield.Bitvector8 {
return bitfield.NewBitvector8()
}

func ConvertToSyncContributionBitVector(b []byte) bitfield.Bitvector8 {
return b
}