diff --git a/internal/blocksync/pool.go b/internal/blocksync/pool.go index bd0df889a..bcbe142c5 100644 --- a/internal/blocksync/pool.go +++ b/internal/blocksync/pool.go @@ -269,19 +269,16 @@ func (pool *BlockPool) RedoRequest(height int64) types.NodeID { // AddBlock validates that the block comes from the peer it was expected from and calls the requester to store it. // TODO: ensure that blocks come in order for each peer. -func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, extCommit *types.ExtendedCommit, blockSize int) { +func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, extCommit *types.ExtendedCommit, blockSize int) error { pool.mtx.Lock() defer pool.mtx.Unlock() if block.Height != extCommit.Height { - pool.logger.Error("heights don't match, not adding block", "block_height", block.Height, "commit_height", extCommit.Height) - return + return fmt.Errorf("heights don't match, not adding block (block height: %d, commit height: %d)", block.Height, extCommit.Height) } requester := pool.requesters[block.Height] if requester == nil { - pool.logger.Error("peer sent us a block we didn't expect", - "peer", peerID, "curHeight", pool.height, "blockHeight", block.Height) diff := pool.height - block.Height if diff < 0 { diff *= -1 @@ -289,7 +286,7 @@ func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, extComm if diff > maxDiffBetweenCurrentAndReceivedBlockHeight { pool.sendError(errors.New("peer sent us a block we didn't expect with a height too far ahead/behind"), peerID) } - return + return fmt.Errorf("peer sent us a block we didn't expect (peer: %s, current height: %d, block height: %d)", peerID, pool.height, block.Height) } if requester.setBlock(block, extCommit, peerID) { @@ -300,9 +297,11 @@ func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, extComm } } else { err := errors.New("requester is different or block already exists") - pool.logger.Error(err.Error(), "peer", peerID, "requester", requester.getPeerID(), "blockHeight", block.Height) pool.sendError(err, peerID) + return fmt.Errorf("%w (peer: %s, requester: %s, block height: %d)", err, peerID, requester.getPeerID(), block.Height) } + + return nil } // MaxPeerHeight returns the highest reported height. diff --git a/internal/blocksync/pool_test.go b/internal/blocksync/pool_test.go index da748ed77..fed86e520 100644 --- a/internal/blocksync/pool_test.go +++ b/internal/blocksync/pool_test.go @@ -46,7 +46,7 @@ func (p testPeer) simulateInput(input inputData) { var blockID types.BlockID var extCommitSigs []types.ExtendedCommitSig extCommit := types.NewExtendedCommit(input.request.Height, 0, blockID, extCommitSigs) - input.pool.AddBlock(input.request.PeerID, block, extCommit, 123) + _ = input.pool.AddBlock(input.request.PeerID, block, extCommit, 123) // TODO: uncommenting this creates a race which is detected by: // https://github.com/golang/go/blob/2bd767b1022dd3254bcec469f0ee164024726486/src/testing/testing.go#L854-L856 // see: https://github.com/tendermint/tendermint/issues/3390#issue-418379890 diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go index c39cc950a..3158b4499 100644 --- a/internal/blocksync/reactor.go +++ b/internal/blocksync/reactor.go @@ -251,7 +251,9 @@ func (r *Reactor) handleMessage(ctx context.Context, envelope *p2p.Envelope, blo return err } - r.pool.AddBlock(envelope.From, block, extCommit, block.Size()) + if err := r.pool.AddBlock(envelope.From, block, extCommit, block.Size()); err != nil { + r.logger.Error("failed to add block", "err", err) + } case *bcproto.StatusRequest: return blockSyncCh.Send(ctx, p2p.Envelope{