Make it possible to construct RPC client without ActorSystem (#725)

app/server/src/main/scala/org/bitcoins/server/Main.scala

@@ -69,7 +69,7 @@ object Main extends App {
   }
 
   val bitcoind = BitcoindInstance.fromDatadir()
-  val bitcoindCli = new BitcoindRpcClient(bitcoind)
+  val bitcoindCli = BitcoindRpcClient.withActorSystem(bitcoind)
   val peer = Peer.fromBitcoind(bitcoind)
 
   val startFut = for {

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

@@ -70,7 +70,7 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
       instance.authCredentials
         .isInstanceOf[BitcoindAuthCredentials.CookieBased])
 
-    val cli = new BitcoindRpcClient(instance)
+    val cli = BitcoindRpcClient.withActorSystem(instance)
     testClientStart(cli)
   }
 
@@ -89,7 +89,7 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
     assert(
       instance.authCredentials
         .isInstanceOf[BitcoindAuthCredentials.PasswordBased])
-    testClientStart(new BitcoindRpcClient(instance))
+    testClientStart(BitcoindRpcClient.withActorSystem(instance))
   }
 
   // the values in this conf was generated by executing
@@ -124,12 +124,12 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
         datadir = conf.datadir
       )
 
-    testClientStart(new BitcoindRpcClient(instance))
+    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 client = new BitcoindRpcClient(instance)
+    val client = BitcoindRpcClient.withActorSystem(instance)
 
     for {
       _ <- client.start()

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

@@ -120,7 +120,7 @@ class TestRpcUtilTest extends BitcoindRpcTest {
 
   it should "be able to create a single node, wait for it to start and then delete it" in {
     val instance = BitcoindRpcTestUtil.instance()
-    val client = new BitcoindRpcClient(instance)
+    val client = BitcoindRpcClient.withActorSystem(instance)
     val startedF = client.start()
 
     startedF.map { _ =>

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

@@ -19,7 +19,8 @@ class BlockchainRpcTest extends BitcoindRpcTest {
   lazy val pruneClientF: Future[BitcoindRpcClient] = clientsF.flatMap {
     case (_, _) =>
       val pruneClient =
-        new BitcoindRpcClient(BitcoindRpcTestUtil.instance(pruneMode = true))
+        BitcoindRpcClient.withActorSystem(
+          BitcoindRpcTestUtil.instance(pruneMode = true))
 
       clientAccum += pruneClient
 

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

@@ -36,7 +36,7 @@ class MempoolRpcTest extends BitcoindRpcTest {
           BitcoindInstance.fromConfig(configNoBroadcast)
 
         val clientWithoutBroadcast =
-          new BitcoindRpcClient(instanceWithoutBroadcast)
+          BitcoindRpcClient.withActorSystem(instanceWithoutBroadcast)
         clientAccum += clientWithoutBroadcast
 
         val pairs = Vector(client -> clientWithoutBroadcast,

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

@@ -34,7 +34,8 @@ class WalletRpcTest extends BitcoindRpcTest {
 
   // This client's wallet is encrypted
   lazy val walletClientF: Future[BitcoindRpcClient] = clientsF.flatMap { _ =>
-    val walletClient = new BitcoindRpcClient(BitcoindRpcTestUtil.instance())
+    val walletClient =
+      BitcoindRpcClient.withActorSystem(BitcoindRpcTestUtil.instance())
     clientAccum += walletClient
 
     for {

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

@@ -44,14 +44,40 @@ class BitcoindRpcClient(val instance: BitcoindInstance)(
 
 object BitcoindRpcClient {
 
+  /** The name we give to actor systems we create. We use this
+    * information to know which actor systems to shut down */
+  private[rpc] val ActorSystemName = "bitcoind-rpc-client-created-by-bitcoin-s"
+
+  /**
+    * Creates an RPC client from the given instance.
+    *
+    * Behind the scenes, we create an actor system for
+    * you. You can use `withActorSystem` if you want to
+    * manually specify an actor system for the RPC client.
+    */
+  def apply(instance: BitcoindInstance): BitcoindRpcClient = {
+    implicit val system = ActorSystem.create(ActorSystemName)
+    withActorSystem(instance)
+  }
+
+  /**
+    * Creates an RPC client from the given instance,
+    * together with the given actor system. This is for
+    * advanced users, wher you need fine grained control
+    * over the RPC client.
+    */
+  def withActorSystem(instance: BitcoindInstance)(
+      implicit system: ActorSystem): BitcoindRpcClient =
+    new BitcoindRpcClient(instance)
+
   /**
     * Constructs a RPC client from the given datadir, or
     * the default datadir if no directory is provided
     */
-  def fromDatadir(datadir: File = BitcoindConfig.DEFAULT_DATADIR)(
-      implicit system: ActorSystem): BitcoindRpcClient = {
+  def fromDatadir(
+      datadir: File = BitcoindConfig.DEFAULT_DATADIR): BitcoindRpcClient = {
     val instance = BitcoindInstance.fromDatadir(datadir)
-    val cli = new BitcoindRpcClient(instance)
+    val cli = BitcoindRpcClient(instance)
     cli
   }
 }

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

@@ -7,6 +7,7 @@ import org.bitcoins.rpc.serializers.JsonSerializers._
 import play.api.libs.json._
 
 import scala.concurrent.Future
+import org.bitcoins.core.util.FutureUtil
 
 /**
   * RPC calls related to administration of a given node
@@ -72,6 +73,13 @@ trait NodeRpc { self: Client =>
   }
 
   def stop(): Future[String] = {
-    bitcoindCall[String]("stop")
+    for {
+      res <- bitcoindCall[String]("stop")
+      _ <- {
+        if (system.name == BitcoindRpcClient.ActorSystemName) {
+          system.terminate()
+        } else FutureUtil.unit
+      }
+    } yield res
   }
 }

bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/v16/BitcoindV16RpcClient.scala

@@ -77,9 +77,31 @@ class BitcoindV16RpcClient(override val instance: BitcoindInstance)(
 
 object BitcoindV16RpcClient {
 
-  def fromUnknownVersion(rpcClient: BitcoindRpcClient)(
-      implicit actorSystem: ActorSystem): Try[BitcoindV16RpcClient] =
+  /**
+    * Creates an RPC client from the given instance.
+    *
+    * Behind the scenes, we create an actor system for
+    * you. You can use `withActorSystem` if you want to
+    * manually specify an actor system for the RPC client.
+    */
+  def apply(instance: BitcoindInstance): BitcoindV16RpcClient = {
+    implicit val system = ActorSystem.create(BitcoindRpcClient.ActorSystemName)
+    withActorSystem(instance)
+  }
+
+  /**
+    * Creates an RPC client from the given instance,
+    * together with the given actor system. This is for
+    * advanced users, wher you need fine grained control
+    * over the RPC client.
+    */
+  def withActorSystem(instance: BitcoindInstance)(
+      implicit system: ActorSystem): BitcoindV16RpcClient =
+    new BitcoindV16RpcClient(instance)
+
+  def fromUnknownVersion(
+      rpcClient: BitcoindRpcClient): Try[BitcoindV16RpcClient] =
     Try {
-      new BitcoindV16RpcClient(rpcClient.instance)
+      new BitcoindV16RpcClient(rpcClient.instance)(rpcClient.system)
     }
 }

bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/v17/BitcoindV17RpcClient.scala

@@ -105,9 +105,31 @@ class BitcoindV17RpcClient(override val instance: BitcoindInstance)(
 
 object BitcoindV17RpcClient {
 
-  def fromUnknownVersion(rpcClient: BitcoindRpcClient)(
-      implicit actorSystem: ActorSystem): Try[BitcoindV17RpcClient] =
+  /**
+    * Creates an RPC client from the given instance.
+    *
+    * Behind the scenes, we create an actor system for
+    * you. You can use `withActorSystem` if you want to
+    * manually specify an actor system for the RPC client.
+    */
+  def apply(instance: BitcoindInstance): BitcoindV17RpcClient = {
+    implicit val system = ActorSystem.create(BitcoindRpcClient.ActorSystemName)
+    withActorSystem(instance)
+  }
+
+  /**
+    * Creates an RPC client from the given instance,
+    * together with the given actor system. This is for
+    * advanced users, wher you need fine grained control
+    * over the RPC client.
+    */
+  def withActorSystem(instance: BitcoindInstance)(
+      implicit system: ActorSystem): BitcoindV17RpcClient =
+    new BitcoindV17RpcClient(instance)
+
+  def fromUnknownVersion(
+      rpcClient: BitcoindRpcClient): Try[BitcoindV17RpcClient] =
     Try {
-      new BitcoindV17RpcClient(rpcClient.instance)
+      new BitcoindV17RpcClient(rpcClient.instance)(rpcClient.system)
     }
 }

docs/chain/sync-chain.md

@@ -10,7 +10,6 @@ able to read this chain on subsequent runs, assuming we are connected
 to the same `bitcoind` instance.
 
 ```scala mdoc:compile-only
-import akka.actor.ActorSystem
 import org.bitcoins.chain.blockchain._
 import org.bitcoins.chain.blockchain.sync._
 import org.bitcoins.chain.models._
@@ -20,8 +19,7 @@ import org.bitcoins.testkit.chain._
 
 import scala.concurrent._
 
-implicit val system = ActorSystem()
-implicit val exectionContext = system.dispatcher
+implicit val ec = ExecutionContext.global
 
 // We are assuming that a `bitcoind` regtest node is running the background.
 // You can see our `bitcoind` guides to see how to connect
@@ -31,7 +29,7 @@ import org.bitcoins.rpc.config.BitcoindInstance
 import org.bitcoins.rpc.client.common.BitcoindRpcClient
 
 val bitcoindInstance = BitcoindInstance.fromDatadir()
-val rpcCli = new BitcoindRpcClient(bitcoindInstance)
+val rpcCli = BitcoindRpcClient(bitcoindInstance)
 
 // Next, we need to create a way to monitor the chain:
 
@@ -79,6 +77,5 @@ val syncResultF = syncedChainApiF.flatMap { chainApi =>
 
 syncResultF.onComplete { case result =>
   println(s"Sync result=${result}")
-  system.terminate()
 }
 ```

docs/rpc/bitcoind.md

@@ -14,14 +14,12 @@ version lines. It can be set up to work with both local and remote Bitcoin Core
 
 ```scala mdoc:compile-only
 import scala.concurrent._
-import akka.actor.ActorSystem
 
 import org.bitcoins.{rpc, core}
 import core.currency.Bitcoins
 import rpc.client.common._
 
-implicit val system = ActorSystem.create()
-implicit val ec: ExecutionContext = system.dispatcher
+implicit val ec: ExecutionContext = ExecutionContext.global
 
 // this reads authentication credentials and
 // connection details from the default data
@@ -51,7 +49,6 @@ Now that we have a secure connection between our remote `bitcoind`, we're
 ready to create the connection with our RPC client
 
 ```scala mdoc:compile-only
-import akka.actor.ActorSystem
 import java.net.URI
 import scala.concurrent._
 
@@ -76,14 +73,12 @@ val bitcoindInstance = {
   )
 }
 
-implicit val system: ActorSystem = ActorSystem.create()
-implicit val ec: ExecutionContext = system.dispatcher
+implicit val ec: ExecutionContext = ExecutionContext.global
 
-val rpcCli = new BitcoindRpcClient(bitcoindInstance)
+val rpcCli = BitcoindRpcClient(bitcoindInstance)
 
 rpcCli.getBalance.onComplete { case balance =>
   println(s"Wallet balance=${balance}")
-  system.terminate()
 }
 ```
 
@@ -106,8 +101,7 @@ import org.bitcoins.core.currency._
 
 import scala.concurrent._
 
-implicit val system = akka.actor.ActorSystem()
-implicit val ec = system.dispatcher
+implicit val ec = ExecutionContext.global
 
 // let's assume you have an already running client,
 // so there's no need to start this one

docs/wallet/create-wallet.md

@@ -16,9 +16,7 @@ This code snippet you have a running `bitcoind` instance, locally
 on regtest.
 
 ```scala mdoc:compile-only
-import akka.actor.ActorSystem
-implicit val system = ActorSystem()
-import system.dispatcher
+implicit val ec = scala.concurrent.ExecutionContext.global
 
 import com.typesafe.config.ConfigFactory
 val config = ConfigFactory.parseString {
@@ -53,7 +51,7 @@ import org.bitcoins.rpc.config.BitcoindInstance
 val bitcoindInstance = BitcoindInstance.fromDatadir()
 
 import org.bitcoins.rpc.client.common.BitcoindRpcClient
-val bitcoind = new BitcoindRpcClient(bitcoindInstance)
+val bitcoind = BitcoindRpcClient(bitcoindInstance)
 
 // when this future completes, we have
 // synced our chain handler to our bitcoind

eclair-rpc/src/main/scala/org/bitcoins/eclair/rpc/client/EclairRpcClient.scala

@@ -38,6 +38,7 @@ import scala.concurrent.{ExecutionContext, Future, Promise}
 import scala.sys.process._
 import scala.util.{Failure, Properties, Success}
 import java.nio.file.NoSuchFileException
+import org.bitcoins.core.util.FutureUtil
 
 /**
   * @param binary Path to Eclair Jar. If not present, reads
@@ -718,8 +719,20 @@ class EclairRpcClient(val instance: EclairInstance, binary: Option[File] = None)
     p.future
   }
 
-  def stop(): Option[Unit] = {
-    process.map(_.destroy())
+  /** Returns true if able to shut down
+    * Eclair instance */
+  def stop(): Future[Boolean] = {
+    val res = process.map(_.destroy()) match {
+      case None    => false
+      case Some(_) => true
+    }
+    val actorSystemF = if (system.name == EclairRpcClient.ActorSystemName) {
+      system.terminate()
+    } else {
+      FutureUtil.unit
+    }
+
+    actorSystemF.map(_ => res)
   }
 
   /**
@@ -794,6 +807,30 @@ class EclairRpcClient(val instance: EclairInstance, binary: Option[File] = None)
 
 object EclairRpcClient {
 
+  /** THe name we use to create actor systems. We use this to know which
+    * actor systems to shut down on node shutdown */
+  private[eclair] val ActorSystemName = "eclair-rpc-client-created-by-bitcoin-s"
+
+  /**
+    * Creates an RPC client from the given instance,
+    * together with the given actor system. This is for
+    * advanced users, wher you need fine grained control
+    * over the RPC client.
+    */
+  def apply(
+      instance: EclairInstance,
+      binary: Option[File] = None): EclairRpcClient = {
+    implicit val systme = ActorSystem.create(ActorSystemName)
+    withActorSystem(instance, binary)
+  }
+
+  /**
+    * Constructs a RPC client from the given datadir, or
+    * the default datadir if no directory is provided
+    */
+  def withActorSystem(instance: EclairInstance, binary: Option[File] = None)(
+      implicit system: ActorSystem) = new EclairRpcClient(instance, binary)
+
   /** The current commit we support of Eclair */
   private[bitcoins] val commit = "6906ecb"
 

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

@@ -634,7 +634,7 @@ trait EclairRpcTestUtil extends BitcoinSLogger {
         rpcUri = auth.bitcoindRpcUri,
         authCredentials = auth.bitcoinAuthOpt.get
       )
-      new BitcoindRpcClient(bitcoindInstance)(system)
+      BitcoindRpcClient.withActorSystem(bitcoindInstance)
     }
     bitcoindRpc
   }
@@ -648,20 +648,19 @@ trait EclairRpcTestUtil extends BitcoinSLogger {
     val bitcoindRpc = getBitcoindRpc(eclairRpcClient)
 
     logger.debug(s"shutting down eclair")
-    val killEclairOpt = eclairRpcClient.stop()
+    val stopEclairF = eclairRpcClient.stop()
     val killBitcoindF = BitcoindRpcTestUtil.stopServer(bitcoindRpc)
 
     for {
       _ <- killBitcoindF
+      stopped <- stopEclairF
     } yield {
-      killEclairOpt match {
-        case Some(_) =>
-          logger.debug(
-            "Successfully shutdown eclair and it's corresponding bitcoind")
-        case None =>
-          logger.info(
-            s"Killed a bitcoind instance, but could not find an eclair process to kill")
-      }
+      if (stopped)
+        logger.debug(
+          "Successfully shutdown eclair and it's corresponding bitcoind")
+      else
+        logger.info(
+          s"Killed a bitcoind instance, but could not find an eclair process to kill")
     }
   }
 }

testkit/src/main/scala/org/bitcoins/testkit/fixtures/BitcoinSFixture.scala

@@ -166,7 +166,7 @@ object BitcoinSFixture {
       implicit system: ActorSystem): Future[BitcoindRpcClient] = {
     import system.dispatcher
     val instance = BitcoindRpcTestUtil.instance()
-    val bitcoind = new BitcoindRpcClient(instance)
+    val bitcoind = BitcoindRpcClient.withActorSystem(instance)
 
     bitcoind.start().map(_ => bitcoind)
   }

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

@@ -45,7 +45,6 @@ import org.bitcoins.util.ListUtil
 import scala.annotation.tailrec
 import scala.collection.immutable.Map
 import scala.collection.mutable
-import scala.collection.JavaConverters._
 import scala.concurrent._
 import scala.concurrent.duration.{DurationInt, FiniteDuration}
 import scala.util._
@@ -164,6 +163,7 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
     // default to newest version
     case Unknown => getBinary(BitcoindVersion.newest)
     case known @ (V16 | V17) =>
+      import org.bitcoins.core.compat.JavaConverters._
       val versionFolder = Files
         .list(binaryDirectory)
         .iterator()
@@ -471,8 +471,10 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
       implicit
       system: ActorSystem): Future[(BitcoindRpcClient, BitcoindRpcClient)] = {
     implicit val ec: ExecutionContextExecutor = system.getDispatcher
-    val client1: BitcoindRpcClient = new BitcoindRpcClient(instance())
-    val client2: BitcoindRpcClient = new BitcoindRpcClient(instance())
+    val client1: BitcoindRpcClient =
+      BitcoindRpcClient.withActorSystem(instance())
+    val client2: BitcoindRpcClient =
+      BitcoindRpcClient.withActorSystem(instance())
 
     val start1F = client1.start()
     val start2F = client2.start()
@@ -536,11 +538,13 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
     val clients: Vector[T] = (0 until numNodes).map { _ =>
       val rpc = version match {
         case BitcoindVersion.Unknown =>
-          new BitcoindRpcClient(BitcoindRpcTestUtil.instance())
+          BitcoindRpcClient.withActorSystem(BitcoindRpcTestUtil.instance())
         case BitcoindVersion.V16 =>
-          new BitcoindV16RpcClient(BitcoindRpcTestUtil.v16Instance())
+          BitcoindV16RpcClient.withActorSystem(
+            BitcoindRpcTestUtil.v16Instance())
         case BitcoindVersion.V17 =>
-          new BitcoindV17RpcClient(BitcoindRpcTestUtil.v17Instance())
+          BitcoindV17RpcClient.withActorSystem(
+            BitcoindRpcTestUtil.v17Instance())
       }
 
       // this is safe as long as this method is never
@@ -685,7 +689,7 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
       signer: BitcoindRpcClient,
       transaction: Transaction,
       utxoDeps: Vector[RpcOpts.SignRawTransactionOutputParameter] = Vector.empty
-  )(implicit actorSystemw: ActorSystem): Future[SignRawTransactionResult] =
+  ): Future[SignRawTransactionResult] =
     signer match {
       case v17: BitcoindV17RpcClient =>
         v17.signRawTransactionWithWallet(transaction, utxoDeps)
@@ -852,7 +856,7 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
       s"${instance.datadir} is not in user temp dir! This could lead to bad things happening.")
 
     //start the bitcoind instance so eclair can properly use it
-    val rpc = new BitcoindRpcClient(instance)
+    val rpc = BitcoindRpcClient.withActorSystem(instance)
     val startedF = rpc.start()
 
     val blocksToGenerate = 102