Don't allow peers to update with lower height

blockchain/pool.go

@@ -106,7 +106,7 @@ func (pool *blockPool) updateMaxPeerHeight() {
 }
 
 // Adds a new peer or updates an existing peer with a new height.
-// If the peer is too short it is removed.
+// If a new peer is too short it is not added.
 func (pool *blockPool) updatePeer(peerID p2p.ID, height int64) error {
 	peer := pool.peers[peerID]
 	oldHeight := int64(0)
@@ -115,37 +115,22 @@ func (pool *blockPool) updatePeer(peerID p2p.ID, height int64) error {
 	}
 	pool.logger.Debug("updatePeer", "peerID", peerID, "height", height, "old_height", oldHeight)
 
+	if peer == nil {
 		if height < pool.height {
 			pool.logger.Info("Peer height too small", "peer", peerID, "height", height, "fsm_height", pool.height)
-
-		// Don't add or update a peer that is not useful.
-		if peer != nil {
-			pool.logger.Info("remove short peer", "peer", peerID, "height", height, "fsm_height", pool.height)
-			pool.removePeer(peerID, errPeerTooShort)
-		}
 			return errPeerTooShort
 		}
-
-	if peer == nil {
 		// Add new peer.
 		peer = newBPPeer(peerID, height, pool.toBcR.sendPeerError)
 		peer.setLogger(pool.logger.With("peer", peerID))
 		pool.peers[peerID] = peer
 	} else {
+		// Check if peer is lowering its height. This is not allowed.
+		if height < peer.height {
+			pool.removePeer(peerID, errPeerLowersItsHeight)
+			return errPeerLowersItsHeight
+		}
 		// Update existing peer.
-		// Remove any requests made for heights in range (height, peer.height].
-		for h, block := range pool.peers[peerID].blocks {
-			if h <= height {
-				continue
-			}
-			// Reschedule the requests for all blocks waiting for the peer, or received and not processed yet.
-			if block == nil {
-				// Since block was not yet received it is counted in numPending, decrement.
-				pool.numPending--
-				pool.peers[peerID].numPending--
-			}
-			pool.rescheduleRequest(peerID, h)
-		}
 		peer.height = height
 	}
 
@@ -171,7 +156,7 @@ func (pool *blockPool) deletePeer(peerID p2p.ID) {
 // Removes any blocks and requests associated with the peer and deletes the peer.
 // Also triggers new requests if blocks have been removed.
 func (pool *blockPool) removePeer(peerID p2p.ID, err error) {
-	pool.logger.Info("removing peer", "peerID", peerID)
+	pool.logger.Info("removing peer", "peerID", peerID, "error", err)
 
 	peer := pool.peers[peerID]
 	if peer == nil {

blockchain/pool_test.go

@@ -143,38 +143,21 @@ func TestBlockPoolUpdatePeer(t *testing.T) {
 			poolWanted: makeBlockPool(testBcR, 100, []bpPeer{{id: "P1", height: 123}}, map[int64]tPBlocks{}),
 		},
 		{
-			name:       "decrease the height of P1 from 120 to 110 (still over current processing height)",
+			name:       "decrease the height of P1 from 120 to 110",
 			pool:       makeBlockPool(testBcR, 100, []bpPeer{{id: "P1", height: 120}}, map[int64]tPBlocks{}),
 			args:       testPeer{"P1", 110},
-			poolWanted: makeBlockPool(testBcR, 100, []bpPeer{{id: "P1", height: 110}}, map[int64]tPBlocks{}),
-		},
-		{
-			name:       "decrease the height of P1 from 120 to 90 (under current processing height)",
-			pool:       makeBlockPool(testBcR, 100, []bpPeer{{id: "P1", height: 120}}, map[int64]tPBlocks{}),
-			args:       testPeer{"P1", 90},
-			errWanted:  errPeerTooShort,
+			errWanted:  errPeerLowersItsHeight,
 			poolWanted: makeBlockPool(testBcR, 100, []bpPeer{}, map[int64]tPBlocks{}),
 		},
 		{
 			name: "decrease the height of P1 from 105 to 102 with blocks",
 			pool: makeBlockPool(testBcR, 100, []bpPeer{{id: "P1", height: 105}},
 				map[int64]tPBlocks{
-					100: {"P1", true}, 101: {"P1", true}, 102: {"P1", true},
-					103: {"P1", true}, 104: {"P1", false}}),
+					100: {"P1", true}, 101: {"P1", true}, 102: {"P1", true}}),
 			args:      testPeer{"P1", 102},
-			poolWanted: makeBlockPool(testBcR, 100, []bpPeer{{id: "P1", height: 102}},
-				map[int64]tPBlocks{
-					100: {"P1", true}, 101: {"P1", true}, 102: {"P1", true}}),
-		},
-		{
-			name: "decrease the height of P1 from 102 to 99 with blocks",
-			pool: makeBlockPool(testBcR, 100, []bpPeer{{id: "P1", height: 102}},
-				map[int64]tPBlocks{
-					100: {"P1", true}, 101: {"P1", true}, 102: {"P1", true}}),
-			args: testPeer{"P1", 99},
+			errWanted: errPeerLowersItsHeight,
 			poolWanted: makeBlockPool(testBcR, 100, []bpPeer{},
 				map[int64]tPBlocks{}),
-			errWanted: errPeerTooShort,
 		},
 	}
 

blockchain/reactor_fsm.go

@@ -150,6 +150,7 @@ var (
 	errNoErrorFinished                 = errors.New("FSM is finished")
 	errInvalidEvent                    = errors.New("invalid event in current state")
 	errNoTallerPeer                    = errors.New("FSM timed out on waiting for a peer taller than this node")
+	errPeerLowersItsHeight             = errors.New("peer reports a height lower than previous")
 	errNoPeerResponseForCurrentHeights = errors.New("FSM timed out on peer block response for current heights")
 	errNoPeerResponse                  = errors.New("FSM timed out on peer block response")
 	errBadDataFromPeer                 = errors.New("received from wrong peer or bad block")

blockchain/reactor_fsm_test.go

@@ -802,7 +802,7 @@ func TestFSMPeerRelatedEvents(t *testing.T) {
 				// statusResponseEv from P1
 				makeStepStatusEv("waitForPeer", "waitForBlock", "P1", 200, nil),
 				// statusResponseEv from P1
-				makeStepStatusEv("waitForBlock", "waitForPeer", "P1", 3, errPeerTooShort),
+				makeStepStatusEv("waitForBlock", "waitForPeer", "P1", 3, errPeerLowersItsHeight),
 			},
 		},
 		{