From beee947ddaadaa47d773ea6e0c91e0825cbef131 Mon Sep 17 00:00:00 2001
From: Ori Newman <orinewman1@gmail.com>
Date: Tue, 29 Nov 2022 17:18:07 +0200
Subject: [PATCH] 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       | 28 +++++++++++++++++--
 .../v5/blockrelay/ibd_with_headers_proof.go   |  7 ++++-
 version/version.go                            |  2 +-
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/app/protocol/flows/v5/blockrelay/ibd.go b/app/protocol/flows/v5/blockrelay/ibd.go
index 7438c201e..726d072b6 100644
--- a/app/protocol/flows/v5/blockrelay/ibd.go
+++ b/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,8 +192,21 @@ func (flow *handleIBDFlow) negotiateMissingSyncerChainSegment() (*externalapi.Do
 					return nil, nil, protocolerrors.Errorf(true, "Sent invalid chain block %s", syncerChainHash)
 				}
 
-				currentHighestKnownSyncerChainHash = syncerChainHash
-				break
+				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
 		}
@@ -285,7 +303,11 @@ func (flow *handleIBDFlow) isGenesisVirtualSelectedParent() (bool, error) {
 func (flow *handleIBDFlow) logIBDFinished(isFinishedSuccessfully bool, err error) {
 	successString := "successfully"
 	if !isFinishedSuccessfully {
-		successString = fmt.Sprintf("(interrupted: %s)", err)
+		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)
 }
diff --git a/app/protocol/flows/v5/blockrelay/ibd_with_headers_proof.go b/app/protocol/flows/v5/blockrelay/ibd_with_headers_proof.go
index 455b1c2dc..d3e345d8d 100644
--- a/app/protocol/flows/v5/blockrelay/ibd_with_headers_proof.go
+++ b/app/protocol/flows/v5/blockrelay/ibd_with_headers_proof.go
@@ -85,7 +85,12 @@ func (flow *handleIBDFlow) shouldSyncAndShouldDownloadHeadersProof(
 			return true, true, nil
 		}
 
-		return false, false, 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
diff --git a/version/version.go b/version/version.go
index dcec2bd61..b461619c0 100644
--- a/version/version.go
+++ b/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