diff --git a/beacon-chain/operations/slashings/BUILD.bazel b/beacon-chain/operations/slashings/BUILD.bazel index bcea792ed694..ebb185cd9da9 100644 --- a/beacon-chain/operations/slashings/BUILD.bazel +++ b/beacon-chain/operations/slashings/BUILD.bazel @@ -11,13 +11,16 @@ go_library( importpath = "github.com/prysmaticlabs/prysm/beacon-chain/operations/slashings", visibility = ["//beacon-chain:__subpackages__"], deps = [ + "//beacon-chain/core/blocks:go_default_library", "//beacon-chain/core/helpers:go_default_library", "//beacon-chain/state:go_default_library", "//shared/params:go_default_library", "//shared/sliceutil:go_default_library", + "@com_github_pkg_errors//:go_default_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", + "@io_opencensus_go//trace:go_default_library", ], ) @@ -30,9 +33,9 @@ go_test( ], embed = [":go_default_library"], deps = [ - "//beacon-chain/state:go_default_library", - "//proto/beacon/p2p/v1:go_default_library", + "//beacon-chain/core/helpers:go_default_library", "//shared/params:go_default_library", + "//shared/testutil:go_default_library", "@com_github_gogo_protobuf//proto:go_default_library", "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library", ], diff --git a/beacon-chain/operations/slashings/metrics.go b/beacon-chain/operations/slashings/metrics.go index 1ecaea787f18..471b623e98f3 100644 --- a/beacon-chain/operations/slashings/metrics.go +++ b/beacon-chain/operations/slashings/metrics.go @@ -6,6 +6,12 @@ import ( ) var ( + numPendingAttesterSlashingFailedSigVerify = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "pending_attester_slashing_fail_sig_verify_total", + Help: "Times an pending attester slashing fails sig verification", + }, + ) numPendingAttesterSlashings = promauto.NewGauge( prometheus.GaugeOpts{ Name: "num_pending_attester_slashings", @@ -24,6 +30,12 @@ var ( Help: "Times an attester slashing for an already slashed validator is received", }, ) + numPendingProposerSlashingFailedSigVerify = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "pending_proposer_slashing_fail_sig_verify_total", + Help: "Times an pending proposer slashing fails sig verification", + }, + ) numPendingProposerSlashings = promauto.NewGauge( prometheus.GaugeOpts{ Name: "num_pending_proposer_slashings", diff --git a/beacon-chain/operations/slashings/service.go b/beacon-chain/operations/slashings/service.go index f9543e3bfd24..b3a011ab808d 100644 --- a/beacon-chain/operations/slashings/service.go +++ b/beacon-chain/operations/slashings/service.go @@ -1,15 +1,18 @@ package slashings import ( - "errors" + "context" "fmt" "sort" + "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" + "github.com/prysmaticlabs/prysm/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/sliceutil" + "go.opencensus.io/trace" ) // NewPool returns an initialized attester slashing and proposer slashing pool. @@ -23,9 +26,11 @@ 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() []*ethpb.AttesterSlashing { +func (p *Pool) PendingAttesterSlashings(ctx context.Context) []*ethpb.AttesterSlashing { p.lock.RLock() defer p.lock.RUnlock() + ctx, span := trace.StartSpan(ctx, "operations.PendingAttesterSlashing") + defer span.End() // Update prom metric. numPendingAttesterSlashings.Set(float64(len(p.pendingAttesterSlashing))) @@ -37,6 +42,7 @@ func (p *Pool) PendingAttesterSlashings() []*ethpb.AttesterSlashing { break } if included[slashing.validatorToSlash] { + p.pendingAttesterSlashing = append(p.pendingAttesterSlashing[:i], p.pendingAttesterSlashing[i+1:]...) continue } attSlashing := slashing.attesterSlashing @@ -44,6 +50,7 @@ func (p *Pool) PendingAttesterSlashings() []*ethpb.AttesterSlashing { for _, idx := range slashedVal { included[idx] = true } + pending = append(pending, attSlashing) } @@ -52,9 +59,11 @@ func (p *Pool) PendingAttesterSlashings() []*ethpb.AttesterSlashing { // 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() []*ethpb.ProposerSlashing { +func (p *Pool) PendingProposerSlashings(ctx context.Context) []*ethpb.ProposerSlashing { p.lock.RLock() defer p.lock.RUnlock() + ctx, span := trace.StartSpan(ctx, "operations.PendingProposerSlashing") + defer span.End() // Update prom metric. numPendingProposerSlashings.Set(float64(len(p.pendingProposerSlashing))) @@ -71,9 +80,20 @@ func (p *Pool) PendingProposerSlashings() []*ethpb.ProposerSlashing { // InsertAttesterSlashing into the pool. This method is a no-op if the attester slashing already exists in the pool, // has been included into a block recently, or the validator is already exited. -func (p *Pool) InsertAttesterSlashing(state *beaconstate.BeaconState, slashing *ethpb.AttesterSlashing) error { +func (p *Pool) InsertAttesterSlashing( + ctx context.Context, + state *beaconstate.BeaconState, + slashing *ethpb.AttesterSlashing, +) error { p.lock.Lock() defer p.lock.Unlock() + ctx, span := trace.StartSpan(ctx, "operations.InsertAttesterSlashing") + defer span.End() + + if err := blocks.VerifyAttesterSlashing(ctx, state, slashing); err != nil { + numPendingAttesterSlashingFailedSigVerify.Inc() + return errors.Wrap(err, "could not verify attester slashing") + } slashedVal := sliceutil.IntersectionUint64(slashing.Attestation_1.AttestingIndices, slashing.Attestation_2.AttestingIndices) for _, val := range slashedVal { @@ -115,9 +135,21 @@ func (p *Pool) InsertAttesterSlashing(state *beaconstate.BeaconState, slashing * // InsertProposerSlashing into the pool. This method is a no-op if the pending slashing already exists, // has been included recently, the validator is already exited, or the validator was already slashed. -func (p *Pool) InsertProposerSlashing(state *beaconstate.BeaconState, slashing *ethpb.ProposerSlashing) error { +func (p *Pool) InsertProposerSlashing( + ctx context.Context, + state *beaconstate.BeaconState, + slashing *ethpb.ProposerSlashing, +) error { p.lock.Lock() defer p.lock.Unlock() + ctx, span := trace.StartSpan(ctx, "operations.InsertProposerSlashing") + defer span.End() + + if err := blocks.VerifyProposerSlashing(state, slashing); err != nil { + numPendingAttesterSlashingFailedSigVerify.Inc() + return errors.Wrap(err, "could not verify proposer slashing") + } + idx := slashing.ProposerIndex ok, err := p.validatorSlashingPreconditionCheck(state, idx) if err != nil { diff --git a/beacon-chain/operations/slashings/service_attester_test.go b/beacon-chain/operations/slashings/service_attester_test.go index 3ca0067afeae..807f2b3a692b 100644 --- a/beacon-chain/operations/slashings/service_attester_test.go +++ b/beacon-chain/operations/slashings/service_attester_test.go @@ -1,15 +1,16 @@ package slashings import ( + "context" "reflect" "strings" "testing" "github.com/gogo/protobuf/proto" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" - beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state" - p2ppb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/testutil" ) func attesterSlashingForValIdx(valIdx ...uint64) *ethpb.AttesterSlashing { @@ -30,25 +31,6 @@ func pendingSlashingForValIdx(valIdx ...uint64) *PendingAttesterSlashing { } } -func generateNPendingSlashings(n uint64) []*PendingAttesterSlashing { - pendingAttSlashings := make([]*PendingAttesterSlashing, n) - for i := uint64(0); i < n; i++ { - pendingAttSlashings[i] = &PendingAttesterSlashing{ - attesterSlashing: attesterSlashingForValIdx(i), - validatorToSlash: i, - } - } - return pendingAttSlashings -} - -func generateNAttSlashings(n uint64) []*ethpb.AttesterSlashing { - attSlashings := make([]*ethpb.AttesterSlashing, n) - for i := uint64(0); i < n; i++ { - attSlashings[i] = attesterSlashingForValIdx(i) - } - return attSlashings -} - func TestPool_InsertAttesterSlashing(t *testing.T) { type fields struct { pending []*PendingAttesterSlashing @@ -57,8 +39,44 @@ func TestPool_InsertAttesterSlashing(t *testing.T) { err string } type args struct { - slashing *ethpb.AttesterSlashing + slashings []*ethpb.AttesterSlashing + } + + beaconState, privKeys := testutil.DeterministicGenesisState(t, 64) + 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 } + if err := beaconState.SetSlot(helpers.StartSlot(1)); err != nil { + t.Fatal(err) + } + + // We mark the following validators with some preconditions. + exitedVal, _ := beaconState.ValidatorAtIndex(uint64(2)) + exitedVal.ExitEpoch = 0 + futureExitedVal, _ := beaconState.ValidatorAtIndex(uint64(4)) + futureExitedVal.ExitEpoch = 17 + slashedVal, _ := beaconState.ValidatorAtIndex(uint64(5)) + slashedVal.Slashed = true + if err := beaconState.UpdateValidatorAtIndex(uint64(2), exitedVal); err != nil { + t.Fatal(err) + } + if err := beaconState.UpdateValidatorAtIndex(uint64(4), futureExitedVal); err != nil { + t.Fatal(err) + } + if err := beaconState.UpdateValidatorAtIndex(uint64(5), slashedVal); err != nil { + t.Fatal(err) + } + tests := []struct { name string fields fields @@ -73,150 +91,71 @@ func TestPool_InsertAttesterSlashing(t *testing.T) { included: make(map[uint64]bool), }, args: args{ - slashing: attesterSlashingForValIdx(1), + slashings: slashings[0:1], }, want: []*PendingAttesterSlashing{ { - attesterSlashing: attesterSlashingForValIdx(1), - validatorToSlash: 1, - }, - }, - }, - { - name: "Empty list two validators slashed", - fields: fields{ - pending: make([]*PendingAttesterSlashing, 0), - included: make(map[uint64]bool), - }, - args: args{ - slashing: ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{0, 1}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{0, 1}, - }, - }, - }, - want: []*PendingAttesterSlashing{ - { - attesterSlashing: ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{0, 1}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{0, 1}, - }, - }, + attesterSlashing: slashings[0], validatorToSlash: 0, }, - { - attesterSlashing: ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{0, 1}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{0, 1}, - }, - }, - validatorToSlash: 1, - }, }, }, { - name: "Empty list two validators slashed out of three", + name: "Empty list two validators slashed", fields: fields{ pending: make([]*PendingAttesterSlashing, 0), included: make(map[uint64]bool), }, args: args{ - slashing: ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{1, 2, 3}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{1, 3}, - }, - }, - }, - want: []*PendingAttesterSlashing{ - { - attesterSlashing: ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{1, 2, 3}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{1, 3}, - }, - }, - validatorToSlash: 1, - }, - { - attesterSlashing: ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{1, 2, 3}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{1, 3}, - }, - }, - validatorToSlash: 3, - }, + slashings: slashings[0:2], }, + want: pendingSlashings[0:2], }, { name: "Duplicate identical slashing", fields: fields{ pending: []*PendingAttesterSlashing{ - - pendingSlashingForValIdx(1), + pendingSlashings[1], }, included: make(map[uint64]bool), }, args: args{ - slashing: attesterSlashingForValIdx(1), - }, - want: []*PendingAttesterSlashing{ - - pendingSlashingForValIdx(1), + slashings: slashings[1:2], }, + want: pendingSlashings[1:2], }, { - name: "Slashing for exited validator", + name: "Slashing for already slashed validator", fields: fields{ pending: []*PendingAttesterSlashing{}, included: make(map[uint64]bool), }, args: args{ - slashing: attesterSlashingForValIdx(2), + slashings: slashings[5:6], }, want: []*PendingAttesterSlashing{}, }, { - name: "Slashing for futuristic exited validator", + name: "Slashing for exited validator", fields: fields{ pending: []*PendingAttesterSlashing{}, included: make(map[uint64]bool), }, args: args{ - slashing: attesterSlashingForValIdx(4), - }, - want: []*PendingAttesterSlashing{ - pendingSlashingForValIdx(4), + slashings: slashings[2:3], }, + want: []*PendingAttesterSlashing{}, }, { - name: "Slashing for slashed validator", + name: "Slashing for futuristic exited validator", fields: fields{ pending: []*PendingAttesterSlashing{}, included: make(map[uint64]bool), - wantErr: true, - err: "cannot be slashed", }, args: args{ - slashing: attesterSlashingForValIdx(5), + slashings: slashings[4:5], }, - want: []*PendingAttesterSlashing{}, + want: pendingSlashings[4:5], }, { name: "Already included", @@ -227,7 +166,7 @@ func TestPool_InsertAttesterSlashing(t *testing.T) { }, }, args: args{ - slashing: attesterSlashingForValIdx(1), + slashings: slashings[1:2], }, want: []*PendingAttesterSlashing{}, }, @@ -235,36 +174,15 @@ func TestPool_InsertAttesterSlashing(t *testing.T) { name: "Maintains sorted order", fields: fields{ pending: []*PendingAttesterSlashing{ - pendingSlashingForValIdx(0), - pendingSlashingForValIdx(2), + pendingSlashings[0], + pendingSlashings[2], }, included: make(map[uint64]bool), }, args: args{ - slashing: attesterSlashingForValIdx(1), + slashings: slashings[1:2], }, - want: generateNPendingSlashings(3), - }, - } - validators := []*ethpb.Validator{ - { // 0 - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - }, - { // 1 - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - }, - { // 2 - Already exited. - ExitEpoch: 15, - }, - { // 3 - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - }, - { // 4 - Will be exited. - ExitEpoch: 17, - }, - { // 5 - Slashed. - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - Slashed: true, + want: pendingSlashings[0:3], }, } for _, tt := range tests { @@ -273,19 +191,19 @@ func TestPool_InsertAttesterSlashing(t *testing.T) { pendingAttesterSlashing: tt.fields.pending, included: tt.fields.included, } - s, err := beaconstate.InitializeFromProtoUnsafe(&p2ppb.BeaconState{ - Slot: 16 * params.BeaconConfig().SlotsPerEpoch, - Validators: validators, - }) - if err != nil { - t.Fatal(err) + var err error + for i := 0; i < len(tt.args.slashings); i++ { + err = p.InsertAttesterSlashing(context.Background(), beaconState, tt.args.slashings[i]) } - err = p.InsertAttesterSlashing(s, tt.args.slashing) if err != nil && tt.fields.wantErr && !strings.Contains(err.Error(), tt.fields.err) { t.Fatalf("Wanted err: %v, received %v", tt.fields.err, err) } if len(p.pendingAttesterSlashing) != len(tt.want) { - t.Fatalf("Mismatched lengths of pending list. Got %d, wanted %d.", len(p.pendingAttesterSlashing), len(tt.want)) + t.Fatalf( + "Mismatched lengths of pending list. Got %d, wanted %d.", + len(p.pendingAttesterSlashing), + len(tt.want), + ) } for i := range p.pendingAttesterSlashing { if p.pendingAttesterSlashing[i].validatorToSlash != tt.want[i].validatorToSlash { @@ -309,6 +227,52 @@ func TestPool_InsertAttesterSlashing(t *testing.T) { } } +func TestPool_InsertAttesterSlashing_SigFailsVerify_ClearPool(t *testing.T) { + conf := params.BeaconConfig() + conf.MaxAttesterSlashings = 2 + params.OverrideBeaconConfig(conf) + beaconState, privKeys := testutil.DeterministicGenesisState(t, 64) + pendingSlashings := make([]*PendingAttesterSlashing, 2) + slashings := make([]*ethpb.AttesterSlashing, 2) + for i := 0; i < 2; 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 + } + // We mess up the signature of the second slashing. + badSig := make([]byte, 96) + copy(badSig, "muahaha") + pendingSlashings[1].attesterSlashing.Attestation_1.Signature = badSig + slashings[1].Attestation_1.Signature = badSig + p := &Pool{ + pendingAttesterSlashing: make([]*PendingAttesterSlashing, 0), + } + if err := p.InsertAttesterSlashing( + context.Background(), + beaconState, + slashings[0], + ); err != nil { + t.Fatal(err) + } + if err := p.InsertAttesterSlashing( + context.Background(), + beaconState, + slashings[1], + ); err == nil { + t.Error("Expected error when inserting slashing with bad sig, got nil") + } + // We expect to only have 1 pending attester slashing in the pool. + if len(p.pendingAttesterSlashing) != 1 { + t.Error("Expected failed attester slashing to have been cleared from pool") + } +} + func TestPool_MarkIncludedAttesterSlashing(t *testing.T) { type fields struct { pending []*PendingAttesterSlashing @@ -450,6 +414,23 @@ func TestPool_PendingAttesterSlashings(t *testing.T) { type fields struct { pending []*PendingAttesterSlashing } + conf := params.BeaconConfig() + conf.MaxAttesterSlashings = 1 + params.OverrideBeaconConfig(conf) + beaconState, privKeys := testutil.DeterministicGenesisState(t, 64) + 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 @@ -465,34 +446,16 @@ func TestPool_PendingAttesterSlashings(t *testing.T) { { name: "All eligible", fields: fields{ - pending: generateNPendingSlashings(1), + pending: pendingSlashings, }, - want: generateNAttSlashings(1), + want: slashings[0:1], }, { name: "Multiple indices", fields: fields{ - pending: []*PendingAttesterSlashing{ - pendingSlashingForValIdx(1, 5, 8), - }, - }, - want: []*ethpb.AttesterSlashing{ - attesterSlashingForValIdx(1, 5, 8), + pending: pendingSlashings[3:6], }, - }, - { - name: "All eligible, over max", - fields: fields{ - pending: generateNPendingSlashings(6), - }, - want: generateNAttSlashings(1), - }, - { - name: "No duplicate slashings for grouped", - fields: fields{ - pending: generateNPendingSlashings(16), - }, - want: generateNAttSlashings(1), + want: slashings[3:4], }, } for _, tt := range tests { @@ -500,70 +463,43 @@ func TestPool_PendingAttesterSlashings(t *testing.T) { p := &Pool{ pendingAttesterSlashing: tt.fields.pending, } - if got := p.PendingAttesterSlashings(); !reflect.DeepEqual(tt.want, got) { + if got := p.PendingAttesterSlashings( + context.Background(), + ); !reflect.DeepEqual(tt.want, got) { t.Errorf("Unexpected return from PendingAttesterSlashings, wanted %v, received %v", tt.want, got) } }) } } -func TestPool_PendingAttesterSlashings_2Max(t *testing.T) { +func TestPool_PendingAttesterSlashings_NoDuplicates(t *testing.T) { conf := params.BeaconConfig() conf.MaxAttesterSlashings = 2 params.OverrideBeaconConfig(conf) - - type fields struct { - pending []*PendingAttesterSlashing + beaconState, privKeys := testutil.DeterministicGenesisState(t, 64) + pendingSlashings := make([]*PendingAttesterSlashing, 3) + slashings := make([]*ethpb.AttesterSlashing, 3) + for i := 0; i < 2; 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: "No duplicates with grouped att slashings", - fields: fields{ - pending: []*PendingAttesterSlashing{ - { - attesterSlashing: attesterSlashingForValIdx(4, 12, 40), - validatorToSlash: 4, - }, - { - attesterSlashing: attesterSlashingForValIdx(6, 8, 24), - validatorToSlash: 6, - }, - { - attesterSlashing: attesterSlashingForValIdx(6, 8, 24), - validatorToSlash: 8, - }, - { - attesterSlashing: attesterSlashingForValIdx(4, 12, 40), - validatorToSlash: 12, - }, - { - attesterSlashing: attesterSlashingForValIdx(6, 8, 24), - validatorToSlash: 24, - }, - { - attesterSlashing: attesterSlashingForValIdx(4, 12, 40), - validatorToSlash: 40, - }, - }, - }, - want: []*ethpb.AttesterSlashing{ - attesterSlashingForValIdx(4, 12, 40), - attesterSlashingForValIdx(6, 8, 24), - }, - }, + // We duplicate the last slashing. + pendingSlashings[2] = pendingSlashings[1] + slashings[2] = slashings[1] + p := &Pool{ + pendingAttesterSlashing: pendingSlashings, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - p := &Pool{ - pendingAttesterSlashing: tt.fields.pending, - } - if got := p.PendingAttesterSlashings(); !reflect.DeepEqual(tt.want, got) { - t.Errorf("Unexpected return from PendingAttesterSlashings, wanted %v, received %v", tt.want, got) - } - }) + want := slashings[0:2] + if got := p.PendingAttesterSlashings( + context.Background(), + ); !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 209b3c24c59b..21c3e00cc656 100644 --- a/beacon-chain/operations/slashings/service_proposer_test.go +++ b/beacon-chain/operations/slashings/service_proposer_test.go @@ -1,15 +1,16 @@ package slashings import ( + "context" "reflect" "strings" "testing" "github.com/gogo/protobuf/proto" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" - beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state" - p2ppb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/testutil" ) func proposerSlashingForValIdx(valIdx uint64) *ethpb.ProposerSlashing { @@ -18,14 +19,6 @@ func proposerSlashingForValIdx(valIdx uint64) *ethpb.ProposerSlashing { } } -func generateNProposerSlashings(n uint64) []*ethpb.ProposerSlashing { - proposerSlashings := make([]*ethpb.ProposerSlashing, n) - for i := uint64(0); i < n; i++ { - proposerSlashings[i] = proposerSlashingForValIdx(i) - } - return proposerSlashings -} - func TestPool_InsertProposerSlashing(t *testing.T) { type fields struct { wantErr bool @@ -34,8 +27,40 @@ func TestPool_InsertProposerSlashing(t *testing.T) { included map[uint64]bool } type args struct { - slashing *ethpb.ProposerSlashing + slashings []*ethpb.ProposerSlashing } + + beaconState, privKeys := testutil.DeterministicGenesisState(t, 64) + slashings := make([]*ethpb.ProposerSlashing, 20) + 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 + } + + if err := beaconState.SetSlot(helpers.StartSlot(1)); err != nil { + t.Fatal(err) + } + + // We mark the following validators with some preconditions. + exitedVal, _ := beaconState.ValidatorAtIndex(uint64(2)) + exitedVal.ExitEpoch = 0 + futureExitedVal, _ := beaconState.ValidatorAtIndex(uint64(4)) + futureExitedVal.ExitEpoch = 17 + slashedVal, _ := beaconState.ValidatorAtIndex(uint64(5)) + slashedVal.Slashed = true + if err := beaconState.UpdateValidatorAtIndex(uint64(2), exitedVal); err != nil { + t.Fatal(err) + } + if err := beaconState.UpdateValidatorAtIndex(uint64(4), futureExitedVal); err != nil { + t.Fatal(err) + } + if err := beaconState.UpdateValidatorAtIndex(uint64(5), slashedVal); err != nil { + t.Fatal(err) + } + tests := []struct { name string fields fields @@ -49,22 +74,22 @@ func TestPool_InsertProposerSlashing(t *testing.T) { included: make(map[uint64]bool), }, args: args{ - slashing: proposerSlashingForValIdx(0), + slashings: slashings[0:1], }, - want: generateNProposerSlashings(1), + want: slashings[0:1], }, { name: "Duplicate identical slashing", fields: fields{ - pending: generateNProposerSlashings(1), + pending: slashings[0:1], included: make(map[uint64]bool), wantErr: true, err: "slashing object already exists in pending proposer slashings", }, args: args{ - slashing: proposerSlashingForValIdx(0), + slashings: slashings[0:1], }, - want: generateNProposerSlashings(1), + want: slashings[0:1], }, { name: "Slashing for exited validator", @@ -75,7 +100,7 @@ func TestPool_InsertProposerSlashing(t *testing.T) { err: "cannot be slashed", }, args: args{ - slashing: proposerSlashingForValIdx(2), + slashings: slashings[2:3], }, want: []*ethpb.ProposerSlashing{}, }, @@ -86,11 +111,9 @@ func TestPool_InsertProposerSlashing(t *testing.T) { included: make(map[uint64]bool), }, args: args{ - slashing: proposerSlashingForValIdx(4), - }, - want: []*ethpb.ProposerSlashing{ - proposerSlashingForValIdx(4), + slashings: slashings[4:5], }, + want: slashings[4:5], }, { name: "Slashing for slashed validator", @@ -98,10 +121,10 @@ func TestPool_InsertProposerSlashing(t *testing.T) { pending: []*ethpb.ProposerSlashing{}, included: make(map[uint64]bool), wantErr: true, - err: "cannot be slashed", + err: "not slashable", }, args: args{ - slashing: proposerSlashingForValIdx(5), + slashings: slashings[5:6], }, want: []*ethpb.ProposerSlashing{}, }, @@ -116,7 +139,7 @@ func TestPool_InsertProposerSlashing(t *testing.T) { err: "cannot be slashed", }, args: args{ - slashing: proposerSlashingForValIdx(1), + slashings: slashings[1:2], }, want: []*ethpb.ProposerSlashing{}, }, @@ -124,56 +147,31 @@ func TestPool_InsertProposerSlashing(t *testing.T) { name: "Maintains sorted order", fields: fields{ pending: []*ethpb.ProposerSlashing{ - proposerSlashingForValIdx(0), - proposerSlashingForValIdx(4), + slashings[0], + slashings[2], }, included: make(map[uint64]bool), }, args: args{ - slashing: proposerSlashingForValIdx(1), + slashings: slashings[1:2], }, want: []*ethpb.ProposerSlashing{ - proposerSlashingForValIdx(0), - proposerSlashingForValIdx(1), - proposerSlashingForValIdx(4), + slashings[0], + slashings[1], + slashings[2], }, }, } - validators := []*ethpb.Validator{ - { // 0 - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - }, - { // 1 - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - }, - { // 2 - Already exited. - ExitEpoch: 15, - }, - { // 3 - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - }, - { // 4 - Will be exited. - ExitEpoch: 17, - }, - { // 5 - Slashed. - ExitEpoch: params.BeaconConfig().FarFutureEpoch, - Slashed: true, - }, - } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { p := &Pool{ pendingProposerSlashing: tt.fields.pending, included: tt.fields.included, } - beaconState, err := beaconstate.InitializeFromProtoUnsafe(&p2ppb.BeaconState{ - Slot: 16 * params.BeaconConfig().SlotsPerEpoch, - Validators: validators, - }) - if err != nil { - t.Fatal(err) + var err error + for i := 0; i < len(tt.args.slashings); i++ { + err = p.InsertProposerSlashing(context.Background(), beaconState, tt.args.slashings[i]) } - err = p.InsertProposerSlashing(beaconState, tt.args.slashing) if err != nil && tt.fields.wantErr && !strings.Contains(err.Error(), tt.fields.err) { t.Fatalf("Wanted err: %v, received %v", tt.fields.err, err) } @@ -200,6 +198,47 @@ func TestPool_InsertProposerSlashing(t *testing.T) { } } +func TestPool_InsertProposerSlashing_SigFailsVerify_ClearPool(t *testing.T) { + conf := params.BeaconConfig() + conf.MaxAttesterSlashings = 2 + params.OverrideBeaconConfig(conf) + beaconState, privKeys := testutil.DeterministicGenesisState(t, 64) + slashings := make([]*ethpb.ProposerSlashing, 2) + for i := 0; i < 2; i++ { + sl, err := testutil.GenerateProposerSlashingForValidator(beaconState, privKeys[i], uint64(i)) + if err != nil { + t.Fatal(err) + } + slashings[i] = sl + } + // We mess up the signature of the second slashing. + badSig := make([]byte, 96) + copy(badSig, "muahaha") + slashings[1].Header_1.Signature = badSig + p := &Pool{ + pendingProposerSlashing: make([]*ethpb.ProposerSlashing, 0), + } + // We only want a single slashing to remain. + if err := p.InsertProposerSlashing( + context.Background(), + beaconState, + slashings[0], + ); err != nil { + t.Fatal(err) + } + if err := p.InsertProposerSlashing( + context.Background(), + beaconState, + slashings[1], + ); err == nil { + t.Error("Expected slashing with bad signature to fail, received nil") + } + // We expect to only have 1 pending proposer slashing in the pool. + if len(p.pendingProposerSlashing) != 1 { + t.Error("Expected failed proposer slashing to have been cleared from pool") + } +} + func TestPool_MarkIncludedProposerSlashing(t *testing.T) { type fields struct { pending []*ethpb.ProposerSlashing @@ -336,8 +375,14 @@ func TestPool_PendingProposerSlashings(t *testing.T) { type fields struct { pending []*ethpb.ProposerSlashing } - type args struct { - validatorToSlash uint64 + beaconState, privKeys := testutil.DeterministicGenesisState(t, 64) + slashings := make([]*ethpb.ProposerSlashing, 20) + 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 } tests := []struct { name string @@ -354,16 +399,16 @@ func TestPool_PendingProposerSlashings(t *testing.T) { { name: "All eligible", fields: fields{ - pending: generateNProposerSlashings(6), + pending: slashings[:params.BeaconConfig().MaxProposerSlashings], }, - want: generateNProposerSlashings(6), + want: slashings[:params.BeaconConfig().MaxProposerSlashings], }, { - name: "All eligible, more than max", + name: "Multiple indices", fields: fields{ - pending: generateNProposerSlashings(24), + pending: slashings[3:6], }, - want: generateNProposerSlashings(16), + want: slashings[3:6], }, } for _, tt := range tests { @@ -371,8 +416,10 @@ func TestPool_PendingProposerSlashings(t *testing.T) { p := &Pool{ pendingProposerSlashing: tt.fields.pending, } - if got := p.PendingProposerSlashings(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("PendingProposerSlashings() = %v, want %v", got, tt.want) + if got := p.PendingProposerSlashings( + context.Background(), + ); !reflect.DeepEqual(tt.want, got) { + t.Errorf("Unexpected return from PendingProposerSlashings, wanted %v, received %v", tt.want, got) } }) } diff --git a/beacon-chain/rpc/beacon/slashings.go b/beacon-chain/rpc/beacon/slashings.go index 2df632007327..50a9ba93b8f6 100644 --- a/beacon-chain/rpc/beacon/slashings.go +++ b/beacon-chain/rpc/beacon/slashings.go @@ -20,7 +20,7 @@ func (bs *Server) SubmitProposerSlashing( if err != nil { return nil, status.Errorf(codes.Internal, "Could not retrieve head state: %v", err) } - if err := bs.SlashingsPool.InsertProposerSlashing(beaconState, req); err != nil { + if err := bs.SlashingsPool.InsertProposerSlashing(ctx, beaconState, req); err != nil { return nil, status.Errorf(codes.Internal, "Could not insert proposer slashing into pool: %v", err) } return ðpb.SubmitSlashingResponse{ @@ -39,7 +39,7 @@ func (bs *Server) SubmitAttesterSlashing( if err != nil { return nil, status.Errorf(codes.Internal, "Could not retrieve head state: %v", err) } - if err := bs.SlashingsPool.InsertAttesterSlashing(beaconState, req); err != nil { + if err := bs.SlashingsPool.InsertAttesterSlashing(ctx, beaconState, req); err != nil { return nil, status.Errorf(codes.Internal, "Could not insert attester slashing into pool: %v", err) } slashedIndices := sliceutil.IntersectionUint64(req.Attestation_1.AttestingIndices, req.Attestation_2.AttestingIndices) diff --git a/beacon-chain/rpc/beacon/slashings_test.go b/beacon-chain/rpc/beacon/slashings_test.go index c068ecb17726..08fff1fcdd86 100644 --- a/beacon-chain/rpc/beacon/slashings_test.go +++ b/beacon-chain/rpc/beacon/slashings_test.go @@ -2,42 +2,28 @@ package beacon import ( "context" - "strconv" "testing" "github.com/gogo/protobuf/proto" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" mock "github.com/prysmaticlabs/prysm/beacon-chain/blockchain/testing" "github.com/prysmaticlabs/prysm/beacon-chain/operations/slashings" - stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state" - pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" - "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/testutil" ) func TestServer_SubmitProposerSlashing(t *testing.T) { ctx := context.Background() - vals := make([]*ethpb.Validator, 10) - for i := 0; i < len(vals); i++ { - key := make([]byte, 48) - copy(key, strconv.Itoa(i)) - vals[i] = ðpb.Validator{ - PublicKey: key[:], - WithdrawalCredentials: make([]byte, 32), - EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance, - Slashed: false, - } + st, privs := testutil.DeterministicGenesisState(t, 64) + slashedVal, err := st.ValidatorAtIndex(5) + if err != nil { + t.Fatal(err) } - // We mark the validator at index 5 as already slashed. - vals[5].Slashed = true - - st, err := stateTrie.InitializeFromProto(&pbp2p.BeaconState{ - Slot: 0, - Validators: vals, - }) - if err != nil { + slashedVal.Slashed = true + if err := st.UpdateValidatorAtIndex(5, slashedVal); err != nil { t.Fatal(err) } + bs := &Server{ HeadFetcher: &mock.ChainService{ State: st, @@ -50,27 +36,11 @@ func TestServer_SubmitProposerSlashing(t *testing.T) { wanted := ðpb.SubmitSlashingResponse{ SlashedIndices: []uint64{2}, } - slashing := ðpb.ProposerSlashing{ - ProposerIndex: 2, - Header_1: ðpb.SignedBeaconBlockHeader{ - Header: ðpb.BeaconBlockHeader{ - Slot: 0, - ParentRoot: nil, - StateRoot: nil, - BodyRoot: nil, - }, - Signature: make([]byte, 96), - }, - Header_2: ðpb.SignedBeaconBlockHeader{ - Header: ðpb.BeaconBlockHeader{ - Slot: 0, - ParentRoot: nil, - StateRoot: nil, - BodyRoot: nil, - }, - Signature: make([]byte, 96), - }, + slashing, err := testutil.GenerateProposerSlashingForValidator(st, privs[2], uint64(2)) + if err != nil { + t.Fatal(err) } + res, err := bs.SubmitProposerSlashing(ctx, slashing) if err != nil { t.Fatal(err) @@ -79,9 +49,13 @@ func TestServer_SubmitProposerSlashing(t *testing.T) { t.Errorf("Wanted %v, received %v", wanted, res) } + slashing, err = testutil.GenerateProposerSlashingForValidator(st, privs[5], uint64(5)) + if err != nil { + t.Fatal(err) + } + // We do not want a proposer slashing for an already slashed validator // (the validator at index 5) to be included in the pool. - slashing.ProposerIndex = 5 if _, err := bs.SubmitProposerSlashing(ctx, slashing); err == nil { t.Error("Expected including a proposer slashing for an already slashed validator to fail") } @@ -89,29 +63,19 @@ func TestServer_SubmitProposerSlashing(t *testing.T) { func TestServer_SubmitAttesterSlashing(t *testing.T) { ctx := context.Background() - vals := make([]*ethpb.Validator, 10) - for i := 0; i < len(vals); i++ { - key := make([]byte, 48) - copy(key, strconv.Itoa(i)) - vals[i] = ðpb.Validator{ - PublicKey: key[:], - WithdrawalCredentials: make([]byte, 32), - EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance, - Slashed: false, - } - } - // We mark the validators at index 5, 6, 7 as already slashed. - vals[5].Slashed = true - vals[6].Slashed = true - vals[7].Slashed = true - - st, err := stateTrie.InitializeFromProto(&pbp2p.BeaconState{ - Slot: 0, - Validators: vals, - }) + // We mark the validators at index 5, 6 as already slashed. + st, privs := testutil.DeterministicGenesisState(t, 64) + slashedVal, err := st.ValidatorAtIndex(5) if err != nil { t.Fatal(err) } + + // We mark the validator at index 5 as already slashed. + slashedVal.Slashed = true + if err := st.UpdateValidatorAtIndex(5, slashedVal); err != nil { + t.Fatal(err) + } + bs := &Server{ HeadFetcher: &mock.ChainService{ State: st, @@ -119,19 +83,16 @@ func TestServer_SubmitAttesterSlashing(t *testing.T) { SlashingsPool: slashings.NewPool(), } - slashing := ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{1, 2, 3}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{2, 3, 4}, - }, + slashing, err := testutil.GenerateAttesterSlashingForValidator(st, privs[2], uint64(2)) + if err != nil { + t.Fatal(err) } + // We want the intersection of the slashing attesting indices // to be slashed, so we expect validators 2 and 3 to be in the response // slashed indices. wanted := ðpb.SubmitSlashingResponse{ - SlashedIndices: []uint64{2, 3}, + SlashedIndices: []uint64{2}, } res, err := bs.SubmitAttesterSlashing(ctx, slashing) if err != nil { @@ -141,16 +102,12 @@ func TestServer_SubmitAttesterSlashing(t *testing.T) { t.Errorf("Wanted %v, received %v", wanted, res) } + slashing, err = testutil.GenerateAttesterSlashingForValidator(st, privs[5], uint64(5)) + if err != nil { + t.Fatal(err) + } // If any of the attesting indices in the slashing object have already // been slashed, we should fail to insert properly into the attester slashing pool. - slashing = ðpb.AttesterSlashing{ - Attestation_1: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{5, 6, 7}, - }, - Attestation_2: ðpb.IndexedAttestation{ - AttestingIndices: []uint64{6, 7, 8}, - }, - } if _, err := bs.SubmitAttesterSlashing(ctx, slashing); err == nil { t.Error("Expected including a attester slashing for an already slashed validator to fail") } diff --git a/beacon-chain/rpc/validator/proposer.go b/beacon-chain/rpc/validator/proposer.go index 0bbc3f87272e..fe8a63c3adb1 100644 --- a/beacon-chain/rpc/validator/proposer.go +++ b/beacon-chain/rpc/validator/proposer.go @@ -89,8 +89,8 @@ func (vs *Server) GetBlock(ctx context.Context, req *ethpb.BlockRequest) (*ethpb Deposits: deposits, Attestations: atts, RandaoReveal: req.RandaoReveal, - ProposerSlashings: vs.SlashingsPool.PendingProposerSlashings(), - AttesterSlashings: vs.SlashingsPool.PendingAttesterSlashings(), + ProposerSlashings: vs.SlashingsPool.PendingProposerSlashings(ctx), + AttesterSlashings: vs.SlashingsPool.PendingAttesterSlashings(ctx), VoluntaryExits: vs.ExitPool.PendingExits(head, req.Slot), Graffiti: graffiti[:], }, diff --git a/beacon-chain/rpc/validator/proposer_test.go b/beacon-chain/rpc/validator/proposer_test.go index 6e083bc4f852..f017ad618b0e 100644 --- a/beacon-chain/rpc/validator/proposer_test.go +++ b/beacon-chain/rpc/validator/proposer_test.go @@ -99,7 +99,7 @@ func TestGetBlock_OK(t *testing.T) { privKeys[0], 0, /* validator index */ ) - if err := proposerServer.SlashingsPool.InsertProposerSlashing(beaconState, proposerSlashing); err != nil { + if err := proposerServer.SlashingsPool.InsertProposerSlashing(context.Background(), beaconState, proposerSlashing); err != nil { t.Fatal(err) } @@ -109,7 +109,7 @@ func TestGetBlock_OK(t *testing.T) { privKeys[1], 1, /* validator index */ ) - if err := proposerServer.SlashingsPool.InsertAttesterSlashing(beaconState, attesterSlashing); err != nil { + if err := proposerServer.SlashingsPool.InsertAttesterSlashing(context.Background(), beaconState, attesterSlashing); err != nil { t.Fatal(err) }