Make tests to not require pre-installed bitcoind (#766)

* Make tests to not require pre-installed bitcoind

* update docs

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/BitcoindInstanceTest.scala

@@ -1,25 +1,26 @@
 package org.bitcoins.rpc
 import java.io.{File, PrintWriter}
+import java.net.URI
 import java.nio.file.{Files, Path}
 
+import akka.stream.StreamTcpException
+import org.bitcoins.core.config.RegTest
 import org.bitcoins.core.currency.Bitcoins
 import org.bitcoins.rpc.client.common.BitcoindRpcClient
-import org.bitcoins.rpc.config.BitcoindInstance
-import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil
-import org.bitcoins.testkit.util.BitcoindRpcTest
-
-import scala.io.Source
-import akka.stream.StreamTcpException
-import java.nio.file.Paths
-import scala.util.Properties
-import org.bitcoins.rpc.config.BitcoindConfig
-import org.bitcoins.rpc.config.BitcoindAuthCredentials
+import org.bitcoins.rpc.config.{
+  BitcoindAuthCredentials,
+  BitcoindConfig,
+  BitcoindInstance
+}
 import org.bitcoins.rpc.util.RpcUtil
-import org.bitcoins.core.config.RegTest
-import java.net.URI
-import scala.concurrent.Future
+import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil
+import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil.newestBitcoindBinary
+import org.bitcoins.testkit.util.BitcoindRpcTest
 import org.scalatest.compatible.Assertion
 
+import scala.concurrent.Future
+import scala.io.Source
+
 class BitcoindInstanceTest extends BitcoindRpcTest {
 
   private val sampleConf: Seq[String] = {
@@ -65,7 +66,7 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
     """.stripMargin
 
     val conf = BitcoindConfig(confStr, BitcoindRpcTestUtil.tmpDir())
-    val instance = BitcoindInstance.fromConfig(conf)
+    val instance = BitcoindInstance.fromConfig(conf, newestBitcoindBinary)
     assert(
       instance.authCredentials
         .isInstanceOf[BitcoindAuthCredentials.CookieBased])
@@ -85,7 +86,7 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
       """.stripMargin
 
     val conf = BitcoindConfig(confStr, BitcoindRpcTestUtil.tmpDir())
-    val instance = BitcoindInstance.fromConfig(conf)
+    val instance = BitcoindInstance.fromConfig(conf, newestBitcoindBinary)
     assert(
       instance.authCredentials
         .isInstanceOf[BitcoindAuthCredentials.PasswordBased])
@@ -121,14 +122,16 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
         uri = new URI(s"http://localhost:$port"),
         rpcUri = new URI(s"http://localhost:$rpcPort"),
         authCredentials = authCredentials,
-        datadir = conf.datadir
+        datadir = conf.datadir,
+        binary = newestBitcoindBinary
       )
 
     testClientStart(BitcoindRpcClient.withActorSystem(instance))
   }
 
   it should "parse a bitcoin.conf file, start bitcoind, mine some blocks and quit" in {
-    val instance = BitcoindInstance.fromDatadir(datadir.toFile)
+    val instance =
+      BitcoindInstance.fromDatadir(datadir.toFile, newestBitcoindBinary)
     val client = BitcoindRpcClient.withActorSystem(instance)
 
     for {

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/TestRpcUtilTest.scala

@@ -4,7 +4,6 @@ import java.io.File
 
 import org.bitcoins.core.currency.Bitcoins
 import org.bitcoins.rpc.client.common.BitcoindRpcClient
-import org.bitcoins.rpc.client.common.RpcOpts.AddNodeArgument
 import org.bitcoins.rpc.util.AsyncUtil.RpcRetryException
 import org.bitcoins.rpc.util.{AsyncUtil, RpcUtil}
 import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/common/BlockchainRpcTest.scala

@@ -2,7 +2,7 @@ package org.bitcoins.rpc.common
 
 import org.bitcoins.core.currency.Bitcoins
 import org.bitcoins.core.number.UInt32
-import org.bitcoins.rpc.client.common.BitcoindRpcClient
+import org.bitcoins.rpc.client.common.{BitcoindRpcClient, BitcoindVersion}
 import org.bitcoins.rpc.client.common.RpcOpts.{AddNodeArgument, AddressType}
 import org.bitcoins.rpc.util.AsyncUtil
 import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil
@@ -14,13 +14,14 @@ import org.bitcoins.core.config.RegTest
 class BlockchainRpcTest extends BitcoindRpcTest {
 
   lazy val clientsF: Future[(BitcoindRpcClient, BitcoindRpcClient)] =
-    BitcoindRpcTestUtil.createNodePair(clientAccum = clientAccum)
+    BitcoindRpcTestUtil.createNodePairV17(clientAccum = clientAccum)
 
   lazy val pruneClientF: Future[BitcoindRpcClient] = clientsF.flatMap {
     case (_, _) =>
       val pruneClient =
         BitcoindRpcClient.withActorSystem(
-          BitcoindRpcTestUtil.instance(pruneMode = true))
+          BitcoindRpcTestUtil.instance(pruneMode = true,
+                                       versionOpt = Some(BitcoindVersion.V17)))
 
       clientAccum += pruneClient
 

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/common/MempoolRpcTest.scala

@@ -1,9 +1,7 @@
 package org.bitcoins.rpc.common
 
 import java.io.File
-import java.nio.file.Files
 
-import com.typesafe.config.ConfigValueFactory
 import org.bitcoins.core.currency.Bitcoins
 import org.bitcoins.core.number.UInt32
 import org.bitcoins.core.protocol.script.ScriptSignature
@@ -17,11 +15,10 @@ import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil
 import org.bitcoins.testkit.util.BitcoindRpcTest
 
 import scala.concurrent.Future
-import java.nio.file.Path
 
 class MempoolRpcTest extends BitcoindRpcTest {
   lazy val clientsF: Future[(BitcoindRpcClient, BitcoindRpcClient)] =
-    BitcoindRpcTestUtil.createNodePair(clientAccum = clientAccum)
+    BitcoindRpcTestUtil.createNodePairV18(clientAccum = clientAccum)
 
   lazy val clientWithoutBroadcastF: Future[BitcoindRpcClient] =
     clientsF.flatMap {
@@ -33,7 +30,8 @@ class MempoolRpcTest extends BitcoindRpcTest {
             .withOption("walletbroadcast", 0.toString)
 
         val instanceWithoutBroadcast =
-          BitcoindInstance.fromConfig(configNoBroadcast)
+          BitcoindInstance.fromConfig(configNoBroadcast,
+                                      BitcoindRpcTestUtil.newestBitcoindBinary)
 
         val clientWithoutBroadcast =
           BitcoindRpcClient.withActorSystem(instanceWithoutBroadcast)

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/common/MiningRpcTest.scala

@@ -9,7 +9,7 @@ import org.bitcoins.rpc.BitcoindP2PException.NotConnected
 
 class MiningRpcTest extends BitcoindRpcTest {
   lazy val clientsF: Future[(BitcoindRpcClient, BitcoindRpcClient)] =
-    BitcoindRpcTestUtil.createNodePair(clientAccum = clientAccum)
+    BitcoindRpcTestUtil.createNodePairV17(clientAccum = clientAccum)
 
   behavior of "MiningRpc"
 

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/common/RawTransactionRpcTest.scala

@@ -20,7 +20,7 @@ import org.bitcoins.rpc.BitcoindException.InvalidAddressOrKey
 
 class RawTransactionRpcTest extends BitcoindRpcTest {
   lazy val clientsF: Future[(BitcoindRpcClient, BitcoindRpcClient)] =
-    BitcoindRpcTestUtil.createNodePair(clientAccum = clientAccum)
+    BitcoindRpcTestUtil.createNodePairV17(clientAccum = clientAccum)
 
   behavior of "RawTransactionRpc"
 

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/common/WalletRpcTest.scala

@@ -33,7 +33,7 @@ import scala.concurrent.Future
 class WalletRpcTest extends BitcoindRpcTest {
   lazy val clientsF: Future[
     (BitcoindRpcClient, BitcoindRpcClient, BitcoindRpcClient)] =
-    BitcoindRpcTestUtil.createNodeTriple(clientAccum = clientAccum)
+    BitcoindRpcTestUtil.createNodeTripleV17(clientAccum = clientAccum)
 
   // This client's wallet is encrypted
   lazy val walletClientF: Future[BitcoindRpcClient] = clientsF.flatMap { _ =>

bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/common/BitcoindRpcClient.scala

@@ -75,8 +75,9 @@ object BitcoindRpcClient {
     * the default datadir if no directory is provided
     */
   def fromDatadir(
-      datadir: File = BitcoindConfig.DEFAULT_DATADIR): BitcoindRpcClient = {
-    val instance = BitcoindInstance.fromDatadir(datadir)
+      datadir: File = BitcoindConfig.DEFAULT_DATADIR,
+      binary: File): BitcoindRpcClient = {
+    val instance = BitcoindInstance.fromDatadir(datadir, binary)
     val cli = BitcoindRpcClient(instance)
     cli
   }

bitcoind-rpc/src/main/scala/org/bitcoins/rpc/config/BitcoindInstance.scala

@@ -1,6 +1,6 @@
 package org.bitcoins.rpc.config
 
-import java.io.File
+import java.io.{File, FileNotFoundException}
 import java.net.URI
 import java.nio.file.{Files, Paths}
 
@@ -9,7 +9,6 @@ import org.bitcoins.core.util.BitcoinSLogger
 import org.bitcoins.rpc.client.common.BitcoindVersion
 
 import scala.sys.process._
-
 import org.bitcoins.core.util.BitcoinSLogger
 import org.bitcoins.core.config.NetworkParameters
 
@@ -51,6 +50,9 @@ sealed trait BitcoindInstance extends BitcoinSLogger {
         .last
 
     foundVersion match {
+      case _: String
+          if foundVersion.equals(BitcoindVersion.Experimental.toString) =>
+        BitcoindVersion.Experimental
       case _: String if foundVersion.startsWith(BitcoindVersion.V16.toString) =>
         BitcoindVersion.V16
       case _: String if foundVersion.startsWith(BitcoindVersion.V17.toString) =>
@@ -95,16 +97,22 @@ object BitcoindInstance {
 
   lazy val DEFAULT_BITCOIND_LOCATION: File = {
 
+    def findExecutableOnPath(name: String): Option[File] =
+      sys.env
+        .getOrElse("PATH", "")
+        .split(File.pathSeparator)
+        .map(directory => new File(directory, name))
+        .find(file => file.isFile && file.canExecute)
+
     val cmd =
       if (Properties.isWin) {
-        "which bitcoind.exe".!!
+        findExecutableOnPath("bitcoind.exe")
       } else {
-        "which bitcoind".!!
+        findExecutableOnPath("bitcoind")
       }
 
-    val path = cmd
-    new File(path.trim)
-
+    cmd.getOrElse(
+      throw new FileNotFoundException("Cannot find a path to bitcoind"))
   }
 
   /** Constructs a `bitcoind` instance from the given datadir, using the
@@ -113,7 +121,9 @@ object BitcoindInstance {
     * @throws IllegalArgumentException if the given datadir does not exist
     */
   def fromDatadir(
-      datadir: File = BitcoindConfig.DEFAULT_DATADIR): BitcoindInstance = {
+      datadir: File = BitcoindConfig.DEFAULT_DATADIR,
+      binary: File = DEFAULT_BITCOIND_LOCATION
+  ): BitcoindInstance = {
     require(datadir.exists, s"${datadir.getPath} does not exist!")
     require(datadir.isDirectory, s"${datadir.getPath} is not a directory!")
 
@@ -121,9 +131,9 @@ object BitcoindInstance {
     if (Files.exists(configPath)) {
 
       val file = configPath.toFile()
-      fromConfigFile(file)
+      fromConfigFile(file, binary)
     } else {
-      fromConfig(BitcoindConfig.empty)
+      fromConfig(BitcoindConfig.empty, binary)
     }
   }
 
@@ -134,18 +144,21 @@ object BitcoindInstance {
     * @throws  IllegalArgumentException if the given config file does not exist
     */
   def fromConfigFile(
-      file: File = BitcoindConfig.DEFAULT_CONF_FILE): BitcoindInstance = {
+      file: File = BitcoindConfig.DEFAULT_CONF_FILE,
+      binary: File = DEFAULT_BITCOIND_LOCATION
+  ): BitcoindInstance = {
     require(file.exists, s"${file.getPath} does not exist!")
     require(file.isFile, s"${file.getPath} is not a file!")
 
     val conf = BitcoindConfig(file, file.getParentFile)
 
-    fromConfig(conf)
+    fromConfig(conf, binary)
   }
 
   /** Constructs a `bitcoind` instance from the given config */
   def fromConfig(
-      config: BitcoindConfig
+      config: BitcoindConfig,
+      binary: File = DEFAULT_BITCOIND_LOCATION
   ): BitcoindInstance = {
 
     val authCredentials = BitcoindAuthCredentials.fromConfig(config)
@@ -154,6 +167,7 @@ object BitcoindInstance {
                      config.rpcUri,
                      authCredentials,
                      zmqConfig = ZmqConfig.fromConfig(config),
+                     binary = binary,
                      datadir = config.datadir)
   }
 }

docs/rpc/bitcoind.md

@@ -18,13 +18,14 @@ import scala.concurrent._
 import org.bitcoins.{rpc, core}
 import core.currency.Bitcoins
 import rpc.client.common._
+import java.io._
 
 implicit val ec: ExecutionContext = ExecutionContext.global
 
 // this reads authentication credentials and
 // connection details from the default data
 // directory on your platform
-val client = BitcoindRpcClient.fromDatadir()
+val client = BitcoindRpcClient.fromDatadir(binary=new File("/path/to/bitcoind"), datadir=new File("/path/to/bitcoind-datadir"))
 
 val balance: Future[Bitcoins] = for {
   _ <- client.start()
@@ -98,6 +99,7 @@ import org.bitcoins.rpc.BitcoindWalletException
 import org.bitcoins.core.crypto._
 import org.bitcoins.core.protocol._
 import org.bitcoins.core.currency._
+import java.io._
 
 import scala.concurrent._
 
@@ -105,7 +107,7 @@ implicit val ec = ExecutionContext.global
 
 // let's assume you have an already running client,
 // so there's no need to start this one
-val cli = BitcoindRpcClient.fromDatadir()
+val cli = BitcoindRpcClient.fromDatadir(binary=new File("/path/to/bitcoind"), datadir=new File("/path/to/bitcoind-datadir"))
 
 // let's also assume you have a bitcoin address
 val address: BitcoinAddress = ???

testkit/src/main/scala/org/bitcoins/testkit/eclair/rpc/EclairRpcTestUtil.scala

@@ -627,7 +627,8 @@ trait EclairRpcTestUtil extends BitcoinSLogger {
         network = instance.network,
         uri = new URI("http://localhost:18333"),
         rpcUri = auth.bitcoindRpcUri,
-        authCredentials = auth.bitcoinAuthOpt.get
+        authCredentials = auth.bitcoinAuthOpt.get,
+        binary = BitcoindRpcTestUtil.newestBitcoindBinary
       )
       BitcoindRpcClient.withActorSystem(bitcoindInstance)
     }

testkit/src/main/scala/org/bitcoins/testkit/rpc/BitcoindRpcTestUtil.scala

@@ -171,6 +171,8 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
     baseBinaryDirectory.resolve("bitcoind")
   }
 
+  def newestBitcoindBinary: File = getBinary(BitcoindVersion.newest)
+
   private def getBinary(version: BitcoindVersion): File = version match {
     // default to newest version
     case Unknown => getBinary(BitcoindVersion.newest)
@@ -224,23 +226,16 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
     val binary: File = versionOpt match {
       case Some(version) => getBinary(version)
       case None =>
-        Try {
-          BitcoindInstance.DEFAULT_BITCOIND_LOCATION
-        }.recoverWith {
-          case _: RuntimeException =>
-            if (Files.exists(
-                  BitcoindRpcTestUtil.binaryDirectory
-                )) {
-              Success(getBinary(BitcoindVersion.newest))
-            } else {
-              Failure(new RuntimeException(
-                "Could not locate bitcoind. Make sure it is installed on your PATH, or if working with Bitcoin-S directly, try running 'sbt downloadBitcoind'"))
-            }
-
-        } match {
-          case Failure(exception) => throw exception
-          case Success(value)     => value
+        if (Files.exists(
+              BitcoindRpcTestUtil.binaryDirectory
+            )) {
+          newestBitcoindBinary
+        } else {
+          throw new RuntimeException(
+            "Could not locate bitcoind. Make sure it is installed on your PATH, or if working with Bitcoin-S " +
+              "directly, try running 'sbt downloadBitcoind'")
         }
+
     }
     val instance = BitcoindInstance(network = network,
                                     uri = uri,
@@ -708,6 +703,13 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
     createNodeTripleInternal(BitcoindVersion.V17, clientAccum)
   }
 
+  def createNodeTripleV18(
+      clientAccum: RpcClientAccum = Vector.newBuilder
+  )(implicit system: ActorSystem): Future[
+    (BitcoindV18RpcClient, BitcoindV18RpcClient, BitcoindV18RpcClient)] = {
+    createNodeTripleInternal(BitcoindVersion.V18, clientAccum)
+  }
+
   def createRawCoinbaseTransaction(
       sender: BitcoindRpcClient,
       receiver: BitcoindRpcClient,