Skip to content

Commit

Permalink
Fix propose protect error handling (#7188)
Browse files Browse the repository at this point in the history
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
  • Loading branch information
shayzluf and prylabs-bulldozer[bot] authored Sep 11, 2020
1 parent b1e2238 commit af46fc7
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 11 deletions.
7 changes: 6 additions & 1 deletion slasher/rpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,13 @@ func (ss *Server) IsSlashableBlock(ctx context.Context, req *ethpb.SignedBeaconB
return nil, err
}
pkMap, err := ss.beaconClient.FindOrGetPublicKeys(ctx, []uint64{req.Header.ProposerIndex})
if err != nil {
return nil, err
}
if err := helpers.VerifyBlockHeaderSigningRoot(
req.Header, pkMap[req.Header.ProposerIndex], req.Signature, domain); err != nil {
req.Header,
pkMap[req.Header.ProposerIndex],
req.Signature, domain); err != nil {
return nil, err
}

Expand Down
6 changes: 5 additions & 1 deletion validator/client/propose_protect.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ func (v *validator) postBlockSignUpdate(ctx context.Context, pubKey [48]byte, bl
if err != nil {
return errors.Wrap(err, "failed to get block header from block")
}
if !v.protector.CommitBlock(ctx, sbh) {
valid, err := v.protector.CommitBlock(ctx, sbh)
if err != nil {
return err
}
if !valid {
if v.emitAccountMetrics {
ValidatorProposeFailVecSlasher.WithLabelValues(fmtKey).Inc()
}
Expand Down
8 changes: 4 additions & 4 deletions validator/slashing-protection/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ func (s *Service) CheckBlockSafety(ctx context.Context, blockHeader *ethpb.Beaco

// CommitBlock this function is part of slashing protection for block proposals it performs
// validation and db update. To be used after the block is proposed.
func (s *Service) CommitBlock(ctx context.Context, blockHeader *ethpb.SignedBeaconBlockHeader) bool {
func (s *Service) CommitBlock(ctx context.Context, blockHeader *ethpb.SignedBeaconBlockHeader) (bool, error) {
ps, err := s.slasherClient.IsSlashableBlock(ctx, blockHeader)
if err != nil {
log.Errorf("External slashing block protection returned an error: %v", err)
return false
return false, err
}
if ps != nil && ps.ProposerSlashing != nil {
log.Warn("External slashing proposal protection found the block to be slashable")
return false
return false, nil
}
return true
return true, nil
}

// CheckAttestationSafety implements the slashing protection for attestations without db update.
Expand Down
8 changes: 6 additions & 2 deletions validator/slashing-protection/external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,13 @@ func TestService_CommitBlock(t *testing.T) {
BodyRoot: bytesutil.PadTo([]byte("body"), 32),
},
}
assert.Equal(t, false, s.CommitBlock(context.Background(), blk), "Expected commit block to fail verification")
slashable, err := s.CommitBlock(context.Background(), blk)
assert.NoError(t, err)
assert.Equal(t, false, slashable, "Expected commit block to fail verification")
s = &Service{slasherClient: mockSlasher.MockSlasher{SlashBlock: false}}
assert.Equal(t, true, s.CommitBlock(context.Background(), blk), "Expected commit block to pass verification")
slashable, err = s.CommitBlock(context.Background(), blk)
assert.NoError(t, err)
assert.Equal(t, true, slashable, "Expected commit block to pass verification")
}

func TestService_VerifyBlock(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion validator/slashing-protection/protector.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ type Protector interface {
CheckAttestationSafety(ctx context.Context, attestation *eth.IndexedAttestation) bool
CommitAttestation(ctx context.Context, attestation *eth.IndexedAttestation) bool
CheckBlockSafety(ctx context.Context, blockHeader *eth.BeaconBlockHeader) bool
CommitBlock(ctx context.Context, blockHeader *eth.SignedBeaconBlockHeader) bool
CommitBlock(ctx context.Context, blockHeader *eth.SignedBeaconBlockHeader) (bool, error)
Status() error
}
4 changes: 2 additions & 2 deletions validator/testing/protector_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit af46fc7

Please sign in to comment.