Fix bug where we just need to awaitSyncAndIBD() rather than attempt to sync() (#5158)

* Fix bug where we just ned to awaitAllSync() rather than attempt to sync()

* Use awaitSyncAndIBD, mv awaitSyncAndIBD to NodeTestUtil

* Replace more NodeUnitTest.syncNeutrinoNode() with NodeTestUtil.awaitSyncAndIBD

* Replace more NodeUnitTest.syncNeutrinoNode() with NodeTestUtil.awaitSyncAndIBD

node-test/src/test/scala/org/bitcoins/node/NeutrinoNodeTest.scala

@@ -10,11 +10,7 @@ import org.bitcoins.node.models.{PeerDAO, PeerDb}
 import org.bitcoins.server.BitcoinSAppConfig
 import org.bitcoins.testkit.BitcoinSTestAppConfig
 import org.bitcoins.testkit.node.fixture.NeutrinoNodeConnectedWithBitcoinds
-import org.bitcoins.testkit.node.{
-  NodeTestUtil,
-  NodeTestWithCachedBitcoindPair,
-  NodeUnitTest
-}
+import org.bitcoins.testkit.node.{NodeTestUtil, NodeTestWithCachedBitcoindPair}
 import org.bitcoins.testkit.util.{AkkaUtil, TorUtil}
 import org.scalatest.{Assertion, FutureOutcome, Outcome}
 
@@ -69,7 +65,7 @@ class NeutrinoNodeTest extends NodeTestWithCachedBitcoindPair {
 
     for {
       _ <- connAndInit
-      _ <- NodeUnitTest.syncNeutrinoNode(node, bitcoinds.head)
+      _ <- NodeTestUtil.awaitSyncAndIBD(node, bitcoinds.head)
     } yield {
       succeed
     }
@@ -187,10 +183,10 @@ class NeutrinoNodeTest extends NodeTestWithCachedBitcoindPair {
           .retryUntilSatisfied(peers.size == 2,
                                interval = 1.second,
                                maxTries = 30)
-        _ <- NodeUnitTest.syncNeutrinoNode(node, bitcoind)
         _ <- Future
           .sequence(peers.map(peerManager.isConnected))
           .flatMap(p => assert(p.forall(_ == true)))
+        _ <- NodeTestUtil.awaitSyncAndIBD(node, bitcoind)
         res <- Future
           .sequence(peers.map(peerManager.isConnected))
           .flatMap(p => assert(p.forall(_ == true)))
@@ -221,7 +217,7 @@ class NeutrinoNodeTest extends NodeTestWithCachedBitcoindPair {
       //itself out of IBD. bitcoind will not sendheaders
       //when it believes itself, or it's peer is in IBD
       val gen1F = for {
-        _ <- NodeUnitTest.syncNeutrinoNode(node, bitcoind)
+        _ <- NodeTestUtil.awaitSyncAndIBD(node, bitcoind)
         x <- bitcoind.generate(1)
       } yield x
 
@@ -269,7 +265,7 @@ class NeutrinoNodeTest extends NodeTestWithCachedBitcoindPair {
       val bitcoind = nodeConnectedWithBitcoind.bitcoinds(0)
 
       for {
-        _ <- NodeUnitTest.syncNeutrinoNode(node, bitcoind)
+        _ <- NodeTestUtil.awaitSyncAndIBD(node, bitcoind)
         _ <- AkkaUtil.nonBlockingSleep(3.seconds)
         //have to generate the block headers independent of one another
         //rather than just calling generateToAddress(2,junkAddress)
@@ -291,7 +287,7 @@ class NeutrinoNodeTest extends NodeTestWithCachedBitcoindPair {
       val bitcoind = nodeConnectedWithBitcoind.bitcoinds(0)
 
       for {
-        _ <- NodeUnitTest.syncNeutrinoNode(node, bitcoind)
+        _ <- NodeTestUtil.awaitSyncAndIBD(node, bitcoind)
         _ <- node.stop()
         //drop all compact filter headers / filters
         _ <- CompactFilterHeaderDAO()(executionContext, node.chainConfig)

node/src/main/scala/org/bitcoins/node/PeerManager.scala

@@ -958,8 +958,8 @@ case class PeerManager(
     val chainApi: ChainApi = ChainHandler.fromDatabase()
     val headerF = chainApi.getBestBlockHeader()
     for {
-      _ <- getHeaderSyncHelper(syncPeerOpt)
       _ <- chainApi.setSyncing(true)
+      _ <- getHeaderSyncHelper(syncPeerOpt)
       cancellable = createFilterSyncJob(chainApi, syncPeerOpt)
       _ = {
         syncFilterCancellableOpt = Some(cancellable)

testkit/src/main/scala/org/bitcoins/testkit/node/NodeTestUtil.scala

@@ -1,6 +1,7 @@
 package org.bitcoins.testkit.node
 
 import akka.actor.ActorSystem
+import org.bitcoins.asyncutil.AsyncUtil
 import org.bitcoins.core.api.node.Peer
 import org.bitcoins.core.api.tor.Socks5ProxyParams
 import org.bitcoins.crypto.DoubleSha256DigestBE
@@ -178,6 +179,31 @@ abstract class NodeTestUtil extends P2PLogger {
     } yield ()
   }
 
+  def awaitSyncAndIBD(node: NeutrinoNode, bitcoind: BitcoindRpcClient)(implicit
+      system: ActorSystem): Future[Unit] = {
+    import system.dispatcher
+
+    for {
+      _ <- NodeTestUtil.awaitSync(node, bitcoind)
+      _ <- AsyncUtil.retryUntilSatisfiedF(
+        () => {
+          val chainApi = node.chainApiFromDb()
+          val syncingF = chainApi.flatMap(_.isSyncing())
+          val isIBDF = chainApi.flatMap(_.isIBD())
+          for {
+            syncing <- syncingF
+            isIBD <- isIBDF
+          } yield {
+            !syncing && !isIBD
+          }
+        },
+        interval = 1.second,
+        maxTries = 5
+      )
+    } yield ()
+
+  }
+
   /** get our neutrino node's uri from a test bitcoind instance to send rpc commands for our node.
     * The peer must be initialized by the node.
     */

testkit/src/main/scala/org/bitcoins/testkit/node/NodeUnitTest.scala

@@ -21,7 +21,6 @@ import org.scalatest.FutureOutcome
 
 import java.net.InetSocketAddress
 import java.time.Instant
-import scala.concurrent.duration.DurationInt
 import scala.concurrent.{ExecutionContext, Future}
 
 trait NodeUnitTest extends BaseNodeTest {
@@ -470,7 +469,7 @@ object NodeUnitTest extends P2PLogger {
           //do nothing as we are already syncing
           logger.info(
             s"Node is already syncing, skipping initiating a new sync.")
-          awaitSyncAndIBD(node, bitcoind).map(_ => node)
+          NodeTestUtil.awaitSyncAndIBD(node, bitcoind).map(_ => node)
         } else {
           neutrinoNodeSyncHelper(node, bitcoind)
         }
@@ -494,31 +493,6 @@ object NodeUnitTest extends P2PLogger {
     } yield node
   }
 
-  private def awaitSyncAndIBD(node: NeutrinoNode, bitcoind: BitcoindRpcClient)(
-      implicit system: ActorSystem): Future[Unit] = {
-    import system.dispatcher
-
-    for {
-      _ <- NodeTestUtil.awaitSync(node, bitcoind)
-      _ <- AsyncUtil.retryUntilSatisfiedF(
-        () => {
-          val chainApi = node.chainApiFromDb()
-          val syncingF = chainApi.flatMap(_.isSyncing())
-          val isIBDF = chainApi.flatMap(_.isIBD())
-          for {
-            syncing <- syncingF
-            isIBD <- isIBDF
-          } yield {
-            !syncing && !isIBD
-          }
-        },
-        interval = 1.second,
-        maxTries = 5
-      )
-    } yield ()
-
-  }
-
   /** This is needed for postgres, we do not drop tables in between individual tests with postgres
     * rather an entire test suite shares the same postgres database.
     * therefore, we need to clean the database after each test, so that migrations can be applied during