Refactor test case to be more idiomatic in hopes this kills CI failures (#2524)

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

@@ -9,6 +9,7 @@ import org.bitcoins.rpc.util.RpcUtil
 import org.bitcoins.server.BitcoinSAppConfig
 import org.bitcoins.testkit.BitcoinSTestAppConfig
 import org.bitcoins.testkit.fixtures.UsesExperimentalBitcoind
+import org.bitcoins.testkit.node.fixture.NeutrinoNodeConnectedWithBitcoind
 import org.bitcoins.testkit.node.{
   NeutrinoNodeFundedWalletBitcoind,
   NodeTestUtil,
@@ -25,46 +26,18 @@ class NeutrinoNodeTest extends NodeUnitTest {
   implicit override protected def getFreshConfig: BitcoinSAppConfig =
     BitcoinSTestAppConfig.getNeutrinoWithEmbeddedDbTestConfig(pgUrl)
 
-  override type FixtureParam = NeutrinoNodeFundedWalletBitcoind
+  override type FixtureParam = NeutrinoNodeConnectedWithBitcoind
 
   override def withFixture(test: OneArgAsyncTest): FutureOutcome =
-    withNeutrinoNodeFundedWalletBitcoind(test,
+    withNeutrinoNodeConnectedToBitcoind(test, Some(BitcoindVersion.V21))
-                                         getBIP39PasswordOpt(),
-                                         Some(BitcoindVersion.V21))
-
-  private var assertionP: Promise[Boolean] = Promise()
-  after {
-    //reset assertion after a test runs, because we
-    //are doing mutation to work around our callback
-    //limitations, we can't currently modify callbacks
-    //after a NeutrinoNode is constructed :-(
-    assertionP = Promise()
-  }
-  //what is going on here??
-  private val utxos: Set[ScriptPubKey] = Set.empty
-
-  private def blockCallback(block: Block): Future[Unit] = {
-    val scriptPubKeys =
-      block.transactions.flatMap(tx => tx.outputs.map(_.scriptPubKey)).toSet
-    assertionP
-      //is this trivially false always?
-      .success(utxos.intersect(scriptPubKeys) == utxos)
-      .future
-      .map(_ => ())
-  }
-
-  def callbacks: NodeCallbacks = {
-    NodeCallbacks(onBlockReceived = Vector(blockCallback(_)))
-  }
 
   behavior of "NeutrinoNode"
 
   it must "receive notification that a block occurred on the p2p network" taggedAs UsesExperimentalBitcoind in {
-    nodeConnectedWithBitcoind: NeutrinoNodeFundedWalletBitcoind =>
+    nodeConnectedWithBitcoind: NeutrinoNodeConnectedWithBitcoind =>
       val node = nodeConnectedWithBitcoind.node
 
-      val _ = node.nodeAppConfig.addCallbacks(callbacks)
+      val bitcoind = nodeConnectedWithBitcoind.bitcoind
-      val bitcoind = nodeConnectedWithBitcoind.bitcoindRpc
 
       val assert1F = for {
         _ <- node.isConnected.map(assert(_))
@@ -75,12 +48,10 @@ class NeutrinoNodeTest extends NodeUnitTest {
         .flatMap(bitcoind.generateToAddress(1, _))
         .map(_.head)
 
-      //sync our spv node expecting to get that generated hash
       val syncF = for {
         _ <- assert1F
         _ <- hashF
-        sync <- node.sync()
+      } yield ()
-      } yield sync
 
       syncF.flatMap { _ =>
         NodeTestUtil
@@ -90,9 +61,9 @@ class NeutrinoNodeTest extends NodeUnitTest {
   }
 
   it must "stay in sync with a bitcoind instance" taggedAs UsesExperimentalBitcoind in {
-    nodeConnectedWithBitcoind: NeutrinoNodeFundedWalletBitcoind =>
+    nodeConnectedWithBitcoind: NeutrinoNodeConnectedWithBitcoind =>
       val node = nodeConnectedWithBitcoind.node
-      val bitcoind = nodeConnectedWithBitcoind.bitcoindRpc
+      val bitcoind = nodeConnectedWithBitcoind.bitcoind
 
       //we need to generate 1 block for bitcoind to consider
       //itself out of IBD. bitcoind will not sendheaders
@@ -105,9 +76,7 @@ class NeutrinoNodeTest extends NodeUnitTest {
       //both our spv node and our bitcoind node _should_ both be at the genesis block (regtest)
       //at this point so no actual syncing is happening
       val initSyncF = gen1F.flatMap { hashes =>
-        val syncF = node.sync()
         for {
-          _ <- syncF
           _ <- NodeTestUtil.awaitBestHash(hashes.head, node)
         } yield ()
       }
@@ -123,39 +92,8 @@ class NeutrinoNodeTest extends NodeUnitTest {
       startGenF.flatMap { cancellable =>
         //we should expect 5 headers have been announced to us via
         //the send headers message.
-        val ExpectedCount = 119
-
-        def hasBlocksF =
-          RpcUtil.retryUntilSatisfiedF(conditionF = () => {
-                                         node
-                                           .chainApiFromDb()
-                                           .flatMap(_.getBlockCount())
-                                           .map(_ == ExpectedCount)
-                                       },
-                                       interval = 1000.millis)
-
-        def hasFilterHeadersF =
-          RpcUtil.retryUntilSatisfiedF(conditionF = () => {
-                                         node
-                                           .chainApiFromDb()
-                                           .flatMap(_.getFilterHeaderCount())
-                                           .map(_ == ExpectedCount)
-                                       },
-                                       interval = 1000.millis)
-
-        def hasFiltersF =
-          RpcUtil.retryUntilSatisfiedF(conditionF = () => {
-                                         node
-                                           .chainApiFromDb()
-                                           .flatMap(_.getFilterCount())
-                                           .map(_ == ExpectedCount)
-                                       },
-                                       interval = 1000.millis)
-
         for {
-          _ <- hasBlocksF
+          _ <- NodeTestUtil.awaitSync(node, bitcoind)
-          _ <- hasFilterHeadersF
-          _ <- hasFiltersF
         } yield {
           val isCancelled = cancellable.cancel()
           if (!isCancelled) {

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

@@ -35,7 +35,11 @@ import org.bitcoins.testkit.EmbeddedPg
 import org.bitcoins.testkit.chain.ChainUnitTest
 import org.bitcoins.testkit.fixtures.BitcoinSFixture
 import org.bitcoins.testkit.keymanager.KeyManagerTestUtil
-import org.bitcoins.testkit.node.NodeUnitTest.{createPeer, emptyPeer}
+import org.bitcoins.testkit.node.NodeUnitTest.{
+  createPeer,
+  emptyPeer,
+  syncNeutrinoNode
+}
 import org.bitcoins.testkit.node.fixture.{
   NeutrinoNodeConnectedWithBitcoind,
   NodeConnectedWithBitcoind,
@@ -262,7 +266,9 @@ trait NodeUnitTest extends BitcoinSFixture with EmbeddedPg {
         node <- NodeUnitTest.createNeutrinoNode(bitcoind)(system,
                                                           appConfig.chainConf,
                                                           appConfig.nodeConf)
-      } yield NeutrinoNodeConnectedWithBitcoind(node, bitcoind)
+        startedNode <- node.start()
+        syncedNode <- syncNeutrinoNode(startedNode, bitcoind)
+      } yield NeutrinoNodeConnectedWithBitcoind(syncedNode, bitcoind)
     }
     makeDependentFixture(
       build = nodeWithBitcoindBuilder,