From 8c96279c82ce843466df849a107d70b7f207ce12 Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 21:38:21 +0800 Subject: [PATCH 1/9] fix slashing checks --- beacon-chain/sync/BUILD.bazel | 1 + .../sync/validate_attester_slashing.go | 3 +- .../sync/validate_attester_slashing_test.go | 43 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/beacon-chain/sync/BUILD.bazel b/beacon-chain/sync/BUILD.bazel index f29c55ab52fb..c453bd121a27 100644 --- a/beacon-chain/sync/BUILD.bazel +++ b/beacon-chain/sync/BUILD.bazel @@ -70,6 +70,7 @@ go_library( "//beacon-chain/core/signing:go_default_library", "//beacon-chain/core/transition:go_default_library", "//beacon-chain/core/transition/interop:go_default_library", + "//beacon-chain/core/validators:go_default_library", "//beacon-chain/db:go_default_library", "//beacon-chain/db/filters:go_default_library", "//beacon-chain/execution:go_default_library", diff --git a/beacon-chain/sync/validate_attester_slashing.go b/beacon-chain/sync/validate_attester_slashing.go index 29f88e23463f..067f0a8d0f26 100644 --- a/beacon-chain/sync/validate_attester_slashing.go +++ b/beacon-chain/sync/validate_attester_slashing.go @@ -6,6 +6,7 @@ import ( pubsub "github.com/libp2p/go-libp2p-pubsub" "github.com/libp2p/go-libp2p/core/peer" "github.com/prysmaticlabs/prysm/v4/beacon-chain/core/blocks" + v "github.com/prysmaticlabs/prysm/v4/beacon-chain/core/validators" "github.com/prysmaticlabs/prysm/v4/container/slice" "github.com/prysmaticlabs/prysm/v4/monitoring/tracing" ethpb "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1" @@ -50,7 +51,7 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg if err != nil { return pubsub.ValidationIgnore, err } - if err := blocks.VerifyAttesterSlashing(ctx, headState, slashing); err != nil { + if _, err := blocks.ProcessAttesterSlashing(ctx, headState, slashing, v.SlashValidator); err != nil { return pubsub.ValidationReject, err } diff --git a/beacon-chain/sync/validate_attester_slashing_test.go b/beacon-chain/sync/validate_attester_slashing_test.go index f92bad4a5df0..d64ce22884d8 100644 --- a/beacon-chain/sync/validate_attester_slashing_test.go +++ b/beacon-chain/sync/validate_attester_slashing_test.go @@ -111,6 +111,49 @@ 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 all validators as withdrawn + vals := s.Validators() + for _, vv := range vals { + vv.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.ErrorContains(t, "unable to slash any validator", 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() From 3020ea477f1af3e7ab5475cd2881c208d080c01f Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:05:38 +0800 Subject: [PATCH 2/9] fix to make it more performant --- .../sync/validate_attester_slashing.go | 22 ++++++++++++++++--- .../sync/validate_attester_slashing_test.go | 22 ++++++++++++++----- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/beacon-chain/sync/validate_attester_slashing.go b/beacon-chain/sync/validate_attester_slashing.go index 067f0a8d0f26..cb939e59d043 100644 --- a/beacon-chain/sync/validate_attester_slashing.go +++ b/beacon-chain/sync/validate_attester_slashing.go @@ -5,11 +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" - v "github.com/prysmaticlabs/prysm/v4/beacon-chain/core/validators" + "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" ) @@ -51,10 +54,23 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg if err != nil { return pubsub.ValidationIgnore, err } - if _, err := blocks.ProcessAttesterSlashing(ctx, headState, slashing, v.SlashValidator); err != nil { + if err := blocks.VerifyAttesterSlashing(ctx, headState, slashing); err != nil { return pubsub.ValidationReject, err } - + slashedVals := slice.IntersectionUint64(slashing.Attestation_1.AttestingIndices, slashing.Attestation_2.AttestingIndices) + 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 + } + } + 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 diff --git a/beacon-chain/sync/validate_attester_slashing_test.go b/beacon-chain/sync/validate_attester_slashing_test.go index d64ce22884d8..f0373f7c2283 100644 --- a/beacon-chain/sync/validate_attester_slashing_test.go +++ b/beacon-chain/sync/validate_attester_slashing_test.go @@ -116,11 +116,10 @@ func TestValidateAttesterSlashing_InvalidSlashing_WithdrawableEpoch(t *testing.T ctx := context.Background() slashing, s := setupValidAttesterSlashing(t) - // Set all validators as withdrawn + // Set only one of the validators as withdrawn vals := s.Validators() - for _, vv := range vals { - vv.WithdrawableEpoch = primitives.Epoch(1) - } + vals[0].WithdrawableEpoch = primitives.Epoch(1) + require.NoError(t, s.SetValidators(vals)) r := &Service{ @@ -148,7 +147,20 @@ func TestValidateAttesterSlashing_InvalidSlashing_WithdrawableEpoch(t *testing.T }, } res, err := r.validateAttesterSlashing(ctx, "foobar", msg) - assert.ErrorContains(t, "unable to slash any validator", err) + 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") From f87e5890f4fdbe47db5d949c997f0a6d6c93b136 Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:08:36 +0800 Subject: [PATCH 3/9] gaz --- beacon-chain/sync/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon-chain/sync/BUILD.bazel b/beacon-chain/sync/BUILD.bazel index c453bd121a27..f29c55ab52fb 100644 --- a/beacon-chain/sync/BUILD.bazel +++ b/beacon-chain/sync/BUILD.bazel @@ -70,7 +70,6 @@ go_library( "//beacon-chain/core/signing:go_default_library", "//beacon-chain/core/transition:go_default_library", "//beacon-chain/core/transition/interop:go_default_library", - "//beacon-chain/core/validators:go_default_library", "//beacon-chain/db:go_default_library", "//beacon-chain/db/filters:go_default_library", "//beacon-chain/execution:go_default_library", From dd7a77d01cff5a35cf9c71b59867c57ed0388f1d Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:13:54 +0800 Subject: [PATCH 4/9] fix up --- beacon-chain/sync/validate_attester_slashing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/sync/validate_attester_slashing.go b/beacon-chain/sync/validate_attester_slashing.go index cb939e59d043..7784b4c98561 100644 --- a/beacon-chain/sync/validate_attester_slashing.go +++ b/beacon-chain/sync/validate_attester_slashing.go @@ -57,7 +57,7 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg if err := blocks.VerifyAttesterSlashing(ctx, headState, slashing); err != nil { return pubsub.ValidationReject, err } - slashedVals := slice.IntersectionUint64(slashing.Attestation_1.AttestingIndices, slashing.Attestation_2.AttestingIndices) + slashedVals := blocks.SlashableAttesterIndices(slashing) isSlashable := false for _, v := range slashedVals { val, err := headState.ValidatorAtIndexReadOnly(primitives.ValidatorIndex(v)) From d72b44fa434af600b44259fafdc92bc463a6480e Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:19:58 +0800 Subject: [PATCH 5/9] potuz's comment --- beacon-chain/sync/validate_attester_slashing.go | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/sync/validate_attester_slashing.go b/beacon-chain/sync/validate_attester_slashing.go index 7784b4c98561..16b907431ce3 100644 --- a/beacon-chain/sync/validate_attester_slashing.go +++ b/beacon-chain/sync/validate_attester_slashing.go @@ -66,6 +66,7 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg } if helpers.IsSlashableValidator(val.ActivationEpoch(), val.WithdrawableEpoch(), val.Slashed(), slots.ToEpoch(headState.Slot())) { isSlashable = true + break } } if !isSlashable { From 52164993a47ae77cddd2a3b3b81e1112cb717d33 Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:30:20 +0800 Subject: [PATCH 6/9] potuz's comment --- beacon-chain/sync/validate_attester_slashing.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon-chain/sync/validate_attester_slashing.go b/beacon-chain/sync/validate_attester_slashing.go index 16b907431ce3..36360ac4288e 100644 --- a/beacon-chain/sync/validate_attester_slashing.go +++ b/beacon-chain/sync/validate_attester_slashing.go @@ -43,7 +43,8 @@ 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) { @@ -57,7 +58,6 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg if err := blocks.VerifyAttesterSlashing(ctx, headState, slashing); err != nil { return pubsub.ValidationReject, err } - slashedVals := blocks.SlashableAttesterIndices(slashing) isSlashable := false for _, v := range slashedVals { val, err := headState.ValidatorAtIndexReadOnly(primitives.ValidatorIndex(v)) From 4c80b3b4aea29840d816aca52ad955258631399d Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:33:29 +0800 Subject: [PATCH 7/9] fix cache --- beacon-chain/sync/validate_attester_slashing.go | 6 ++---- beacon-chain/sync/validate_attester_slashing_test.go | 4 +++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/beacon-chain/sync/validate_attester_slashing.go b/beacon-chain/sync/validate_attester_slashing.go index 36360ac4288e..00342cdb0d8b 100644 --- a/beacon-chain/sync/validate_attester_slashing.go +++ b/beacon-chain/sync/validate_attester_slashing.go @@ -47,7 +47,7 @@ func (s *Service) validateAttesterSlashing(ctx context.Context, pid peer.ID, msg 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 } @@ -79,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() diff --git a/beacon-chain/sync/validate_attester_slashing_test.go b/beacon-chain/sync/validate_attester_slashing_test.go index f0373f7c2283..fe8756331885 100644 --- a/beacon-chain/sync/validate_attester_slashing_test.go +++ b/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" @@ -345,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)) } } From eec7f38ee679f7bd837741b33f137df6695e2a8b Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:37:31 +0800 Subject: [PATCH 8/9] change index in test for better case --- beacon-chain/sync/validate_attester_slashing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/sync/validate_attester_slashing_test.go b/beacon-chain/sync/validate_attester_slashing_test.go index fe8756331885..7a9534f3d93a 100644 --- a/beacon-chain/sync/validate_attester_slashing_test.go +++ b/beacon-chain/sync/validate_attester_slashing_test.go @@ -119,7 +119,7 @@ func TestValidateAttesterSlashing_InvalidSlashing_WithdrawableEpoch(t *testing.T slashing, s := setupValidAttesterSlashing(t) // Set only one of the validators as withdrawn vals := s.Validators() - vals[0].WithdrawableEpoch = primitives.Epoch(1) + vals[1].WithdrawableEpoch = primitives.Epoch(1) require.NoError(t, s.SetValidators(vals)) From d1462e661c0b985e70e6356f0524044e198638d3 Mon Sep 17 00:00:00 2001 From: nisdas Date: Mon, 17 Apr 2023 22:45:27 +0800 Subject: [PATCH 9/9] gaz --- beacon-chain/sync/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/sync/BUILD.bazel b/beacon-chain/sync/BUILD.bazel index f29c55ab52fb..80f47e2e5413 100644 --- a/beacon-chain/sync/BUILD.bazel +++ b/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",