Refactor replaceable transactions

eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala

@@ -19,7 +19,7 @@ package fr.acinq.eclair.channel
 import akka.actor.{ActorRef, PossiblyHarmful, typed}
 import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
 import fr.acinq.bitcoin.scalacompat.{ByteVector32, DeterministicWallet, OutPoint, Satoshi, SatoshiLong, Transaction, TxId, TxOut}
-import fr.acinq.eclair.blockchain.fee.{ConfirmationTarget, FeeratePerKw}
+import fr.acinq.eclair.blockchain.fee.{ConfirmationTarget, FeeratePerKw, OnChainFeeConf}
 import fr.acinq.eclair.channel.LocalFundingStatus.DualFundedUnconfirmedFundingTx
 import fr.acinq.eclair.channel.fund.InteractiveTxBuilder._
 import fr.acinq.eclair.channel.fund.{InteractiveTxBuilder, InteractiveTxSigningSession}
@@ -313,7 +313,7 @@ sealed trait CommitPublished {
   def commitTx: Transaction
   /** Map of relevant outpoints that have been spent and the confirmed transaction that spends them. */
   def irrevocablySpent: Map[OutPoint, Transaction]
-
+  /** Returns true if the commitment transaction is confirmed. */
   def isConfirmed: Boolean = {
     // NB: if multiple transactions end up in the same block, the first confirmation we receive may not be the commit tx.
     // However if the confirmed tx spends from the commit tx, we know that the commit tx is already confirmed and we know
@@ -329,11 +329,26 @@ sealed trait CommitPublished {
  * @param htlcTxs                  txs claiming HTLCs. There will be one entry for each pending HTLC. The value will be
  *                                 None only for incoming HTLCs for which we don't have the preimage (we can't claim them yet).
  * @param claimHtlcDelayedTxs      3rd-stage txs (spending the output of HTLC txs).
- * @param claimAnchorTxs           txs spending anchor outputs to bump the feerate of the commitment tx (if applicable).
- *                                 We currently only claim our local anchor, but it would be nice to claim both when it
- *                                 is economical to do so to avoid polluting the utxo set.
+ * @param claimAnchorTxs           txs spending our anchor output to bump the feerate of the commitment tx (if applicable).
  */
 case class LocalCommitPublished(commitTx: Transaction, claimMainDelayedOutputTx: Option[ClaimLocalDelayedOutputTx], htlcTxs: Map[OutPoint, Option[HtlcTx]], claimHtlcDelayedTxs: List[HtlcDelayedTx], claimAnchorTxs: List[ClaimAnchorOutputTx], irrevocablySpent: Map[OutPoint, Transaction]) extends CommitPublished {
+  // We previously used a list of anchor transactions because we included the confirmation target, but that's obsolete and should be overridden on updates.
+  val claimAnchorTx_opt: Option[ClaimAnchorOutputTx] = claimAnchorTxs.headOption
+
+  /** Compute the confirmation target that should be used to get the [[commitTx]] confirmed. */
+  def confirmationTarget(onChainFeeConf: OnChainFeeConf): Option[ConfirmationTarget] = {
+    if (isConfirmed) {
+      None
+    } else {
+      htlcTxs.values.flatten.map(_.htlcExpiry.blockHeight).minOption match {
+        // If there are pending HTLCs, we must get the commit tx confirmed before they timeout.
+        case Some(htlcExpiry) => Some(ConfirmationTarget.Absolute(htlcExpiry))
+        // Otherwise, we don't have funds at risk, so we can aim for a slower confirmation.
+        case None => Some(ConfirmationTarget.Priority(onChainFeeConf.feeTargets.closing))
+      }
+    }
+  }
+
   /**
    * A local commit is considered done when:
    * - all commitment tx outputs that we can spend have been spent and confirmed (even if the spending tx was not ours)
@@ -363,11 +378,26 @@ case class LocalCommitPublished(commitTx: Transaction, claimMainDelayedOutputTx:
  * @param claimMainOutputTx tx claiming our main output (if we have one).
  * @param claimHtlcTxs      txs claiming HTLCs. There will be one entry for each pending HTLC. The value will be None
  *                          only for incoming HTLCs for which we don't have the preimage (we can't claim them yet).
- * @param claimAnchorTxs    txs spending anchor outputs to bump the feerate of the commitment tx (if applicable).
- *                          We currently only claim our local anchor, but it would be nice to claim both when it is
- *                          economical to do so to avoid polluting the utxo set.
+ * @param claimAnchorTxs    txs spending our anchor output to bump the feerate of the commitment tx (if applicable).
  */
 case class RemoteCommitPublished(commitTx: Transaction, claimMainOutputTx: Option[ClaimRemoteCommitMainOutputTx], claimHtlcTxs: Map[OutPoint, Option[ClaimHtlcTx]], claimAnchorTxs: List[ClaimAnchorOutputTx], irrevocablySpent: Map[OutPoint, Transaction]) extends CommitPublished {
+  // We previously used a list of anchor transactions because we included the confirmation target, but that's obsolete and should be overridden on updates.
+  val claimAnchorTx_opt: Option[ClaimAnchorOutputTx] = claimAnchorTxs.headOption
+
+  /** Compute the confirmation target that should be used to get the [[commitTx]] confirmed. */
+  def confirmationTarget(onChainFeeConf: OnChainFeeConf): Option[ConfirmationTarget] = {
+    if (isConfirmed) {
+      None
+    } else {
+      claimHtlcTxs.values.flatten.map(_.htlcExpiry.blockHeight).minOption match {
+        // If there are pending HTLCs, we must get the commit tx confirmed before they timeout.
+        case Some(htlcExpiry) => Some(ConfirmationTarget.Absolute(htlcExpiry))
+        // Otherwise, we don't have funds at risk, so we can aim for a slower confirmation.
+        case None => Some(ConfirmationTarget.Priority(onChainFeeConf.feeTargets.closing))
+      }
+    }
+  }
+
   /**
    * A remote commit is considered done when all commitment tx outputs that we can spend have been spent and confirmed
    * (even if the spending tx was not ours).

eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala

@@ -237,6 +237,19 @@ case class RemoteCommit(index: Long, spec: CommitmentSpec, txid: TxId, remotePer
 /** We have the next remote commit when we've sent our commit_sig but haven't yet received their revoke_and_ack. */
 case class NextRemoteCommit(sig: CommitSig, commit: RemoteCommit)
 
+/**
+ * If we ignore revoked commitments, there can be at most three concurrent commitment transactions during a force-close:
+ *  - the local commitment
+ *  - the remote commitment
+ *  - the next remote commitment, if we sent commit_sig but haven't yet received revoke_and_ack
+ */
+case class CommitTxIds(localCommitTxId: TxId, remoteCommitTxId: TxId, nextRemoteCommitTxId_opt: Option[TxId]) {
+  val txIds: Set[TxId] = nextRemoteCommitTxId_opt match {
+    case Some(nextRemoteCommitTxId) => Set(localCommitTxId, remoteCommitTxId, nextRemoteCommitTxId)
+    case None => Set(localCommitTxId, remoteCommitTxId)
+  }
+}
+
 /**
  * A minimal commitment for a given funding tx.
  *
@@ -255,6 +268,7 @@ case class Commitment(fundingTxIndex: Long,
                       localCommit: LocalCommit, remoteCommit: RemoteCommit, nextRemoteCommit_opt: Option[NextRemoteCommit]) {
   val commitInput: InputInfo = localCommit.commitTxAndRemoteSig.commitTx.input
   val fundingTxId: TxId = commitInput.outPoint.txid
+  val commitTxIds: CommitTxIds = CommitTxIds(localCommit.commitTxAndRemoteSig.commitTx.tx.txid, remoteCommit.txid, nextRemoteCommit_opt.map(_.commit.txid))
   val capacity: Satoshi = commitInput.txOut.amount
   /** Once the funding transaction is confirmed, short_channel_id matching this transaction. */
   val shortChannelId_opt: Option[RealShortChannelId] = localFundingStatus match {
@@ -735,6 +749,7 @@ case class FullCommitment(params: ChannelParams, changes: CommitmentChanges,
   val remoteParams: RemoteParams = params.remoteParams
   val commitInput: InputInfo = localCommit.commitTxAndRemoteSig.commitTx.input
   val fundingTxId: TxId = commitInput.outPoint.txid
+  val commitTxIds: CommitTxIds = CommitTxIds(localCommit.commitTxAndRemoteSig.commitTx.tx.txid, remoteCommit.txid, nextRemoteCommit_opt.map(_.commit.txid))
   val capacity: Satoshi = commitInput.txOut.amount
   val commitment: Commitment = Commitment(fundingTxIndex, firstRemoteCommitIndex, remoteFundingPubKey, localFundingStatus, remoteFundingStatus, localCommit, remoteCommit, nextRemoteCommit_opt)
 

eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala

@@ -850,25 +850,6 @@ object Helpers {
       if (localPaysCommitTxFees) commitInput.txOut.amount - commitTx.txOut.map(_.amount).sum else 0 sat
     }
 
-    /**
-     * This function checks if the proposed confirmation target is more aggressive than whatever confirmation target
-     * we previously had. Note that absolute targets are always considered more aggressive than relative targets.
-     */
-    private def shouldUpdateAnchorTxs(anchorTxs: List[ClaimAnchorOutputTx], confirmationTarget: ConfirmationTarget): Boolean = {
-      anchorTxs
-        .collect { case tx: ClaimAnchorOutputTx => tx.confirmationTarget }
-        .forall {
-          case ConfirmationTarget.Absolute(current) => confirmationTarget match {
-            case ConfirmationTarget.Absolute(proposed) => proposed < current
-            case _: ConfirmationTarget.Priority => false
-          }
-          case ConfirmationTarget.Priority(current) => confirmationTarget match {
-            case _: ConfirmationTarget.Absolute => true
-            case ConfirmationTarget.Priority(proposed) => current < proposed
-          }
-        }
-    }
-
     object LocalClose {
 
       /**
@@ -900,23 +881,17 @@ object Helpers {
         )
         val spendAnchors = htlcTxs.nonEmpty || onChainFeeConf.spendAnchorWithoutHtlcs
         if (spendAnchors) {
-          // If we don't have pending HTLCs, we don't have funds at risk, so we can aim for a slower confirmation.
-          val confirmCommitBefore = htlcTxs.values.flatten.map(htlcTx => ConfirmationTarget.Absolute(htlcTx.htlcExpiry.blockHeight)).minByOption(_.confirmBefore).getOrElse(ConfirmationTarget.Priority(onChainFeeConf.feeTargets.closing))
-          claimAnchors(fundingKey, commitmentKeys, lcp, confirmCommitBefore, commitment.params.commitmentFormat)
+          claimAnchors(fundingKey, commitmentKeys, lcp, commitment.params.commitmentFormat)
         } else {
           lcp
         }
       }
 
-      def claimAnchors(fundingKey: PrivateKey, commitKeys: LocalCommitmentKeys, lcp: LocalCommitPublished, confirmationTarget: ConfirmationTarget, commitmentFormat: CommitmentFormat)(implicit log: LoggingAdapter): LocalCommitPublished = {
-        if (shouldUpdateAnchorTxs(lcp.claimAnchorTxs, confirmationTarget)) {
-          val claimAnchorTx = withTxGenerationLog("local-anchor") {
-            ClaimAnchorOutputTx.createUnsignedTx(fundingKey, commitKeys.publicKeys, lcp.commitTx, confirmationTarget, commitmentFormat)
-          }
-          lcp.copy(claimAnchorTxs = claimAnchorTx.toList)
-        } else {
-          lcp
+      def claimAnchors(fundingKey: PrivateKey, commitKeys: LocalCommitmentKeys, lcp: LocalCommitPublished, commitmentFormat: CommitmentFormat)(implicit log: LoggingAdapter): LocalCommitPublished = {
+        val claimAnchorTx = withTxGenerationLog("local-anchor") {
+          ClaimAnchorOutputTx.createUnsignedTx(fundingKey, commitKeys.publicKeys, lcp.commitTx, commitmentFormat)
         }
+        lcp.copy(claimAnchorTxs = claimAnchorTx.toList)
       }
 
       /**
@@ -1015,23 +990,17 @@ object Helpers {
         )
         val spendAnchors = htlcTxs.nonEmpty || onChainFeeConf.spendAnchorWithoutHtlcs
         if (spendAnchors) {
-          // If we don't have pending HTLCs, we don't have funds at risk, so we use the normal closing priority.
-          val confirmCommitBefore = htlcTxs.values.flatten.map(htlcTx => ConfirmationTarget.Absolute(htlcTx.htlcExpiry.blockHeight)).minByOption(_.confirmBefore).getOrElse(ConfirmationTarget.Priority(onChainFeeConf.feeTargets.closing))
-          claimAnchors(fundingKey, commitKeys, rcp, confirmCommitBefore, commitment.params.commitmentFormat)
+          claimAnchors(fundingKey, commitKeys, rcp, commitment.params.commitmentFormat)
         } else {
           rcp
         }
       }
 
-      def claimAnchors(fundingKey: PrivateKey, commitKeys: RemoteCommitmentKeys, rcp: RemoteCommitPublished, confirmationTarget: ConfirmationTarget, commitmentFormat: CommitmentFormat)(implicit log: LoggingAdapter): RemoteCommitPublished = {
-        if (shouldUpdateAnchorTxs(rcp.claimAnchorTxs, confirmationTarget)) {
-          val claimAnchorTx = withTxGenerationLog("remote-anchor") {
-            ClaimAnchorOutputTx.createUnsignedTx(fundingKey, commitKeys.publicKeys, rcp.commitTx, confirmationTarget, commitmentFormat)
-          }
-          rcp.copy(claimAnchorTxs = claimAnchorTx.toList)
-        } else {
-          rcp
+      def claimAnchors(fundingKey: PrivateKey, commitKeys: RemoteCommitmentKeys, rcp: RemoteCommitPublished, commitmentFormat: CommitmentFormat)(implicit log: LoggingAdapter): RemoteCommitPublished = {
+        val claimAnchorTx = withTxGenerationLog("remote-anchor") {
+          ClaimAnchorOutputTx.createUnsignedTx(fundingKey, commitKeys.publicKeys, rcp.commitTx, commitmentFormat)
         }
+        rcp.copy(claimAnchorTxs = claimAnchorTx.toList)
       }
 
       /**

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

@@ -38,8 +38,8 @@ import fr.acinq.eclair.channel.Monitoring.{Metrics, Tags}
 import fr.acinq.eclair.channel._
 import fr.acinq.eclair.channel.fund.InteractiveTxBuilder._
 import fr.acinq.eclair.channel.fund.{InteractiveTxBuilder, InteractiveTxFunder, InteractiveTxSigningSession}
-import fr.acinq.eclair.channel.publish.TxPublisher
 import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishReplaceableTx, SetChannelId}
+import fr.acinq.eclair.channel.publish.{ReplaceableLocalCommitAnchor, ReplaceableRemoteCommitAnchor, TxPublisher}
 import fr.acinq.eclair.crypto.keymanager.ChannelKeys
 import fr.acinq.eclair.db.DbEventHandler.ChannelEvent.EventType
 import fr.acinq.eclair.db.PendingCommandsDb
@@ -2168,27 +2168,36 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
         case commitmentFormat: Transactions.AnchorOutputsCommitmentFormat =>
           val commitment = d.commitments.latest
           val fundingKey = channelKeys.fundingKey(commitment.fundingTxIndex)
-          val lcp1 = d.localCommitPublished.map(lcp => Closing.LocalClose.claimAnchors(fundingKey, commitment.localKeys(channelKeys), lcp, c.confirmationTarget, commitmentFormat))
-          val rcp1 = d.remoteCommitPublished.map(rcp => Closing.RemoteClose.claimAnchors(fundingKey, commitment.remoteKeys(channelKeys, commitment.remoteCommit.remotePerCommitmentPoint), rcp, c.confirmationTarget, commitmentFormat))
-          val nrcp1 = d.nextRemoteCommitPublished.map(nrcp => Closing.RemoteClose.claimAnchors(fundingKey, commitment.remoteKeys(channelKeys, commitment.nextRemoteCommit_opt.get.commit.remotePerCommitmentPoint), nrcp, c.confirmationTarget, commitmentFormat))
+          val localAnchor_opt = for {
+            lcp <- d.localCommitPublished
+            commitKeys = commitment.localKeys(channelKeys)
+            anchorTx <- Closing.LocalClose.claimAnchors(fundingKey, commitKeys, lcp, commitmentFormat).claimAnchorTx_opt
+          } yield PublishReplaceableTx(ReplaceableLocalCommitAnchor(anchorTx, fundingKey, commitKeys, lcp.commitTx, commitment), c.confirmationTarget)
+          val remoteAnchor_opt = for {
+            rcp <- d.remoteCommitPublished
+            commitKeys = commitment.remoteKeys(channelKeys, commitment.remoteCommit.remotePerCommitmentPoint)
+            anchorTx <- Closing.RemoteClose.claimAnchors(fundingKey, commitKeys, rcp, commitmentFormat).claimAnchorTx_opt
+          } yield PublishReplaceableTx(ReplaceableRemoteCommitAnchor(anchorTx, fundingKey, commitKeys, rcp.commitTx, commitment), c.confirmationTarget)
+          val nextRemoteAnchor_opt = for {
+            nrcp <- d.nextRemoteCommitPublished
+            commitKeys = commitment.remoteKeys(channelKeys, commitment.nextRemoteCommit_opt.get.commit.remotePerCommitmentPoint)
+            anchorTx <- Closing.RemoteClose.claimAnchors(fundingKey, commitKeys, nrcp, commitmentFormat).claimAnchorTx_opt
+          } yield PublishReplaceableTx(ReplaceableRemoteCommitAnchor(anchorTx, fundingKey, commitKeys, nrcp.commitTx, commitment), c.confirmationTarget)
           // We favor the remote commitment(s) because they're more interesting than the local commitment (no CSV delays).
-          if (rcp1.nonEmpty) {
-            rcp1.foreach(rcp => rcp.claimAnchorTxs.foreach { tx => txPublisher ! PublishReplaceableTx(tx, channelKeys, d.commitments.latest, rcp.commitTx, tx.confirmationTarget) })
+          if (remoteAnchor_opt.nonEmpty) {
+            remoteAnchor_opt.foreach { publishTx => txPublisher ! publishTx }
             c.replyTo ! RES_SUCCESS(c, d.channelId)
-            stay() using d.copy(remoteCommitPublished = rcp1) storing()
-          } else if (nrcp1.nonEmpty) {
-            nrcp1.foreach(rcp => rcp.claimAnchorTxs.foreach { tx => txPublisher ! PublishReplaceableTx(tx, channelKeys, d.commitments.latest, rcp.commitTx, tx.confirmationTarget) })
+          } else if (nextRemoteAnchor_opt.nonEmpty) {
+            nextRemoteAnchor_opt.foreach { publishTx => txPublisher ! publishTx }
             c.replyTo ! RES_SUCCESS(c, d.channelId)
-            stay() using d.copy(nextRemoteCommitPublished = nrcp1) storing()
-          } else if (lcp1.nonEmpty) {
-            lcp1.foreach(lcp => lcp.claimAnchorTxs.foreach { tx => txPublisher ! PublishReplaceableTx(tx, channelKeys, d.commitments.latest, lcp.commitTx, tx.confirmationTarget) })
+          } else if (localAnchor_opt.nonEmpty) {
+            localAnchor_opt.foreach { publishTx => txPublisher ! publishTx }
             c.replyTo ! RES_SUCCESS(c, d.channelId)
-            stay() using d.copy(localCommitPublished = lcp1) storing()
           } else {
             log.warning("cannot bump force-close fees, local or remote commit not published")
             c.replyTo ! RES_FAILURE(c, CommandUnavailableInThisState(d.channelId, "rbf-force-close", stateName))
-            stay()
           }
+          stay()
         case _ =>
           log.warning("cannot bump force-close fees, channel is not using anchor outputs")
           c.replyTo ! RES_FAILURE(c, CommandUnavailableInThisState(d.channelId, "rbf-force-close", stateName))