From 56737681aded0d65342bfa2c92d5589e2ad833a2 Mon Sep 17 00:00:00 2001
From: Torkel Rogstad <torkel@rogstad.io>
Date: Thu, 11 Jul 2019 16:20:09 +0200
Subject: [PATCH] Fix bug where cookie auth threw on isStartedF

---
 .../bitcoins/rpc/BitcoindInstanceTest.scala   | 43 +++++++++++--------
 .../bitcoins/rpc/client/common/Client.scala   | 35 +++++++++------
 2 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/BitcoindInstanceTest.scala b/bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/BitcoindInstanceTest.scala
index 0decb45ca9..ca6dda846d 100644
--- a/bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/BitcoindInstanceTest.scala
+++ b/bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/BitcoindInstanceTest.scala
@@ -17,6 +17,8 @@ import org.bitcoins.rpc.config.BitcoindAuthCredentials
 import org.bitcoins.rpc.util.RpcUtil
 import org.bitcoins.core.config.RegTest
 import java.net.URI
+import scala.concurrent.Future
+import org.scalatest.compatible.Assertion
 
 class BitcoindInstanceTest extends BitcoindRpcTest {
 
@@ -34,6 +36,24 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
     pw.close()
   }
 
+  /**
+    * Tests that the client can call the isStartedF method
+    * without throwing and then start
+    */
+  private def testClientStart(client: BitcoindRpcClient): Future[Assertion] = {
+    clientAccum += client
+    for {
+      firstStarted <- client.isStartedF
+      _ <- client.start()
+      secondStarted <- client.isStartedF
+
+      _ <- client.getBalance
+    } yield {
+      assert(!firstStarted)
+      assert(secondStarted)
+    }
+  }
+
   behavior of "BitcoindInstance"
 
   it should "start a bitcoind with cookie based authentication" in {
@@ -49,12 +69,9 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
     assert(
       instance.authCredentials
         .isInstanceOf[BitcoindAuthCredentials.CookieBased])
-    for {
-      cli <- BitcoindRpcTestUtil.startedBitcoindRpcClient(instance,
-                                                          clientAccum =
-                                                            clientAccum)
-      _ <- cli.getBalance
-    } yield succeed
+
+    val cli = new BitcoindRpcClient(instance)
+    testClientStart(cli)
   }
 
   it should "start a bitcoind with user and password based authentication" in {
@@ -72,12 +89,7 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
     assert(
       instance.authCredentials
         .isInstanceOf[BitcoindAuthCredentials.PasswordBased])
-    for {
-      cli <- BitcoindRpcTestUtil.startedBitcoindRpcClient(instance,
-                                                          clientAccum =
-                                                            clientAccum)
-      _ <- cli.getBalance
-    } yield succeed
+    testClientStart(new BitcoindRpcClient(instance))
   }
 
   // the values in this conf was generated by executing
@@ -112,12 +124,7 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
         datadir = conf.datadir
       )
 
-    for {
-      cli <- BitcoindRpcTestUtil.startedBitcoindRpcClient(instance,
-                                                          clientAccum =
-                                                            clientAccum)
-      _ <- cli.getBalance
-    } yield succeed
+    testClientStart(new BitcoindRpcClient(instance))
   }
 
   it should "parse a bitcoin.conf file, start bitcoind, mine some blocks and quit" in {
diff --git a/bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/common/Client.scala b/bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/common/Client.scala
index 120cf99531..e57e13b43e 100644
--- a/bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/common/Client.scala
+++ b/bitcoind-rpc/src/main/scala/org/bitcoins/rpc/client/common/Client.scala
@@ -175,24 +175,31 @@ trait Client extends BitcoinSLogger {
     * Checks whether the underlying bitcoind daemon is running
     */
   def isStartedF: Future[Boolean] = {
-    val request = buildRequest(instance, "ping", JsArray.empty)
-    val responseF = sendRequest(request)
+    def tryPing: Future[Boolean] = {
+      val request = buildRequest(instance, "ping", JsArray.empty)
+      val responseF = sendRequest(request)
 
-    val payloadF: Future[JsValue] = responseF.flatMap(getPayload)
+      val payloadF: Future[JsValue] = responseF.flatMap(getPayload)
 
-    // Ping successful if no error can be parsed from the payload
-    val parsedF = payloadF.map { payload =>
-      (payload \ errorKey).validate[RpcError] match {
-        case _: JsSuccess[RpcError] => false
-        case _: JsError             => true
+      // Ping successful if no error can be parsed from the payload
+      val parsedF = payloadF.map { payload =>
+        (payload \ errorKey).validate[RpcError] match {
+          case _: JsSuccess[RpcError] => false
+          case _: JsError             => true
+        }
+      }
+
+      parsedF.recover {
+        case exc: StreamTcpException
+            if exc.getMessage.contains("Connection refused") =>
+          false
       }
     }
-
-    parsedF.recover {
-      case exc: StreamTcpException
-          if exc.getMessage.contains("Connection refused") =>
-        false
-
+    instance.authCredentials match {
+      case cookie: CookieBased if Files.notExists(cookie.cookiePath) =>
+        // if the cookie file doesn't exist we're not started
+        Future.successful(false)
+      case (CookieBased(_, _) | PasswordBased(_, _)) => tryPing
     }
   }