Fix IBD sync conditions (#2174)

* Fix IBD sync conditions

* Fix syntax

* Fix Sprintf

* Bump version

* On negotiation check only blocks in future of PP

* Only log error and add comment

* Fix comment

app/protocol/flows/v5/blockrelay/ibd.go

@@ -175,6 +175,11 @@ func (flow *handleIBDFlow) negotiateMissingSyncerChainSegment() (*externalapi.Do
 	chainNegotiationRestartCounter := 0
 	chainNegotiationZoomCounts := 0
 	initialLocatorLen := len(locatorHashes)
+	pruningPoint, err := flow.Domain().Consensus().PruningPoint()
+	if err != nil {
+		return nil, nil, err
+	}
+
 	for {
 		var lowestUnknownSyncerChainHash, currentHighestKnownSyncerChainHash *externalapi.DomainHash
 		for _, syncerChainHash := range locatorHashes {
@@ -187,9 +192,22 @@ func (flow *handleIBDFlow) negotiateMissingSyncerChainSegment() (*externalapi.Do
 					return nil, nil, protocolerrors.Errorf(true, "Sent invalid chain block %s", syncerChainHash)
 				}
 
+				isPruningPointOnSyncerChain, err := flow.Domain().Consensus().IsInSelectedParentChainOf(pruningPoint, syncerChainHash)
+				if err != nil {
+					log.Errorf("Error checking isPruningPointOnSyncerChain: %s", err)
+				}
+
+				// We're only interested in syncer chain blocks that have our pruning
+				// point in their selected chain. Otherwise, it means one of the following:
+				// 1) We will not switch the virtual selected chain to the syncers chain since it will violate finality
+				//    (hence we can ignore it unless merged by others).
+				// 2) syncerChainHash is actually in the past of our pruning point so there's no
+				//    point in syncing from it.
+				if err == nil && isPruningPointOnSyncerChain {
 					currentHighestKnownSyncerChainHash = syncerChainHash
 					break
 				}
+			}
 			lowestUnknownSyncerChainHash = syncerChainHash
 		}
 		// No unknown blocks, break. Note this can only happen in the first iteration
@@ -285,7 +303,11 @@ func (flow *handleIBDFlow) isGenesisVirtualSelectedParent() (bool, error) {
 func (flow *handleIBDFlow) logIBDFinished(isFinishedSuccessfully bool, err error) {
 	successString := "successfully"
 	if !isFinishedSuccessfully {
+		if err != nil {
 			successString = fmt.Sprintf("(interrupted: %s)", err)
+		} else {
+			successString = fmt.Sprintf("(interrupted)")
+		}
 	}
 	log.Infof("IBD with peer %s finished %s", flow.peer, successString)
 }

app/protocol/flows/v5/blockrelay/ibd_with_headers_proof.go

@@ -85,12 +85,17 @@ func (flow *handleIBDFlow) shouldSyncAndShouldDownloadHeadersProof(
 			return true, true, nil
 		}
 
+		if highestKnownSyncerChainHash == nil {
+			log.Infof("Stopping IBD since IBD from this node will cause a finality conflict")
 			return false, false, nil
 		}
 
 		return false, true, nil
 	}
 
+	return false, true, nil
+}
+
 func (flow *handleIBDFlow) checkIfHighHashHasMoreBlueWorkThanSelectedTipAndPruningDepthMoreBlueScore(relayBlock *externalapi.DomainBlock) (bool, error) {
 	virtualSelectedParent, err := flow.Domain().Consensus().GetVirtualSelectedParent()
 	if err != nil {

version/version.go

@@ -11,7 +11,7 @@ const validCharacters = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrs
 const (
 	appMajor uint = 0
 	appMinor uint = 12
-	appPatch uint = 10
+	appPatch uint = 11
 )
 
 // appBuild is defined as a variable so it can be overridden during the build