blocksync: fixed behaviour when validation fails; appliead PR comments

internal/blocksync/reactor.go

@@ -190,13 +190,10 @@ func (r *Reactor) OnStop() {
 func (r *Reactor) respondToPeer(ctx context.Context, msg *bcproto.BlockRequest, peerID types.NodeID, blockSyncCh *p2p.Channel) error {
 	block := r.store.LoadBlockProto(msg.Height)
 	if block != nil {
-		blockCommit := r.store.LoadBlockCommitProto(msg.Height)
-		if blockCommit != nil {
-			return blockSyncCh.Send(ctx, p2p.Envelope{
-				To:      peerID,
-				Message: &bcproto.BlockResponse{Block: block},
-			})
-		}
+		return blockSyncCh.Send(ctx, p2p.Envelope{
+			To:      peerID,
+			Message: &bcproto.BlockResponse{Block: block},
+		})
 	}
 
 	r.logger.Info("peer requesting a block we do not have", "peer", peerID, "height", msg.Height)
@@ -495,7 +492,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
 
 			newBlockParts, err2 := newBlock.MakePartSet(types.BlockPartSizeBytes)
 			if err2 != nil {
-				r.logger.Error("failed to make ",
+				r.logger.Error("failed to make block at ",
 					"height", newBlock.Height,
 					"err", err2.Error())
 				return
@@ -520,12 +517,13 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
 						}); serr != nil {
 							return
 						}
+						continue
 					case ErrInvalidVerifyBlock:
 						r.logger.Error(
 							err.Error(),
 							"last_commit", verifyBlock.LastCommit,
-							"block_id", newBlockID,
-							"height", r.lastTrustedBlock.block.Height,
+							"verify_block_id", newBlockID,
+							"verify_block_height", newBlock.Height,
 						)
 						peerID := r.pool.RedoRequest(r.lastTrustedBlock.block.Height + 2)
 						if serr := blockSyncCh.SendError(ctx, p2p.PeerError{
@@ -534,6 +532,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
 						}); serr != nil {
 							return
 						}
+						continue
 					}
 
 				}
@@ -548,13 +547,19 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
 				}
 				oldHash := r.initialState.Validators.Hash()
 				if !bytes.Equal(oldHash, newBlock.ValidatorsHash) {
-
-					fmt.Println(
-
+					r.logger.Error("The validator set provided by the new block does not match the expected validator set",
 						"initial hash ", r.initialState.Validators.Hash(),
 						"new hash ", newBlock.ValidatorsHash,
 					)
-					return
+
+					peerID := r.pool.RedoRequest(r.lastTrustedBlock.block.Height + 1)
+					if serr := blockSyncCh.SendError(ctx, p2p.PeerError{
+						NodeID: peerID,
+						Err:    ErrValidationFailed{},
+					}); serr != nil {
+						return
+					}
+					continue
 				}
 				r.lastTrustedBlock = &BlockResponse{block: newBlock, commit: verifyBlock.LastCommit}
 			}

internal/blocksync/verify.go

@@ -82,7 +82,8 @@ func (e ErrValidationFailed) Error() string {
 	return "failed to verify next block"
 }
 
-func VerifyNextBlock(newBlock *types.Block, newBlockID types.BlockID, verifyBlock *types.Block, trustedBlock *types.Block, trustedCommit *types.Commit, validators *types.ValidatorSet) error {
+func VerifyNextBlock(newBlock *types.Block, newBlockID types.BlockID, verifyBlock *types.Block, trustedBlock *types.Block,
+	trustedCommit *types.Commit, validators *types.ValidatorSet) error {
 
 	// If the blockID in LastCommit of NewBlock does not match the trusted block
 	// we can assume NewBlock is not correct
@@ -99,7 +100,8 @@ func VerifyNextBlock(newBlock *types.Block, newBlockID types.BlockID, verifyBloc
 	// Verify NewBlock usign the validator set obtained after applying the last block
 	// Note: VerifyAdjacent in the LightClient relies on a trusting period which is not applicable here
 	// ToDo: We need witness verification here as well and backwards verification from a state where we can trust validators
-	if err := VerifyAdjacent(&types.SignedHeader{Header: &trustedBlock.Header, Commit: trustedCommit}, &types.SignedHeader{Header: &newBlock.Header, Commit: verifyBlock.LastCommit}, validators); err != nil {
+	if err := VerifyAdjacent(&types.SignedHeader{Header: &trustedBlock.Header, Commit: trustedCommit},
+		&types.SignedHeader{Header: &newBlock.Header, Commit: verifyBlock.LastCommit}, validators); err != nil {
 		return ErrValidationFailed{Reason: err}
 	}
 

internal/store/store.go

@@ -126,7 +126,7 @@ func (bs *BlockStore) LoadBaseMeta() *types.BlockMeta {
 	return nil
 }
 
-// LoadBlock returns the block with the given height in protobuf.
+// LoadBlockProto returns the block with the given height in protobuf.
 // If no block is found for that height, it returns nil.
 func (bs *BlockStore) LoadBlockProto(height int64) *tmproto.Block {
 	var blockMeta = bs.LoadBlockMeta(height)
@@ -157,30 +157,11 @@ func (bs *BlockStore) LoadBlockProto(height int64) *tmproto.Block {
 // LoadBlock returns the block with the given height.
 // If no block is found for that height, it returns nil.
 func (bs *BlockStore) LoadBlock(height int64) *types.Block {
-	var blockMeta = bs.LoadBlockMeta(height)
-	if blockMeta == nil {
+	blockProto := bs.LoadBlockProto(height)
+	if blockProto == nil {
 		return nil
 	}
-
-	pbb := new(tmproto.Block)
-	buf := []byte{}
-	for i := 0; i < int(blockMeta.BlockID.PartSetHeader.Total); i++ {
-		part := bs.LoadBlockPart(height, i)
-		// If the part is missing (e.g. since it has been deleted after we
-		// loaded the block meta) we consider the whole block to be missing.
-		if part == nil {
-			return nil
-		}
-		buf = append(buf, part.Bytes...)
-	}
-	err := proto.Unmarshal(buf, pbb)
-	if err != nil {
-		// NOTE: The existence of meta should imply the existence of the
-		// block. So, make sure meta is only saved after blocks are saved.
-		panic(fmt.Errorf("error reading block: %w", err))
-	}
-
-	block, err := types.BlockFromProto(pbb)
+	block, err := types.BlockFromProto(blockProto)
 	if err != nil {
 		panic(fmt.Errorf("error from proto block: %w", err))
 	}
@@ -282,26 +263,6 @@ func (bs *BlockStore) LoadBlockMeta(height int64) *types.BlockMeta {
 	return blockMeta
 }
 
-// LoadBlockCommit returns the Commit for the given height in protobuf.
-// This commit consists of the +2/3 and other Precommit-votes for block at `height`,
-// and it comes from the block.LastCommit for `height+1`.
-// If no commit is found for the given height, it returns nil.
-func (bs *BlockStore) LoadBlockCommitProto(height int64) *tmproto.Commit {
-	var pbc = new(tmproto.Commit)
-	bz, err := bs.db.Get(blockCommitKey(height))
-	if err != nil {
-		panic(err)
-	}
-	if len(bz) == 0 {
-		return nil
-	}
-	err = proto.Unmarshal(bz, pbc)
-	if err != nil {
-		panic(fmt.Errorf("error reading block commit: %w", err))
-	}
-	return pbc
-}
-
 // LoadBlockCommit returns the Commit for the given height.
 // This commit consists of the +2/3 and other Precommit-votes for block at `height`,
 // and it comes from the block.LastCommit for `height+1`.