Unwatch previous funding tx after splice (#3218)

After a splice transaction confirms, we don't need to keep watching the
previous funding output: it has been irrevocably spent and needlessly
consumes resources in the `Watcher` actor.

Those watches are cleaned up when the `Channel` actor dies, which does
not happen if the channel isn't closed and the `Peer` actor is kept
alive while disconnected.

Whenever we receive a new block, we log the number of watches we have.
This lets us detect whether we're missing some clean-up of old watches
in edge cases.

Author: t-bast

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

@@ -132,6 +132,7 @@ object ZmqWatcher {
 
   case class WatchFundingSpent(replyTo: ActorRef[WatchFundingSpentTriggered], txId: TxId, outputIndex: Int, hints: Set[TxId]) extends WatchSpent[WatchFundingSpentTriggered]
   case class WatchFundingSpentTriggered(spendingTx: Transaction) extends WatchSpentTriggered
+  case class UnwatchFundingSpent(txId: TxId, outputIndex: Int) extends Command
 
   case class WatchOutputSpent(replyTo: ActorRef[WatchOutputSpentTriggered], txId: TxId, outputIndex: Int, amount: Satoshi, hints: Set[TxId]) extends WatchSpent[WatchOutputSpentTriggered]
   case class WatchOutputSpentTriggered(amount: Satoshi, spendingTx: Transaction) extends WatchSpentTriggered
@@ -321,6 +322,12 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
         blockHeight.set(currentHeight.toLong)
         context.system.eventStream ! EventStream.Publish(CurrentBlockHeight(currentHeight))
         // TODO: should we try to mitigate the herd effect and not check all watches immediately?
+        val watchExternalChannelCount = watches.keySet.count(_.isInstanceOf[WatchExternalChannelSpent])
+        val watchFundingSpentCount = watches.keySet.count(_.isInstanceOf[WatchFundingSpent])
+        val watchOutputSpentCount = watches.keySet.count(_.isInstanceOf[WatchOutputSpent])
+        val watchPublishedCount = watches.keySet.count(_.isInstanceOf[WatchPublished])
+        val watchConfirmedCount = watches.keySet.count(_.isInstanceOf[WatchConfirmed[_]])
+        log.info("{} watched utxos: external-channels={}, funding-spent={}, output-spent={}, tx-published={}, tx-confirmed={}", watchedUtxos.size, watchExternalChannelCount, watchFundingSpentCount, watchOutputSpentCount, watchPublishedCount, watchConfirmedCount)
         KamonExt.timeFuture(Metrics.NewBlockCheckConfirmedDuration.withoutTags()) {
           Future.sequence(watches.collect {
             case (w: WatchPublished, _) => checkPublished(w)
@@ -401,6 +408,11 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
         val watchedUtxos1 = deprecatedWatches.foldLeft(watchedUtxos) { case (m, w) => removeWatchedUtxos(m, w) }
         watching(watches -- deprecatedWatches, watchedUtxos1, analyzedBlocks)
 
+      case UnwatchFundingSpent(txId, outputIndex) =>
+        val deprecatedWatches = watches.keySet.collect { case w: WatchFundingSpent if w.txId == txId && w.outputIndex == outputIndex => w }
+        val watchedUtxos1 = deprecatedWatches.foldLeft(watchedUtxos) { case (m, w) => removeWatchedUtxos(m, w) }
+        watching(watches -- deprecatedWatches, watchedUtxos1, analyzedBlocks)
+
       case ValidateRequest(replyTo, ann) =>
         client.validate(ann).map(replyTo ! _)
         Behaviors.same

eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/CommonFundingHandlers.scala

@@ -102,6 +102,8 @@ trait CommonFundingHandlers extends CommonHandlers {
             // Children splice transactions may already spend that confirmed funding transaction.
             val spliceSpendingTxs = commitments1.all.collect { case c if c.fundingTxIndex == commitment.fundingTxIndex + 1 => c.fundingTxId }
             watchFundingSpent(commitment, additionalKnownSpendingTxs = spliceSpendingTxs.toSet, None)
+            // We can unwatch the previous funding transaction(s), which have been spent by this splice transaction.
+            d.commitments.all.collect { case c if c.fundingTxIndex < commitment.fundingTxIndex => blockchain ! UnwatchFundingSpent(c.fundingTxId, c.fundingInput.index.toInt) }
             // In the dual-funding/splicing case we can forget all other transactions (RBF attempts), they have been
             // double-spent by the tx that just confirmed.
             val conflictingTxs = d.commitments.active // note how we use the unpruned original commitments

eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcherSpec.scala

@@ -370,6 +370,32 @@ class ZmqWatcherSpec extends TestKitBaseClass with AnyFunSuiteLike with Bitcoind
     })
   }
 
+  test("unwatch funding spent") {
+    withWatcher(f => {
+      import f._
+
+      val (priv, address) = createExternalAddress()
+      val tx = sendToAddress(address, Btc(1), probe)
+      val outputIndex = tx.txOut.indexWhere(_.publicKeyScript == Script.write(Script.pay2wpkh(priv.publicKey)))
+      val (tx1, _) = createUnspentTxChain(tx, priv)
+
+      watcher ! WatchFundingSpent(probe.ref, tx.txid, outputIndex, Set.empty)
+      probe.expectNoMessage(100 millis)
+
+      bitcoinClient.publishTransaction(tx1)
+      probe.expectMsg(WatchFundingSpentTriggered(tx1))
+      probe.expectNoMessage(100 millis)
+
+      watcher ! UnwatchFundingSpent(tx.txid, outputIndex)
+      probe.expectNoMessage(100 millis)
+      // Let's confirm tx and tx1: seeing tx1 in a block should trigger both WatchSpentTriggered events again, but we unwatched the transaction.
+      bitcoinClient.getBlockHeight().pipeTo(probe.ref)
+      probe.expectMsgType[BlockHeight]
+      generateBlocks(1)
+      probe.expectNoMessage(100 millis)
+    })
+  }
+
   test("watch for unknown spent transactions") {
     withWatcher(f => {
       import f._