[ETCM-316] Cleanup

input-output-hk · Feb 28, 2021 · 241d34b · 241d34b
1 parent 2f7be23
commit 241d34b
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 24 deletions.
diff --git a/src/main/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolver.scala b/src/main/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolver.scala
@@ -60,14 +60,15 @@ class RecentBlocksSearch(blockchain: Blockchain) {
       candidateHeaders: Seq[BlockHeader],
       bestBlockNumber: BigInt
   ): Option[BigInt] = {
+    def isParent(potentialParent: BigInt, childCandidate: BlockHeader): Boolean =
+      blockchain.getBlockHeaderByNumber(potentialParent).exists { _.isParentOf(childCandidate) }
     NonEmptyList.fromList(candidateHeaders.reverse.toList).flatMap { remoteHeaders =>
       val blocksToBeCompared = bestBlockNumber.until(bestBlockNumber - remoteHeaders.size).by(-1).toList
       remoteHeaders.toList
         .zip(blocksToBeCompared)
-        .find { case (childCandidate, parent) =>
-          blockchain.getBlockHeaderByNumber(parent).exists { _.isParentOf(childCandidate) }
+        .collectFirst {
+          case (childCandidate, parent) if isParent(parent, childCandidate) => parent
         }
-        .map(_._2)
     }
   }
 
@@ -107,10 +108,9 @@ object BinarySearchSupport extends Logger {
       else if (parentNum == min && childNum == max) ContinueBinarySearch(searchState.copy(minBlockNumber = childNum))
       else ContinueBinarySearch(searchState.copy(minBlockNumber = parentNum))
     } else { // no parent/child -> chains have diverged before parent block
-      if (min == 1 && max == 1) NoCommonBlock
+      if (min == 1 && max <= 2) NoCommonBlock
       else if (min == max) BinarySearchCompleted(parentOf(parentNum))
-      // else if (parentNum == max) BinarySearchCompleted(parentNum)
-      else ContinueBinarySearch(searchState.copy(maxBlockNumber = parentOf(parentNum)))
+      else ContinueBinarySearch(searchState.copy(maxBlockNumber = parentOf(parentNum).max(1)))
     }
   }
 

diff --git a/src/main/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolverActor.scala b/src/main/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolverActor.scala
@@ -208,7 +208,6 @@ class FastSyncBranchResolverActor(
     restart()
   }
 
-  // TODO track restarts and give up after a couple of times
   private def restart(): Unit = {
     context.become(waitingForPeerWithHighestBlock)
     self ! StartBranchResolver

diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml
@@ -8,7 +8,7 @@
         </encoder>
     </appender>
 
-    <logger name="io.iohk.ethereum" level="DEBUG" />
+    <logger name="io.iohk.ethereum" level="INFO" />
     <logger name="io.iohk.ethereum.vm.VM" level="OFF" />
 
     <root level="INFO">

diff --git a/src/test/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolverActorSpec.scala b/src/test/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolverActorSpec.scala
@@ -79,12 +79,16 @@ class FastSyncBranchResolverActorSpec
         val fastSyncBranchResolver =
           creatFastSyncBranchResolver(sender.ref, etcPeerManager, CacheBasedBlacklist.empty(BlacklistMaxElements))
 
-        val (peer, peerInfo) = getBestPeer
-        (for {
+        val expectation: PartialFunction[Any, BranchResolvedSuccessful] = {
+          case r @ BranchResolvedSuccessful(num, _) if num == BigInt(5) => r
+        }
+
+        val response = (for {
           _ <- Task(sender.send(fastSyncBranchResolver, StartBranchResolver))
-          response <- Task(sender.expectMsg(BranchResolvedSuccessful(5, peer)))
+          response <- Task(sender.expectMsgPF()(expectation))
           _ <- Task(stopController(fastSyncBranchResolver))
         } yield response).runSyncUnsafe()
+        assert(getBestPeers.contains(response.masterPeer))
       }
 
       "The chain is repaired doing binary searching with the new master peer and then remove the last invalid blocks" - {
@@ -118,13 +122,12 @@ class FastSyncBranchResolverActorSpec
             case r @ BranchResolvedSuccessful(num, _) if num == BigInt(5) => r
           }
 
-          val (peer, peerInfo) = getBestPeer
-          log.debug(s"*** peers: ${handshakedPeers.map(p => (p._1.id, p._2.maxBlockNumber))}")
-          (for {
+          val response = (for {
             _ <- Task(sender.send(fastSyncBranchResolver, StartBranchResolver))
             response <- Task(sender.expectMsgPF()(expectation))
             _ <- Task(stopController(fastSyncBranchResolver))
           } yield response).runSyncUnsafe()
+          assert(getBestPeers.contains(response.masterPeer))
         }
         "highest common block is in the first half" in new TestSetup {
           override implicit lazy val system = self.system
@@ -157,13 +160,12 @@ class FastSyncBranchResolverActorSpec
             case r @ BranchResolvedSuccessful(num, _) if num == BigInt(3) => r
           }
 
-          val (peer, peerInfo) = getBestPeer
-          log.debug(s"*** peers: ${handshakedPeers.map(p => (p._1.id, p._2.maxBlockNumber))}")
-          (for {
+          val response = (for {
             _ <- Task(sender.send(fastSyncBranchResolver, StartBranchResolver))
             response <- Task(sender.expectMsgPF()(expectation))
             _ <- Task(stopController(fastSyncBranchResolver))
           } yield response).runSyncUnsafe()
+          assert(getBestPeers.contains(response.masterPeer))
         }
 
         "highest common block is in the second half" in new TestSetup {
@@ -196,13 +198,12 @@ class FastSyncBranchResolverActorSpec
             case r @ BranchResolvedSuccessful(num, _) if num == BigInt(6) => r
           }
 
-          val (peer, peerInfo) = getBestPeer
-          log.debug(s"*** peers: ${handshakedPeers.map(p => (p._1.id, p._2.maxBlockNumber))}")
-          (for {
+          val response = (for {
             _ <- Task(sender.send(fastSyncBranchResolver, StartBranchResolver))
             response <- Task(sender.expectMsgPF()(expectation))
             _ <- Task(stopController(fastSyncBranchResolver))
           } yield response).runSyncUnsafe()
+          assert(getBestPeers.contains(response.masterPeer))
         }
       }
 
@@ -223,15 +224,14 @@ class FastSyncBranchResolverActorSpec
           1 -> firstBatchBlockHeaders,
           2 -> List(blocksSavedInPeer.get(3).get), // block 4
           3 -> List(blocksSavedInPeer.get(1).get), // block 2
-          4 -> List(blocksSavedInPeer.get(0).get) // block 1
+          4 -> List(blocksSavedInPeer.get(1).get) // block 1
         )
 
         saveBlocks(blocksSaved)
         val etcPeerManager = createEtcPeerManager(handshakedPeers, blocksSentFromPeer)
         val fastSyncBranchResolver =
           creatFastSyncBranchResolver(sender.ref, etcPeerManager, CacheBasedBlacklist.empty(BlacklistMaxElements))
 
-        val (peer, peerInfo) = getBestPeer
         log.debug(s"*** peers: ${handshakedPeers.map(p => (p._1.id, p._2.maxBlockNumber))}")
         (for {
           _ <- Task(sender.send(fastSyncBranchResolver, StartBranchResolver))
@@ -304,8 +304,9 @@ class FastSyncBranchResolverActorSpec
       awaitCond(gracefulStop(actorRef, actorAskTimeout.duration).futureValue)
     }
 
-    def getBestPeer: (Peer, PeerInfo) = {
-      handshakedPeers.toList.sortBy { case (_, peerInfo) => peerInfo.maxBlockNumber }(Ordering[BigInt].reverse).head
+    def getBestPeers: List[Peer] = {
+      val maxBlock = handshakedPeers.toList.map { case (_, peerInfo) => peerInfo.maxBlockNumber }.max
+      handshakedPeers.toList.filter { case (_, peerInfo) => peerInfo.maxBlockNumber == maxBlock }.map(_._1)
     }
   }
 }