From cde0d3dc237a121a0f04460343b689585895e9b7 Mon Sep 17 00:00:00 2001
From: t-bast <bastien@acinq.fr>
Date: Tue, 20 May 2025 16:01:37 +0200
Subject: [PATCH 1/4] Move `confirmationTarget` to closing helper function

We move functions that compute the confirmation target outside of
`LocalCommitPublished` and `RemoteCommitPublished` to helper functions
in the `Closing` object.

We also slightly refactor `doPublish` functions to remove the pattern
matching on the commitment format, which was actually unnecessary since
we introduced the `redeemableHtlcTxs` function.
---
 .../fr/acinq/eclair/channel/ChannelData.scala | 30 +-----------
 .../fr/acinq/eclair/channel/Helpers.scala     | 21 ++++++++
 .../eclair/channel/fsm/ErrorHandlers.scala    | 49 ++++++++++---------
 3 files changed, 47 insertions(+), 53 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala
index 3b08caa8a5..196f5c3497 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala
+++ b/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, OnChainFeeConf}
+import fr.acinq.eclair.blockchain.fee.{ConfirmationTarget, FeeratePerKw}
 import fr.acinq.eclair.channel.LocalFundingStatus.DualFundedUnconfirmedFundingTx
 import fr.acinq.eclair.channel.fund.InteractiveTxBuilder._
 import fr.acinq.eclair.channel.fund.{InteractiveTxBuilder, InteractiveTxSigningSession}
@@ -335,20 +335,6 @@ case class LocalCommitPublished(commitTx: Transaction, claimMainDelayedOutputTx:
   // 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)
@@ -384,20 +370,6 @@ case class RemoteCommitPublished(commitTx: Transaction, claimMainOutputTx: Optio
   // 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).
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
index 144d277e01..6ee6d1ad25 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -850,6 +850,27 @@ object Helpers {
       if (localPaysCommitTxFees) commitInput.txOut.amount - commitTx.txOut.map(_.amount).sum else 0 sat
     }
 
+    /** Return the confirmation target that should be used for our local commitment. */
+    def confirmationTarget(localCommit: LocalCommit, localDustLimit: Satoshi, commitmentFormat: CommitmentFormat, onChainFeeConf: OnChainFeeConf): ConfirmationTarget = {
+      confirmationTarget(localCommit.spec, localDustLimit, commitmentFormat, onChainFeeConf)
+    }
+
+    /** Return the confirmation target that should be used for the given remote commitment. */
+    def confirmationTarget(remoteCommit: RemoteCommit, remoteDustLimit: Satoshi, commitmentFormat: CommitmentFormat, onChainFeeConf: OnChainFeeConf): ConfirmationTarget = {
+      confirmationTarget(remoteCommit.spec, remoteDustLimit, commitmentFormat, onChainFeeConf)
+    }
+
+    private def confirmationTarget(spec: CommitmentSpec, dustLimit: Satoshi, commitmentFormat: CommitmentFormat, onChainFeeConf: OnChainFeeConf): ConfirmationTarget = {
+      val offeredHtlcs = Transactions.trimOfferedHtlcs(dustLimit, spec, commitmentFormat).map(_.add)
+      val receivedHtlcs = Transactions.trimReceivedHtlcs(dustLimit, spec, commitmentFormat).map(_.add)
+      (offeredHtlcs ++ receivedHtlcs).map(_.cltvExpiry).minOption match {
+        // If there are pending HTLCs, we must get the commit tx confirmed before they timeout.
+        case Some(htlcExpiry) => ConfirmationTarget.Absolute(htlcExpiry.blockHeight)
+        // Otherwise, we don't have funds at risk, so we can aim for a slower confirmation.
+        case None => ConfirmationTarget.Priority(onChainFeeConf.feeTargets.closing)
+      }
+    }
+
     object LocalClose {
 
       /**
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala
index e7cfe24ea3..2fa5760460 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala
@@ -233,19 +233,17 @@ trait ErrorHandlers extends CommonHandlers {
 
     val fundingKey = channelKeys.fundingKey(commitment.fundingTxIndex)
     val commitKeys = commitment.localKeys(channelKeys)
-    val localPaysCommitTxFees = commitment.localParams.paysCommitTxFees
-    val publishQueue = commitment.params.commitmentFormat match {
-      case Transactions.DefaultCommitmentFormat =>
-        val htlcTxs = redeemableHtlcTxs(commitTx, commitKeys, commitment)
-        List(PublishFinalTx(commitTx, commitment.commitInput.outPoint, commitment.capacity, "commit-tx", Closing.commitTxFee(commitment.commitInput, commitTx, localPaysCommitTxFees), None)) ++ (claimMainDelayedOutputTx.map(tx => PublishFinalTx(tx, tx.fee, None)) ++ htlcTxs ++ claimHtlcDelayedTxs.map(tx => PublishFinalTx(tx, tx.fee, None)))
-      case _: Transactions.AnchorOutputsCommitmentFormat =>
-        val claimAnchor = for {
-          confirmationTarget <- localCommitPublished.confirmationTarget(nodeParams.onChainFeeConf)
-          anchorTx <- claimAnchorTx_opt
-        } yield PublishReplaceableTx(ReplaceableLocalCommitAnchor(anchorTx, fundingKey, commitKeys, commitTx, commitment), confirmationTarget)
-        val htlcTxs = redeemableHtlcTxs(commitTx, commitKeys, commitment)
-        List(PublishFinalTx(commitTx, commitment.commitInput.outPoint, commitment.capacity, "commit-tx", Closing.commitTxFee(commitment.commitInput, commitTx, localPaysCommitTxFees), None)) ++ claimAnchor ++ claimMainDelayedOutputTx.map(tx => PublishFinalTx(tx, tx.fee, None)) ++ htlcTxs ++ claimHtlcDelayedTxs.map(tx => PublishFinalTx(tx, tx.fee, None))
+    val publishCommitTx = PublishFinalTx(commitTx, commitment.commitInput.outPoint, commitment.capacity, "commit-tx", Closing.commitTxFee(commitment.commitInput, commitTx, commitment.localParams.paysCommitTxFees), None)
+    val publishAnchorTx_opt = claimAnchorTx_opt match {
+      case Some(anchorTx) if !localCommitPublished.isConfirmed =>
+        val confirmationTarget = Closing.confirmationTarget(commitment.localCommit, commitment.localParams.dustLimit, commitment.params.commitmentFormat, nodeParams.onChainFeeConf)
+        Some(PublishReplaceableTx(ReplaceableLocalCommitAnchor(anchorTx, fundingKey, commitKeys, commitTx, commitment), confirmationTarget))
+      case _ => None
     }
+    val publishMainDelayedTx_opt = claimMainDelayedOutputTx.map(tx => PublishFinalTx(tx, tx.fee, None))
+    val publishHtlcTxs = redeemableHtlcTxs(commitTx, commitKeys, commitment)
+    val publishHtlcDelayedTxs = claimHtlcDelayedTxs.map(tx => PublishFinalTx(tx, tx.fee, None))
+    val publishQueue = Seq(publishCommitTx) ++ publishAnchorTx_opt ++ publishMainDelayedTx_opt ++ publishHtlcTxs ++ publishHtlcDelayedTxs
     publishIfNeeded(publishQueue, irrevocablySpent)
 
     // We watch:
@@ -259,7 +257,7 @@ trait ErrorHandlers extends CommonHandlers {
     // We watch outputs of the commitment tx that both parties may spend.
     // We also watch our local anchor: this ensures that we will correctly detect when it's confirmed and count its fees
     // in the audit DB, even if we restart before confirmation.
-    val watchSpentQueue = htlcTxs.keys.toSeq ++ claimAnchorTx_opt.collect { case tx if !localCommitPublished.isConfirmed => tx.input.outPoint }
+    val watchSpentQueue = htlcTxs.keys.toSeq ++ publishAnchorTx_opt.map(_.input)
     watchSpentIfNeeded(commitTx, watchSpentQueue, irrevocablySpent)
   }
 
@@ -331,17 +329,20 @@ trait ErrorHandlers extends CommonHandlers {
     import remoteCommitPublished._
 
     val fundingKey = channelKeys.fundingKey(commitment.fundingTxIndex)
-    val remotePerCommitmentPoint = commitment.nextRemoteCommit_opt match {
-      case Some(c) if remoteCommitPublished.commitTx.txid == c.commit.txid => c.commit.remotePerCommitmentPoint
-      case _ => commitment.remoteCommit.remotePerCommitmentPoint
+    val remoteCommit = commitment.nextRemoteCommit_opt match {
+      case Some(c) if remoteCommitPublished.commitTx.txid == c.commit.txid => c.commit
+      case _ => commitment.remoteCommit
     }
-    val commitKeys = commitment.remoteKeys(channelKeys, remotePerCommitmentPoint)
-    val claimAnchor = for {
-      confirmationTarget <- remoteCommitPublished.confirmationTarget(nodeParams.onChainFeeConf)
-      anchorTx <- claimAnchorTx_opt
-    } yield PublishReplaceableTx(ReplaceableRemoteCommitAnchor(anchorTx, fundingKey, commitKeys, commitTx, commitment), confirmationTarget)
-    val htlcTxs = redeemableClaimHtlcTxs(remoteCommitPublished, commitKeys, commitment)
-    val publishQueue = claimAnchor ++ claimMainOutputTx.map(tx => PublishFinalTx(tx, tx.fee, None)).toSeq ++ htlcTxs
+    val commitKeys = commitment.remoteKeys(channelKeys, remoteCommit.remotePerCommitmentPoint)
+    val publishAnchorTx_opt = claimAnchorTx_opt match {
+      case Some(anchorTx) if !remoteCommitPublished.isConfirmed =>
+        val confirmationTarget = Closing.confirmationTarget(remoteCommit, commitment.remoteParams.dustLimit, commitment.params.commitmentFormat, nodeParams.onChainFeeConf)
+        Some(PublishReplaceableTx(ReplaceableRemoteCommitAnchor(anchorTx, fundingKey, commitKeys, commitTx, commitment), confirmationTarget))
+      case _ => None
+    }
+    val publishMainTx_opt = claimMainOutputTx.map(tx => PublishFinalTx(tx, tx.fee, None))
+    val publishHtlcTxs = redeemableClaimHtlcTxs(remoteCommitPublished, commitKeys, commitment)
+    val publishQueue = publishAnchorTx_opt ++ publishMainTx_opt ++ publishHtlcTxs
     publishIfNeeded(publishQueue, irrevocablySpent)
 
     // We watch:
@@ -353,7 +354,7 @@ trait ErrorHandlers extends CommonHandlers {
     // We watch outputs of the commitment tx that both parties may spend.
     // We also watch our local anchor: this ensures that we will correctly detect when it's confirmed and count its fees
     // in the audit DB, even if we restart before confirmation.
-    val watchSpentQueue = claimHtlcTxs.keys.toSeq ++ claimAnchorTx_opt.collect { case tx if !remoteCommitPublished.isConfirmed => tx.input.outPoint }
+    val watchSpentQueue = claimHtlcTxs.keys.toSeq ++ publishAnchorTx_opt.map(_.input)
     watchSpentIfNeeded(commitTx, watchSpentQueue, irrevocablySpent)
   }
 

From 6f58e0f17109e36d8bf58f26fdcb89f4b4cd01fd Mon Sep 17 00:00:00 2001
From: t-bast <bastien@acinq.fr>
Date: Tue, 20 May 2025 16:45:07 +0200
Subject: [PATCH 2/4] Refactor anchor tx creation

This is a trivial refactoring where we explicitly create a single anchor
transaction instead of replacing an empty list. We also clean up
comments and variable names in force-close helpers.
---
 .../fr/acinq/eclair/channel/Helpers.scala     | 95 +++++++------------
 .../fr/acinq/eclair/channel/fsm/Channel.scala |  6 +-
 2 files changed, 39 insertions(+), 62 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
index 6ee6d1ad25..756c017671 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -873,46 +873,36 @@ object Helpers {
 
     object LocalClose {
 
-      /**
-       * Claim all the HTLCs that we've received from our current commit tx. This will be done using 2nd stage HTLC transactions.
-       *
-       * @param commitment our commitment data, which includes payment preimages
-       * @return a list of transactions (one per output of the commit tx that we can claim)
-       */
+      /** Claim all the outputs that belong to us in our local commitment transaction. */
       def claimCommitTxOutputs(channelKeys: ChannelKeys, commitment: FullCommitment, commitTx: Transaction, feerates: FeeratesPerKw, onChainFeeConf: OnChainFeeConf, finalScriptPubKey: ByteVector)(implicit log: LoggingAdapter): LocalCommitPublished = {
         require(commitment.localCommit.commitTxAndRemoteSig.commitTx.tx.txid == commitTx.txid, "txid mismatch, provided tx is not the current local commit tx")
         val fundingKey = channelKeys.fundingKey(commitment.fundingTxIndex)
         val commitmentKeys = commitment.localKeys(channelKeys)
-        val feeratePerKwDelayed = onChainFeeConf.getClosingFeerate(feerates)
-
-        // first we will claim our main output as soon as the delay is over
-        val mainDelayedTx = withTxGenerationLog("local-main-delayed") {
-          ClaimLocalDelayedOutputTx.createSignedTx(commitmentKeys, commitTx, commitment.localParams.dustLimit, commitment.remoteParams.toSelfDelay, finalScriptPubKey, feeratePerKwDelayed, commitment.params.commitmentFormat)
+        val feerateDelayed = onChainFeeConf.getClosingFeerate(feerates)
+        val mainDelayedTx_opt = withTxGenerationLog("local-main-delayed") {
+          ClaimLocalDelayedOutputTx.createSignedTx(commitmentKeys, commitTx, commitment.localParams.dustLimit, commitment.remoteParams.toSelfDelay, finalScriptPubKey, feerateDelayed, commitment.params.commitmentFormat)
         }
-
-        val htlcTxs: Map[OutPoint, Option[HtlcTx]] = claimHtlcOutputs(commitmentKeys, commitment)
-
-        val lcp = LocalCommitPublished(
+        val htlcTxs = claimHtlcOutputs(commitmentKeys, commitment)
+        val spendAnchors = htlcTxs.nonEmpty || onChainFeeConf.spendAnchorWithoutHtlcs
+        val anchorTx_opt = if (spendAnchors) {
+          claimAnchor(fundingKey, commitmentKeys, commitTx, commitment.params.commitmentFormat)
+        } else {
+          None
+        }
+        LocalCommitPublished(
           commitTx = commitTx,
-          claimMainDelayedOutputTx = mainDelayedTx,
+          claimMainDelayedOutputTx = mainDelayedTx_opt,
           htlcTxs = htlcTxs,
           claimHtlcDelayedTxs = Nil, // we will claim these once the htlc txs are confirmed
-          claimAnchorTxs = Nil,
+          claimAnchorTxs = anchorTx_opt.toList,
           irrevocablySpent = Map.empty
         )
-        val spendAnchors = htlcTxs.nonEmpty || onChainFeeConf.spendAnchorWithoutHtlcs
-        if (spendAnchors) {
-          claimAnchors(fundingKey, commitmentKeys, lcp, commitment.params.commitmentFormat)
-        } 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)
+      def claimAnchor(fundingKey: PrivateKey, commitKeys: LocalCommitmentKeys, commitTx: Transaction, commitmentFormat: CommitmentFormat)(implicit log: LoggingAdapter): Option[ClaimAnchorOutputTx] = {
+        withTxGenerationLog("local-anchor") {
+          ClaimAnchorOutputTx.createUnsignedTx(fundingKey, commitKeys.publicKeys, commitTx, commitmentFormat)
         }
-        lcp.copy(claimAnchorTxs = claimAnchorTx.toList)
       }
 
       /**
@@ -988,56 +978,43 @@ object Helpers {
 
     object RemoteClose {
 
-      /**
-       * Claim all the HTLCs that we've received from their current commit tx, if the channel used option_static_remotekey
-       * we don't need to claim our main output because it directly pays to one of our wallet's p2wpkh addresses.
-       *
-       * @param commitment   our commitment data, which includes payment preimages
-       * @param remoteCommit the remote commitment data to use to claim outputs (it can be their current or next commitment)
-       * @param commitTx     the remote commitment transaction that has just been published
-       * @return a list of transactions (one per output of the commit tx that we can claim)
-       */
+      /** Claim all the outputs that belong to us in the remote commitment transaction (which can be either their current or next commitment). */
       def claimCommitTxOutputs(channelKeys: ChannelKeys, commitment: FullCommitment, remoteCommit: RemoteCommit, commitTx: Transaction, feerates: FeeratesPerKw, onChainFeeConf: OnChainFeeConf, finalScriptPubKey: ByteVector)(implicit log: LoggingAdapter): RemoteCommitPublished = {
         require(remoteCommit.txid == commitTx.txid, "txid mismatch, provided tx is not the current remote commit tx")
         val fundingKey = channelKeys.fundingKey(commitment.fundingTxIndex)
         val commitKeys = commitment.remoteKeys(channelKeys, remoteCommit.remotePerCommitmentPoint)
-        val htlcTxs: Map[OutPoint, Option[ClaimHtlcTx]] = claimHtlcOutputs(channelKeys, commitKeys, commitment, remoteCommit, feerates, finalScriptPubKey)
-        val rcp = RemoteCommitPublished(
+        val mainTx_opt = claimMainOutput(commitment.params, commitKeys, commitTx, feerates, onChainFeeConf, finalScriptPubKey)
+        val htlcTxs = claimHtlcOutputs(channelKeys, commitKeys, commitment, remoteCommit, feerates, finalScriptPubKey)
+        val spendAnchors = htlcTxs.nonEmpty || onChainFeeConf.spendAnchorWithoutHtlcs
+        val anchorTx_opt = if (spendAnchors) {
+          claimAnchor(fundingKey, commitKeys, commitTx, commitment.params.commitmentFormat)
+        } else {
+          None
+        }
+        RemoteCommitPublished(
           commitTx = commitTx,
-          claimMainOutputTx = claimMainOutput(commitment.params, commitKeys, commitTx, feerates, onChainFeeConf, finalScriptPubKey),
+          claimMainOutputTx = mainTx_opt,
           claimHtlcTxs = htlcTxs,
-          claimAnchorTxs = Nil,
+          claimAnchorTxs = anchorTx_opt.toList,
           irrevocablySpent = Map.empty
         )
-        val spendAnchors = htlcTxs.nonEmpty || onChainFeeConf.spendAnchorWithoutHtlcs
-        if (spendAnchors) {
-          claimAnchors(fundingKey, commitKeys, rcp, commitment.params.commitmentFormat)
-        } 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)
+      def claimAnchor(fundingKey: PrivateKey, commitKeys: RemoteCommitmentKeys, commitTx: Transaction, commitmentFormat: CommitmentFormat)(implicit log: LoggingAdapter): Option[ClaimAnchorOutputTx] = {
+        withTxGenerationLog("remote-anchor") {
+          ClaimAnchorOutputTx.createUnsignedTx(fundingKey, commitKeys.publicKeys, commitTx, commitmentFormat)
         }
-        rcp.copy(claimAnchorTxs = claimAnchorTx.toList)
       }
 
-      /**
-       * Claim our main output only
-       *
-       * @param commitTx the remote commitment transaction that has just been published
-       * @return an optional [[ClaimRemoteCommitMainOutputTx]] transaction claiming our main output
-       */
+      /** Claim our main output from the remote commitment transaction, if available. */
       def claimMainOutput(params: ChannelParams, commitKeys: RemoteCommitmentKeys, commitTx: Transaction, feerates: FeeratesPerKw, onChainFeeConf: OnChainFeeConf, finalScriptPubKey: ByteVector)(implicit log: LoggingAdapter): Option[ClaimRemoteCommitMainOutputTx] = {
-        val feeratePerKwMain = onChainFeeConf.getClosingFeerate(feerates)
+        val feerate = onChainFeeConf.getClosingFeerate(feerates)
         params.commitmentFormat match {
           case DefaultCommitmentFormat => withTxGenerationLog("remote-main") {
-            ClaimP2WPKHOutputTx.createSignedTx(commitKeys, commitTx, params.localParams.dustLimit, finalScriptPubKey, feeratePerKwMain, params.commitmentFormat)
+            ClaimP2WPKHOutputTx.createSignedTx(commitKeys, commitTx, params.localParams.dustLimit, finalScriptPubKey, feerate, params.commitmentFormat)
           }
           case _: AnchorOutputsCommitmentFormat => withTxGenerationLog("remote-main-delayed") {
-            ClaimRemoteDelayedOutputTx.createSignedTx(commitKeys, commitTx, params.localParams.dustLimit, finalScriptPubKey, feeratePerKwMain, params.commitmentFormat)
+            ClaimRemoteDelayedOutputTx.createSignedTx(commitKeys, commitTx, params.localParams.dustLimit, finalScriptPubKey, feerate, params.commitmentFormat)
           }
         }
       }
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
index a6cfa0f9fe..2f8faa2fdd 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
@@ -2171,17 +2171,17 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
           val localAnchor_opt = for {
             lcp <- d.localCommitPublished
             commitKeys = commitment.localKeys(channelKeys)
-            anchorTx <- Closing.LocalClose.claimAnchors(fundingKey, commitKeys, lcp, commitmentFormat).claimAnchorTx_opt
+            anchorTx <- Closing.LocalClose.claimAnchor(fundingKey, commitKeys, lcp.commitTx, commitmentFormat)
           } 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
+            anchorTx <- Closing.RemoteClose.claimAnchor(fundingKey, commitKeys, rcp.commitTx, commitmentFormat)
           } 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
+            anchorTx <- Closing.RemoteClose.claimAnchor(fundingKey, commitKeys, nrcp.commitTx, commitmentFormat)
           } 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 (remoteAnchor_opt.nonEmpty) {

From e825da411b54934c5947173018a8f082e895289a Mon Sep 17 00:00:00 2001
From: t-bast <bastien@acinq.fr>
Date: Tue, 20 May 2025 17:21:26 +0200
Subject: [PATCH 3/4] Refactor upstream trimmed or timed out HTLCs

We stop relying on `LocalCommitPublished` and `RemoteCommitPublished` to
identify when HTLCs should be failed upstream because they are either
trimmed or their timeout transaction is confirmed. We instead rely on
commitment data for local commitments, and re-create the commit outputs
on-the-fly for remote commitments.
---
 .../fr/acinq/eclair/channel/Helpers.scala     | 82 ++++++++++---------
 .../fr/acinq/eclair/channel/fsm/Channel.scala |  4 +-
 .../relay/PostRestartHtlcCleaner.scala        |  9 +-
 .../fr/acinq/eclair/channel/HelpersSpec.scala | 28 ++++---
 4 files changed, 66 insertions(+), 57 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
index 756c017671..06feabade0 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -906,7 +906,8 @@ object Helpers {
       }
 
       /**
-       * Claim the outputs of a local commit tx corresponding to HTLCs.
+       * Claim the outputs of a local commit tx corresponding to HTLCs. If we don't have the preimage for a received
+       * * HTLC, we still include an entry in the map because we may receive that preimage later.
        */
       def claimHtlcOutputs(commitKeys: LocalCommitmentKeys, commitment: FullCommitment)(implicit log: LoggingAdapter): Map[OutPoint, Option[HtlcTx]] = {
         // We collect all the preimages we wanted to reveal to our peer.
@@ -919,14 +920,14 @@ object Helpers {
         // We collect incoming HTLCs that we haven't relayed: they may have been signed by our peer, but we haven't
         // received their revocation yet.
         val nonRelayedIncomingHtlcs: Set[Long] = commitment.changes.remoteChanges.all.collect { case add: UpdateAddHtlc => add.id }.toSet
-
         commitment.localCommit.htlcTxsAndRemoteSigs.collect {
-          case HtlcTxAndRemoteSig(txInfo@HtlcSuccessTx(_, _, paymentHash, _, _), remoteSig) =>
-            if (hash2Preimage.contains(paymentHash)) {
+          case HtlcTxAndRemoteSig(txInfo: HtlcSuccessTx, remoteSig) =>
+            if (hash2Preimage.contains(txInfo.paymentHash)) {
               // We immediately spend incoming htlcs for which we have the preimage.
+              val preimage = hash2Preimage(txInfo.paymentHash)
               Some(txInfo.input.outPoint -> withTxGenerationLog("htlc-success") {
                 val localSig = txInfo.sign(commitKeys, commitment.params.commitmentFormat, Map.empty)
-                Right(txInfo.addSigs(commitKeys, localSig, remoteSig, hash2Preimage(paymentHash), commitment.params.commitmentFormat))
+                Right(txInfo.addSigs(commitKeys, localSig, remoteSig, preimage, commitment.params.commitmentFormat))
               })
             } else if (failedIncomingHtlcs.contains(txInfo.htlcId)) {
               // We can ignore incoming htlcs that we started failing: our peer will claim them after the timeout.
@@ -1019,16 +1020,22 @@ object Helpers {
         }
       }
 
+      /** Create outputs of the remote commitment transaction, allowing us for example to identify HTLC outputs. */
+      def makeRemoteCommitTxOutputs(channelKeys: ChannelKeys, commitKeys: RemoteCommitmentKeys, commitment: FullCommitment, remoteCommit: RemoteCommit): Seq[CommitmentOutput] = {
+        val fundingKey = channelKeys.fundingKey(commitment.fundingTxIndex)
+        makeCommitTxOutputs(commitment.remoteFundingPubKey, fundingKey.publicKey, commitKeys.publicKeys, !commitment.localParams.paysCommitTxFees, commitment.remoteParams.dustLimit, commitment.localParams.toSelfDelay, remoteCommit.spec, commitment.params.commitmentFormat)
+      }
+
       /**
-       * Claim our htlc outputs only from the remote commitment.
+       * Claim the outputs of a remote commit tx corresponding to HTLCs. If we don't have the preimage for a received
+       * * HTLC, we still include an entry in the map because we may receive that preimage later.
        */
       def claimHtlcOutputs(channelKeys: ChannelKeys, commitKeys: RemoteCommitmentKeys, commitment: FullCommitment, remoteCommit: RemoteCommit, feerates: FeeratesPerKw, finalScriptPubKey: ByteVector)(implicit log: LoggingAdapter): Map[OutPoint, Option[ClaimHtlcTx]] = {
-        val fundingKey = channelKeys.fundingKey(commitment.fundingTxIndex)
-        val outputs = makeCommitTxOutputs(commitment.remoteFundingPubKey, fundingKey.publicKey, commitKeys.publicKeys, !commitment.localParams.paysCommitTxFees, commitment.remoteParams.dustLimit, commitment.localParams.toSelfDelay, remoteCommit.spec, commitment.params.commitmentFormat)
+        val outputs = makeRemoteCommitTxOutputs(channelKeys, commitKeys, commitment, remoteCommit)
         val remoteCommitTx = makeCommitTx(commitment.commitInput, remoteCommit.index, commitment.params.remoteParams.paymentBasepoint, commitKeys.ourPaymentBasePoint, !commitment.params.localParams.isChannelOpener, outputs)
         require(remoteCommitTx.tx.txid == remoteCommit.txid, "txid mismatch, cannot recompute the current remote commit tx")
         // We need to use a rather high fee for htlc-claim because we compete with the counterparty.
-        val feeratePerKwHtlc = feerates.fast
+        val feerateHtlc = feerates.fast
 
         // We collect all the preimages we wanted to reveal to our peer.
         val hash2Preimage: Map[ByteVector32, ByteVector32] = commitment.changes.localChanges.all.collect { case u: UpdateFulfillHtlc => u.paymentPreimage }.map(r => Crypto.sha256(r) -> r).toMap
@@ -1046,8 +1053,9 @@ object Helpers {
           case OutgoingHtlc(add: UpdateAddHtlc) =>
             if (hash2Preimage.contains(add.paymentHash)) {
               // We immediately spend incoming htlcs for which we have the preimage.
+              val preimage = hash2Preimage(add.paymentHash)
               withTxGenerationLog("claim-htlc-success") {
-                ClaimHtlcSuccessTx.createSignedTx(commitKeys, remoteCommitTx.tx, commitment.localParams.dustLimit, outputs, finalScriptPubKey, add, hash2Preimage(add.paymentHash), feeratePerKwHtlc, commitment.params.commitmentFormat)
+                ClaimHtlcSuccessTx.createSignedTx(commitKeys, remoteCommitTx.tx, commitment.localParams.dustLimit, outputs, finalScriptPubKey, add, preimage, feerateHtlc, commitment.params.commitmentFormat)
               }.map(claimHtlcTx => claimHtlcTx.input.outPoint -> Some(claimHtlcTx))
             } else if (failedIncomingHtlcs.contains(add.id)) {
               // We can ignore incoming htlcs that we started failing: our peer will claim them after the timeout.
@@ -1067,7 +1075,7 @@ object Helpers {
             // claim the output, we will learn the preimage from their transaction, otherwise we will get our funds
             // back after the timeout.
             withTxGenerationLog("claim-htlc-timeout") {
-              ClaimHtlcTimeoutTx.createSignedTx(commitKeys, remoteCommitTx.tx, commitment.localParams.dustLimit, outputs, finalScriptPubKey, add, feeratePerKwHtlc, commitment.params.commitmentFormat)
+              ClaimHtlcTimeoutTx.createSignedTx(commitKeys, remoteCommitTx.tx, commitment.localParams.dustLimit, outputs, finalScriptPubKey, add, feerateHtlc, commitment.params.commitmentFormat)
             }.map(claimHtlcTx => claimHtlcTx.input.outPoint -> Some(claimHtlcTx))
         }.flatten.toMap
       }
@@ -1120,7 +1128,7 @@ object Helpers {
         val feeratePenalty = feerates.fast
 
         // First we will claim our main output right away.
-        val mainTx = commitmentFormat match {
+        val mainTx_opt = commitmentFormat match {
           case DefaultCommitmentFormat => withTxGenerationLog("remote-main") {
             ClaimP2WPKHOutputTx.createSignedTx(commitKeys, commitTx, localParams.dustLimit, finalScriptPubKey, feerateMain, commitmentFormat)
           }
@@ -1130,7 +1138,7 @@ object Helpers {
         }
 
         // Then we punish them by stealing their main output.
-        val mainPenaltyTx = withTxGenerationLog("main-penalty") {
+        val mainPenaltyTx_opt = withTxGenerationLog("main-penalty") {
           MainPenaltyTx.createSignedTx(commitKeys, revocationKey, commitTx, localParams.dustLimit, finalScriptPubKey, localParams.toSelfDelay, feeratePenalty, commitmentFormat)
         }
 
@@ -1143,8 +1151,8 @@ object Helpers {
 
         RevokedCommitPublished(
           commitTx = commitTx,
-          claimMainOutputTx = mainTx,
-          mainPenaltyTx = mainPenaltyTx,
+          claimMainOutputTx = mainTx_opt,
+          mainPenaltyTx = mainPenaltyTx_opt,
           htlcPenaltyTxs = htlcPenaltyTxs.toList,
           claimHtlcDelayedPenaltyTxs = Nil, // we will generate and spend those if they publish their HtlcSuccessTx or HtlcTimeoutTx
           irrevocablySpent = Map.empty
@@ -1238,26 +1246,25 @@ object Helpers {
      * more htlcs have timed out and need to be failed in an upstream channel. Trimmed htlcs can be failed as soon as
      * the commitment tx has been confirmed.
      *
-     * @param tx a tx that has reached mindepth
      * @return a set of htlcs that need to be failed upstream
      */
-    def trimmedOrTimedOutHtlcs(commitmentFormat: CommitmentFormat, localCommit: LocalCommit, localCommitPublished: LocalCommitPublished, localDustLimit: Satoshi, tx: Transaction)(implicit log: LoggingAdapter): Set[UpdateAddHtlc] = {
+    def trimmedOrTimedOutHtlcs(commitmentFormat: CommitmentFormat, localCommit: LocalCommit, localDustLimit: Satoshi, confirmedTx: Transaction)(implicit log: LoggingAdapter): Set[UpdateAddHtlc] = {
       val untrimmedHtlcs = Transactions.trimOfferedHtlcs(localDustLimit, localCommit.spec, commitmentFormat).map(_.add)
-      if (tx.txid == localCommit.commitTxAndRemoteSig.commitTx.tx.txid) {
+      if (confirmedTx.txid == localCommit.commitTxAndRemoteSig.commitTx.tx.txid) {
         // The commitment tx is confirmed: we can immediately fail all dust htlcs (they don't have an output in the tx).
         localCommit.spec.htlcs.collect(outgoing) -- untrimmedHtlcs
       } else {
         // Maybe this is a timeout tx: in that case we can resolve and fail the corresponding htlc.
-        tx.txIn.flatMap(txIn => localCommitPublished.htlcTxs.get(txIn.outPoint) match {
+        confirmedTx.txIn.flatMap(txIn => localCommit.htlcTxsAndRemoteSigs.map(_.htlcTx).find(_.input.outPoint == txIn.outPoint) match {
           // This may also be our peer claiming the HTLC by revealing the preimage: in that case we have already
           // extracted the preimage with [[extractPreimages]] and relayed it upstream.
-          case Some(Some(htlcTimeoutTx: HtlcTimeoutTx)) if Scripts.extractPreimagesFromClaimHtlcSuccess(tx).isEmpty =>
+          case Some(htlcTimeoutTx: HtlcTimeoutTx) if Scripts.extractPreimagesFromClaimHtlcSuccess(confirmedTx).isEmpty =>
             untrimmedHtlcs.find(_.id == htlcTimeoutTx.htlcId) match {
               case Some(htlc) =>
-                log.info("htlc-timeout tx for htlc #{} paymentHash={} expiry={} has been confirmed (tx={})", htlcTimeoutTx.htlcId, htlc.paymentHash, tx.lockTime, tx)
+                log.info("htlc-timeout tx for htlc #{} paymentHash={} expiry={} has been confirmed (tx={})", htlcTimeoutTx.htlcId, htlc.paymentHash, confirmedTx.lockTime, confirmedTx)
                 Some(htlc)
               case None =>
-                log.error("could not find htlc #{} for htlc-timeout tx={}", htlcTimeoutTx.htlcId, tx)
+                log.error("could not find htlc #{} for htlc-timeout tx={}", htlcTimeoutTx.htlcId, confirmedTx)
                 None
             }
           case _ => None
@@ -1270,30 +1277,29 @@ object Helpers {
      * more htlcs have timed out and need to be failed in an upstream channel. Trimmed htlcs can be failed as soon as
      * the commitment tx has been confirmed.
      *
-     * @param tx a tx that has reached mindepth
      * @return a set of htlcs that need to be failed upstream
      */
-    def trimmedOrTimedOutHtlcs(commitmentFormat: CommitmentFormat, remoteCommit: RemoteCommit, remoteCommitPublished: RemoteCommitPublished, remoteDustLimit: Satoshi, tx: Transaction)(implicit log: LoggingAdapter): Set[UpdateAddHtlc] = {
-      val untrimmedHtlcs = Transactions.trimReceivedHtlcs(remoteDustLimit, remoteCommit.spec, commitmentFormat).map(_.add)
-      if (tx.txid == remoteCommit.txid) {
+    def trimmedOrTimedOutHtlcs(channelKeys: ChannelKeys, commitment: FullCommitment, remoteCommit: RemoteCommit, confirmedTx: Transaction)(implicit log: LoggingAdapter): Set[UpdateAddHtlc] = {
+      if (confirmedTx.txid == remoteCommit.txid) {
         // The commitment tx is confirmed: we can immediately fail all dust htlcs (they don't have an output in the tx).
+        val untrimmedHtlcs = Transactions.trimReceivedHtlcs(commitment.remoteParams.dustLimit, remoteCommit.spec, commitment.params.commitmentFormat).map(_.add)
         remoteCommit.spec.htlcs.collect(incoming) -- untrimmedHtlcs
-      } else {
-        // Maybe this is a timeout tx: in that case we can resolve and fail the corresponding htlc.
-        tx.txIn.flatMap(txIn => remoteCommitPublished.claimHtlcTxs.get(txIn.outPoint) match {
+      } else if (confirmedTx.txIn.exists(_.outPoint.txid == remoteCommit.txid)) {
+        // The transaction spends the commitment tx: maybe it is a timeout tx, in which case we can resolve and fail the
+        // corresponding htlc.
+        val commitKeys = commitment.remoteKeys(channelKeys, remoteCommit.remotePerCommitmentPoint)
+        val outputs = RemoteClose.makeRemoteCommitTxOutputs(channelKeys, commitKeys, commitment, remoteCommit)
+        confirmedTx.txIn.filter(_.outPoint.txid == remoteCommit.txid).flatMap(txIn => outputs(txIn.outPoint.index.toInt) match {
           // This may also be our peer claiming the HTLC by revealing the preimage: in that case we have already
           // extracted the preimage with [[extractPreimages]] and relayed it upstream.
-          case Some(Some(claimHtlcTimeoutTx: ClaimHtlcTimeoutTx)) if Scripts.extractPreimagesFromHtlcSuccess(tx).isEmpty =>
-            untrimmedHtlcs.find(_.id == claimHtlcTimeoutTx.htlcId) match {
-              case Some(htlc) =>
-                log.info("claim-htlc-timeout tx for htlc #{} paymentHash={} expiry={} has been confirmed (tx={})", claimHtlcTimeoutTx.htlcId, htlc.paymentHash, tx.lockTime, tx)
-                Some(htlc)
-              case None =>
-                log.error("could not find htlc #{} for claim-htlc-timeout tx={}", claimHtlcTimeoutTx.htlcId, tx)
-                None
-            }
+          // Note: we're looking at the remote commitment, so it's an incoming HTLC for them (outgoing for us).
+          case CommitmentOutput.InHtlc(htlc, _, _) if Scripts.extractPreimagesFromHtlcSuccess(confirmedTx).isEmpty =>
+            log.info("claim-htlc-timeout tx for htlc #{} paymentHash={} expiry={} has been confirmed (tx={})", htlc.add.id, htlc.add.paymentHash, htlc.add.cltvExpiry, confirmedTx)
+            Some(htlc.add)
           case _ => None
         }).toSet
+      } else {
+        Set.empty
       }
     }
 
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
index 2f8faa2fdd..ad5416954f 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
@@ -2109,8 +2109,8 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
       }
       // we may need to fail some htlcs in case a commitment tx was published and they have reached the timeout threshold
       val timedOutHtlcs = Closing.isClosingTypeAlreadyKnown(d1) match {
-        case Some(c: Closing.LocalClose) => Closing.trimmedOrTimedOutHtlcs(d.commitments.params.commitmentFormat, c.localCommit, c.localCommitPublished, d.commitments.params.localParams.dustLimit, tx)
-        case Some(c: Closing.RemoteClose) => Closing.trimmedOrTimedOutHtlcs(d.commitments.params.commitmentFormat, c.remoteCommit, c.remoteCommitPublished, d.commitments.params.remoteParams.dustLimit, tx)
+        case Some(c: Closing.LocalClose) => Closing.trimmedOrTimedOutHtlcs(d.commitments.params.commitmentFormat, c.localCommit, d.commitments.params.localParams.dustLimit, tx)
+        case Some(c: Closing.RemoteClose) => Closing.trimmedOrTimedOutHtlcs(channelKeys, d.commitments.latest, c.remoteCommit, tx)
         case Some(_: Closing.RevokedClose) => Set.empty[UpdateAddHtlc] // revoked commitments are handled using [[overriddenOutgoingHtlcs]] below
         case Some(_: Closing.RecoveryClose) => Set.empty[UpdateAddHtlc] // we lose htlc outputs in dataloss protection scenarios (future remote commit)
         case Some(_: Closing.MutualClose) => Set.empty[UpdateAddHtlc]
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/PostRestartHtlcCleaner.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/PostRestartHtlcCleaner.scala
index 41978cf4d9..52db0a055a 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/PostRestartHtlcCleaner.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/PostRestartHtlcCleaner.scala
@@ -72,7 +72,7 @@ class PostRestartHtlcCleaner(nodeParams: NodeParams, register: ActorRef, initial
         val nonStandardIncomingHtlcs: Seq[IncomingHtlc] = nodeParams.pluginParams.collect { case p: CustomCommitmentsPlugin => p.getIncomingHtlcs(nodeParams, log) }.flatten
         val htlcsIn: Seq[IncomingHtlc] = getIncomingHtlcs(channels, nodeParams.db.payments, nodeParams.privateKey, nodeParams.features) ++ nonStandardIncomingHtlcs
         val nonStandardRelayedOutHtlcs: Map[Origin.Cold, Set[(ByteVector32, Long)]] = nodeParams.pluginParams.collect { case p: CustomCommitmentsPlugin => p.getHtlcsRelayedOut(htlcsIn, nodeParams, log) }.flatten.toMap
-        val relayedOut: Map[Origin.Cold, Set[(ByteVector32, Long)]] = getHtlcsRelayedOut(channels, htlcsIn) ++ nonStandardRelayedOutHtlcs
+        val relayedOut: Map[Origin.Cold, Set[(ByteVector32, Long)]] = getHtlcsRelayedOut(nodeParams, channels, htlcsIn) ++ nonStandardRelayedOutHtlcs
 
         val settledHtlcs: Set[(ByteVector32, Long)] = nodeParams.db.pendingCommands.listSettlementCommands().map { case (channelId, cmd) => (channelId, cmd.id) }.toSet
         val notRelayed = htlcsIn.filterNot(htlcIn => {
@@ -403,7 +403,7 @@ object PostRestartHtlcCleaner {
       .toMap
 
   /** @return pending outgoing HTLCs, grouped by their upstream origin. */
-  private def getHtlcsRelayedOut(channels: Seq[PersistentChannelData], htlcsIn: Seq[IncomingHtlc])(implicit log: LoggingAdapter): Map[Origin.Cold, Set[(ByteVector32, Long)]] = {
+  private def getHtlcsRelayedOut(nodeParams: NodeParams, channels: Seq[PersistentChannelData], htlcsIn: Seq[IncomingHtlc])(implicit log: LoggingAdapter): Map[Origin.Cold, Set[(ByteVector32, Long)]] = {
     val htlcsOut = channels
       .collect { case c: ChannelDataWithCommitments => c }
       .flatMap { c =>
@@ -428,9 +428,10 @@ object PostRestartHtlcCleaner {
               case None => Set.empty
             }
             val params = d.commitments.params
+            val channelKeys = nodeParams.channelKeyManager.channelKeys(params.channelConfig, params.localParams.fundingKeyPath)
             val timedOutHtlcs: Set[Long] = (closingType_opt match {
-              case Some(c: Closing.LocalClose) => confirmedTxs.flatMap(tx => Closing.trimmedOrTimedOutHtlcs(params.commitmentFormat, c.localCommit, c.localCommitPublished, params.localParams.dustLimit, tx))
-              case Some(c: Closing.RemoteClose) => confirmedTxs.flatMap(tx => Closing.trimmedOrTimedOutHtlcs(params.commitmentFormat, c.remoteCommit, c.remoteCommitPublished, params.remoteParams.dustLimit, tx))
+              case Some(c: Closing.LocalClose) => confirmedTxs.flatMap(tx => Closing.trimmedOrTimedOutHtlcs(params.commitmentFormat, c.localCommit, params.localParams.dustLimit, tx))
+              case Some(c: Closing.RemoteClose) => confirmedTxs.flatMap(tx => Closing.trimmedOrTimedOutHtlcs(channelKeys, d.commitments.latest, c.remoteCommit, tx))
               case Some(_: Closing.RevokedClose) => Set.empty // revoked commitments are handled using [[overriddenOutgoingHtlcs]] above
               case Some(_: Closing.RecoveryClose) => Set.empty // we lose htlc outputs in dataloss protection scenarios (future remote commit)
               case Some(_: Closing.MutualClose) => Set.empty
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala
index 074b1e1825..04353d75e1 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/HelpersSpec.scala
@@ -107,9 +107,11 @@ class HelpersSpec extends TestKitBaseClass with AnyFunSuiteLike with ChannelStat
     import f._
 
     val dustLimit = alice.underlyingActor.nodeParams.channelConf.dustLimit
-    val commitmentFormat = alice.stateData.asInstanceOf[DATA_CLOSING].commitments.params.commitmentFormat
     val localCommit = alice.stateData.asInstanceOf[DATA_CLOSING].commitments.latest.localCommit
-    val remoteCommit = bob.stateData.asInstanceOf[DATA_CLOSING].commitments.latest.remoteCommit
+    val remoteKeys = bob.underlyingActor.channelKeys
+    val remoteCommitment = bob.stateData.asInstanceOf[DATA_CLOSING].commitments.latest
+    val remoteCommit = remoteCommitment.remoteCommit
+    val commitmentFormat = remoteCommitment.params.commitmentFormat
 
     val htlcTimeoutTxs = getHtlcTimeoutTxs(aliceCommitPublished)
     val htlcSuccessTxs = getHtlcSuccessTxs(aliceCommitPublished)
@@ -120,34 +122,34 @@ class HelpersSpec extends TestKitBaseClass with AnyFunSuiteLike with ChannelStat
     val claimHtlcSuccessTxsModifiedFees = claimHtlcSuccessTxs.map(tx => tx.modify(_.tx.txOut).setTo(Seq(tx.tx.txOut.head.copy(amount = 5000 sat))))
 
     val aliceTimedOutHtlcs = htlcTimeoutTxs.map(htlcTimeout => {
-      val timedOutHtlcs = Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, aliceCommitPublished, dustLimit, htlcTimeout.tx)
+      val timedOutHtlcs = Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, dustLimit, htlcTimeout.tx)
       assert(timedOutHtlcs.size == 1)
       timedOutHtlcs.head
     })
     assert(aliceTimedOutHtlcs.toSet == aliceHtlcs)
 
     val bobTimedOutHtlcs = claimHtlcTimeoutTxs.map(claimHtlcTimeout => {
-      val timedOutHtlcs = Closing.trimmedOrTimedOutHtlcs(commitmentFormat, remoteCommit, bobCommitPublished, dustLimit, claimHtlcTimeout.tx)
+      val timedOutHtlcs = Closing.trimmedOrTimedOutHtlcs(remoteKeys, remoteCommitment, remoteCommit, claimHtlcTimeout.tx)
       assert(timedOutHtlcs.size == 1)
       timedOutHtlcs.head
     })
     assert(bobTimedOutHtlcs.toSet == bobHtlcs)
 
     val bobTimedOutHtlcs2 = claimHtlcTimeoutTxsModifiedFees.map(claimHtlcTimeout => {
-      val timedOutHtlcs = Closing.trimmedOrTimedOutHtlcs(commitmentFormat, remoteCommit, bobCommitPublished, dustLimit, claimHtlcTimeout.tx)
+      val timedOutHtlcs = Closing.trimmedOrTimedOutHtlcs(remoteKeys, remoteCommitment, remoteCommit, claimHtlcTimeout.tx)
       assert(timedOutHtlcs.size == 1)
       timedOutHtlcs.head
     })
     assert(bobTimedOutHtlcs2.toSet == bobHtlcs)
 
-    htlcSuccessTxs.foreach(htlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, aliceCommitPublished, dustLimit, htlcSuccess.tx).isEmpty))
-    htlcSuccessTxs.foreach(htlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, remoteCommit, bobCommitPublished, dustLimit, htlcSuccess.tx).isEmpty))
-    claimHtlcSuccessTxs.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, aliceCommitPublished, dustLimit, claimHtlcSuccess.tx).isEmpty))
-    claimHtlcSuccessTxsModifiedFees.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, aliceCommitPublished, dustLimit, claimHtlcSuccess.tx).isEmpty))
-    claimHtlcSuccessTxs.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, remoteCommit, bobCommitPublished, dustLimit, claimHtlcSuccess.tx).isEmpty))
-    claimHtlcSuccessTxsModifiedFees.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, remoteCommit, bobCommitPublished, dustLimit, claimHtlcSuccess.tx).isEmpty))
-    htlcTimeoutTxs.foreach(htlcTimeout => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, remoteCommit, bobCommitPublished, dustLimit, htlcTimeout.tx).isEmpty))
-    claimHtlcTimeoutTxs.foreach(claimHtlcTimeout => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, aliceCommitPublished, dustLimit, claimHtlcTimeout.tx).isEmpty))
+    htlcSuccessTxs.foreach(htlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, dustLimit, htlcSuccess.tx).isEmpty))
+    htlcSuccessTxs.foreach(htlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(remoteKeys, remoteCommitment, remoteCommit, htlcSuccess.tx).isEmpty))
+    claimHtlcSuccessTxs.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, dustLimit, claimHtlcSuccess.tx).isEmpty))
+    claimHtlcSuccessTxsModifiedFees.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, dustLimit, claimHtlcSuccess.tx).isEmpty))
+    claimHtlcSuccessTxs.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(remoteKeys, remoteCommitment, remoteCommit, claimHtlcSuccess.tx).isEmpty))
+    claimHtlcSuccessTxsModifiedFees.foreach(claimHtlcSuccess => assert(Closing.trimmedOrTimedOutHtlcs(remoteKeys, remoteCommitment, remoteCommit, claimHtlcSuccess.tx).isEmpty))
+    htlcTimeoutTxs.foreach(htlcTimeout => assert(Closing.trimmedOrTimedOutHtlcs(remoteKeys, remoteCommitment, remoteCommit, htlcTimeout.tx).isEmpty))
+    claimHtlcTimeoutTxs.foreach(claimHtlcTimeout => assert(Closing.trimmedOrTimedOutHtlcs(commitmentFormat, localCommit, dustLimit, claimHtlcTimeout.tx).isEmpty))
   }
 
   test("find timed out htlcs") {

From b267ad026fc8d79aba038a3bc1ed289c33714f37 Mon Sep 17 00:00:00 2001
From: t-bast <bastien@acinq.fr>
Date: Tue, 20 May 2025 19:00:18 +0200
Subject: [PATCH 4/4] Publish 3rd-stage penalty txs on tx confirmation

In case of a revoked close, we previously published 3rd-stage penalty
txs as soon as we saw our peer's HTLC transactions in our mempool.
This isn't necessary and may actually help them get confirmed since
it is essentially a CPFP. We now instead wait for the revoked HTLC
transaction to be confirmed before publishing a penalty transaction
claiming its outputs.
---
 .../fr/acinq/eclair/channel/Helpers.scala     |   4 +-
 .../fr/acinq/eclair/channel/fsm/Channel.scala |  34 +++---
 .../channel/states/h/ClosingStateSpec.scala   | 115 ++++++++----------
 .../integration/ChannelIntegrationSpec.scala  |   2 +
 4 files changed, 74 insertions(+), 81 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
index 06feabade0..4bd2fc1765 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@@ -1185,8 +1185,8 @@ object Helpers {
               val commitmentKeys = RemoteCommitmentKeys(params, channelKeys, remotePerCommitmentSecret.publicKey)
               val revocationKey = channelKeys.revocationKey(remotePerCommitmentSecret)
               // We need to use a high fee when spending HTLC txs because after a delay they can also be spent by the counterparty.
-              val feeratePerKwPenalty = feerates.fastest
-              val penaltyTxs = ClaimHtlcDelayedOutputPenaltyTx.createSignedTxs(commitmentKeys, revocationKey, htlcTx, params.localParams.dustLimit, params.localParams.toSelfDelay, finalScriptPubKey, feeratePerKwPenalty, params.commitmentFormat).flatMap(claimHtlcDelayedOutputPenaltyTx => {
+              val feeratePenalty = feerates.fastest
+              val penaltyTxs = ClaimHtlcDelayedOutputPenaltyTx.createSignedTxs(commitmentKeys, revocationKey, htlcTx, params.localParams.dustLimit, params.localParams.toSelfDelay, finalScriptPubKey, feeratePenalty, params.commitmentFormat).flatMap(claimHtlcDelayedOutputPenaltyTx => {
                 withTxGenerationLog("htlc-delayed-penalty") {
                   claimHtlcDelayedOutputPenaltyTx.map(signedTx => {
                     // We need to make sure that the tx is indeed valid.
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
index ad5416954f..5ac35fbcf3 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
@@ -2030,13 +2030,12 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
       }
 
     case Event(WatchOutputSpentTriggered(_, tx), d: DATA_CLOSING) =>
-      // one of the outputs of the local/remote/revoked commit was spent
-      // we just put a watch to be notified when it is confirmed
+      // One of the outputs of the local/remote/revoked commit transaction or of an HTLC transaction was spent.
+      // We put a watch to be notified when the transaction confirms: it may double-spend one of our transactions.
       blockchain ! WatchTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepth)
-      // when a remote or local commitment tx containing outgoing htlcs is published on the network,
-      // we watch it in order to extract payment preimage if funds are pulled by the counterparty
-      // we can then use these preimages to fulfill origin htlcs
-      log.debug(s"processing bitcoin output spent by txid={} tx={}", tx.txid, tx)
+      // If this is an HTLC transaction, it may reveal preimages that we haven't received yet.
+      // If we successfully extract those preimages, we can forward them upstream.
+      log.debug("processing bitcoin output spent by txid={} tx={}", tx.txid, tx)
       val extracted = Closing.extractPreimages(d.commitments.latest, tx)
       extracted.foreach { case (htlc, preimage) =>
         d.commitments.originChannels.get(htlc.id) match {
@@ -2044,20 +2043,12 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
             log.info("fulfilling htlc #{} paymentHash={} origin={}", htlc.id, htlc.paymentHash, origin)
             relayer ! RES_ADD_SETTLED(origin, htlc, HtlcResult.OnChainFulfill(preimage))
           case None =>
-            // if we don't have the origin, it means that we already have forwarded the fulfill so that's not a big deal.
-            // this can happen if they send a signature containing the fulfill, then fail the channel before we have time to sign it
+            // If we don't have the origin, it means that we already have forwarded the fulfill so that's not a big deal.
+            // This can happen if they send a signature containing the fulfill, then fail the channel before we have time to sign it.
             log.warning("cannot fulfill htlc #{} paymentHash={} (origin not found)", htlc.id, htlc.paymentHash)
         }
       }
-      val revokedCommitPublished1 = d.revokedCommitPublished.map { rev =>
-        // this transaction may be an HTLC transaction spending a revoked commitment
-        // in that case, we immediately publish an HTLC-penalty transaction spending its output(s)
-        val (rev1, penaltyTxs) = Closing.RevokedClose.claimHtlcTxOutputs(d.commitments.params, channelKeys, d.commitments.remotePerCommitmentSecrets, rev, tx, nodeParams.currentBitcoinCoreFeerates, d.finalScriptPubKey)
-        penaltyTxs.foreach(claimTx => txPublisher ! PublishFinalTx(claimTx, claimTx.fee, None))
-        penaltyTxs.foreach(claimTx => blockchain ! WatchOutputSpent(self, tx.txid, claimTx.input.outPoint.index.toInt, claimTx.amountIn, hints = Set(claimTx.tx.txid)))
-        rev1
-      }
-      stay() using d.copy(revokedCommitPublished = revokedCommitPublished1) storing()
+      stay()
 
     case Event(WatchTxConfirmedTriggered(blockHeight, _, tx), d: DATA_CLOSING) =>
       log.info("txid={} has reached mindepth, updating closing state", tx.txid)
@@ -2076,7 +2067,14 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
         remoteCommitPublished = d.remoteCommitPublished.map(Closing.updateRemoteCommitPublished(_, tx)),
         nextRemoteCommitPublished = d.nextRemoteCommitPublished.map(Closing.updateRemoteCommitPublished(_, tx)),
         futureRemoteCommitPublished = d.futureRemoteCommitPublished.map(Closing.updateRemoteCommitPublished(_, tx)),
-        revokedCommitPublished = d.revokedCommitPublished.map(Closing.updateRevokedCommitPublished(_, tx))
+        revokedCommitPublished = d.revokedCommitPublished.map(rvk => {
+          // If the tx is one of our peer's HTLC txs, they were able to claim the output before us.
+          // In that case, we immediately publish a penalty transaction spending their HTLC tx to steal their funds.
+          val (rvk1, penaltyTxs) = Closing.RevokedClose.claimHtlcTxOutputs(d.commitments.params, channelKeys, d.commitments.remotePerCommitmentSecrets, rvk, tx, nodeParams.currentBitcoinCoreFeerates, d.finalScriptPubKey)
+          penaltyTxs.foreach(claimTx => txPublisher ! PublishFinalTx(claimTx, claimTx.fee, None))
+          penaltyTxs.foreach(claimTx => blockchain ! WatchOutputSpent(self, tx.txid, claimTx.input.outPoint.index.toInt, claimTx.amountIn, hints = Set(claimTx.tx.txid)))
+          Closing.updateRevokedCommitPublished(rvk1, tx)
+        })
       )
       // if the local commitment tx just got confirmed, let's send an event telling when we will get the main output refund
       if (d1.localCommitPublished.exists(_.commitTx.txid == tx.txid)) {
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala
index 80ae2dc76e..63c1c36dcf 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala
@@ -1899,7 +1899,7 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     testInputRestoredRevokedTx(f, ChannelFeatures(Features.StaticRemoteKey, Features.AnchorOutputsZeroFeeHtlcTx))
   }
 
-  def testOutputSpentRevokedTx(f: FixtureParam, channelFeatures: ChannelFeatures): Unit = {
+  def testRevokedHtlcTxConfirmed(f: FixtureParam, channelFeatures: ChannelFeatures): Unit = {
     import f._
     val revokedCloseFixture = prepareRevokedClose(f, channelFeatures)
     assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.params.channelFeatures == channelFeatures)
@@ -1931,91 +1931,83 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     (1 to 5).foreach(_ => alice2blockchain.expectMsgType[WatchOutputSpent]) // main output penalty and 4 htlc penalties
     alice2blockchain.expectNoMessage(100 millis)
 
-    // bob manages to claim 2 htlc outputs before alice can penalize him: 1 htlc-success and 1 htlc-timeout.
-    val (fulfilledHtlc, _) = revokedCloseFixture.htlcsAlice.head
-    val (failedHtlc, _) = revokedCloseFixture.htlcsBob.last
-    val bobHtlcSuccessTx1 = bobRevokedCommit.htlcTxsAndRemoteSigs.collectFirst {
-      case HtlcTxAndRemoteSig(txInfo: HtlcSuccessTx, _) if txInfo.htlcId == fulfilledHtlc.id =>
-        assert(fulfilledHtlc.paymentHash == txInfo.paymentHash)
-        txInfo
-    }.get
-    val bobHtlcTimeoutTx = bobRevokedCommit.htlcTxsAndRemoteSigs.collectFirst {
-      case HtlcTxAndRemoteSig(txInfo: HtlcTimeoutTx, _) if txInfo.htlcId == failedHtlc.id =>
-        txInfo
-    }.get
-    val bobOutpoints = Seq(bobHtlcSuccessTx1, bobHtlcTimeoutTx).map(_.input.outPoint).toSet
-    assert(bobOutpoints.size == 2)
+    // the revoked commit and main penalty transactions confirm
+    alice ! WatchTxConfirmedTriggered(BlockHeight(100), 3, rvk.commitTx)
+    alice ! WatchTxConfirmedTriggered(BlockHeight(110), 0, rvk.mainPenaltyTx.get.tx)
+    if (!channelFeatures.paysDirectlyToWallet) {
+      alice ! WatchTxConfirmedTriggered(BlockHeight(110), 1, rvk.claimMainOutputTx.get.tx)
+    }
 
-    // alice reacts by publishing penalty txs that spend bob's htlc transactions
+    // bob publishes one of his HTLC-success transactions
+    val (fulfilledHtlc, _) = revokedCloseFixture.htlcsAlice.head
+    val bobHtlcSuccessTx1 = bobRevokedCommit.htlcTxsAndRemoteSigs.collectFirst { case HtlcTxAndRemoteSig(txInfo: HtlcSuccessTx, _) if txInfo.htlcId == fulfilledHtlc.id => txInfo }.get
+    assert(bobHtlcSuccessTx1.paymentHash == fulfilledHtlc.paymentHash)
     alice ! WatchOutputSpentTriggered(bobHtlcSuccessTx1.amountIn, bobHtlcSuccessTx1.tx)
-    awaitCond(alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.size == 1)
-    val claimHtlcSuccessPenalty1 = alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.last
-    Transaction.correctlySpends(claimHtlcSuccessPenalty1.tx, bobHtlcSuccessTx1.tx :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)
     assert(alice2blockchain.expectMsgType[WatchTxConfirmed].txId == bobHtlcSuccessTx1.tx.txid)
-    assert(alice2blockchain.expectMsgType[PublishFinalTx].tx == claimHtlcSuccessPenalty1.tx)
-    val watchSpent1 = alice2blockchain.expectMsgType[WatchOutputSpent]
-    assert(watchSpent1.txId == bobHtlcSuccessTx1.tx.txid)
-    assert(watchSpent1.outputIndex == claimHtlcSuccessPenalty1.input.outPoint.index)
-    alice2blockchain.expectNoMessage(100 millis)
 
+    // bob publishes one of his HTLC-timeout transactions
+    val (failedHtlc, _) = revokedCloseFixture.htlcsBob.last
+    val bobHtlcTimeoutTx = bobRevokedCommit.htlcTxsAndRemoteSigs.collectFirst { case HtlcTxAndRemoteSig(txInfo: HtlcTimeoutTx, _) if txInfo.htlcId == failedHtlc.id => txInfo }.get
+    assert(bobHtlcTimeoutTx.paymentHash == failedHtlc.paymentHash)
     alice ! WatchOutputSpentTriggered(bobHtlcTimeoutTx.amountIn, bobHtlcTimeoutTx.tx)
-    awaitCond(alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.size == 2)
-    val claimHtlcTimeoutPenalty = alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.last
-    Transaction.correctlySpends(claimHtlcTimeoutPenalty.tx, bobHtlcTimeoutTx.tx :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)
     assert(alice2blockchain.expectMsgType[WatchTxConfirmed].txId == bobHtlcTimeoutTx.tx.txid)
-    assert(alice2blockchain.expectMsgType[PublishFinalTx].tx == claimHtlcTimeoutPenalty.tx)
-    val watchSpent2 = alice2blockchain.expectMsgType[WatchOutputSpent]
-    assert(watchSpent2.txId == bobHtlcTimeoutTx.tx.txid)
-    assert(watchSpent2.outputIndex == claimHtlcTimeoutPenalty.input.outPoint.index)
-    alice2blockchain.expectNoMessage(100 millis)
 
     // bob RBFs his htlc-success with a different transaction
     val bobHtlcSuccessTx2 = bobHtlcSuccessTx1.tx.copy(txIn = TxIn(OutPoint(randomTxId(), 0), Nil, 0) +: bobHtlcSuccessTx1.tx.txIn)
     assert(bobHtlcSuccessTx2.txid !== bobHtlcSuccessTx1.tx.txid)
     alice ! WatchOutputSpentTriggered(bobHtlcSuccessTx1.amountIn, bobHtlcSuccessTx2)
-    awaitCond(alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.size == 3)
-    val claimHtlcSuccessPenalty2 = alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.last
-    assert(claimHtlcSuccessPenalty1.tx.txid != claimHtlcSuccessPenalty2.tx.txid)
-    Transaction.correctlySpends(claimHtlcSuccessPenalty2.tx, bobHtlcSuccessTx2 :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)
     assert(alice2blockchain.expectMsgType[WatchTxConfirmed].txId == bobHtlcSuccessTx2.txid)
-    assert(alice2blockchain.expectMsgType[PublishFinalTx].tx == claimHtlcSuccessPenalty2.tx)
-    val watchSpent3 = alice2blockchain.expectMsgType[WatchOutputSpent]
-    assert(watchSpent3.txId == bobHtlcSuccessTx2.txid)
-    assert(watchSpent3.outputIndex == claimHtlcSuccessPenalty2.input.outPoint.index)
+
+    // bob's HTLC-timeout confirms: alice reacts by publishing a penalty tx
+    alice ! WatchTxConfirmedTriggered(BlockHeight(115), 0, bobHtlcTimeoutTx.tx)
+    awaitCond(alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.size == 1)
+    val claimHtlcTimeoutPenalty = alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.head
+    Transaction.correctlySpends(claimHtlcTimeoutPenalty.tx, bobHtlcTimeoutTx.tx :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)
+    assert(alice2blockchain.expectMsgType[PublishFinalTx].tx == claimHtlcTimeoutPenalty.tx)
+    inside(alice2blockchain.expectMsgType[WatchOutputSpent]) { w =>
+      assert(w.txId == bobHtlcTimeoutTx.tx.txid)
+      assert(w.outputIndex == claimHtlcTimeoutPenalty.input.outPoint.index)
+    }
+    alice2blockchain.expectNoMessage(100 millis)
+
+    // bob's htlc-success RBF confirms: alice reacts by publishing a penalty tx
+    alice ! WatchTxConfirmedTriggered(BlockHeight(115), 1, bobHtlcSuccessTx2)
+    awaitCond(alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.size == 2)
+    val claimHtlcSuccessPenalty = alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.last
+    Transaction.correctlySpends(claimHtlcSuccessPenalty.tx, bobHtlcSuccessTx2 :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)
+    assert(alice2blockchain.expectMsgType[PublishFinalTx].tx == claimHtlcSuccessPenalty.tx)
+    inside(alice2blockchain.expectMsgType[WatchOutputSpent]) { w =>
+      assert(w.txId == bobHtlcSuccessTx2.txid)
+      assert(w.outputIndex == claimHtlcSuccessPenalty.input.outPoint.index)
+    }
     alice2blockchain.expectNoMessage(100 millis)
 
     // transactions confirm: alice can move to the closed state
-    val remainingHtlcPenaltyTxs = rvk.htlcPenaltyTxs.filterNot(htlcPenalty => bobOutpoints.contains(htlcPenalty.input.outPoint))
+    val bobHtlcOutpoints = Set(bobHtlcTimeoutTx.input.outPoint, bobHtlcSuccessTx1.input.outPoint)
+    val remainingHtlcPenaltyTxs = rvk.htlcPenaltyTxs.filterNot(htlcPenalty => bobHtlcOutpoints.contains(htlcPenalty.input.outPoint))
     assert(remainingHtlcPenaltyTxs.size == 2)
-    alice ! WatchTxConfirmedTriggered(BlockHeight(100), 3, rvk.commitTx)
-    alice ! WatchTxConfirmedTriggered(BlockHeight(110), 0, rvk.mainPenaltyTx.get.tx)
-    if (!channelFeatures.paysDirectlyToWallet) {
-      alice ! WatchTxConfirmedTriggered(BlockHeight(110), 1, rvk.claimMainOutputTx.get.tx)
-    }
     alice ! WatchTxConfirmedTriggered(BlockHeight(110), 2, remainingHtlcPenaltyTxs.head.tx)
     alice ! WatchTxConfirmedTriggered(BlockHeight(115), 2, remainingHtlcPenaltyTxs.last.tx)
-    alice ! WatchTxConfirmedTriggered(BlockHeight(115), 0, bobHtlcTimeoutTx.tx)
-    alice ! WatchTxConfirmedTriggered(BlockHeight(115), 1, bobHtlcSuccessTx2)
+    alice ! WatchTxConfirmedTriggered(BlockHeight(120), 0, claimHtlcTimeoutPenalty.tx)
     assert(alice.stateName == CLOSING)
 
-    alice ! WatchTxConfirmedTriggered(BlockHeight(120), 0, claimHtlcTimeoutPenalty.tx)
-    alice ! WatchTxConfirmedTriggered(BlockHeight(121), 0, claimHtlcSuccessPenalty2.tx)
+    alice ! WatchTxConfirmedTriggered(BlockHeight(121), 0, claimHtlcSuccessPenalty.tx)
     awaitCond(alice.stateName == CLOSED)
   }
 
-  test("recv WatchOutputSpentTriggered (one revoked tx, counterparty published htlc-success tx, option_static_remotekey)", Tag(ChannelStateTestsTags.StaticRemoteKey)) { f =>
-    testOutputSpentRevokedTx(f, ChannelFeatures(Features.StaticRemoteKey))
+  test("recv WatchTxConfirmedTriggered (revoked htlc-success tx, option_static_remotekey)", Tag(ChannelStateTestsTags.StaticRemoteKey)) { f =>
+    testRevokedHtlcTxConfirmed(f, ChannelFeatures(Features.StaticRemoteKey))
   }
 
-  test("recv WatchOutputSpentTriggered (one revoked tx, counterparty published htlc-success tx, anchor outputs)", Tag(ChannelStateTestsTags.AnchorOutputs)) { f =>
-    testOutputSpentRevokedTx(f, ChannelFeatures(Features.StaticRemoteKey, Features.AnchorOutputs))
+  test("recv WatchTxConfirmedTriggered (revoked htlc-success tx, anchor outputs)", Tag(ChannelStateTestsTags.AnchorOutputs)) { f =>
+    testRevokedHtlcTxConfirmed(f, ChannelFeatures(Features.StaticRemoteKey, Features.AnchorOutputs))
   }
 
-  test("recv WatchOutputSpentTriggered (one revoked tx, counterparty published htlc-success tx, anchor outputs zero fee htlc txs)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
-    testOutputSpentRevokedTx(f, ChannelFeatures(Features.StaticRemoteKey, Features.AnchorOutputsZeroFeeHtlcTx))
+  test("recv WatchTxConfirmedTriggered (revoked htlc-success tx, anchor outputs zero fee htlc txs)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
+    testRevokedHtlcTxConfirmed(f, ChannelFeatures(Features.StaticRemoteKey, Features.AnchorOutputsZeroFeeHtlcTx))
   }
 
-  test("recv WatchOutputSpentTriggered (one revoked tx, counterparty published aggregated htlc tx)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
+  test("recv WatchTxConfirmedTriggered (revoked aggregated htlc tx)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
     import f._
 
     // bob publishes one of his revoked txs
@@ -2069,12 +2061,13 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
 
     // alice reacts by publishing penalty txs that spend bob's htlc transaction
     alice ! WatchOutputSpentTriggered(bobHtlcTxs(0).amountIn, bobHtlcTx)
+    assert(alice2blockchain.expectMsgType[WatchTxConfirmed].txId == bobHtlcTx.txid)
+    alice ! WatchTxConfirmedTriggered(BlockHeight(129), 7, bobHtlcTx)
     awaitCond(alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs.size == 4)
     val claimHtlcDelayedPenaltyTxs = alice.stateData.asInstanceOf[DATA_CLOSING].revokedCommitPublished.head.claimHtlcDelayedPenaltyTxs
     val spentOutpoints = Set(OutPoint(bobHtlcTx, 1), OutPoint(bobHtlcTx, 2), OutPoint(bobHtlcTx, 3), OutPoint(bobHtlcTx, 4))
     assert(claimHtlcDelayedPenaltyTxs.map(_.input.outPoint).toSet == spentOutpoints)
     claimHtlcDelayedPenaltyTxs.foreach(claimHtlcPenalty => Transaction.correctlySpends(claimHtlcPenalty.tx, bobHtlcTx :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS))
-    assert(alice2blockchain.expectMsgType[WatchTxConfirmed].txId == bobHtlcTx.txid)
     val publishedPenaltyTxs = Set(
       alice2blockchain.expectMsgType[PublishFinalTx],
       alice2blockchain.expectMsgType[PublishFinalTx],
@@ -2141,15 +2134,15 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     alice2relayer.expectNoMessage(100 millis)
   }
 
-  test("recv WatchTxConfirmedTriggered (one revoked tx, pending htlcs)") { f =>
+  test("recv WatchTxConfirmedTriggered (revoked commit tx, pending htlcs)") { f =>
     testRevokedTxConfirmed(f, ChannelFeatures(Features.StaticRemoteKey))
   }
 
-  test("recv WatchTxConfirmedTriggered (one revoked tx, pending htlcs, anchor outputs)", Tag(ChannelStateTestsTags.AnchorOutputs)) { f =>
+  test("recv WatchTxConfirmedTriggered (revoked commit tx, pending htlcs, anchor outputs)", Tag(ChannelStateTestsTags.AnchorOutputs)) { f =>
     testRevokedTxConfirmed(f, ChannelFeatures(Features.StaticRemoteKey, Features.AnchorOutputs))
   }
 
-  test("recv WatchTxConfirmedTriggered (one revoked tx, pending htlcs, anchor outputs zero fee htlc txs)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
+  test("recv WatchTxConfirmedTriggered (revoked commit tx, pending htlcs, anchor outputs zero fee htlc txs)", Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
     testRevokedTxConfirmed(f, ChannelFeatures(Features.StaticRemoteKey, Features.AnchorOutputsZeroFeeHtlcTx))
   }
 
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/integration/ChannelIntegrationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/integration/ChannelIntegrationSpec.scala
index ae85c0a1bd..3a0132e1fc 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/integration/ChannelIntegrationSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/integration/ChannelIntegrationSpec.scala
@@ -525,6 +525,8 @@ class StandardChannelIntegrationSpec extends ChannelIntegrationSpec {
       // 3rd stage txs (txs spending htlc txs) are not tested if C publishes the htlc-penalty transaction before F publishes its htlc-timeout
       case Failure(e: JsonRPCError) => assert(e.error.message == "txn-mempool-conflict")
     }
+    // we generate enough blocks for HTLC txs to be confirmed, in case they were successfully published
+    generateBlocks(8)
     // at this point C should have 5 recv transactions: F's main output and all htlc outputs (taken as punishment)
     // C's main output uses static_remotekey, so C doesn't need to claim it
     awaitCond({