Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back disable slashing broadcast flag #8141

Merged
merged 6 commits into from
Jan 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions beacon-chain/rpc/beacon/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ go_library(
"//shared/bytesutil:go_default_library",
"//shared/cmd:go_default_library",
"//shared/event:go_default_library",
"//shared/featureconfig:go_default_library",
"//shared/pagination:go_default_library",
"//shared/params:go_default_library",
"//shared/sliceutil:go_default_library",
Expand Down Expand Up @@ -95,6 +96,7 @@ go_test(
"//shared/attestationutil:go_default_library",
"//shared/bytesutil:go_default_library",
"//shared/cmd:go_default_library",
"//shared/featureconfig:go_default_library",
"//shared/mock:go_default_library",
"//shared/params:go_default_library",
"//shared/testutil:go_default_library",
Expand Down
14 changes: 9 additions & 5 deletions beacon-chain/rpc/beacon/slashings.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package beacon

import (
"context"
"github.com/prysmaticlabs/prysm/shared/featureconfig"

ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"github.com/prysmaticlabs/prysm/shared/sliceutil"
Expand All @@ -23,8 +24,10 @@ func (bs *Server) SubmitProposerSlashing(
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)
}
if err := bs.Broadcaster.Broadcast(ctx, req); err != nil {
return nil, err
if !featureconfig.Get().DisableBroadcastSlashings {
if err := bs.Broadcaster.Broadcast(ctx, req); err != nil {
return nil, err
}
}

return &ethpb.SubmitSlashingResponse{
Expand All @@ -46,10 +49,11 @@ func (bs *Server) SubmitAttesterSlashing(
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)
}
if err := bs.Broadcaster.Broadcast(ctx, req); err != nil {
return nil, err
if !featureconfig.Get().DisableBroadcastSlashings {
if err := bs.Broadcaster.Broadcast(ctx, req); err != nil {
return nil, err
}
}

slashedIndices := sliceutil.IntersectionUint64(req.Attestation_1.AttestingIndices, req.Attestation_2.AttestingIndices)
return &ethpb.SubmitSlashingResponse{
SlashedIndices: slashedIndices,
Expand Down
94 changes: 94 additions & 0 deletions beacon-chain/rpc/beacon/slashings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package beacon

import (
"context"
"github.com/gogo/protobuf/proto"
ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"github.com/prysmaticlabs/prysm/shared/featureconfig"
"testing"

mock "github.com/prysmaticlabs/prysm/beacon-chain/blockchain/testing"
Expand Down Expand Up @@ -71,3 +74,94 @@ func TestServer_SubmitAttesterSlashing(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, true, mb.BroadcastCalled, "Expected broadcast to be called when flag is set")
}

func TestServer_SubmitProposerSlashing_DontBroadcast(t *testing.T) {
resetCfg := featureconfig.InitWithReset(&featureconfig.Flags{DisableBroadcastSlashings: true})
defer resetCfg()
ctx := context.Background()
st, privs := testutil.DeterministicGenesisState(t, 64)
slashedVal, err := st.ValidatorAtIndex(5)
require.NoError(t, err)
// We mark the validator at index 5 as already slashed.
slashedVal.Slashed = true
require.NoError(t, st.UpdateValidatorAtIndex(5, slashedVal))

mb := &mockp2p.MockBroadcaster{}
bs := &Server{
HeadFetcher: &mock.ChainService{
State: st,
},
SlashingsPool: slashings.NewPool(),
Broadcaster: mb,
}

// We want a proposer slashing for validator with index 2 to
// be included in the pool.
wanted := &ethpb.SubmitSlashingResponse{
SlashedIndices: []uint64{2},
}
slashing, err := testutil.GenerateProposerSlashingForValidator(st, privs[2], uint64(2))
require.NoError(t, err)

res, err := bs.SubmitProposerSlashing(ctx, slashing)
require.NoError(t, err)
if !proto.Equal(wanted, res) {
t.Errorf("Wanted %v, received %v", wanted, res)
}

assert.Equal(t, false, mb.BroadcastCalled, "Expected broadcast not to be called by default")

slashing, err = testutil.GenerateProposerSlashingForValidator(st, privs[5], uint64(5))
require.NoError(t, err)

// We do not want a proposer slashing for an already slashed validator
// (the validator at index 5) to be included in the pool.
_, err = bs.SubmitProposerSlashing(ctx, slashing)
require.ErrorContains(t, "Could not insert proposer slashing into pool", err)
}

func TestServer_SubmitAttesterSlashing_DontBroadcast(t *testing.T) {
resetCfg := featureconfig.InitWithReset(&featureconfig.Flags{DisableBroadcastSlashings: true})
defer resetCfg()
ctx := context.Background()
// We mark the validators at index 5, 6 as already slashed.
st, privs := testutil.DeterministicGenesisState(t, 64)
slashedVal, err := st.ValidatorAtIndex(5)
require.NoError(t, err)

// We mark the validator at index 5 as already slashed.
slashedVal.Slashed = true
require.NoError(t, st.UpdateValidatorAtIndex(5, slashedVal))

mb := &mockp2p.MockBroadcaster{}
bs := &Server{
HeadFetcher: &mock.ChainService{
State: st,
},
SlashingsPool: slashings.NewPool(),
Broadcaster: mb,
}

slashing, err := testutil.GenerateAttesterSlashingForValidator(st, privs[2], uint64(2))
require.NoError(t, 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 := &ethpb.SubmitSlashingResponse{
SlashedIndices: []uint64{2},
}
res, err := bs.SubmitAttesterSlashing(ctx, slashing)
require.NoError(t, err)
if !proto.Equal(wanted, res) {
t.Errorf("Wanted %v, received %v", wanted, res)
}
assert.Equal(t, false, mb.BroadcastCalled, "Expected broadcast not to be called by default")

slashing, err = testutil.GenerateAttesterSlashingForValidator(st, privs[5], uint64(5))
require.NoError(t, 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.
_, err = bs.SubmitAttesterSlashing(ctx, slashing)
assert.NotNil(t, err, "Expected including a attester slashing for an already slashed validator to fail")
}
5 changes: 5 additions & 0 deletions shared/featureconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ type Flags struct {
// Slasher toggles.
EnableHistoricalDetection bool // EnableHistoricalDetection disables historical attestation detection and performs detection on the chain head immediately.
DisableLookback bool // DisableLookback updates slasher to not use the lookback and update validator histories until epoch 0.
DisableBroadcastSlashings bool // DisableBroadcastSlashings disables p2p broadcasting of proposer and attester slashings.

// Cache toggles.
EnableSSZCache bool // EnableSSZCache see https://github.com/prysmaticlabs/prysm/pull/4558.
Expand Down Expand Up @@ -182,6 +183,10 @@ func ConfigureBeaconChain(ctx *cli.Context) {
log.Warn("Using a larger gossip history for the node")
cfg.EnableLargerGossipHistory = true
}
if ctx.Bool(disableBroadcastSlashingFlag.Name) {
log.Warn("Disabling slashing broadcasting to p2p network")
cfg.DisableBroadcastSlashings = true
}
Init(cfg)
}

Expand Down
7 changes: 6 additions & 1 deletion shared/featureconfig/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ var (
Usage: "(Danger): Disables the cache for attesting history in the validator DB, greatly increasing " +
"disk reads and writes as well as increasing time required for attestations to be produced",
}
disableBroadcastSlashingFlag = &cli.BoolFlag{
Name: "disable-broadcast-slashings",
Usage: "Disables broadcasting slashings submitted to the beacon node.",
}
)

// devModeFlags holds list of flags that are set when development mode is on.
Expand Down Expand Up @@ -122,7 +126,7 @@ var SlasherFlags = append(deprecatedFlags, []cli.Flag{
}...)

// E2EValidatorFlags contains a list of the validator feature flags to be tested in E2E.
var E2EValidatorFlags = []string{}
var E2EValidatorFlags = make([]string, 0)

// BeaconChainFlags contains a list of all the feature flags that apply to the beacon-chain client.
var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
Expand All @@ -141,6 +145,7 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{
checkPtInfoCache,
disablePruningDepositProofs,
disableSyncBacktracking,
disableBroadcastSlashingFlag,
}...)

// E2EBeaconChainFlags contains a list of the beacon chain feature flags to be tested in E2E.
Expand Down