Use OutPoint type instead of txid + output index (no functional changes)

Author: sstone

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/OnChainWallet.scala

@@ -81,7 +81,7 @@ trait OnChainChannelFunder {
    * Note that if this function returns false, that doesn't mean the output cannot be spent. The output could be unknown
    * (not in the blockchain nor in the mempool) but could reappear later and be spendable at that point.
    */
-  def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean]
+  def isTransactionOutputSpendable(outPoint: OutPoint, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean]
 
   /** Rollback a transaction that we failed to commit: this probably translates to "release locks on utxos". */
   def rollback(tx: Transaction)(implicit ec: ExecutionContext): Future[Boolean]

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala

@@ -103,6 +103,8 @@ object ZmqWatcher {
     def txId: TxId
     /** Index of the outpoint to watch. */
     def outputIndex: Int
+    /** outpoint to watch */
+    def outPoint: OutPoint = OutPoint(txId, outputIndex)
     /**
      * TxIds of potential spending transactions; most of the time we know the txs, and it allows for optimizations.
      * This argument can safely be ignored by watcher implementations.
@@ -435,14 +437,14 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
           // This is an external channels: funds are not at risk, so we don't need to scan the blockchain to find the
           // spending transaction, it is costly and unnecessary. We simply check whether the output has already been
           // spent by a confirmed transaction.
-          client.isTransactionOutputSpent(w.txId, w.outputIndex).collect {
+          client.isTransactionOutputSpent(w.outPoint).collect {
             case true =>
               // The output has been spent, so we trigger the watch without including the spending transaction.
               context.self ! TriggerEvent(w.replyTo, w, WatchExternalChannelSpentTriggered(w.shortChannelId, None))
           }
         case _ =>
           // The parent tx was published, we need to make sure this particular output has not been spent.
-          client.isTransactionOutputSpendable(w.txId, w.outputIndex, includeMempool = true).collect {
+          client.isTransactionOutputSpendable(w.outPoint, includeMempool = true).collect {
             case false =>
               // The output has been spent, let's find the spending tx.
               // If we know some potential spending txs, we try to fetch them directly.
@@ -456,7 +458,7 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
                     case None =>
                       // The hints didn't help us, let's search for the spending transaction in the mempool.
                       log.info("{}:{} has already been spent, looking for the spending tx in the mempool", w.txId, w.outputIndex)
-                      client.lookForMempoolSpendingTx(w.txId, w.outputIndex).map(Some(_)).recover { case _ => None }.map {
+                      client.lookForMempoolSpendingTx(w.outPoint).map(Some(_)).recover { case _ => None }.map {
                         case Some(spendingTx) =>
                           log.info("found tx spending {}:{} in the mempool: txid={}", w.txId, w.outputIndex, spendingTx.txid)
                           context.self ! ProcessNewTransaction(spendingTx)
@@ -465,7 +467,7 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
                           // before we set the watch. We have to scan the blockchain to find it, which is expensive
                           // since bitcoind doesn't provide indexes for this scenario.
                           log.warn("{}:{} has already been spent, spending tx not in the mempool, looking in the blockchain...", w.txId, w.outputIndex)
-                          client.lookForSpendingTx(None, w.txId, w.outputIndex, nodeParams.channelConf.maxChannelSpentRescanBlocks).map { spendingTx =>
+                          client.lookForSpendingTx(None, w.outPoint, nodeParams.channelConf.maxChannelSpentRescanBlocks).map { spendingTx =>
                             log.warn("found the spending tx of {}:{} in the blockchain: txid={}", w.txId, w.outputIndex, spendingTx.txid)
                             context.self ! ProcessNewTransaction(spendingTx)
                           }.recover {

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala

@@ -116,17 +116,17 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val lockUtxos: Bool
    * (not in the blockchain nor in the mempool) but could reappear later and be spendable at that point. If you want to
    * ensure that an output is not spendable anymore, you should use [[isTransactionOutputSpent]].
    */
-  def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] =
+  def isTransactionOutputSpendable(outPoint: OutPoint, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] =
     for {
-      json <- rpcClient.invoke("gettxout", txid, outputIndex, includeMempool)
+      json <- rpcClient.invoke("gettxout", outPoint.txid, outPoint.index, includeMempool)
     } yield json != JNull
 
   /**
    * Return true if this output has already been spent by a confirmed transaction.
    * Note that a reorg may invalidate the result of this function and make a spent output spendable again.
    */
-  def isTransactionOutputSpent(txid: TxId, outputIndex: Int)(implicit ec: ExecutionContext): Future[Boolean] = {
-    getTxConfirmations(txid).flatMap {
+  def isTransactionOutputSpent(outPoint: OutPoint)(implicit ec: ExecutionContext): Future[Boolean] = {
+    getTxConfirmations(outPoint.txid).flatMap {
       case Some(confirmations) if confirmations > 0 =>
         // There is an important limitation when using isTransactionOutputSpendable: if it returns false, it can mean a
         // few different things:
@@ -137,7 +137,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val lockUtxos: Bool
         // The only way to make sure that our output has been spent is to verify that it is coming from a confirmed
         // transaction and that it has been spent by another confirmed transaction. We want to ignore the mempool to
         // only consider spending transactions that have been confirmed.
-        isTransactionOutputSpendable(txid, outputIndex, includeMempool = false).map(r => !r)
+        isTransactionOutputSpendable(outPoint, includeMempool = false).map(r => !r)
       case _ =>
         // If the output itself isn't in the blockchain, it cannot be spent by a confirmed transaction.
         Future.successful(false)
@@ -166,18 +166,18 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val lockUtxos: Bool
         // themselves been double-spent, we will never be able to consider our transaction double-spent. With the
         // information we have, these unknown inputs could eventually reappear and the transaction could be broadcast
         // again.
-        Future.sequence(tx.txIn.map(txIn => isTransactionOutputSpent(txIn.outPoint.txid, txIn.outPoint.index.toInt))).map(_.exists(_ == true))
+        Future.sequence(tx.txIn.map(txIn => isTransactionOutputSpent(txIn.outPoint))).map(_.exists(_ == true))
       }
     } yield doubleSpent
 
   /** Search for mempool transaction spending a given output. */
-  def lookForMempoolSpendingTx(txid: TxId, outputIndex: Int)(implicit ec: ExecutionContext): Future[Transaction] = {
-    rpcClient.invoke("gettxspendingprevout", Seq(OutpointArg(txid, outputIndex))).collect {
+  def lookForMempoolSpendingTx(outPoint: OutPoint)(implicit ec: ExecutionContext): Future[Transaction] = {
+    rpcClient.invoke("gettxspendingprevout", Seq(OutpointArg(outPoint.txid, outPoint.index))).collect {
       case JArray(results) => results.flatMap(result => (result \ "spendingtxid").extractOpt[String].map(TxId.fromValidHex))
     }.flatMap { spendingTxIds =>
       spendingTxIds.headOption match {
         case Some(spendingTxId) => getTransaction(spendingTxId)
-        case None => Future.failed(new RuntimeException(s"mempool doesn't contain any transaction spending $txid:$outputIndex"))
+        case None => Future.failed(new RuntimeException(s"mempool doesn't contain any transaction spending $outPoint"))
       }
     }
   }
@@ -188,23 +188,22 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val lockUtxos: Bool
    * will have already claimed all possible outputs and there's nothing we can do about it.
    *
    * @param blockHash_opt hash of a block *after* the output has been spent. If not provided, we will use the blockchain tip.
-   * @param txid          id of the transaction output that has been spent.
-   * @param outputIndex   index of the transaction output that has been spent.
+   * @param outPoint      transaction output that has been spent.
    * @param limit         maximum number of previous blocks to scan.
    * @return the transaction spending the given output.
    */
-  def lookForSpendingTx(blockHash_opt: Option[BlockHash], txid: TxId, outputIndex: Int, limit: Int)(implicit ec: ExecutionContext): Future[Transaction] = {
+  def lookForSpendingTx(blockHash_opt: Option[BlockHash], outPoint: OutPoint, limit: Int)(implicit ec: ExecutionContext): Future[Transaction] = {
     for {
       blockId <- blockHash_opt match {
         case Some(blockHash) => Future.successful(BlockId(blockHash))
         // NB: bitcoind confusingly returns the blockId instead of the blockHash.
         case None => rpcClient.invoke("getbestblockhash").collect { case JString(blockId) => BlockId(ByteVector32.fromValidHex(blockId)) }
       }
       block <- getBlock(blockId)
-      res <- block.tx.asScala.find(tx => tx.txIn.asScala.exists(i => i.outPoint.txid == KotlinUtils.scala2kmp(txid) && i.outPoint.index == outputIndex)) match {
+      res <- block.tx.asScala.find(tx => tx.txIn.asScala.exists(i => i.outPoint == KotlinUtils.scala2kmp(outPoint))) match {
         case Some(tx) => Future.successful(KotlinUtils.kmp2scala(tx))
-        case None if limit > 0 => lookForSpendingTx(Some(KotlinUtils.kmp2scala(block.header.hashPreviousBlock)), txid, outputIndex, limit - 1)
-        case None => Future.failed(new RuntimeException(s"couldn't find tx spending $txid:$outputIndex in the blockchain"))
+        case None if limit > 0 => lookForSpendingTx(Some(KotlinUtils.kmp2scala(block.header.hashPreviousBlock)), outPoint, limit - 1)
+        case None => Future.failed(new RuntimeException(s"couldn't find tx spending $outPoint in the blockchain"))
       }
     } yield res
   }
@@ -755,12 +754,13 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient, val lockUtxos: Bool
         TxId.fromValidHex(txs(txIndex).extract[String])
       }.getOrElse(TxId(ByteVector32.Zeroes)))
       tx <- getRawTransaction(txid)
-      unspent <- isTransactionOutputSpendable(txid, outputIndex, includeMempool = true)
+      outPoint = OutPoint(txid, outputIndex)
+      unspent <- isTransactionOutputSpendable(outPoint, includeMempool = true)
       fundingTxStatus <- if (unspent) {
         Future.successful(UtxoStatus.Unspent)
       } else {
         // if this returns true, it means that the spending tx is *not* in the blockchain
-        isTransactionOutputSpendable(txid, outputIndex, includeMempool = false).map(res => UtxoStatus.Spent(spendingTxConfirmed = !res))
+        isTransactionOutputSpendable(outPoint, includeMempool = false).map(res => UtxoStatus.Spent(spendingTxConfirmed = !res))
       }
     } yield ValidateResult(c, Right((Transaction.read(tx), fundingTxStatus)))
   } recover {

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala

@@ -758,7 +758,7 @@ private class InteractiveTxBuilder(replyTo: ActorRef[InteractiveTxBuilder.Respon
             // unconfirmed inputs, because if they are valid but not in our mempool we would incorrectly consider
             // them unspendable (unknown). We want to reject unspendable inputs to immediately fail the funding
             // attempt, instead of waiting to detect the double-spend later.
-            wallet.isTransactionOutputSpendable(outpoint.txid, outpoint.index.toInt, includeMempool = true)
+            wallet.isTransactionOutputSpendable(outpoint, includeMempool = true)
           case _ => Future.successful(false)
         }
         case Success(false) => Future.successful(false)

eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/MempoolTxMonitor.scala

@@ -200,8 +200,8 @@ private class MempoolTxMonitor(nodeParams: NodeParams,
   private def checkInputStatus(input: OutPoint): Unit = {
     val checkInputTask = for {
       parentConfirmations <- bitcoinClient.getTxConfirmations(input.txid)
-      spendableMempoolExcluded <- bitcoinClient.isTransactionOutputSpendable(input.txid, input.index.toInt, includeMempool = false)
-      spendableMempoolIncluded <- bitcoinClient.isTransactionOutputSpendable(input.txid, input.index.toInt, includeMempool = true)
+      spendableMempoolExcluded <- bitcoinClient.isTransactionOutputSpendable(input, includeMempool = false)
+      spendableMempoolIncluded <- bitcoinClient.isTransactionOutputSpendable(input, includeMempool = true)
     } yield computeInputStatus(parentConfirmations, spendableMempoolExcluded, spendableMempoolIncluded)
     context.pipeToSelf(checkInputTask) {
       case Success(status) => status

eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPrePublisher.scala

@@ -99,7 +99,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams,
     context.pipeToSelf(bitcoinClient.getTxConfirmations(fundingOutpoint.txid).flatMap {
       case Some(_) =>
         // The funding transaction was found, let's see if we can still spend it.
-        bitcoinClient.isTransactionOutputSpendable(fundingOutpoint.txid, fundingOutpoint.index.toInt, includeMempool = true).flatMap {
+        bitcoinClient.isTransactionOutputSpendable(fundingOutpoint, includeMempool = true).flatMap {
           case true =>
             // The funding output is unspent: let's publish our anchor transaction to get our local commit confirmed.
             Future.successful(ParentTxOk)
@@ -162,7 +162,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams,
       case Some(_) =>
         // The funding transaction was found, let's see if we can still spend it. Note that in this case, we only look
         // at *confirmed* spending transactions (unlike the local commit case).
-        bitcoinClient.isTransactionOutputSpendable(fundingOutpoint.txid, fundingOutpoint.index.toInt, includeMempool = false).flatMap {
+        bitcoinClient.isTransactionOutputSpendable(fundingOutpoint, includeMempool = false).flatMap {
           case true =>
             // The funding output is unspent, or spent by an *unconfirmed* transaction: let's publish our anchor
             // transaction, we may be able to replace our local commit with this (more interesting) remote commit.
@@ -221,7 +221,7 @@ private class ReplaceableTxPrePublisher(nodeParams: NodeParams,
       case Some(_) =>
         // If the HTLC output is already spent by a confirmed transaction, there is no need for RBF: either this is one
         // of our transactions (which thus has a high enough feerate), or it was a race with our peer and we lost.
-        bitcoinClient.isTransactionOutputSpent(input.txid, input.index.toInt).map {
+        bitcoinClient.isTransactionOutputSpent(input).map {
           case true => HtlcOutputAlreadySpent
           case false => ParentTxOk
         }

eclair-core/src/test/scala/fr/acinq/eclair/blockchain/DummyOnChainWallet.scala

@@ -77,7 +77,7 @@ class DummyOnChainWallet extends OnChainWallet with OnChainAddressCache {
 
   override def getTxConfirmations(txid: TxId)(implicit ec: ExecutionContext): Future[Option[Int]] = Future.failed(new RuntimeException("transaction not found"))
 
-  override def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] = Future.successful(true)
+  override def isTransactionOutputSpendable(outPoint: OutPoint, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] = Future.successful(true)
 
   override def rollback(tx: Transaction)(implicit ec: ExecutionContext): Future[Boolean] = {
     rolledback = rolledback + tx
@@ -218,7 +218,7 @@ class SingleKeyOnChainWallet extends OnChainWallet with OnChainAddressCache {
 
   override def getTxConfirmations(txid: TxId)(implicit ec: ExecutionContext): Future[Option[Int]] = Future.successful(None)
 
-  override def isTransactionOutputSpendable(txid: TxId, outputIndex: Int, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] = Future.successful(true)
+  override def isTransactionOutputSpendable(outPoint: OutPoint, includeMempool: Boolean)(implicit ec: ExecutionContext): Future[Boolean] = Future.successful(true)
 
   override def rollback(tx: Transaction)(implicit ec: ExecutionContext): Future[Boolean] = {
     rolledback = rolledback :+ tx