Abort HTLC tx publisher if remote commit confirms (#2080)

If the remote commit confirms before our local commit, there is no reason
to try to publish our HTLC transactions, we will instead directly claim
the htlc outputs from the remote commit.

We previously checked timelocks before checking preconditions, which in
this case means we would be waiting for a confirmation on our local commit
forever. We now check preconditions before timelocks, and added a
precondition that verifies that the remote commit isn't confirmed before
publishing our HTLC txs.

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

@@ -51,6 +51,7 @@ object ReplaceableTxPublisher {
   private case object TimeLocksOk extends Command
   private case object CommitTxAlreadyConfirmed extends RuntimeException with Command
   private case object RemoteCommitTxPublished extends RuntimeException with Command
+  private case object RemoteCommitTxConfirmed extends Command
   private case object PreconditionsOk extends Command
   private case class FundingFailed(reason: Throwable) extends Command
   private case class SignFundedTx(tx: ReplaceableTransactionWithInputInfo, fee: Satoshi) extends Command
@@ -178,23 +179,12 @@ private class ReplaceableTxPublisher(nodeParams: NodeParams,
 
   def start(): Behavior[Command] = {
     Behaviors.receiveMessagePartial {
-      case Publish(replyTo, cmd, targetFeerate) => checkTimeLocks(replyTo, cmd, targetFeerate)
-      case Stop => Behaviors.stopped
-    }
-  }
-
-  def checkTimeLocks(replyTo: ActorRef[TxPublisher.PublishTxResult], cmd: TxPublisher.PublishReplaceableTx, targetFeerate: FeeratePerKw): Behavior[Command] = {
-    val timeLocksChecker = context.spawn(TxTimeLocksMonitor(nodeParams, watcher, loggingInfo), "time-locks-monitor")
-    timeLocksChecker ! CheckTx(context.messageAdapter[TxTimeLocksMonitor.TimeLocksOk](_ => TimeLocksOk), cmd.txInfo.tx, cmd.desc)
-    Behaviors.receiveMessagePartial {
-      case TimeLocksOk =>
+      case Publish(replyTo, cmd, targetFeerate) =>
         cmd.txInfo match {
           case _: ClaimLocalAnchorOutputTx => checkAnchorPreconditions(replyTo, cmd, targetFeerate)
           case htlcTx: HtlcTx => checkHtlcPreconditions(replyTo, cmd, htlcTx, targetFeerate)
         }
-      case Stop =>
-        timeLocksChecker ! TxTimeLocksMonitor.Stop
-        Behaviors.stopped
+      case Stop => Behaviors.stopped
     }
   }
 
@@ -243,23 +233,48 @@ private class ReplaceableTxPublisher(nodeParams: NodeParams,
   }
 
   def checkHtlcPreconditions(replyTo: ActorRef[TxPublisher.PublishTxResult], cmd: TxPublisher.PublishReplaceableTx, htlcTx: HtlcTx, targetFeerate: FeeratePerKw): Behavior[Command] = {
-    val htlcFeerate = cmd.commitments.localCommit.spec.htlcTxFeerate(cmd.commitments.commitmentFormat)
-    // HTLC transactions have a 1-block relative delay when using anchor outputs, which ensures our commit tx has already
-    // been confirmed (we don't need to check again here).
-    HtlcTxAndWitnessData(htlcTx, cmd.commitments) match {
-      case Some(txWithWitnessData) if targetFeerate <= htlcFeerate =>
-        val channelKeyPath = keyManager.keyPath(cmd.commitments.localParams, cmd.commitments.channelConfig)
-        val localPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, cmd.commitments.localCommit.index)
-        val localHtlcBasepoint = keyManager.htlcPoint(channelKeyPath)
-        val localSig = keyManager.sign(htlcTx, localHtlcBasepoint, localPerCommitmentPoint, TxOwner.Local, cmd.commitments.commitmentFormat)
-        val signedHtlcTx = txWithWitnessData.addSigs(localSig, cmd.commitments.commitmentFormat)
-        log.info("publishing {} without adding inputs: txid={}", htlcTx.desc, signedHtlcTx.tx.txid)
-        publish(replyTo, cmd, signedHtlcTx.tx, htlcTx.fee)
-      case Some(_) =>
-        fund(replyTo, cmd, targetFeerate)
-      case None =>
-        log.error("witness data not found for htlcId={}, skipping...", htlcTx.htlcId)
-        sendResult(replyTo, TxPublisher.TxRejected(loggingInfo.id, cmd, TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = false)))
+    // We verify that:
+    //  - their commit is not confirmed: if it is, there is no need to publish our HTLC transactions
+    context.pipeToSelf(bitcoinClient.getTxConfirmations(cmd.commitments.remoteCommit.txid)) {
+      case Success(Some(depth)) if depth >= nodeParams.minDepthBlocks => RemoteCommitTxConfirmed
+      case Success(_) => PreconditionsOk
+      case Failure(_) => PreconditionsOk // if our checks fail, we don't want it to prevent us from publishing HTLC transactions
+    }
+    Behaviors.receiveMessagePartial {
+      case PreconditionsOk =>
+        HtlcTxAndWitnessData(htlcTx, cmd.commitments) match {
+          case Some(txWithWitnessData) => checkTimeLocks(replyTo, cmd, txWithWitnessData, targetFeerate)
+          case None =>
+            log.error("witness data not found for htlcId={}, skipping...", htlcTx.htlcId)
+            sendResult(replyTo, TxPublisher.TxRejected(loggingInfo.id, cmd, TxPublisher.TxRejectedReason.TxSkipped(retryNextBlock = false)))
+        }
+      case RemoteCommitTxConfirmed =>
+        log.warn("cannot publish {}: remote commit has been confirmed", cmd.desc)
+        sendResult(replyTo, TxPublisher.TxRejected(loggingInfo.id, cmd, TxPublisher.TxRejectedReason.ConflictingTxConfirmed))
+      case Stop => Behaviors.stopped
+    }
+  }
+
+  def checkTimeLocks(replyTo: ActorRef[TxPublisher.PublishTxResult], cmd: TxPublisher.PublishReplaceableTx, htlcTxWithWitnessData: HtlcTxAndWitnessData, targetFeerate: FeeratePerKw): Behavior[Command] = {
+    val timeLocksChecker = context.spawn(TxTimeLocksMonitor(nodeParams, watcher, loggingInfo), "time-locks-monitor")
+    timeLocksChecker ! CheckTx(context.messageAdapter[TxTimeLocksMonitor.TimeLocksOk](_ => TimeLocksOk), cmd.txInfo.tx, cmd.desc)
+    Behaviors.receiveMessagePartial {
+      case TimeLocksOk =>
+        val htlcFeerate = cmd.commitments.localCommit.spec.htlcTxFeerate(cmd.commitments.commitmentFormat)
+        if (targetFeerate <= htlcFeerate) {
+          val channelKeyPath = keyManager.keyPath(cmd.commitments.localParams, cmd.commitments.channelConfig)
+          val localPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, cmd.commitments.localCommit.index)
+          val localHtlcBasepoint = keyManager.htlcPoint(channelKeyPath)
+          val localSig = keyManager.sign(htlcTxWithWitnessData.txInfo, localHtlcBasepoint, localPerCommitmentPoint, TxOwner.Local, cmd.commitments.commitmentFormat)
+          val signedHtlcTx = htlcTxWithWitnessData.addSigs(localSig, cmd.commitments.commitmentFormat)
+          log.info("publishing {} without adding inputs: txid={}", cmd.desc, signedHtlcTx.tx.txid)
+          publish(replyTo, cmd, signedHtlcTx.tx, signedHtlcTx.fee)
+        } else {
+          fund(replyTo, cmd, targetFeerate)
+        }
+      case Stop =>
+        timeLocksChecker ! TxTimeLocksMonitor.Stop
+        Behaviors.stopped
     }
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/channel/publish/ReplaceableTxPublisherSpec.scala

@@ -29,7 +29,7 @@ import fr.acinq.eclair.blockchain.bitcoind.rpc.{BitcoinCoreClient, BitcoinJsonRP
 import fr.acinq.eclair.blockchain.fee.FeeratePerKw
 import fr.acinq.eclair.channel._
 import fr.acinq.eclair.channel.publish.ReplaceableTxPublisher.{Publish, Stop}
-import fr.acinq.eclair.channel.publish.TxPublisher.TxRejectedReason.{ConflictingTxUnconfirmed, CouldNotFund, TxSkipped, WalletInputGone}
+import fr.acinq.eclair.channel.publish.TxPublisher.TxRejectedReason._
 import fr.acinq.eclair.channel.publish.TxPublisher._
 import fr.acinq.eclair.channel.states.{ChannelStateTestsHelperMethods, ChannelStateTestsTags}
 import fr.acinq.eclair.transactions.Transactions.{ClaimLocalAnchorOutputTx, HtlcSuccessTx, HtlcTimeoutTx}
@@ -446,6 +446,53 @@ class ReplaceableTxPublisherSpec extends TestKitBaseClass with AnyFunSuiteLike w
     })
   }
 
+  test("remote commit tx confirmed, not publishing htlc tx") {
+    withFixture(Seq(500 millibtc), ChannelTypes.AnchorOutputsZeroFeeHtlcTx, f => {
+      import f._
+
+      // Add htlcs in both directions and ensure that preimages are available.
+      addHtlc(5_000_000 msat, alice, bob, alice2bob, bob2alice)
+      crossSign(alice, bob, alice2bob, bob2alice)
+      val (r, htlc) = addHtlc(4_000_000 msat, bob, alice, bob2alice, alice2bob)
+      crossSign(bob, alice, bob2alice, alice2bob)
+      probe.send(alice, CMD_FULFILL_HTLC(htlc.id, r, replyTo_opt = Some(probe.ref)))
+      probe.expectMsgType[CommandSuccess[CMD_FULFILL_HTLC]]
+
+      // Force-close channel.
+      probe.send(alice, CMD_FORCECLOSE(probe.ref))
+      probe.expectMsgType[CommandSuccess[CMD_FORCECLOSE]]
+      alice2blockchain.expectMsgType[PublishRawTx]
+      assert(alice2blockchain.expectMsgType[PublishReplaceableTx].txInfo.isInstanceOf[ClaimLocalAnchorOutputTx])
+      alice2blockchain.expectMsgType[PublishRawTx] // claim main output
+      val htlcSuccess = alice2blockchain.expectMsgType[PublishReplaceableTx]
+      assert(htlcSuccess.txInfo.isInstanceOf[HtlcSuccessTx])
+      val htlcTimeout = alice2blockchain.expectMsgType[PublishReplaceableTx]
+      assert(htlcTimeout.txInfo.isInstanceOf[HtlcTimeoutTx])
+
+      // Ensure remote commit tx confirms.
+      val remoteCommitTx = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.fullySignedLocalCommitTx(bob.underlyingActor.nodeParams.channelKeyManager)
+      wallet.publishTransaction(remoteCommitTx.tx).pipeTo(probe.ref)
+      probe.expectMsg(remoteCommitTx.tx.txid)
+      generateBlocks(5)
+
+      // Verify that HTLC transactions immediately fail to publish.
+      val targetFeerate = FeeratePerKw(15_000 sat)
+      val htlcSuccessPublisher = createPublisher()
+      htlcSuccessPublisher ! Publish(probe.ref, htlcSuccess, targetFeerate)
+      val result1 = probe.expectMsgType[TxRejected]
+      assert(result1.cmd === htlcSuccess)
+      assert(result1.reason === ConflictingTxConfirmed)
+      htlcSuccessPublisher ! Stop
+
+      val htlcTimeoutPublisher = createPublisher()
+      htlcTimeoutPublisher ! Publish(probe.ref, htlcTimeout, targetFeerate)
+      val result2 = probe.expectMsgType[TxRejected]
+      assert(result2.cmd === htlcTimeout)
+      assert(result2.reason === ConflictingTxConfirmed)
+      htlcTimeoutPublisher ! Stop
+    })
+  }
+
   def closeChannelWithHtlcs(f: Fixture): (Transaction, PublishReplaceableTx, PublishReplaceableTx) = {
     import f._