blockchain: remove duplication of validate basic (#5418)

blockchain/msgs.go

@@ -83,10 +83,8 @@ func ValidateMsg(pb proto.Message) error {
 			return errors.New("negative Height")
 		}
 	case *bcproto.BlockResponse:
-		_, err := types.BlockFromProto(msg.Block)
-		if err != nil {
-			return err
-		}
+		// validate basic is called later when converting from proto
+		return nil
 	case *bcproto.NoBlockResponse:
 		if msg.Height < 0 {
 			return errors.New("negative Height")

blockchain/v0/reactor.go

@@ -227,6 +227,7 @@ func (bcR *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 		bi, err := types.BlockFromProto(msg.Block)
 		if err != nil {
 			bcR.Logger.Error("Block content is invalid", "err", err)
+			bcR.Switch.StopPeerForError(src, err)
 			return
 		}
 		bcR.pool.AddBlock(src.ID(), bi, len(msgBytes))

blockchain/v1/reactor.go

@@ -285,6 +285,7 @@ func (bcR *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 		bi, err := types.BlockFromProto(msg.Block)
 		if err != nil {
 			bcR.Logger.Error("error transition block from protobuf", "err", err)
+			_ = bcR.swReporter.Report(behaviour.BadMessage(src.ID(), err.Error()))
 			return
 		}
 		msgForFSM := bcReactorMessage{

blockchain/v2/reactor.go

@@ -469,6 +469,7 @@ func (r *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) {
 		bi, err := types.BlockFromProto(msg.Block)
 		if err != nil {
 			r.logger.Error("error transitioning block from protobuf", "err", err)
+			_ = r.reporter.Report(behaviour.BadMessage(src.ID(), err.Error()))
 			return
 		}
 		if r.events != nil {

blockchain/v2/scheduler_test.go

@@ -1377,6 +1377,9 @@ func checkScResults(t *testing.T, wantErr bool, err error, wantEvent Event, even
 		t.Errorf("error = %v, wantErr %v", err, wantErr)
 		return
 	}
+	if !assert.IsType(t, wantEvent, event) {
+		t.Log(fmt.Sprintf("Wrong type received, got: %v", event))
+	}
 	switch wantEvent := wantEvent.(type) {
 	case scPeerError:
 		assert.Equal(t, wantEvent.peerID, event.(scPeerError).peerID)
@@ -1460,7 +1463,7 @@ func TestScHandleBlockResponse(t *testing.T) {
 				pendingTime: map[int64]time.Time{6: now},
 			},
 			args:      args{event: block6FromP1},
-			wantEvent: scBlockReceived{peerID: "P1", block: makeScBlock(6)},
+			wantEvent: scBlockReceived{peerID: "P1", block: block6FromP1.block},
 		},
 	}
 
@@ -2047,6 +2050,8 @@ func TestScHandle(t *testing.T) {
 		priorityNormal
 	}
 
+	block1, block2, block3 := makeScBlock(1), makeScBlock(2), makeScBlock(3)
+
 	t0 := time.Now()
 	tick := make([]time.Time, 100)
 	for i := range tick {
@@ -2135,8 +2140,8 @@ func TestScHandle(t *testing.T) {
 					},
 				},
 				{ // block response 1
-					args:      args{event: bcBlockResponse{peerID: "P1", time: tick[4], size: 100, block: makeScBlock(1)}},
-					wantEvent: scBlockReceived{peerID: "P1", block: makeScBlock(1)},
+					args:      args{event: bcBlockResponse{peerID: "P1", time: tick[4], size: 100, block: block1}},
+					wantEvent: scBlockReceived{peerID: "P1", block: block1},
 					wantSc: &scTestParams{
 						startTime:   now,
 						peers:       map[string]*scPeer{"P1": {height: 3, state: peerStateReady, lastTouched: tick[4]}},
@@ -2148,8 +2153,8 @@ func TestScHandle(t *testing.T) {
 					},
 				},
 				{ // block response 2
-					args:      args{event: bcBlockResponse{peerID: "P1", time: tick[5], size: 100, block: makeScBlock(2)}},
-					wantEvent: scBlockReceived{peerID: "P1", block: makeScBlock(2)},
+					args:      args{event: bcBlockResponse{peerID: "P1", time: tick[5], size: 100, block: block2}},
+					wantEvent: scBlockReceived{peerID: "P1", block: block2},
 					wantSc: &scTestParams{
 						startTime:   now,
 						peers:       map[string]*scPeer{"P1": {height: 3, state: peerStateReady, lastTouched: tick[5]}},
@@ -2161,8 +2166,8 @@ func TestScHandle(t *testing.T) {
 					},
 				},
 				{ // block response 3
-					args:      args{event: bcBlockResponse{peerID: "P1", time: tick[6], size: 100, block: makeScBlock(3)}},
-					wantEvent: scBlockReceived{peerID: "P1", block: makeScBlock(3)},
+					args:      args{event: bcBlockResponse{peerID: "P1", time: tick[6], size: 100, block: block3}},
+					wantEvent: scBlockReceived{peerID: "P1", block: block3},
 					wantSc: &scTestParams{
 						startTime: now,
 						peers:     map[string]*scPeer{"P1": {height: 3, state: peerStateReady, lastTouched: tick[6]}},

types/validator_set.go

@@ -661,6 +661,9 @@ func (vals *ValidatorSet) UpdateWithChangeSet(changes []*Validator) error {
 // with a bonus for including more than +2/3 of the signatures.
 func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID,
 	height int64, commit *Commit) error {
+	if commit == nil {
+		return errors.New("nil commit")
+	}
 
 	if vals.Size() != len(commit.Signatures) {
 		return NewErrInvalidCommitSignatures(vals.Size(), len(commit.Signatures))
@@ -718,6 +721,9 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID,
 // signatures.
 func (vals *ValidatorSet) VerifyCommitLight(chainID string, blockID BlockID,
 	height int64, commit *Commit) error {
+	if commit == nil {
+		return errors.New("nil commit")
+	}
 
 	if vals.Size() != len(commit.Signatures) {
 		return NewErrInvalidCommitSignatures(vals.Size(), len(commit.Signatures))
@@ -770,10 +776,13 @@ func (vals *ValidatorSet) VerifyCommitLight(chainID string, blockID BlockID,
 // This method is primarily used by the light client and does not check all the
 // signatures.
 func (vals *ValidatorSet) VerifyCommitLightTrusting(chainID string, commit *Commit, trustLevel tmmath.Fraction) error {
-	// sanity check
+	// sanity checks
 	if trustLevel.Denominator == 0 {
 		return errors.New("trustLevel has zero Denominator")
 	}
+	if commit == nil {
+		return errors.New("nil commit")
+	}
 
 	var (
 		talliedVotingPower int64