Fix bug where we didn't set spendingTxId when transitioning from Reserved -> PendingConfirmationsSpent (#3909)

* Fix bug where we didn't set spendingTxId when transitioning from Reserved -> PendingConfirmationsSpent

* Try to add unit test

* WIP

* WIP2

* Get reserved state working

* Fix assertion, cleanup logs

* Cleanup test case

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

@@ -1,7 +1,8 @@
 package org.bitcoins.wallet
 
+import grizzled.slf4j.Logging
 import org.bitcoins.core.api.wallet.db.SpendingInfoDb
-import org.bitcoins.core.currency.Satoshis
+import org.bitcoins.core.currency.{Bitcoins, Satoshis}
 import org.bitcoins.core.number._
 import org.bitcoins.core.protocol.BitcoinAddress
 import org.bitcoins.core.protocol.script._
@@ -21,7 +22,9 @@ import org.scalatest.{FutureOutcome, Outcome}
 
 import scala.concurrent.Future
 
-class UTXOLifeCycleTest extends BitcoinSWalletTestCachedBitcoindNewest {
+class UTXOLifeCycleTest
+    extends BitcoinSWalletTestCachedBitcoindNewest
+    with Logging {
 
   behavior of "Wallet Txo States"
 
@@ -514,4 +517,90 @@ class UTXOLifeCycleTest extends BitcoinSWalletTestCachedBitcoindNewest {
         assert(reserved.outPoint == utxos.head.outPoint)
       }
   }
+
+  it must "mark a utxo as reserved that is still receiving confirmations and not unreserve the utxo" in {
+    param =>
+      val WalletWithBitcoindRpc(wallet, bitcoind) = param
+      val addressF = wallet.getNewAddress()
+      val txIdF =
+        addressF.flatMap(addr => bitcoind.sendToAddress(addr, Bitcoins.one))
+      val throwAwayAddrF = bitcoind.getNewAddress
+      for {
+        txId <- txIdF
+        //generate a few blocks to make the utxo pending confirmations received
+        throwAwayAddr <- throwAwayAddrF
+        hashes <- bitcoind.generateToAddress(blocks = 1, throwAwayAddr)
+        block <- bitcoind.getBlockRaw(hashes.head)
+        _ <- wallet.processBlock(block)
+
+        //make sure the utxo is pending confirmations received
+        utxos <- wallet.listUtxos(TxoState.PendingConfirmationsReceived)
+        _ = assert(utxos.length == 1)
+        utxo = utxos.head
+        _ = assert(utxo.txid == txId)
+        _ = assert(utxo.state == TxoState.PendingConfirmationsReceived)
+        //now mark the utxo as reserved
+        _ <- wallet.markUTXOsAsReserved(Vector(utxo))
+        //confirm it is reserved
+        _ <- wallet
+          .listUtxos(TxoState.Reserved)
+          .map(utxos =>
+            assert(utxos.contains(utxo.copyWithState(TxoState.Reserved))))
+
+        //now process another block
+        hashes2 <- bitcoind.generateToAddress(blocks = 1, throwAwayAddr)
+        block2 <- bitcoind.getBlockRaw(hashes2.head)
+        _ <- wallet.processBlock(block2)
+
+        //the utxo should still be reserved
+        reservedUtxos <- wallet.listUtxos(TxoState.Reserved)
+        reservedUtxo = reservedUtxos.head
+      } yield {
+        assert(reservedUtxo.txid == txId)
+        assert(reservedUtxo.state == TxoState.Reserved)
+      }
+  }
+
+  it must "transition a reserved utxo to spent when we are offline" in {
+    param =>
+      val WalletWithBitcoindRpc(wallet, bitcoind) = param
+      val bitcoindAddrF = bitcoind.getNewAddress
+      val amt = Satoshis(100000)
+      val utxoCountF = wallet.listUtxos()
+      for {
+        bitcoindAdr <- bitcoindAddrF
+        utxoCount <- utxoCountF
+        //build a spending transaction
+        tx <- wallet.sendToAddress(bitcoindAdr, amt, SatoshisPerVirtualByte.one)
+        c <- wallet.listUtxos()
+        _ = assert(c.length == utxoCount.length)
+        txIdBE <- bitcoind.sendRawTransaction(tx)
+
+        //find all utxos that we can use to fund a transaction
+        utxos <- wallet
+          .listUtxos()
+          .map(_.filter(u => TxoState.receivedStates.contains(u.state)))
+        broadcastReceived <- wallet.listUtxos(TxoState.BroadcastReceived)
+        _ = assert(broadcastReceived.length == 1) //change output
+
+        //mark all utxos as reserved
+        _ <- wallet.markUTXOsAsReserved(utxos)
+        newReservedUtxos <- wallet.listUtxos(TxoState.Reserved)
+
+        //make sure all utxos are reserved
+        _ = assert(newReservedUtxos.length == utxoCount.length)
+        blockHash <- bitcoind.generateToAddress(1, bitcoindAdr).map(_.head)
+        block <- bitcoind.getBlockRaw(blockHash)
+        _ <- wallet.processBlock(block)
+        broadcastSpentUtxo <- wallet.listUtxos(
+          TxoState.PendingConfirmationsSpent)
+        finalReservedUtxos <- wallet.listUtxos(TxoState.Reserved)
+      } yield {
+        assert(broadcastSpentUtxo.length == 1)
+        //make sure spendingTxId got set correctly
+        assert(broadcastSpentUtxo.head.spendingTxIdOpt.get == txIdBE)
+        //make sure no utxos get unreserved when processing the block
+        assert(finalReservedUtxos.length == newReservedUtxos.length)
+      }
+  }
 }

wallet/src/main/scala/org/bitcoins/wallet/internal/TransactionProcessing.scala

@@ -408,7 +408,6 @@ private[bitcoins] trait TransactionProcessing extends WalletLogger {
           out
             .copyWithSpendingTxId(spendingTxId)
             .copyWithState(state = BroadcastSpent)
-
         Some(updated)
       case TxoState.BroadcastSpent =>
         if (!out.spendingTxIdOpt.contains(spendingTxId)) {
@@ -483,31 +482,18 @@ private[bitcoins] trait TransactionProcessing extends WalletLogger {
           logger.debug(
             s"Updating block_hash of txo=${transaction.txIdBE.hex}, new block hash=${blockHash.hex}")
 
-          // If the utxo was marked reserved we want to update it to spent now
-          // since it has been included in a block
-          val unreservedTxo = foundTxo.state match {
-            case TxoState.Reserved =>
-              foundTxo.copyWithState(TxoState.PendingConfirmationsSpent)
-            case TxoState.PendingConfirmationsReceived |
-                TxoState.ConfirmedReceived |
-                TxoState.PendingConfirmationsSpent | TxoState.ConfirmedSpent |
-                TxoState.DoesNotExist | TxoState.ImmatureCoinbase |
-                BroadcastReceived | BroadcastSpent =>
-              foundTxo
-          }
-
           val updateTxDbF = insertTransaction(transaction, blockHashOpt)
 
           // Update Txo State
           updateTxDbF.flatMap(_ =>
-            updateUtxoConfirmedState(unreservedTxo).flatMap {
+            updateUtxoConfirmedState(foundTxo).flatMap {
               case Some(txo) =>
                 logger.debug(
                   s"Updated block_hash of txo=${txo.txid.hex} new block hash=${blockHash.hex}")
                 Future.successful(txo)
               case None =>
                 // State was not updated so we need to update it so it's block hash is in the database
-                spendingInfoDAO.update(unreservedTxo)
+                spendingInfoDAO.update(foundTxo)
             })
         case None =>
           logger.debug(

wallet/src/main/scala/org/bitcoins/wallet/internal/UtxoHandling.scala

@@ -315,6 +315,8 @@ private[wallet] trait UtxoHandling extends WalletLogger {
 
   override def markUTXOsAsReserved(
       utxos: Vector[SpendingInfoDb]): Future[Vector[SpendingInfoDb]] = {
+    val outPoints = utxos.map(_.outPoint)
+    logger.info(s"Reserving utxos=$outPoints")
     val updated = utxos.map(_.copyWithState(TxoState.Reserved))
     for {
       utxos <- spendingInfoDAO.markAsReserved(updated)
@@ -327,7 +329,7 @@ private[wallet] trait UtxoHandling extends WalletLogger {
       tx: Transaction): Future[Vector[SpendingInfoDb]] = {
     for {
       utxos <- spendingInfoDAO.findOutputsBeingSpent(tx)
-      reserved <- markUTXOsAsReserved(utxos.toVector)
+      reserved <- markUTXOsAsReserved(utxos)
     } yield reserved
   }