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

Add Slasher double block detection to E2E #5936

Merged
merged 7 commits into from
May 21, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions beacon-chain/rpc/validator/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"math/rand"
"time"

"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"

"github.com/pkg/errors"
ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"github.com/prysmaticlabs/go-ssz"
Expand All @@ -19,6 +17,7 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/core/state"
"github.com/prysmaticlabs/prysm/beacon-chain/core/state/interop"
stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"
dbpb "github.com/prysmaticlabs/prysm/proto/beacon/db"
"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/featureconfig"
Expand Down
129 changes: 107 additions & 22 deletions endtoend/evaluators/slashing.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

ptypes "github.com/gogo/protobuf/types"
"github.com/pkg/errors"
eth "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"github.com/prysmaticlabs/go-bitfield"
"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
Expand All @@ -19,33 +20,33 @@ import (
// InjectDoubleVote broadcasts a double vote into the beacon node pool for the slasher to detect.
var InjectDoubleVote = types.Evaluator{
Name: "inject_double_vote_%d",
Policy: beforeEpoch(2),
Policy: onEpoch(1),
Evaluation: insertDoubleAttestationIntoPool,
}

// ProposeDoubleBlock broadcasts a double block to the beacon node for the slasher to detect.
var ProposeDoubleBlock = types.Evaluator{
Name: "propose_double_block_%d",
Policy: onEpoch(1),
Evaluation: proposeDoubleBlock,
}

// ValidatorsSlashed ensures the expected amount of validators are slashed.
var ValidatorsSlashed = types.Evaluator{
Name: "validators_slashed_epoch_%d",
Policy: afterNthEpoch(0),
Policy: afterNthEpoch(1),
Evaluation: validatorsSlashed,
}

// SlashedValidatorsLoseBalance checks if the validators slashed lose the right balance.
var SlashedValidatorsLoseBalance = types.Evaluator{
Name: "slashed_validators_lose_valance_epoch_%d",
Policy: afterNthEpoch(0),
Policy: afterNthEpoch(1),
Evaluation: validatorsLoseBalance,
}

var slashedIndices []uint64

// Not including first epoch because of issues with genesis.
func beforeEpoch(epoch uint64) func(uint64) bool {
return func(currentEpoch uint64) bool {
return currentEpoch < epoch
}
}

func validatorsSlashed(conns ...*grpc.ClientConn) error {
conn := conns[0]
ctx := context.Background()
Expand All @@ -55,8 +56,8 @@ func validatorsSlashed(conns ...*grpc.ClientConn) error {
if err != nil {
return err
}
if len(changes.SlashedIndices) != 2 && len(changes.SlashedIndices) != 4 {
return fmt.Errorf("expected 2 or 4 indices to be slashed, received %d", len(changes.SlashedIndices))
if len(changes.SlashedIndices) != len(slashedIndices) {
return fmt.Errorf("expected %d indices to be slashed, received %d", len(slashedIndices), len(changes.SlashedIndices))
}
return nil
}
Expand Down Expand Up @@ -100,7 +101,7 @@ func insertDoubleAttestationIntoPool(conns ...*grpc.ClientConn) error {
ctx := context.Background()
chainHead, err := beaconClient.GetChainHead(ctx, &ptypes.Empty{})
if err != nil {
return err
return errors.Wrap(err, "could not get chain head")
}

_, privKeys, err := testutil.DeterministicDepositsAndKeys(64)
Expand All @@ -116,7 +117,7 @@ func insertDoubleAttestationIntoPool(conns ...*grpc.ClientConn) error {
PublicKeys: pubKeys,
})
if err != nil {
return err
return errors.Wrap(err, "could not get duties")
}

var committeeIndex uint64
Expand Down Expand Up @@ -147,11 +148,11 @@ func insertDoubleAttestationIntoPool(conns ...*grpc.ClientConn) error {
}
resp, err := valClient.DomainData(ctx, req)
if err != nil {
return err
return errors.Wrap(err, "could not get domain data")
}
signingRoot, err := helpers.ComputeSigningRoot(attData, resp.SignatureDomain)
if err != nil {
return err
return errors.Wrap(err, "could not compute signing root")
}

valsToSlash := uint64(2)
Expand All @@ -169,14 +170,98 @@ func insertDoubleAttestationIntoPool(conns ...*grpc.ClientConn) error {
Data: attData,
Signature: privKeys[committee[i]].Sign(signingRoot[:]).Marshal(),
}
for _, conn := range conns {
client := eth.NewBeaconNodeValidatorClient(conn)
_, err = client.ProposeAttestation(ctx, att)
if err != nil {
return err
}
// We only broadcast to conns[0] here since we can trust that at least 1 node will be online.
// Only broadcasting the attestation to one node also helps test slashing propagation.
client := eth.NewBeaconNodeValidatorClient(conns[0])
if _, err = client.ProposeAttestation(ctx, att); err != nil {
return errors.Wrap(err, "could not propose attestation")
}
slashedIndices = append(slashedIndices, committee[i])
}
return nil
}

func proposeDoubleBlock(conns ...*grpc.ClientConn) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can return more informative errors in each of the conditionals in this func, something like errors.Wrap(err, "could not do X")

conn := conns[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happen if len(conns) != 1 is empty? Probably best to check the length and return error

Copy link
Contributor Author

@0xKiwi 0xKiwi May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will always be something here, the test would fail much earlier elsewhere if it wasn't. That would mean we don't have any beacon nodes to connect to. At least 1 node is a guarantee here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. But better to be defensive. Someone brand new can be writing an e2e test and use proposeDoubleBlock but 100% unaware conns[0] vs [1], [2] will be used

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why pass in conns, why do we need a list? why not just use conn?

Copy link
Contributor Author

@0xKiwi 0xKiwi May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting the check here would be very odd, these style of tests follow an interface for all of E2E. Only conns[0] is used here but in the dozen or so other E2E tests, the other conns are used. Using multiple conns here just adds little benefit. E2E is designed with the assumption that there will always be at least 1 beacon node, because if there isn't, the test wouldn't progress.

Copy link
Contributor Author

@0xKiwi 0xKiwi May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are at least run with 1 node, so pinging for any chain data that shouldn't be node dependent is done with the node index that we know is running.

Copy link
Contributor Author

@0xKiwi 0xKiwi May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, only using one beacon node here helps test --broadcast-slashing, its why I removed the connection to other nodes from injectDoubleVote.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I got confused. Perhaps we should add a comment to mention only con0 is explicitly used but input takes in a list of conns.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, that sounds good 👍

valClient := eth.NewBeaconNodeValidatorClient(conn)
beaconClient := eth.NewBeaconChainClient(conn)

ctx := context.Background()
chainHead, err := beaconClient.GetChainHead(ctx, &ptypes.Empty{})
if err != nil {
return errors.Wrap(err, "could not get chain head")
}

_, privKeys, err := testutil.DeterministicDepositsAndKeys(64)
if err != nil {
return err
}
pubKeys := make([][]byte, len(privKeys))
for i, priv := range privKeys {
pubKeys[i] = priv.PublicKey().Marshal()
}
duties, err := valClient.GetDuties(ctx, &eth.DutiesRequest{
Epoch: chainHead.HeadEpoch,
PublicKeys: pubKeys,
})
if err != nil {
return errors.Wrap(err, "could not get duties")
}

var proposerIndex uint64
for i, duty := range duties.CurrentEpochDuties {
if sliceutil.IsInUint64(chainHead.HeadSlot-1, duty.ProposerSlots) {
proposerIndex = uint64(i)
break
}
}

hashLen := 32
blk := &eth.BeaconBlock{
Slot: chainHead.HeadSlot - 1,
ParentRoot: bytesutil.PadTo([]byte("bad parent root"), hashLen),
StateRoot: bytesutil.PadTo([]byte("bad state root"), hashLen),
ProposerIndex: proposerIndex,
Body: &eth.BeaconBlockBody{
Eth1Data: &eth.Eth1Data{
BlockHash: bytesutil.PadTo([]byte("bad block hash"), hashLen),
DepositRoot: bytesutil.PadTo([]byte("bad deposit root"), hashLen),
DepositCount: 1,
},
RandaoReveal: bytesutil.PadTo([]byte("bad randao"), params.BeaconConfig().BLSSignatureLength),
Graffiti: bytesutil.PadTo([]byte("teehee"), hashLen),
ProposerSlashings: []*eth.ProposerSlashing{},
AttesterSlashings: []*eth.AttesterSlashing{},
Attestations: []*eth.Attestation{},
Deposits: []*eth.Deposit{},
VoluntaryExits: []*eth.SignedVoluntaryExit{},
},
}

req := &eth.DomainRequest{
Epoch: chainHead.HeadEpoch,
Domain: params.BeaconConfig().DomainBeaconProposer[:],
}
resp, err := valClient.DomainData(ctx, req)
if err != nil {
return errors.Wrap(err, "could not get domain data")
}
signingRoot, err := helpers.ComputeSigningRoot(blk, resp.SignatureDomain)
if err != nil {
return errors.Wrap(err, "could not compute signing root")
}
sig := privKeys[proposerIndex].Sign(signingRoot[:]).Marshal()
signedBlk := &eth.SignedBeaconBlock{
Block: blk,
Signature: sig,
}

// We only broadcast to conns[0] here since we can trust that at least 1 node will be online.
// Only broadcasting the attestation to one node also helps test slashing propagation.
client := eth.NewBeaconNodeValidatorClient(conns[0])
if _, err = client.ProposeBlock(ctx, signedBlk); err == nil {
return errors.New("expected block to fail processing")
}
slashedIndices = append(slashedIndices, proposerIndex)
return nil
}
3 changes: 2 additions & 1 deletion endtoend/minimal_slashing_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestEndToEnd_Slashing_MinimalConfig(t *testing.T) {
minimalConfig := &types.E2EConfig{
BeaconFlags: []string{"--minimal-config", "--custom-genesis-delay=25"},
ValidatorFlags: []string{"--minimal-config"},
EpochsToRun: 2,
EpochsToRun: 3,
TestSync: false,
TestSlasher: true,
Evaluators: []types.Evaluator{
Expand All @@ -26,6 +26,7 @@ func TestEndToEnd_Slashing_MinimalConfig(t *testing.T) {
ev.ValidatorsSlashed,
ev.SlashedValidatorsLoseBalance,
ev.InjectDoubleVote,
ev.ProposeDoubleBlock,
},
}
if err := e2eParams.Init(2); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion endtoend/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type E2EConfig struct {
type Evaluator struct {
Name string
Policy func(currentEpoch uint64) bool
Evaluation func(conn ...*grpc.ClientConn) error
Evaluation func(conn ...*grpc.ClientConn) error // A variable amount of conns is allowed to be passed in for evaluations to check all nodes if needed.
}

// BeaconNodeInfo contains the info of ports and other required information
Expand Down
5 changes: 3 additions & 2 deletions shared/featureconfig/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ var (
Hidden: true,
}
deprecatedEnableDomainDataCacheFlag = &cli.BoolFlag{
Name: "enable-domain-data-cache",
Usage: deprecatedUsage,
Name: "enable-domain-data-cache",
Usage: deprecatedUsage,
Hidden: true,
}
)
Expand Down Expand Up @@ -463,4 +463,5 @@ var E2EBeaconChainFlags = []string{
"--enable-state-ref-copy",
"--enable-new-state-mgmt",
"--enable-init-sync-wrr",
"--broadcast-slashing",
}