Remove amount-based confirmation scaling (#3044)

We previously scaled the number of confirmations based on the channel
capacity: however, this doesn't really work with splicing, as shown by
the following attack scenario:

- a malicious node opens a very large channel
- we thus wait for more confirmations to protect against reorgs
- they then send lightning payments until all of the balance is on our
  side of the channel
- we splice-out to shrink the channel to a minimal size
- if we thus us a smaller number of confirmations (because the channel
  is now small), the malicious node can perform this small reorg and
  publish one of their revoked commitment

There are more involved variations of this attack, which show that the
real amount at stake is potentially much more than a single channel's
current capacity. Instead of trying to find a complex solution that
would likely have subtle edge cases, it's a lot simpler to always use
a static number of confirmations that is large enough to protect against
reorgs entirely.

Author: t-bast

docs/release-notes/eclair-vnext.md

@@ -12,7 +12,20 @@
 
 ### Miscellaneous improvements and bug fixes
 
-<insert changes>
+#### Remove confirmation scaling based on funding amount
+
+We previously scaled the number of confirmations based on the channel funding amount.
+However, this doesn't work with splicing, where the channel capacity may change drastically.
+It's much simpler to always use the same number of confirmations, while choosing a value that is large enough to protect against malicious reorgs.
+We now by default use 8 confirmations, which can be modified in `eclair.conf`:
+
+```conf
+// Minimum number of confirmations for channel transactions to be safe from reorgs.
+eclair.channel.min-depth-blocks = 8
+```
+
+Note however that we require `min-depth` to be at least 6 blocks, since the BOLTs require this before announcing channels.
+See #3044 for more details.
 
 ## Verifying signatures
 

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

@@ -133,7 +133,7 @@ eclair {
 
     to-remote-delay-blocks = 720 // number of blocks that the other node's to-self outputs must be delayed (720 ~ 5 days)
     max-to-local-delay-blocks = 2016 // maximum number of blocks that we are ready to accept for our own delayed outputs (2016 ~ 2 weeks)
-    min-depth-blocks = 6 // minimum number of confirmations for channel transactions, which we will additionally scale based on the amount at stake
+    min-depth-blocks = 8 // minimum number of confirmations for channel transactions to be safe from reorgs
     expiry-delta-blocks = 144
     max-expiry-delta-blocks = 2016 // we won't forward HTLCs with timeouts greater than this delta
     // When we receive the preimage for an HTLC and want to fulfill it but the upstream peer stops responding, we want to

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala

@@ -88,7 +88,7 @@ object ZmqWatcher {
     /** TxId of the transaction to watch. */
     def txId: TxId
     /** Number of confirmations. */
-    def minDepth: Long
+    def minDepth: Int
   }
 
   /**
@@ -140,17 +140,17 @@ object ZmqWatcher {
   case class WatchPublished(replyTo: ActorRef[WatchPublishedTriggered], txId: TxId) extends Watch[WatchPublishedTriggered]
   case class WatchPublishedTriggered(tx: Transaction) extends WatchTriggered
 
-  case class WatchFundingConfirmed(replyTo: ActorRef[WatchFundingConfirmedTriggered], txId: TxId, minDepth: Long) extends WatchConfirmed[WatchFundingConfirmedTriggered]
+  case class WatchFundingConfirmed(replyTo: ActorRef[WatchFundingConfirmedTriggered], txId: TxId, minDepth: Int) extends WatchConfirmed[WatchFundingConfirmedTriggered]
   case class WatchFundingConfirmedTriggered(blockHeight: BlockHeight, txIndex: Int, tx: Transaction) extends WatchConfirmedTriggered
 
   case class RelativeDelay(parentTxId: TxId, delay: Long)
-  case class WatchTxConfirmed(replyTo: ActorRef[WatchTxConfirmedTriggered], txId: TxId, minDepth: Long, delay_opt: Option[RelativeDelay] = None) extends WatchConfirmed[WatchTxConfirmedTriggered]
+  case class WatchTxConfirmed(replyTo: ActorRef[WatchTxConfirmedTriggered], txId: TxId, minDepth: Int, delay_opt: Option[RelativeDelay] = None) extends WatchConfirmed[WatchTxConfirmedTriggered]
   case class WatchTxConfirmedTriggered(blockHeight: BlockHeight, txIndex: Int, tx: Transaction) extends WatchConfirmedTriggered
 
-  case class WatchParentTxConfirmed(replyTo: ActorRef[WatchParentTxConfirmedTriggered], txId: TxId, minDepth: Long) extends WatchConfirmed[WatchParentTxConfirmedTriggered]
+  case class WatchParentTxConfirmed(replyTo: ActorRef[WatchParentTxConfirmedTriggered], txId: TxId, minDepth: Int) extends WatchConfirmed[WatchParentTxConfirmedTriggered]
   case class WatchParentTxConfirmedTriggered(blockHeight: BlockHeight, txIndex: Int, tx: Transaction) extends WatchConfirmedTriggered
 
-  case class WatchAlternativeCommitTxConfirmed(replyTo: ActorRef[WatchAlternativeCommitTxConfirmedTriggered], txId: TxId, minDepth: Long) extends WatchConfirmed[WatchAlternativeCommitTxConfirmedTriggered]
+  case class WatchAlternativeCommitTxConfirmed(replyTo: ActorRef[WatchAlternativeCommitTxConfirmedTriggered], txId: TxId, minDepth: Int) extends WatchConfirmed[WatchAlternativeCommitTxConfirmedTriggered]
   case class WatchAlternativeCommitTxConfirmedTriggered(blockHeight: BlockHeight, txIndex: Int, tx: Transaction) extends WatchConfirmedTriggered
 
   private sealed trait AddWatchResult

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

@@ -7,9 +7,7 @@ import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, Satoshi
 import fr.acinq.eclair.blockchain.fee.{FeeratePerByte, FeeratePerKw, FeeratesPerKw, OnChainFeeConf}
 import fr.acinq.eclair.channel.Helpers.Closing
 import fr.acinq.eclair.channel.Monitoring.{Metrics, Tags}
-import fr.acinq.eclair.channel.fsm.Channel
 import fr.acinq.eclair.channel.fsm.Channel.ChannelConf
-import fr.acinq.eclair.channel.fund.InteractiveTxBuilder.SharedTransaction
 import fr.acinq.eclair.crypto.keymanager.ChannelKeyManager
 import fr.acinq.eclair.crypto.{Generators, ShaChain}
 import fr.acinq.eclair.payment.OutgoingPaymentPacket
@@ -40,6 +38,9 @@ case class ChannelParams(channelId: ByteVector32,
   // We can safely cast to millisatoshis since we verify that it's less than a valid millisatoshi amount.
   val maxHtlcAmount: MilliSatoshi = remoteParams.maxHtlcValueInFlightMsat.toBigInt.min(localParams.maxHtlcValueInFlightMsat.toLong).toLong.msat
 
+  // If we've set the 0-conf feature bit for this peer, we will always use 0-conf with them.
+  val zeroConf: Boolean = localParams.initFeatures.hasFeature(Features.ZeroConf)
+
   /**
    * We update local/global features at reconnection
    */
@@ -49,47 +50,10 @@ case class ChannelParams(channelId: ByteVector32,
   )
 
   /**
-   * Returns the number of confirmations needed to safely handle a funding transaction that we unilaterally funded.
-   * As funder we trust ourselves to not double spend funding txs, so we don't need to scale the number of confirmations
-   * based on the funding amount. We want to wait a few blocks though to ensure that the short_channel_id we obtain will
-   * not be invalidated by a reorg.
+   * Returns the number of confirmations needed to make a channel transaction safe from reorgs.
+   * A malicious miner that can create a longer reorg will be able to steal all of the channel funds.
    */
-  def minDepthFunder(defaultMinDepth: Int): Option[Long] = {
-    if (localParams.initFeatures.hasFeature(Features.ZeroConf)) {
-      None
-    } else {
-      Some(defaultMinDepth.toLong)
-    }
-  }
-
-  /**
-   * Returns the number of confirmations needed to safely handle a funding transaction with remote inputs. We make sure
-   * the cumulative block reward largely exceeds the channel size, because an attacker that could create a reorg would
-   * be able to steal the entire channel funding, but would likely miss block rewards during that process, making it
-   * economically irrational for them.
-   *
-   * @param fundingAmount funding amount of the channel
-   * @return number of confirmations needed, if any
-   */
-  def minDepthFundee(defaultMinDepth: Int, fundingAmount: Satoshi): Option[Long] =
-    if (localParams.initFeatures.hasFeature(Features.ZeroConf)) {
-      None // zero-conf stay zero-conf, whatever the funding amount is
-    } else {
-      Some(ChannelParams.minDepthScaled(defaultMinDepth, fundingAmount))
-    }
-
-  /**
-   * When using dual funding or splices, we wait for multiple confirmations even if we're the initiator because:
-   *  - our peer may also contribute to the funding transaction, even if they don't contribute to the channel funding amount
-   *  - even if they don't, we may RBF the transaction and don't want to handle reorgs
-   */
-  def minDepthDualFunding(defaultMinDepth: Int, sharedTx: SharedTransaction): Option[Long] = {
-    if (localParams.initFeatures.hasFeature(Features.ZeroConf)) {
-      None
-    } else {
-      Some(ChannelParams.minDepthScaled(defaultMinDepth, sharedTx.sharedOutput.amount))
-    }
-  }
+  def minDepth(defaultMinDepth: Int): Option[Int] = if (zeroConf) None else Some(defaultMinDepth)
 
   /** Channel reserve that applies to our funds. */
   def localChannelReserveForCapacity(capacity: Satoshi, isSplice: Boolean): Satoshi = if (channelFeatures.hasFeature(Features.DualFunding) || isSplice) {
@@ -139,20 +103,6 @@ case class ChannelParams(channelId: ByteVector32,
 
 }
 
-object ChannelParams {
-  def minDepthScaled(defaultMinDepth: Int, amount: Satoshi): Int = {
-    if (amount <= Channel.MAX_FUNDING_WITHOUT_WUMBO) {
-      // small amount: not scaled
-      defaultMinDepth
-    } else {
-      val blockReward = 3.125 // this will be too large after the halving in 2028
-      val scalingFactor = 10
-      val blocksToReachFunding = (((scalingFactor * amount.toBtc.toDouble) / blockReward).ceil + 1).toInt
-      defaultMinDepth.max(blocksToReachFunding)
-    }
-  }
-}
-
 // @formatter:off
 case class LocalChanges(proposed: List[UpdateMessage], signed: List[UpdateMessage], acked: List[UpdateMessage]) {
   def all: List[UpdateMessage] = proposed ++ signed ++ acked

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

@@ -112,9 +112,6 @@ object Channel {
     require(balanceThresholds.sortBy(_.available) == balanceThresholds, "channel-update.balance-thresholds must be sorted by available-sat")
 
     def minFundingSatoshis(flags: ChannelFlags): Satoshi = if (flags.announceChannel) minFundingPublicSatoshis else minFundingPrivateSatoshis
-
-    /** The number of confirmations required to be safe from reorgs is always scaled based on the amount at risk. */
-    def minDepthScaled(amount: Satoshi): Int = ChannelParams.minDepthScaled(minDepth, amount)
   }
 
   trait TxPublisherFactory {
@@ -320,14 +317,13 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
                   // once, because at the next restore, the status of the funding tx will be "confirmed".
                   ()
               }
-              watchFundingConfirmed(commitment.fundingTxId, Some(singleFundingMinDepth(data)), herdDelay_opt)
+              watchFundingConfirmed(commitment.fundingTxId, Some(nodeParams.channelConf.minDepth), herdDelay_opt)
             case fundingTx: LocalFundingStatus.DualFundedUnconfirmedFundingTx =>
               publishFundingTx(fundingTx)
-              val minDepth_opt = data.commitments.params.minDepthDualFunding(nodeParams.channelConf.minDepth, fundingTx.sharedTx.tx)
+              val minDepth_opt = data.commitments.params.minDepth(nodeParams.channelConf.minDepth)
               watchFundingConfirmed(fundingTx.sharedTx.txId, minDepth_opt, herdDelay_opt)
             case fundingTx: LocalFundingStatus.ZeroconfPublishedFundingTx =>
-              // This is a zero-conf channel, the min-depth isn't critical: we use the default.
-              watchFundingConfirmed(fundingTx.tx.txid, Some(nodeParams.channelConf.minDepth.toLong), herdDelay_opt)
+              watchFundingConfirmed(fundingTx.tx.txid, Some(nodeParams.channelConf.minDepth), herdDelay_opt)
             case _: LocalFundingStatus.ConfirmedFundingTx =>
               data match {
                 case closing: DATA_CLOSING if Closing.nothingAtStake(closing) || Closing.isClosingTypeAlreadyKnown(closing).isDefined =>
@@ -612,7 +608,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
                     // We don't have their tx_sigs, but they have ours, and could publish the funding tx without telling us.
                     // That's why we move on immediately to the next step, and will update our unsigned funding tx when we
                     // receive their tx_sigs.
-                    val minDepth_opt = d.commitments.params.minDepthDualFunding(nodeParams.channelConf.minDepth, signingSession1.fundingTx.sharedTx.tx)
+                    val minDepth_opt = d.commitments.params.minDepth(nodeParams.channelConf.minDepth)
                     watchFundingConfirmed(signingSession.fundingTx.txId, minDepth_opt, delay_opt = None)
                     val commitments1 = d.commitments.add(signingSession1.commitment)
                     val d1 = d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice)
@@ -1349,7 +1345,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
                   rollbackFundingAttempt(signingSession.fundingTx.tx, previousTxs = Seq.empty) // no splice rbf yet
                   stay() using d.copy(spliceStatus = SpliceStatus.SpliceAborted) sending TxAbort(d.channelId, f.getMessage)
                 case Right(signingSession1) =>
-                  val minDepth_opt = d.commitments.params.minDepthDualFunding(nodeParams.channelConf.minDepth, signingSession1.fundingTx.sharedTx.tx)
+                  val minDepth_opt = d.commitments.params.minDepth(nodeParams.channelConf.minDepth)
                   watchFundingConfirmed(signingSession.fundingTx.txId, minDepth_opt, delay_opt = None)
                   val commitments1 = d.commitments.add(signingSession1.commitment)
                   val d1 = d.copy(commitments = commitments1, spliceStatus = SpliceStatus.NoSplice)
@@ -1368,7 +1364,6 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
       val fundingStatus = LocalFundingStatus.ZeroconfPublishedFundingTx(w.tx, d.commitments.localFundingSigs(w.tx.txid), d.commitments.liquidityPurchase(w.tx.txid))
       d.commitments.updateLocalFundingStatus(w.tx.txid, fundingStatus, d.lastAnnouncedFundingTxId_opt) match {
         case Right((commitments1, commitment)) =>
-          // This is a zero-conf channel, the min-depth isn't critical: we use the default.
           watchFundingConfirmed(w.tx.txid, Some(nodeParams.channelConf.minDepth), delay_opt = None)
           maybeEmitEventsPostSplice(d.aliases, d.commitments, commitments1, d.lastAnnouncement_opt)
           maybeUpdateMaxHtlcAmount(d.channelUpdate.htlcMaximumMsat, commitments1)
@@ -1960,7 +1955,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
         d.commitments.resolveCommitment(tx) match {
           case Some(commitment) =>
             log.warning("a commit tx for an older commitment has been published fundingTxId={} fundingTxIndex={}", tx.txid, commitment.fundingTxIndex)
-            blockchain ! WatchAlternativeCommitTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepthScaled(commitment.capacity))
+            blockchain ! WatchAlternativeCommitTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepth)
             stay()
           case None =>
             // This must be a former funding tx that has already been pruned, because watches are unordered.
@@ -2029,7 +2024,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
     case Event(WatchOutputSpentTriggered(amount, 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
-      blockchain ! WatchTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepthScaled(amount))
+      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
@@ -2064,7 +2059,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
           val (localCommitPublished1, claimHtlcTx_opt) = Closing.LocalClose.claimHtlcDelayedOutput(localCommitPublished, keyManager, d.commitments.latest, tx, nodeParams.currentBitcoinCoreFeerates, nodeParams.onChainFeeConf, d.finalScriptPubKey)
           claimHtlcTx_opt.foreach(claimHtlcTx => {
             txPublisher ! PublishFinalTx(claimHtlcTx, claimHtlcTx.fee, None)
-            blockchain ! WatchTxConfirmed(self, claimHtlcTx.tx.txid, nodeParams.channelConf.minDepthScaled(claimHtlcTx.amountIn), Some(RelativeDelay(tx.txid, d.commitments.params.remoteParams.toSelfDelay.toInt.toLong)))
+            blockchain ! WatchTxConfirmed(self, claimHtlcTx.tx.txid, nodeParams.channelConf.minDepth, Some(RelativeDelay(tx.txid, d.commitments.params.remoteParams.toSelfDelay.toInt.toLong)))
           })
           Closing.updateLocalCommitPublished(localCommitPublished1, tx)
         }),
@@ -2841,7 +2836,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
       } else {
         // This is one of the transactions we published.
         val closingTx = d.findClosingTx(tx).get
-        blockchain ! WatchTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepthScaled(closingTx.amountIn))
+        blockchain ! WatchTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepth)
         stay()
       }
 
@@ -2869,7 +2864,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
           case Some(commitment) =>
             log.warning("a commit tx for an older commitment has been published fundingTxId={} fundingTxIndex={}", tx.txid, commitment.fundingTxIndex)
             // We watch the commitment tx, in the meantime we force close using the latest commitment.
-            blockchain ! WatchAlternativeCommitTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepthScaled(commitment.capacity))
+            blockchain ! WatchAlternativeCommitTxConfirmed(self, tx.txid, nodeParams.channelConf.minDepth)
             spendLocalCurrent(d)
           case None =>
             // This must be a former funding tx that has already been pruned, because watches are unordered.