Testkit wallet with bitcoind uses bitcoind as api (#1499)

* Testkit wallet with bitcoind uses bitcoind as api

* Fix docs

bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/v19/BitcoindV19RpcClient.scala

@@ -15,6 +15,7 @@ import org.bitcoins.core.api.ChainQueryApi.FilterResponse
 import org.bitcoins.core.gcs.FilterType
 import org.bitcoins.core.protocol.transaction.Transaction
 import org.bitcoins.core.script.crypto.HashType
+import org.bitcoins.core.util.FutureUtil
 import org.bitcoins.crypto.ECPrivateKey
 import org.bitcoins.rpc.client.common.{
   BitcoindRpcClient,
@@ -42,16 +43,24 @@ class BitcoindV19RpcClient(override val instance: BitcoindInstance)(
   override def getFiltersBetweenHeights(
       startHeight: Int,
       endHeight: Int): Future[Vector[ChainQueryApi.FilterResponse]] = {
-    Future.sequence(
+    val allHeights = startHeight.to(endHeight)
-      startHeight
+
-        .until(endHeight)
+    def f(range: Vector[Int]): Future[Vector[FilterResponse]] = {
-        .map { height =>
+      val filterFs = range.map { height =>
         for {
           hash <- getBlockHash(height)
           filter <- getBlockFilter(hash, FilterType.Basic)
-          } yield FilterResponse(filter.filter, hash, height)
+        } yield {
+          FilterResponse(filter.filter, hash, height)
         }
-        .toVector)
+      }
+      Future.sequence(filterFs)
+    }
+
+    FutureUtil.batchExecute(elements = allHeights.toVector,
+                            f = f,
+                            init = Vector.empty,
+                            batchSize = 25)
   }
 
   override def getFilterCount: Future[Int] = getBlockCount

docs/wallet/wallet-rescan.md

@@ -17,6 +17,7 @@ To run this example you need to make sure you have access to a bitcoind binary.
 
 ```scala mdoc:invisible
 import org.bitcoins.testkit.BitcoinSTestAppConfig
+import org.bitcoins.testkit.fixtures._
 import org.bitcoins.testkit.wallet._
 import org.bitcoins.server.BitcoinSAppConfig
 import akka.actor.ActorSystem
@@ -33,8 +34,9 @@ implicit val appConfig: BitcoinSAppConfig = BitcoinSTestAppConfig.getNeutrinoTes
 
 //ok now let's spin up a bitcoind and a bitcoin-s wallet with funds in it
 val walletWithBitcoindF = for {
-  w <- BitcoinSWalletTest.createWalletBitcoindNodeChainQueryApi()
+  bitcoind <- BitcoinSFixture.createBitcoindWithFunds()
-} yield w
+  walletWithBitcoind <- BitcoinSWalletTest.createWalletWithBitcoindCallbacks(bitcoind)
+} yield walletWithBitcoind
 
 val walletF = walletWithBitcoindF.map(_.wallet)
 

testkit/src/main/scala/org/bitcoins/testkit/chain/SyncUtil.scala

@@ -1,17 +1,17 @@
 package org.bitcoins.testkit.chain
 
 import org.bitcoins.chain.blockchain.sync.FilterWithHeaderHash
+import org.bitcoins.commons.jsonmodels.bitcoind.GetBlockFilterResult
 import org.bitcoins.core.api.ChainQueryApi.FilterResponse
 import org.bitcoins.core.api.{ChainQueryApi, NodeApi, NodeChainQueryApi}
 import org.bitcoins.core.gcs.FilterType
 import org.bitcoins.core.protocol.BlockStamp
-import org.bitcoins.core.protocol.blockchain.{Block, BlockHeader}
+import org.bitcoins.core.protocol.blockchain.BlockHeader
+import org.bitcoins.core.protocol.transaction.Transaction
 import org.bitcoins.core.util.{BitcoinSLogger, FutureUtil}
+import org.bitcoins.crypto.{DoubleSha256Digest, DoubleSha256DigestBE}
 import org.bitcoins.rpc.client.common.BitcoindRpcClient
 import org.bitcoins.rpc.client.v19.BitcoindV19RpcClient
-import org.bitcoins.commons.jsonmodels.bitcoind.GetBlockFilterResult
-import org.bitcoins.core.protocol.transaction.Transaction
-import org.bitcoins.crypto.{DoubleSha256Digest, DoubleSha256DigestBE}
 import org.bitcoins.wallet.Wallet
 
 import scala.concurrent.{ExecutionContext, Future}
@@ -45,75 +45,41 @@ abstract class SyncUtil extends BitcoinSLogger {
       }
   }
 
-  def getChainQueryApi(bitcoindV19RpcClient: BitcoindV19RpcClient)(
+  def getTestChainQueryApi(bitcoind: BitcoindRpcClient): ChainQueryApi = {
-      implicit ec: ExecutionContext): ChainQueryApi = {
     new ChainQueryApi {
 
       /** Gets the height of the given block */
       override def getBlockHeight(
-          blockHash: DoubleSha256DigestBE): Future[Option[Int]] = {
+          blockHash: DoubleSha256DigestBE): Future[Option[Int]] =
-        bitcoindV19RpcClient
+        bitcoind.getBlockHeight(blockHash)
-          .getBlockHeader(blockHash)
-          .map(b => Some(b.height))
-      }
 
       /** Gets the hash of the block that is what we consider "best" */
       override def getBestBlockHash(): Future[DoubleSha256DigestBE] = {
-        bitcoindV19RpcClient.getBestBlockHash
+        bitcoind.getBestBlockHash
       }
 
       /** Gets number of confirmations for the given block hash */
       override def getNumberOfConfirmations(
           blockHashOpt: DoubleSha256DigestBE): Future[Option[Int]] = {
-        bitcoindV19RpcClient.getBlock(blockHashOpt).map { b =>
+        bitcoind.getNumberOfConfirmations(blockHashOpt)
-          Some(b.confirmations)
-        }
       }
 
       /** Gets the number of compact filters in the database */
       override def getFilterCount: Future[Int] = {
-        //filter count should be same as block height?
+        bitcoind.getFilterCount
-        bitcoindV19RpcClient.getBlockCount
       }
 
       /** Returns the block height of the given block stamp */
-      override def getHeightByBlockStamp(
+      override def getHeightByBlockStamp(blockStamp: BlockStamp): Future[Int] =
-          blockStamp: BlockStamp): Future[Int] = {
+        bitcoind.getHeightByBlockStamp(blockStamp)
-        blockStamp match {
-          case BlockStamp.BlockHash(hash) => getBlockHeight(hash).map(_.get)
-          case BlockStamp.BlockHeight(height) =>
-            Future.successful(height)
-          case BlockStamp.BlockTime(_) =>
-            throw new RuntimeException("Cannot query by block time")
-        }
-      }
 
       override def epochSecondToBlockHeight(time: Long): Future[Int] =
         Future.successful(0)
 
       override def getFiltersBetweenHeights(
           startHeight: Int,
-          endHeight: Int): Future[Vector[FilterResponse]] = {
+          endHeight: Int): Future[Vector[FilterResponse]] =
-        val allHeights = startHeight.to(endHeight)
+        bitcoind.getFiltersBetweenHeights(startHeight, endHeight)
-
-        def f(range: Vector[Int]): Future[Vector[FilterResponse]] = {
-          val filterFs = range.map { height =>
-            for {
-              hash <- bitcoindV19RpcClient.getBlockHash(height)
-              filter <- bitcoindV19RpcClient.getBlockFilter(hash,
-                                                            FilterType.Basic)
-            } yield {
-              FilterResponse(filter.filter, hash, height)
-            }
-          }
-          Future.sequence(filterFs)
-        }
-
-        FutureUtil.batchExecute(elements = allHeights.toVector,
-                                f = f,
-                                init = Vector.empty,
-                                batchSize = 25)
-      }
     }
   }
 
@@ -168,13 +134,9 @@ abstract class SyncUtil extends BitcoinSLogger {
       override def downloadBlocks(
           blockHashes: Vector[DoubleSha256Digest]): Future[Unit] = {
         logger.info(s"Fetching ${blockHashes.length} hashes from bitcoind")
-        val f: Vector[DoubleSha256Digest] => Future[Wallet] = {
+        val f: Vector[DoubleSha256Digest] => Future[Wallet] = { hashes =>
-          case hashes =>
+          val blocksF =
-            val fetchedBlocks: Vector[Future[Block]] = hashes.map {
+            FutureUtil.sequentially(hashes)(bitcoindRpcClient.getBlockRaw)
-              bitcoindRpcClient
-                .getBlockRaw(_)
-            }
-            val blocksF = Future.sequence(fetchedBlocks)
 
           val updatedWalletF = for {
             blocks <- blocksF
@@ -182,7 +144,7 @@ abstract class SyncUtil extends BitcoinSLogger {
             processedWallet <- {
               FutureUtil.foldLeftAsync(wallet, blocks) {
                 case (wallet, block) =>
-                    wallet.processBlock(block).map(_.asInstanceOf[Wallet])
+                  wallet.processBlock(block)
               }
             }
           } yield processedWallet
@@ -213,20 +175,20 @@ abstract class SyncUtil extends BitcoinSLogger {
     }
   }
 
-  def getNodeChainQueryApi(bitcoindV19RpcClient: BitcoindV19RpcClient)(
+  def getNodeChainQueryApi(bitcoind: BitcoindRpcClient)(
       implicit ec: ExecutionContext): NodeChainQueryApi = {
-    val chainQuery = SyncUtil.getChainQueryApi(bitcoindV19RpcClient)
+    val chainQuery = SyncUtil.getTestChainQueryApi(bitcoind)
-    val nodeApi = SyncUtil.getNodeApi(bitcoindV19RpcClient)
+    val nodeApi = SyncUtil.getNodeApi(bitcoind)
     NodeChainQueryApi(nodeApi, chainQuery)
   }
 
   def getNodeChainQueryApiWalletCallback(
-      bitcoindV19RpcClient: BitcoindV19RpcClient,
+      bitcoind: BitcoindRpcClient,
       walletF: Future[Wallet])(
       implicit ec: ExecutionContext): NodeChainQueryApi = {
-    val chainQuery = SyncUtil.getChainQueryApi(bitcoindV19RpcClient)
+    val chainQuery = SyncUtil.getTestChainQueryApi(bitcoind)
     val nodeApi =
-      SyncUtil.getNodeApiWalletCallback(bitcoindV19RpcClient, walletF)
+      SyncUtil.getNodeApiWalletCallback(bitcoind, walletF)
     NodeChainQueryApi(nodeApi, chainQuery)
   }
 }

testkit/src/main/scala/org/bitcoins/testkit/wallet/BitcoinSWalletTest.scala

@@ -12,7 +12,6 @@ import org.bitcoins.core.util.{FutureUtil, TimeUtil}
 import org.bitcoins.core.wallet.fee.{FeeUnit, SatoshisPerVirtualByte}
 import org.bitcoins.crypto.{DoubleSha256Digest, DoubleSha256DigestBE}
 import org.bitcoins.db.AppConfig
-import org.bitcoins.feeprovider.ConstantFeeRateProvider
 import org.bitcoins.keymanager.bip39.BIP39KeyManager
 import org.bitcoins.rpc.client.common.{BitcoindRpcClient, BitcoindVersion}
 import org.bitcoins.rpc.client.v19.BitcoindV19RpcClient
@@ -203,12 +202,30 @@ trait BitcoinSWalletTest
   def withNewWalletAndBitcoind(test: OneArgAsyncTest): FutureOutcome = {
     val builder: () => Future[WalletWithBitcoind] = composeBuildersAndWrap(
       builder = { () =>
-        createDefaultWallet(nodeApi, chainQueryApi)
+        BitcoinSFixture.createBitcoindWithFunds()
       },
-      dependentBuilder = { (wallet: Wallet) =>
+      dependentBuilder = { (bitcoind: BitcoindRpcClient) =>
-        createWalletWithBitcoind(wallet)
+        createWalletWithBitcoind(bitcoind)
       },
-      wrap = (_: WalletApi, walletWithBitcoind: WalletWithBitcoind) =>
+      wrap = (_: BitcoindRpcClient, walletWithBitcoind: WalletWithBitcoind) =>
+        walletWithBitcoind
+    )
+
+    makeDependentFixture(builder, destroy = destroyWalletWithBitcoind)(test)
+  }
+
+  def withNewWalletAndBitcoindV19(test: OneArgAsyncTest): FutureOutcome = {
+    val builder: () => Future[WalletWithBitcoind] = composeBuildersAndWrap(
+      builder = { () =>
+        BitcoinSFixture
+          .createBitcoindWithFunds(Some(BitcoindVersion.V19))
+          .map(_.asInstanceOf[BitcoindV19RpcClient])
+      },
+      dependentBuilder = { (bitcoind: BitcoindV19RpcClient) =>
+        createWalletWithBitcoindV19(bitcoind)
+      },
+      wrap =
+        (_: BitcoindV19RpcClient, walletWithBitcoind: WalletWithBitcoindV19) =>
           walletWithBitcoind
     )
 
@@ -216,17 +233,14 @@ trait BitcoinSWalletTest
   }
 
   def withFundedWalletAndBitcoind(test: OneArgAsyncTest): FutureOutcome = {
-    val builder: () => Future[WalletWithBitcoind] =
+    val builder: () => Future[WalletWithBitcoind] = { () =>
-      composeBuildersAndWrapFuture(
+      for {
-        builder = { () =>
+        bitcoind <- BitcoinSFixture
-          BitcoinSWalletTest.createWallet2Accounts(nodeApi, chainQueryApi)
+          .createBitcoindWithFunds(None)
-        },
+        wallet <- createWalletWithBitcoindCallbacks(bitcoind)
-        dependentBuilder = { (wallet: Wallet) =>
+        fundedWallet <- fundWalletWithBitcoind(wallet)
-          createWalletWithBitcoind(wallet)
+      } yield fundedWallet
-        },
+    }
-        processResult = (_: WalletApi, pair: WalletWithBitcoind) =>
-          fundWalletWithBitcoind(pair)
-      )
 
     makeDependentFixture(builder, destroy = destroyWalletWithBitcoind)(test)
   }
@@ -234,9 +248,14 @@ trait BitcoinSWalletTest
   def withFundedWalletAndBitcoindV19(test: OneArgAsyncTest): FutureOutcome = {
     val builder: () => Future[WalletWithBitcoindV19] = { () =>
       for {
-        walletBitcoind <- createWalletBitcoindNodeChainQueryApi()
+        bitcoind <- BitcoinSFixture
-        fundedWallet <- fundWalletWithBitcoind(walletBitcoind)
+          .createBitcoindWithFunds(Some(BitcoindVersion.V19))
-      } yield fundedWallet
+          .map(_.asInstanceOf[BitcoindV19RpcClient])
+        wallet <- createWalletWithBitcoindCallbacks(bitcoind)
+        fundedWallet <- fundWalletWithBitcoind(wallet)
+      } yield {
+        WalletWithBitcoindV19(fundedWallet.wallet, bitcoind)
+      }
     }
 
     makeDependentFixture(builder, destroy = destroyWalletWithBitcoind)(test)
@@ -406,50 +425,39 @@ object BitcoinSWalletTest extends WalletLogger {
 
   /** Creates a default wallet with bitcoind where the [[ChainQueryApi]] fed to the wallet
     * is implemented by bitcoind */
-  def createWalletBitcoindNodeChainQueryApi(extraConfig: Option[Config] = None)(
+  def createWalletWithBitcoindCallbacks(
+      bitcoind: BitcoindRpcClient,
+      extraConfig: Option[Config] = None)(
       implicit config: BitcoinSAppConfig,
-      system: ActorSystem): Future[WalletWithBitcoindV19] = {
+      system: ActorSystem): Future[WalletWithBitcoind] = {
     import system.dispatcher
-    val bitcoindF = BitcoinSFixture
-      .createBitcoindWithFunds(Some(BitcoindVersion.V19))
-      .map(_.asInstanceOf[BitcoindV19RpcClient])
-    val nodeChainQueryApiF =
-      bitcoindF.map(b => SyncUtil.getNodeChainQueryApi(b))
-    val walletCallbackP = Promise[Wallet]()
-    val walletWithBitcoindV19F = for {
-      bitcoind <- bitcoindF
-      api <- nodeChainQueryApiF
-      wallet <- BitcoinSWalletTest.createWallet2Accounts(api.nodeApi,
-                                                         api.chainQueryApi,
-                                                         extraConfig)
-
     //we need to create a promise so we can inject the wallet with the callback
-      //after we have created it into SyncUtil.getNodeChainQueryApiWalletCallback
+    //after we have created it into SyncUtil.getNodeApiWalletCallback
     //so we don't lose the internal state of the wallet
-
+    val walletCallbackP = Promise[Wallet]()
-      //now unfortunately we have to create _another_ wallet that has the correct callback
+    val walletWithBitcoindF = for {
-      //setup for our wallet so we can receive block updates from bitcoind
+      wallet <- BitcoinSWalletTest.createWallet2Accounts(bitcoind,
-      apiCallback = SyncUtil.getNodeChainQueryApiWalletCallback(
+                                                         bitcoind,
-        bitcoindV19RpcClient = bitcoind,
+                                                         extraConfig)
-        walletF = walletCallbackP.future)
 
       //create the wallet with the appropriate callbacks now that
       //we have them
       walletWithCallback = Wallet(
         keyManager = wallet.keyManager,
-        nodeApi = apiCallback.nodeApi,
+        nodeApi =
-        chainQueryApi = apiCallback.chainQueryApi,
+          SyncUtil.getNodeApiWalletCallback(bitcoind, walletCallbackP.future),
-        feeRateApi = ConstantFeeRateProvider(SatoshisPerVirtualByte.one),
+        chainQueryApi = SyncUtil.getTestChainQueryApi(bitcoind),
+        feeRateApi = bitcoind,
         creationTime = wallet.keyManager.creationTime
       )(wallet.walletConfig, wallet.ec)
       //complete the walletCallbackP so we can handle the callbacks when they are
       //called without hanging forever.
       _ = walletCallbackP.success(walletWithCallback)
-    } yield WalletWithBitcoindV19(walletWithCallback, bitcoind)
+    } yield WalletWithBitcoindRpc(walletWithCallback, bitcoind)
 
-    walletWithBitcoindV19F.failed.foreach(err => walletCallbackP.failure(err))
+    walletWithBitcoindF.failed.foreach(err => walletCallbackP.failure(err))
 
-    walletWithBitcoindV19F
+    walletWithBitcoindF
   }
 
   def createWallet2Accounts(
@@ -490,6 +498,12 @@ object BitcoinSWalletTest extends WalletLogger {
     bitcoindF.map(WalletWithBitcoindRpc(wallet, _))
   }
 
+  def createWalletWithBitcoind(bitcoind: BitcoindRpcClient)(
+      implicit system: ActorSystem,
+      config: BitcoinSAppConfig): Future[WalletWithBitcoind] = {
+    createWalletWithBitcoindCallbacks(bitcoind, None)
+  }
+
   def createWalletWithBitcoindV19(wallet: Wallet)(
       implicit system: ActorSystem): Future[WalletWithBitcoindV19] = {
     import system.dispatcher
@@ -502,6 +516,16 @@ object BitcoinSWalletTest extends WalletLogger {
       created.bitcoind.asInstanceOf[BitcoindV19RpcClient])
   }
 
+  def createWalletWithBitcoindV19(bitcoind: BitcoindV19RpcClient)(
+      implicit system: ActorSystem,
+      config: BitcoinSAppConfig): Future[WalletWithBitcoindV19] = {
+    import system.dispatcher
+    for {
+      created <- createWalletWithBitcoindCallbacks(bitcoind)
+
+    } yield WalletWithBitcoindV19(created.wallet, bitcoind)
+  }
+
   def createWalletWithBitcoind(
       wallet: Wallet,
       bitcoindRpcClient: BitcoindRpcClient

wallet-test/src/test/scala/org/bitcoins/wallet/RescanHandlingTest.scala

@@ -1,7 +1,8 @@
 package org.bitcoins.wallet
 
-import org.bitcoins.core.currency.{Bitcoins, CurrencyUnits}
+import org.bitcoins.core.currency.{Bitcoins, CurrencyUnits, Satoshis}
 import org.bitcoins.core.protocol.BlockStamp
+import org.bitcoins.core.util.FutureUtil
 import org.bitcoins.server.BitcoinSAppConfig
 import org.bitcoins.testkit.BitcoinSTestAppConfig
 import org.bitcoins.testkit.wallet.BitcoinSWalletTest
@@ -11,8 +12,6 @@ import org.bitcoins.testkit.wallet.BitcoinSWalletTest.{
 }
 import org.scalatest.FutureOutcome
 
-import scala.concurrent.Future
-
 class RescanHandlingTest extends BitcoinSWalletTest {
 
   /** Wallet config with data directory set to user temp directory */
@@ -54,6 +53,8 @@ class RescanHandlingTest extends BitcoinSWalletTest {
       val wallet = fixture.wallet
 
       for {
+        balance <- wallet.getBalance()
+        _ = assert(balance != Satoshis.zero)
         utxos <- wallet.spendingInfoDAO.findAll()
         _ = assert(utxos.nonEmpty)
 
@@ -194,11 +195,10 @@ class RescanHandlingTest extends BitcoinSWalletTest {
       val oldestHeightF = for {
         utxos <- utxosF
         blockhashes = utxos.map(_.blockHash)
-        heights <- Future.sequence {
+        heights <- FutureUtil.sequentially(blockhashes) { hash =>
-          blockhashes.map(h =>
+          wallet.chainQueryApi.getBlockHeight(hash.get)
-            wallet.chainQueryApi.getBlockHeight(h.get).map(_.get))
         }
-      } yield heights.min
+      } yield heights.min.get
 
       //ok now that we have the height of the oldest utxo, let's rescan up to then
       val rescanF = for {

wallet-test/src/test/scala/org/bitcoins/wallet/WalletIntegrationTest.scala

@@ -93,10 +93,10 @@ class WalletIntegrationTest extends BitcoinSWalletTest {
         .map(balance => assert(balance == valueFromBitcoind))
       _ <- wallet
         .getConfirmedBalance()
-        .map(confirmed => assert(confirmed == 0.bitcoin))
+        .map(confirmed => assert(confirmed == valueFromBitcoind))
       _ <- wallet
         .getUnconfirmedBalance()
-        .map(unconfirmed => assert(unconfirmed == valueFromBitcoind))
+        .map(unconfirmed => assert(unconfirmed == 0.satoshis))
 
       signedTx <- bitcoind.getNewAddress.flatMap {
         wallet.sendToAddress(_, valueToBitcoind, Some(feeRate))