Remove non-final transactions from XxxCommitPublished (#3097)

When a channel was force-closed, we previously stored partially created
closing transactions for each output of the commitment transaction. In
some cases those transactions didn't include any on-chain fee (HTLC txs
with 0-fee anchors) and weren't signed. The fee is set in the publisher
actors which then sign those transactions.

This was an issue because we used those partial transactions as if they
were the final transaction that would eventually confirm: this made it
look like RBF wasn't an option to callers, and was thus easy to misuse.

We now change our data model to only store the outputs of the commit tx
that we may spend, without any actual spending transaction. We only
store spending transactions once they are confirmed, at which point they
can safely be used as closing transactions by callers such as our
balance checks.

This lets us get rid of some codec migration code related to closing
transactions, and is more future-proof because we now don't need to
encode closing transactions and can thus change their format easily.
This also reduces the size of our channel state during force-close.

We add a `max-closing-feerate` config parameter to cap the feerate used
for closing transactions which aren't at risk of being double-spent.
Node operators should generally use a low value here, and update it when
they need to reclaim liquidity if needed.

Author: t-bast

docs/FAQ.md

@@ -1,6 +1,6 @@
 # FAQ
 
-## What does it mean for a channel to be "enabled" or "disabled" ?
+## What does it mean for a channel to be "enabled" or "disabled"?
 
 A channel is disabled if a `channel_update` message has been broadcast for that channel with the `disable` bit set (see [BOLT 7](https://github.com/lightning/bolts/blob/master/07-routing-gossip.md#the-channel_update-message)). It means that the channel still exists but cannot be used to route payments, until it has been re-enabled.
 
@@ -13,6 +13,16 @@ There are other cases when a channel becomes disabled, for example when its bala
 
 Note that you can have multiple channels between the same nodes, and that some of them can be enabled while others are disabled (i.e. enable/disable is channel-specific, not node-specific).
 
-## How should you stop an Eclair node ?
+## How do I make my closing transactions confirm faster?
+
+When channels are unilaterally closed, there is a delay before which closing transactions can be published: you must wait for this delay before you can get your funds back.
+
+Once published, transactions will be automatically RBF-ed by `eclair` based on your configuration values for the [`eclair.on-chain-fees` section](../eclair-core/src/main/resources/reference.conf).
+
+Note that there is an upper bound on the feerate that will be used, configured by the `eclair.on-chain-fees.max-closing-feerate` parameter.
+If the current feerate is higher than this value, your transactions will not confirm.
+You should update `eclair.on-chain-fees.max-closing-feerate` in your `eclair.conf` and restart your node: your transactions will automatically be RBF-ed using the new feerate.
+
+## How should you stop an Eclair node?
 
 To stop your node you just need to kill its process, there is no API command to do this. The JVM handles the quit signal and notifies the node to perform clean-up. For example, there is a hook to cleanly free DB locks when using Postgres.

docs/release-notes/eclair-vnext.md

@@ -39,6 +39,12 @@ It can be enabled by setting `eclair.features.option_attribution_data = optional
 
 ### Miscellaneous improvements and bug fixes
 
+#### Add `max-closing-feerate` configuration parameter
+
+We added a new configuration value to `eclair.conf` to limit the feerate used for force-close transactions where funds aren't at risk: `eclair.on-chain-fees.max-closing-feerate`.
+This ensures that you won't end up paying a lot of fees during mempool congestion: your node will wait for the feerate to decrease to get your non-urgent transactions confirmed.
+If you need those transactions to confirm because you are low on liquidity, you should update `eclair.on-chain-fees.max-closing-feerate` and restart your node: `eclair` will automatically RBF all available transactions.
+
 #### Remove confirmation scaling based on funding amount
 
 We previously scaled the number of confirmations based on the channel funding amount.

eclair-core/src/main/resources/reference.conf

@@ -271,6 +271,14 @@ eclair {
       closing = medium
     }
 
+    // Maximum feerate that will be used when closing channels for outputs that aren't at risk (main balance and HTLC 3rd-stage transactions).
+    // Using a low value here ensures that you won't be paying high fees when the mempool is congested and you're not in
+    // a hurry to get your channel funds back.
+    // If closing transactions don't confirm and you need to get the funds back quickly, you should increase this value
+    // and restart your node: closing transactions will automatically be RBF-ed to match the current feerate.
+    // This value is in satoshis per byte.
+    max-closing-feerate = 10
+
     feerate-tolerance {
       ratio-low = 0.5 // will allow remote fee rates as low as half our local feerate (only enforced when not using anchor outputs)
       ratio-high = 10.0 // will allow remote fee rates as high as 10 times our local feerate (for all commitment formats)

eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala

@@ -607,6 +607,7 @@ object NodeParams extends Logging {
       ),
       onChainFeeConf = OnChainFeeConf(
         feeTargets = feeTargets,
+        maxClosingFeerate = FeeratePerKw(FeeratePerByte(Satoshi(config.getLong("on-chain-fees.max-closing-feerate")))),
         safeUtxosThreshold = config.getInt("on-chain-fees.safe-utxos-threshold"),
         spendAnchorWithoutHtlcs = config.getBoolean("on-chain-fees.spend-anchor-without-htlcs"),
         anchorWithoutHtlcsMaxFee = Satoshi(config.getLong("on-chain-fees.anchor-without-htlcs-max-fee-satoshis")),

eclair-core/src/main/scala/fr/acinq/eclair/balance/CheckBalance.scala

@@ -23,7 +23,6 @@ import fr.acinq.eclair.channel.Helpers.Closing
 import fr.acinq.eclair.channel.Helpers.Closing._
 import fr.acinq.eclair.channel._
 import fr.acinq.eclair.transactions.DirectedHtlc.incoming
-import fr.acinq.eclair.transactions.Transactions.{ClaimHtlcSuccessTx, HtlcSuccessTx, HtlcTimeoutTx}
 import fr.acinq.eclair.wire.protocol.UpdateAddHtlc
 
 import scala.concurrent.{ExecutionContext, Future}
@@ -69,31 +68,26 @@ object CheckBalance {
     def addLocalClose(lcp: LocalCommitPublished): MainAndHtlcBalance = {
       // If our main transaction isn't deeply confirmed yet, we count it in our off-chain balance.
       // Once it confirms, it will be included in our on-chain balance, so we ignore it in our off-chain balance.
-      val additionalToLocal = lcp.claimMainDelayedOutputTx.map(_.input.outPoint) match {
+      val additionalToLocal = lcp.localOutput_opt match {
         case Some(outpoint) if !lcp.irrevocablySpent.contains(outpoint) => lcp.commitTx.txOut(outpoint.index.toInt).amount
         case _ => 0 sat
       }
-      val additionalHtlcs = lcp.htlcTxs.map {
-        case (outpoint, htlcTx_opt) =>
-          val htlcAmount = lcp.commitTx.txOut(outpoint.index.toInt).amount
-          lcp.irrevocablySpent.get(outpoint) match {
-            case Some(spendingTx) =>
-              // If the HTLC was spent by us, there will be an entry in our 3rd-stage transactions.
-              // Otherwise it was spent by the remote and we don't have anything to add to our balance.
-              val delayedHtlcOutpoint = OutPoint(spendingTx.txid, 0)
-              val htlcSpentByUs = lcp.claimHtlcDelayedTxs.map(_.input.outPoint).contains(delayedHtlcOutpoint)
-              // If our 3rd-stage transaction isn't confirmed yet, we should count it in our off-chain balance.
-              // Once confirmed, we should ignore it since it will appear in our on-chain balance.
-              val htlcDelayedPending = !lcp.irrevocablySpent.contains(delayedHtlcOutpoint)
-              if (htlcSpentByUs && htlcDelayedPending) htlcAmount else 0 sat
-            case None =>
-              // We assume that HTLCs will be fulfilled, so we only count incoming HTLCs in our off-chain balance.
-              htlcTx_opt match {
-                case Some(_: HtlcSuccessTx) => htlcAmount
-                case Some(_: HtlcTimeoutTx) => 0 sat
-                case None => htlcAmount // incoming HTLC for which we don't have the preimage yet
-              }
-          }
+      val additionalHtlcs = lcp.htlcOutputs.map { outpoint =>
+        val htlcAmount = lcp.commitTx.txOut(outpoint.index.toInt).amount
+        lcp.irrevocablySpent.get(outpoint) match {
+          case Some(spendingTx) =>
+            // If the HTLC was spent by us, there will be an entry in our 3rd-stage transactions.
+            // Otherwise it was spent by the remote and we don't have anything to add to our balance.
+            val delayedHtlcOutpoint = OutPoint(spendingTx.txid, 0)
+            val htlcSpentByUs = lcp.htlcDelayedOutputs.contains(delayedHtlcOutpoint)
+            // If our 3rd-stage transaction isn't confirmed yet, we should count it in our off-chain balance.
+            // Once confirmed, we should ignore it since it will appear in our on-chain balance.
+            val htlcDelayedPending = !lcp.irrevocablySpent.contains(delayedHtlcOutpoint)
+            if (htlcSpentByUs && htlcDelayedPending) htlcAmount else 0 sat
+          case None =>
+            // We assume that HTLCs will be fulfilled, so we only count incoming HTLCs in our off-chain balance.
+            if (lcp.incomingHtlcs.contains(outpoint)) htlcAmount else 0 sat
+        }
       }.sum
       MainAndHtlcBalance(toLocal = toLocal + additionalToLocal, htlcs = htlcs + additionalHtlcs)
     }
@@ -102,16 +96,15 @@ object CheckBalance {
     def addRemoteClose(rcp: RemoteCommitPublished): MainAndHtlcBalance = {
       // If our main transaction isn't deeply confirmed yet, we count it in our off-chain balance.
       // Once it confirms, it will be included in our on-chain balance, so we ignore it in our off-chain balance.
-      val additionalToLocal = rcp.claimMainOutputTx.map(_.input.outPoint) match {
+      val additionalToLocal = rcp.localOutput_opt match {
         case Some(outpoint) if !rcp.irrevocablySpent.contains(outpoint) => rcp.commitTx.txOut(outpoint.index.toInt).amount
         case _ => 0 sat
       }
       // If HTLC transactions are confirmed, they will appear in our on-chain balance if we were the one to claim them.
       // We only need to include incoming HTLCs that haven't been claimed yet (since we assume that they will be fulfilled).
       // Note that it is their commitment, so incoming/outgoing are inverted.
-      val additionalHtlcs = rcp.claimHtlcTxs.map {
-        case (outpoint, Some(_: ClaimHtlcSuccessTx)) if !rcp.irrevocablySpent.contains(outpoint) => rcp.commitTx.txOut(outpoint.index.toInt).amount
-        case (outpoint, None) if !rcp.irrevocablySpent.contains(outpoint) => rcp.commitTx.txOut(outpoint.index.toInt).amount // incoming HTLC for which we don't have the preimage yet
+      val additionalHtlcs = rcp.incomingHtlcs.keys.map {
+        case outpoint if !rcp.irrevocablySpent.contains(outpoint) => rcp.commitTx.txOut(outpoint.index.toInt).amount
         case _ => 0 sat
       }.sum
       MainAndHtlcBalance(toLocal = toLocal + additionalToLocal, htlcs = htlcs + additionalHtlcs)
@@ -122,15 +115,15 @@ object CheckBalance {
       // If our main transaction isn't deeply confirmed yet, we count it in our off-chain balance.
       // Once it confirms, it will be included in our on-chain balance, so we ignore it in our off-chain balance.
       // We do the same thing for our main penalty transaction claiming their main output.
-      val additionalToLocal = rvk.claimMainOutputTx.map(_.input.outPoint) match {
+      val additionalToLocal = rvk.localOutput_opt match {
         case Some(outpoint) if !rvk.irrevocablySpent.contains(outpoint) => rvk.commitTx.txOut(outpoint.index.toInt).amount
         case _ => 0 sat
       }
-      val additionalToRemote = rvk.mainPenaltyTx.map(_.input.outPoint) match {
+      val additionalToRemote = rvk.remoteOutput_opt match {
         case Some(outpoint) if !rvk.irrevocablySpent.contains(outpoint) => rvk.commitTx.txOut(outpoint.index.toInt).amount
         case _ => 0 sat
       }
-      val additionalHtlcs = rvk.htlcPenaltyTxs.map(_.input.outPoint).map(htlcOutpoint => {
+      val additionalHtlcs = rvk.htlcOutputs.map(htlcOutpoint => {
         val htlcAmount = rvk.commitTx.txOut(htlcOutpoint.index.toInt).amount
         rvk.irrevocablySpent.get(htlcOutpoint) match {
           case Some(spendingTx) =>
@@ -139,7 +132,7 @@ object CheckBalance {
               case Some(outputIndex) =>
                 // If they managed to get their HTLC transaction confirmed, we published an HTLC-delayed penalty transaction.
                 val delayedHtlcOutpoint = OutPoint(spendingTx.txid, outputIndex)
-                val htlcSpentByThem = rvk.claimHtlcDelayedPenaltyTxs.map(_.input.outPoint).contains(delayedHtlcOutpoint)
+                val htlcSpentByThem = rvk.htlcDelayedOutputs.contains(delayedHtlcOutpoint)
                 // If our 3rd-stage transaction isn't confirmed yet, we should count it in our off-chain balance.
                 // Once confirmed, we should ignore it since it will appear in our on-chain balance.
                 val htlcDelayedPending = !rvk.irrevocablySpent.contains(delayedHtlcOutpoint)
@@ -194,7 +187,7 @@ object CheckBalance {
           // In the recovery case, we can only claim our main output, HTLC outputs are lost.
           // Once our main transaction confirms, the channel will transition to the CLOSED state and our channel funds
           // will appear in our on-chain balance (minus on-chain fees).
-          case Some(c: RecoveryClose) => c.remoteCommitPublished.claimMainOutputTx.map(_.input.outPoint) match {
+          case Some(c: RecoveryClose) => c.remoteCommitPublished.localOutput_opt match {
             case Some(localOutput) =>
               val localBalance = c.remoteCommitPublished.commitTx.txOut(localOutput.index.toInt).amount
               this.copy(closing = this.closing.copy(toLocal = this.closing.toLocal + localBalance))

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/OnChainFeeConf.scala

@@ -91,6 +91,7 @@ case class FeerateTolerance(ratioLow: Double, ratioHigh: Double, anchorOutputMax
 }
 
 case class OnChainFeeConf(feeTargets: FeeTargets,
+                          maxClosingFeerate: FeeratePerKw,
                           safeUtxosThreshold: Int,
                           spendAnchorWithoutHtlcs: Boolean,
                           anchorWithoutHtlcsMaxFee: Satoshi,
@@ -128,5 +129,5 @@ case class OnChainFeeConf(feeTargets: FeeTargets,
     }
   }
 
-  def getClosingFeerate(feerates: FeeratesPerKw): FeeratePerKw = feeTargets.closing.getFeerate(feerates)
+  def getClosingFeerate(feerates: FeeratesPerKw): FeeratePerKw = feeTargets.closing.getFeerate(feerates).min(maxClosingFeerate)
 }