Refactor replaceable transactions (#3075)

We previously included confirmation targets directly in HTLC and anchor
transactions, which didn't really belong there. We now instead set
confirmation targets at publication time, based on the current state of
the force-close attempt.

We also extract witness data for HTLCs (preimage and remote signature)
when creating the command to publish the transaction, instead of doing
it inside the publisher actors.

We improve the checks done in the pre-publisher actors, which now take
into account the min-depth for commit txs before aborting anchor txs.
The `ReplaceableTxFunder` is also simpler now, thanks to a better
encapsulation of the transaction's contents.

We didn't watch our anchor output on the remote commit, which means we
weren't recording in the `AuditDB` the fees we paid to get a remote
commit tx confirmed by spending our anchor output. We only had that
code for the local commit, but we've recently started more aggressively
trying to get the *remote* commitment confirmed, so it's important to
correctly record those fees.

Author: t-bast

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))