From 6e9839f9870d50b37824c6f332724223ea56ab1f Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Thu, 14 May 2020 13:11:28 -0400 Subject: [PATCH] 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 --- beacon-chain/operations/slashings/BUILD.bazel | 2 + beacon-chain/operations/slashings/log.go | 7 ++ beacon-chain/operations/slashings/service.go | 33 ++++++-- .../slashings/service_attester_test.go | 80 ++++++++++++++++++- .../slashings/service_proposer_test.go | 61 +++++++++++++- beacon-chain/rpc/validator/proposer.go | 4 +- beacon-chain/sync/subscriber_test.go | 4 +- slasher/beaconclient/receivers.go | 2 +- 8 files changed, 178 insertions(+), 15 deletions(-) create mode 100644 beacon-chain/operations/slashings/log.go diff --git a/beacon-chain/operations/slashings/BUILD.bazel b/beacon-chain/operations/slashings/BUILD.bazel index 9b03d685f9b..508fb080b7d 100644 --- 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 new file mode 100644 index 00000000000..7c2a498e5db --- /dev/null +++ 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 index fc15ed635b2..e943f73a693 100644 --- 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 index 492a1fdaa3f..aafa6f10d29 100644 --- a/beacon-chain/operations/slashings/service_attester_test.go +++ b/beacon-chain/operations/slashings/service_attester_test.go @@ -475,7 +475,7 @@ 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) } @@ -483,6 +483,82 @@ func TestPool_PendingAttesterSlashings(t *testing.T) { } } +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 index 14c797d7e6f..b2616e4bed1 100644 --- 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 index 287b6df0afb..524959e0f54 100644 --- 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 index 5d058dca0dd..1045661a5e4 100644 --- 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 index ec4b54e9e61..6cbd0c5fafd 100644 --- 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")