Make ChainApi.processHeaders() return a failed future in the case we … (#2436)

* Rename NodeUnitTest.confg -> NodeUnitTest.getFreshConfig()

* Rename CachedBitcoinSAppConfig.config -> CachedBitcoinSAppConfig.cachedConfig

* Make CachedChainAppConfig extend CachedBitcoinSAppConfig

* Make ChainApi.processHeaders() return a failed future in the case we have no valid headers

* Run scalafmt

* Fix test case to check if promise is completed yet

* WIP: Get something working that isn't network specific

Start putting things back in place

Add comment

Revert logback file

Remove BitcoinSLogger object

* Fix unused import

* Get rid of annoying diff

* Fix spacing nit

chain-test/src/test/scala/org/bitcoins/chain/blockchain/ChainHandlerTest.scala

@@ -94,6 +94,16 @@ class ChainHandlerTest extends ChainDbUnitTest {
       } yield succeed
   }
 
+  it must "fail if we give a header that cannot be connected to anything" in {
+    chainHandler: ChainHandler =>
+      val newHeader = BlockHeaderHelper.badPrevHash
+      recoverToSucceededIf[RuntimeException] {
+        for {
+          result <- chainHandler.processHeader(newHeader)
+        } yield result
+      }
+  }
+
   // B
   // C -> D
   it must "handle a very basic reorg where one chain is one block behind the best chain" in {

chain/src/main/scala/org/bitcoins/chain/blockchain/ChainHandler.scala

@@ -135,6 +135,13 @@ class ChainHandler(
         successfullyValidatedHeaders.distinct
       }
 
+      if (headersToBeCreated.isEmpty) {
+        //this means we are given zero headers that were valid.
+        //Return a failure in this case to avoid issue 2365
+        //https://github.com/bitcoin-s/bitcoin-s/issues/2365
+        Future.failed(new RuntimeException(
+          s"Failed to connect any headers to our internal chain state, failures=$blockchainUpdates"))
+      } else {
         val chains = blockchainUpdates.map(_.blockchain)
 
         val createdF = blockHeaderDAO.createAll(headersToBeCreated)
@@ -153,7 +160,8 @@ class ChainHandler(
                   _ <- acc
                   _ <-
                     chainConfig.chainCallbacks
-                    .executeOnBlockHeaderConnectedCallbacks(logger,
+                      .executeOnBlockHeaderConnectedCallbacks(
+                        logger,
                         header.height,
                         header.blockHeader)
                 } yield ()
@@ -167,6 +175,7 @@ class ChainHandler(
         }
       }
     }
+  }
 
   /** @inheritdoc */
   override def processHeaders(

core/src/main/scala/org/bitcoins/core/api/chain/ChainApi.scala

@@ -19,7 +19,9 @@ import scala.concurrent.Future
 trait ChainApi extends ChainQueryApi {
 
   /**
-    * Adds a block header to our chain project
+    * Adds a block header to our chain project.
+    * This will return a failed future when the
+    * given header is invalid.
     * @param header
     * @return
     */
@@ -29,7 +31,9 @@ trait ChainApi extends ChainQueryApi {
 
   /** Process all of the given headers and returns a new [[ChainApi chain api]]
     * that contains these headers. This method processes headers in the order
-    * that they are given. If the headers are out of order, this method will fail
+    * that they are given. If the headers are out of order, this method will fail.
+    *
+    * This method will also fail when there are zero headers given that are valid.
     * @param headers
     * @return
     */

node-test/src/test/scala/org/bitcoins/node/networking/peer/DataMessageHandlerTest.scala

@@ -105,7 +105,9 @@ class DataMessageHandlerTest extends NodeUnitTest {
 
       val callback: OnBlockHeadersReceived = (headers: Vector[BlockHeader]) => {
         Future {
+          if (!resultP.isCompleted) {
             resultP.success(headers)
+          }
           ()
         }
       }