Fix Attester Slashing Validation In Gossip (#12295)

* fix slashing checks

* fix to make it more performant

* gaz

* fix up

* potuz's comment

* potuz's comment

* fix cache

* change index in test for better case

* gaz

---------

Co-authored-by: Potuz <potuz@prysmaticlabs.com>

beacon-chain/sync/BUILD.bazel

@@ -211,6 +211,7 @@ go_test(
         "//consensus-types/primitives:go_default_library",
         "//consensus-types/wrapper:go_default_library",
         "//container/leaky-bucket:go_default_library",
+        "//container/slice:go_default_library",
         "//crypto/bls:go_default_library",
         "//crypto/rand:go_default_library",
         "//encoding/bytesutil:go_default_library",

beacon-chain/sync/validate_attester_slashing.go

@@ -5,10 +5,14 @@ import (
 
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/core/blocks"
+	"github.com/prysmaticlabs/prysm/v4/beacon-chain/core/helpers"
+	"github.com/prysmaticlabs/prysm/v4/consensus-types/primitives"
 	"github.com/prysmaticlabs/prysm/v4/container/slice"
 	"github.com/prysmaticlabs/prysm/v4/monitoring/tracing"
 	ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
+	"github.com/prysmaticlabs/prysm/v4/time/slots"
 	"go.opencensus.io/trace"
 )
 
@@ -39,10 +43,11 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg
 		return pubsub.ValidationReject, errWrongMessage
 	}
 
-	if slashing == nil || slashing.Attestation_1 == nil || slashing.Attestation_2 == nil {
+	slashedVals := blocks.SlashableAttesterIndices(slashing)
+	if slashedVals == nil {
 		return pubsub.ValidationReject, errNilMessage
 	}
-	if s.hasSeenAttesterSlashingIndices(slashing.Attestation_1.AttestingIndices, slashing.Attestation_2.AttestingIndices) {
+	if s.hasSeenAttesterSlashingIndices(slashedVals) {
 		return pubsub.ValidationIgnore, nil
 	}
 
@@ -53,7 +58,20 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg
 	if err := blocks.VerifyAttesterSlashing(ctx, headState, slashing); err != nil {
 		return pubsub.ValidationReject, err
 	}
-
+	isSlashable := false
+	for _, v := range slashedVals {
+		val, err := headState.ValidatorAtIndexReadOnly(primitives.ValidatorIndex(v))
+		if err != nil {
+			return pubsub.ValidationIgnore, err
+		}
+		if helpers.IsSlashableValidator(val.ActivationEpoch(), val.WithdrawableEpoch(), val.Slashed(), slots.ToEpoch(headState.Slot())) {
+			isSlashable = true
+			break
+		}
+	}
+	if !isSlashable {
+		return pubsub.ValidationReject, errors.Errorf("none of the validators are slashable: %v", slashedVals)
+	}
 	s.cfg.chain.ReceiveAttesterSlashing(ctx, slashing)
 
 	msg.ValidatorData = slashing // Used in downstream subscriber
@@ -61,9 +79,7 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg
 }
 
 // Returns true if the node has already received a valid attester slashing with the attesting indices.
-func (s *Service) hasSeenAttesterSlashingIndices(indices1, indices2 []uint64) bool {
-	slashableIndices := slice.IntersectionUint64(indices1, indices2)
-
+func (s *Service) hasSeenAttesterSlashingIndices(slashableIndices []uint64) bool {
 	s.seenAttesterSlashingLock.RLock()
 	defer s.seenAttesterSlashingLock.RUnlock()
 

beacon-chain/sync/validate_attester_slashing_test.go

@@ -18,6 +18,7 @@ import (
 	mockSync "github.com/prysmaticlabs/prysm/v4/beacon-chain/sync/initial-sync/testing"
 	"github.com/prysmaticlabs/prysm/v4/config/params"
 	"github.com/prysmaticlabs/prysm/v4/consensus-types/primitives"
+	"github.com/prysmaticlabs/prysm/v4/container/slice"
 	"github.com/prysmaticlabs/prysm/v4/crypto/bls"
 	ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/v4/testing/assert"
@@ -111,6 +112,61 @@ func TestValidateAttesterSlashing_ValidSlashing(t *testing.T) {
 	assert.NotNil(t, msg.ValidatorData, "Decoded message was not set on the message validator data")
 }
 
+func TestValidateAttesterSlashing_InvalidSlashing_WithdrawableEpoch(t *testing.T) {
+	p := p2ptest.NewTestP2P(t)
+	ctx := context.Background()
+
+	slashing, s := setupValidAttesterSlashing(t)
+	// Set only one of the  validators as withdrawn
+	vals := s.Validators()
+	vals[1].WithdrawableEpoch = primitives.Epoch(1)
+
+	require.NoError(t, s.SetValidators(vals))
+
+	r := &Service{
+		cfg: &config{
+			p2p:         p,
+			chain:       &mock.ChainService{State: s, Genesis: time.Now()},
+			initialSync: &mockSync.Sync{IsSyncing: false},
+		},
+		seenAttesterSlashingCache: make(map[uint64]bool),
+		subHandler:                newSubTopicHandler(),
+	}
+
+	buf := new(bytes.Buffer)
+	_, err := p.Encoding().EncodeGossip(buf, slashing)
+	require.NoError(t, err)
+
+	topic := p2p.GossipTypeMapping[reflect.TypeOf(slashing)]
+	d, err := r.currentForkDigest()
+	assert.NoError(t, err)
+	topic = r.addDigestToTopic(topic, d)
+	msg := &pubsub.Message{
+		Message: &pubsubpb.Message{
+			Data:  buf.Bytes(),
+			Topic: &topic,
+		},
+	}
+	res, err := r.validateAttesterSlashing(ctx, "foobar", msg)
+	assert.NoError(t, err)
+	valid := res == pubsub.ValidationAccept
+
+	assert.Equal(t, true, valid, "Rejected Validation")
+
+	// Set all validators as withdrawn.
+	vals = s.Validators()
+	for _, vv := range vals {
+		vv.WithdrawableEpoch = primitives.Epoch(1)
+	}
+
+	require.NoError(t, s.SetValidators(vals))
+	res, err = r.validateAttesterSlashing(ctx, "foobar", msg)
+	assert.ErrorContains(t, "none of the validators are slashable", err)
+	invalid := res == pubsub.ValidationReject
+
+	assert.Equal(t, true, invalid, "Passed Validation")
+}
+
 func TestValidateAttesterSlashing_CanFilter(t *testing.T) {
 	p := p2ptest.NewTestP2P(t)
 	ctx := context.Background()
@@ -290,6 +346,7 @@ func TestSeenAttesterSlashingIndices(t *testing.T) {
 			seenAttesterSlashingCache: map[uint64]bool{},
 		}
 		r.setAttesterSlashingIndicesSeen(tc.saveIndices1, tc.saveIndices2)
-		assert.Equal(t, tc.seen, r.hasSeenAttesterSlashingIndices(tc.checkIndices1, tc.checkIndices2))
+		slashedVals := slice.IntersectionUint64(tc.checkIndices1, tc.checkIndices2)
+		assert.Equal(t, tc.seen, r.hasSeenAttesterSlashingIndices(slashedVals))
 	}
 }