Validate slashings upon pool exit (#5848)

* Validate slashings upon pool exit * Change in implementation * Fix test * Merge branch 'master' into validate-slashing * Merge refs/heads/master into validate-slashing * Merge refs/heads/master into validate-slashing * Merge refs/heads/master into validate-slashing * Undo e2e changes * Merge branch 'validate-slashing' of github.com:prysmaticlabs/prysm into validate-slashing * Merge refs/heads/master into validate-slashing
prysmaticlabs · May 14, 2020 · 6e9839f · 6e9839f
1 parent f6d8633
commit 6e9839f
Show file tree

Hide file tree

Showing 8 changed files with 178 additions and 15 deletions.
diff --git a/beacon-chain/operations/slashings/BUILD.bazel b/beacon-chain/operations/slashings/BUILD.bazel
@@ -5,6 +5,7 @@ go_library(
     name = "go_default_library",
     srcs = [
         "doc.go",
+        "log.go",
         "metrics.go",
         "service.go",
         "types.go",
@@ -21,6 +22,7 @@ go_library(
         "@com_github_prometheus_client_golang//prometheus:go_default_library",
         "@com_github_prometheus_client_golang//prometheus/promauto:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library",
+        "@com_github_sirupsen_logrus//:go_default_library",
         "@io_opencensus_go//trace:go_default_library",
     ],
 )

diff --git a/beacon-chain/operations/slashings/log.go b/beacon-chain/operations/slashings/log.go
@@ -0,0 +1,7 @@
+package slashings
+
+import (
+	"github.com/sirupsen/logrus"
+)
+
+var log = logrus.WithField("prefix", "pool/slashings")
diff --git a/beacon-chain/operations/slashings/service.go b/beacon-chain/operations/slashings/service.go
@@ -26,7 +26,7 @@ func NewPool() *Pool {
 
 // PendingAttesterSlashings returns attester slashings that are able to be included into a block.
 // This method will not return more than the block enforced MaxAttesterSlashings.
-func (p *Pool) PendingAttesterSlashings(ctx context.Context) []*ethpb.AttesterSlashing {
+func (p *Pool) PendingAttesterSlashings(ctx context.Context, state *beaconstate.BeaconState) []*ethpb.AttesterSlashing {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 	ctx, span := trace.StartSpan(ctx, "operations.PendingAttesterSlashing")
@@ -37,12 +37,19 @@ func (p *Pool) PendingAttesterSlashings(ctx context.Context) []*ethpb.AttesterSl
 
 	included := make(map[uint64]bool)
 	pending := make([]*ethpb.AttesterSlashing, 0, params.BeaconConfig().MaxAttesterSlashings)
-	for i, slashing := range p.pendingAttesterSlashing {
-		if i >= int(params.BeaconConfig().MaxAttesterSlashings) {
+	for i := 0; i < len(p.pendingAttesterSlashing); i++ {
+		slashing := p.pendingAttesterSlashing[i]
+		if len(pending) >= int(params.BeaconConfig().MaxAttesterSlashings) {
 			break
 		}
-		if included[slashing.validatorToSlash] {
+		valid, err := p.validatorSlashingPreconditionCheck(state, slashing.validatorToSlash)
+		if err != nil {
+			log.WithError(err).Error("could not validate attester slashing")
+			continue
+		}
+		if included[slashing.validatorToSlash] || !valid {
 			p.pendingAttesterSlashing = append(p.pendingAttesterSlashing[:i], p.pendingAttesterSlashing[i+1:]...)
+			i--
 			continue
 		}
 		attSlashing := slashing.attesterSlashing
@@ -59,7 +66,7 @@ func (p *Pool) PendingAttesterSlashings(ctx context.Context) []*ethpb.AttesterSl
 
 // PendingProposerSlashings returns proposer slashings that are able to be included into a block.
 // This method will not return more than the block enforced MaxProposerSlashings.
-func (p *Pool) PendingProposerSlashings(ctx context.Context) []*ethpb.ProposerSlashing {
+func (p *Pool) PendingProposerSlashings(ctx context.Context, state *beaconstate.BeaconState) []*ethpb.ProposerSlashing {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 	ctx, span := trace.StartSpan(ctx, "operations.PendingProposerSlashing")
@@ -69,10 +76,22 @@ func (p *Pool) PendingProposerSlashings(ctx context.Context) []*ethpb.ProposerSl
 	numPendingProposerSlashings.Set(float64(len(p.pendingProposerSlashing)))
 
 	pending := make([]*ethpb.ProposerSlashing, 0, params.BeaconConfig().MaxProposerSlashings)
-	for i, slashing := range p.pendingProposerSlashing {
-		if i >= int(params.BeaconConfig().MaxProposerSlashings) {
+	for i := 0; i < len(p.pendingProposerSlashing); i++ {
+		slashing := p.pendingProposerSlashing[i]
+		if len(pending) >= int(params.BeaconConfig().MaxProposerSlashings) {
 			break
 		}
+		valid, err := p.validatorSlashingPreconditionCheck(state, slashing.Header_1.Header.ProposerIndex)
+		if err != nil {
+			log.WithError(err).Error("could not validate proposer slashing")
+			continue
+		}
+		if !valid {
+			p.pendingProposerSlashing = append(p.pendingProposerSlashing[:i], p.pendingProposerSlashing[i+1:]...)
+			i--
+			continue
+		}
+
 		pending = append(pending, slashing)
 	}
 	return pending

diff --git a/beacon-chain/operations/slashings/service_attester_test.go b/beacon-chain/operations/slashings/service_attester_test.go
@@ -475,14 +475,90 @@ func TestPool_PendingAttesterSlashings(t *testing.T) {
 				pendingAttesterSlashing: tt.fields.pending,
 			}
 			if got := p.PendingAttesterSlashings(
-				context.Background(),
+				context.Background(), beaconState,
 			); !reflect.DeepEqual(tt.want, got) {
 				t.Errorf("Unexpected return from PendingAttesterSlashings, wanted %v, received %v", tt.want, got)
 			}
 		})
 	}
 }
 
+func TestPool_PendingAttesterSlashings_Slashed(t *testing.T) {
+	type fields struct {
+		pending []*PendingAttesterSlashing
+	}
+	params.SetupTestConfigCleanup(t)
+	conf := params.BeaconConfig()
+	conf.MaxAttesterSlashings = 2
+	params.OverrideBeaconConfig(conf)
+	beaconState, privKeys := testutil.DeterministicGenesisState(t, 64)
+	val, err := beaconState.ValidatorAtIndex(0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	val.Slashed = true
+	if err := beaconState.UpdateValidatorAtIndex(0, val); err != nil {
+		t.Fatal(err)
+	}
+	val, err = beaconState.ValidatorAtIndex(3)
+	if err != nil {
+		t.Fatal(err)
+	}
+	val.Slashed = true
+	if err := beaconState.UpdateValidatorAtIndex(3, val); err != nil {
+		t.Fatal(err)
+	}
+	val, err = beaconState.ValidatorAtIndex(5)
+	if err != nil {
+		t.Fatal(err)
+	}
+	val.Slashed = true
+	if err := beaconState.UpdateValidatorAtIndex(5, val); err != nil {
+		t.Fatal(err)
+	}
+	pendingSlashings := make([]*PendingAttesterSlashing, 20)
+	slashings := make([]*ethpb.AttesterSlashing, 20)
+	for i := 0; i < len(pendingSlashings); i++ {
+		sl, err := testutil.GenerateAttesterSlashingForValidator(beaconState, privKeys[i], uint64(i))
+		if err != nil {
+			t.Fatal(err)
+		}
+		pendingSlashings[i] = &PendingAttesterSlashing{
+			attesterSlashing: sl,
+			validatorToSlash: uint64(i),
+		}
+		slashings[i] = sl
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   []*ethpb.AttesterSlashing
+	}{
+		{
+			name: "Skips slashed validator",
+			fields: fields{
+				pending: pendingSlashings,
+			},
+			want: slashings[1:3],
+		},
+		{
+			name: "Skips gapped slashed validators",
+			fields: fields{
+				pending: pendingSlashings[2:],
+			},
+			want: []*ethpb.AttesterSlashing{slashings[4], slashings[6]},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &Pool{pendingAttesterSlashing: tt.fields.pending}
+			if got := p.PendingAttesterSlashings(context.Background(), beaconState); !reflect.DeepEqual(tt.want, got) {
+				t.Errorf("Unexpected return from PendingAttesterSlashings, \nwanted %v, \nreceived %v", tt.want, got)
+			}
+		})
+	}
+}
+
 func TestPool_PendingAttesterSlashings_NoDuplicates(t *testing.T) {
 	params.SetupTestConfigCleanup(t)
 	conf := params.BeaconConfig()
@@ -510,7 +586,7 @@ func TestPool_PendingAttesterSlashings_NoDuplicates(t *testing.T) {
 	}
 	want := slashings[0:2]
 	if got := p.PendingAttesterSlashings(
-		context.Background(),
+		context.Background(), beaconState,
 	); !reflect.DeepEqual(want, got) {
 		t.Errorf("Unexpected return from PendingAttesterSlashings, wanted %v, received %v", want, got)
 	}

diff --git a/beacon-chain/operations/slashings/service_proposer_test.go b/beacon-chain/operations/slashings/service_proposer_test.go
@@ -432,10 +432,69 @@ func TestPool_PendingProposerSlashings(t *testing.T) {
 				pendingProposerSlashing: tt.fields.pending,
 			}
 			if got := p.PendingProposerSlashings(
-				context.Background(),
+				context.Background(), beaconState,
 			); !reflect.DeepEqual(tt.want, got) {
 				t.Errorf("Unexpected return from PendingProposerSlashings, wanted %v, received %v", tt.want, got)
 			}
 		})
 	}
 }
+
+func TestPool_PendingProposerSlashings_Slashed(t *testing.T) {
+	type fields struct {
+		pending []*ethpb.ProposerSlashing
+	}
+	beaconState, privKeys := testutil.DeterministicGenesisState(t, 64)
+	val, err := beaconState.ValidatorAtIndex(0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	val.Slashed = true
+	if err := beaconState.UpdateValidatorAtIndex(0, val); err != nil {
+		t.Fatal(err)
+	}
+	val, err = beaconState.ValidatorAtIndex(5)
+	if err != nil {
+		t.Fatal(err)
+	}
+	val.Slashed = true
+	if err := beaconState.UpdateValidatorAtIndex(5, val); err != nil {
+		t.Fatal(err)
+	}
+	slashings := make([]*ethpb.ProposerSlashing, 32)
+	for i := 0; i < len(slashings); i++ {
+		sl, err := testutil.GenerateProposerSlashingForValidator(beaconState, privKeys[i], uint64(i))
+		if err != nil {
+			t.Fatal(err)
+		}
+		slashings[i] = sl
+	}
+	result := make([]*ethpb.ProposerSlashing, 32)
+	copy(result, slashings)
+	tests := []struct {
+		name   string
+		fields fields
+		want   []*ethpb.ProposerSlashing
+	}{
+		{
+			name: "removes slashed",
+			fields: fields{
+				pending: slashings,
+			},
+			want: append(result[1:5], result[6:18]...),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &Pool{
+				pendingProposerSlashing: tt.fields.pending,
+			}
+
+			if got := p.PendingProposerSlashings(
+				context.Background(), beaconState,
+			); !reflect.DeepEqual(tt.want, got) {
+				t.Errorf("Unexpected return from PendingProposerSlashings, \nwanted %v, \nreceived %v", tt.want, got)
+			}
+		})
+	}
+}
diff --git a/beacon-chain/rpc/validator/proposer.go b/beacon-chain/rpc/validator/proposer.go
@@ -97,8 +97,8 @@ func (vs *Server) GetBlock(ctx context.Context, req *ethpb.BlockRequest) (*ethpb
 			Deposits:          deposits,
 			Attestations:      atts,
 			RandaoReveal:      req.RandaoReveal,
-			ProposerSlashings: vs.SlashingsPool.PendingProposerSlashings(ctx),
-			AttesterSlashings: vs.SlashingsPool.PendingAttesterSlashings(ctx),
+			ProposerSlashings: vs.SlashingsPool.PendingProposerSlashings(ctx, head),
+			AttesterSlashings: vs.SlashingsPool.PendingAttesterSlashings(ctx, head),
 			VoluntaryExits:    vs.ExitPool.PendingExits(head, req.Slot),
 			Graffiti:          graffiti[:],
 		},

diff --git a/beacon-chain/sync/subscriber_test.go b/beacon-chain/sync/subscriber_test.go
@@ -125,7 +125,7 @@ func TestSubscribe_ReceivesAttesterSlashing(t *testing.T) {
 	if testutil.WaitTimeout(&wg, time.Second) {
 		t.Fatal("Did not receive PubSub in 1 second")
 	}
-	as := r.slashingPool.PendingAttesterSlashings(ctx)
+	as := r.slashingPool.PendingAttesterSlashings(ctx, beaconState)
 	if len(as) != 1 {
 		t.Errorf("Expected attester slashing: %v to be added to slashing pool. got: %v", attesterSlashing, as[0])
 	}
@@ -188,7 +188,7 @@ func TestSubscribe_ReceivesProposerSlashing(t *testing.T) {
 	if testutil.WaitTimeout(&wg, time.Second) {
 		t.Fatal("Did not receive PubSub in 1 second")
 	}
-	ps := r.slashingPool.PendingProposerSlashings(ctx)
+	ps := r.slashingPool.PendingProposerSlashings(ctx, beaconState)
 	if len(ps) != 1 {
 		t.Errorf("Expected proposer slashing: %v to be added to slashing pool. got: %v", proposerSlashing, ps[0])
 	}

diff --git a/slasher/beaconclient/receivers.go b/slasher/beaconclient/receivers.go
@@ -104,7 +104,7 @@ func (bs *Service) receiveAttestations(ctx context.Context) {
 					break
 				default:
 					log.WithError(err).Errorf("Could not receive attestations from beacon node. rpc status: %v", e.Code())
-					break
+					return
 				}
 			} else {
 				log.WithError(err).Error("Could not receive attestations from beacon node")