Fix opportunistic zero-conf (#2616)

If we didn't plan on using zero-conf, but our peer sends us an early
channel_ready, we can opportunistically switch to zero-conf.

But we can only do that if we're sure that our peer cannot double-spend
the funding transaction. We previously checked their contribution to the
funding output, but that's not enough: they may add inputs to the funding
transaction even if they don't contribute to the funding output.

We were also setting duplicate `WatchPublished` in case we were already
using zero-conf, which is now fixed.

When our peer sends us channel_ready while we're still waiting for
confirmations, we may opportunistically switch to zero-conf, in which
case we have both a WatchPublished and a WatchConfirmed pending.
But it may not actually be a real switch to zero-conf: maybe the
transaction is confirmed, and they simply received the block slightly
before us. In that case, the WatchConfirmed may trigger first, and it
would be inefficient to let the WatchPublished override our funding
status: it will make us set a new WatchConfirmed that will instantly
trigger and rewrite the funding status again.

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

@@ -379,7 +379,7 @@ object Helpers {
 
     /**
      * When using dual funding, we wait for multiple confirmations even if we're the initiator because:
-     *  - our peer may also contribute to the funding transaction
+     *  - 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(channelConf: ChannelConf, localFeatures: Features[InitFeature], isInitiator: Boolean, localAmount: Satoshi, remoteAmount: Satoshi): Option[Long] = {

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

@@ -1563,31 +1563,42 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
     case Event(TickChannelOpenTimeout, _) => stay()
 
     case Event(w: WatchPublishedTriggered, d: PersistentChannelData) =>
-      val fundingStatus = LocalFundingStatus.ZeroconfPublishedFundingTx(w.tx)
+      // When our peer sends us channel_ready while we're still waiting for confirmations, we may opportunistically
-      d.commitments.updateLocalFundingStatus(w.tx.txid, fundingStatus) match {
+      // switch to zero-conf, in which case we have both a WatchPublished and a WatchConfirmed pending. But it may not
-        case Right((commitments1, _)) =>
+      // actually be a real switch to zero-conf: maybe the transaction is confirmed, and they simply received the block
-          log.info(s"zero-conf funding txid=${w.tx.txid} has been published")
+      // slightly before us. In that case, the WatchConfirmed may trigger first, and it would be inefficient to let the
-          watchFundingConfirmed(w.tx.txid, Some(nodeParams.channelConf.minDepthBlocks))
+      // WatchPublished override our funding status: it will make us set a new WatchConfirmed that will instantly
-          val d1 = d match {
+      // trigger and rewrite the funding status again.
-            // NB: we discard remote's stashed channel_ready, they will send it back at reconnection
+      val alreadyConfirmed = d.commitments.active.map(_.localFundingStatus).collect { case LocalFundingStatus.ConfirmedFundingTx(tx) => tx }.exists(_.txid == w.tx.txid)
-            case d: DATA_WAIT_FOR_FUNDING_CONFIRMED =>
+      if (alreadyConfirmed) {
-              val realScidStatus = RealScidStatus.Unknown
+        stay()
-              val shortIds = createShortIds(d.channelId, realScidStatus)
+      } else {
-              DATA_WAIT_FOR_CHANNEL_READY(commitments1, shortIds)
+        val fundingStatus = LocalFundingStatus.ZeroconfPublishedFundingTx(w.tx)
-            case d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED =>
+        d.commitments.updateLocalFundingStatus(w.tx.txid, fundingStatus) match {
-              val realScidStatus = RealScidStatus.Unknown
+          case Right((commitments1, _)) =>
-              val shortIds = createShortIds(d.channelId, realScidStatus)
+            log.info(s"zero-conf funding txid=${w.tx.txid} has been published")
-              DATA_WAIT_FOR_DUAL_FUNDING_READY(commitments1, shortIds)
+            watchFundingConfirmed(w.tx.txid, Some(nodeParams.channelConf.minDepthBlocks))
-            case d: DATA_WAIT_FOR_CHANNEL_READY => d.copy(commitments = commitments1)
+            val d1 = d match {
-            case d: DATA_WAIT_FOR_DUAL_FUNDING_READY => d.copy(commitments = commitments1)
+              // NB: we discard remote's stashed channel_ready, they will send it back at reconnection
-            case d: DATA_NORMAL => d.copy(commitments = commitments1)
+              case d: DATA_WAIT_FOR_FUNDING_CONFIRMED =>
-            case d: DATA_SHUTDOWN => d.copy(commitments = commitments1)
+                val realScidStatus = RealScidStatus.Unknown
-            case d: DATA_NEGOTIATING => d.copy(commitments = commitments1)
+                val shortIds = createShortIds(d.channelId, realScidStatus)
-            case d: DATA_WAIT_FOR_REMOTE_PUBLISH_FUTURE_COMMITMENT => d.copy(commitments = commitments1)
+                DATA_WAIT_FOR_CHANNEL_READY(commitments1, shortIds)
-            case d: DATA_CLOSING => d.copy(commitments = commitments1)
+              case d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED =>
-          }
+                val realScidStatus = RealScidStatus.Unknown
-          stay() using d1 storing()
+                val shortIds = createShortIds(d.channelId, realScidStatus)
-        case Left(_) => stay()
+                DATA_WAIT_FOR_DUAL_FUNDING_READY(commitments1, shortIds)
+              case d: DATA_WAIT_FOR_CHANNEL_READY => d.copy(commitments = commitments1)
+              case d: DATA_WAIT_FOR_DUAL_FUNDING_READY => d.copy(commitments = commitments1)
+              case d: DATA_NORMAL => d.copy(commitments = commitments1)
+              case d: DATA_SHUTDOWN => d.copy(commitments = commitments1)
+              case d: DATA_NEGOTIATING => d.copy(commitments = commitments1)
+              case d: DATA_WAIT_FOR_REMOTE_PUBLISH_FUTURE_COMMITMENT => d.copy(commitments = commitments1)
+              case d: DATA_CLOSING => d.copy(commitments = commitments1)
+            }
+            stay() using d1 storing()
+          case Left(_) => stay()
+        }
       }
 
     case Event(w: WatchFundingConfirmedTriggered, d: PersistentChannelData) =>

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

@@ -30,7 +30,7 @@ import fr.acinq.eclair.channel.publish.TxPublisher.SetChannelId
 import fr.acinq.eclair.crypto.ShaChain
 import fr.acinq.eclair.transactions.Scripts
 import fr.acinq.eclair.wire.protocol._
-import fr.acinq.eclair.{Features, RealShortChannelId, ToMilliSatoshiConversion, UInt64}
+import fr.acinq.eclair.{RealShortChannelId, ToMilliSatoshiConversion, UInt64}
 
 /**
  * Created by t-bast on 19/04/2022.
@@ -618,15 +618,8 @@ trait ChannelOpenDualFunded extends DualFundingHandlers with ErrorHandlers {
     case Event(e: BITCOIN_FUNDING_DOUBLE_SPENT, d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED) => handleDualFundingDoubleSpent(e, d)
 
     case Event(remoteChannelReady: ChannelReady, d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED) =>
-      // We can skip waiting for confirmations if:
+      if (switchToZeroConf(remoteChannelReady, d)) {
-      //  - there is a single version of the funding tx (otherwise we don't know which one to use)
-      //  - they didn't contribute to the funding output or we trust them to not double-spend
-      val canUseZeroConf = remoteChannelReady.alias_opt.isDefined &&
-        d.commitments.active.size == 1 &&
-        (d.latestFundingTx.fundingParams.remoteAmount == 0.sat || d.commitments.params.localParams.initFeatures.hasFeature(Features.ZeroConf))
-      if (canUseZeroConf) {
         log.info("this channel isn't zero-conf, but they sent an early channel_ready with an alias: no need to wait for confirmations")
-        // NB: we will receive a WatchFundingConfirmedTriggered later that will simply be ignored.
         blockchain ! WatchPublished(self, d.commitments.latest.fundingTxId)
       }
       log.debug("received their channel_ready, deferring message")

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

@@ -31,7 +31,7 @@ import fr.acinq.eclair.crypto.ShaChain
 import fr.acinq.eclair.transactions.Transactions.TxOwner
 import fr.acinq.eclair.transactions.{Scripts, Transactions}
 import fr.acinq.eclair.wire.protocol.{AcceptChannel, AnnouncementSignatures, ChannelReady, ChannelTlv, Error, FundingCreated, FundingSigned, OpenChannel, TlvStream}
-import fr.acinq.eclair.{MilliSatoshiLong, RealShortChannelId, UInt64, randomKey, toLongId}
+import fr.acinq.eclair.{Features, MilliSatoshiLong, RealShortChannelId, UInt64, randomKey, toLongId}
 import scodec.bits.ByteVector
 
 import scala.util.{Failure, Success}
@@ -372,9 +372,16 @@ trait ChannelOpenSingleFunded extends SingleFundingHandlers with ErrorHandlers {
 
   when(WAIT_FOR_FUNDING_CONFIRMED)(handleExceptions {
     case Event(remoteChannelReady: ChannelReady, d: DATA_WAIT_FOR_FUNDING_CONFIRMED) =>
-      if (remoteChannelReady.alias_opt.isDefined && d.commitments.params.localParams.isInitiator) {
+      // We are here if:
+      //  - we're using zero-conf, but our peer was very fast and we received their channel_ready before our watcher
+      //    notification that the funding tx has been successfully published: in that case we don't put a duplicate watch
+      //  - we're not using zero-conf, but our peer decided to trust us anyway, in which case we can skip waiting for
+      //    confirmations if we're the initiator (no risk of double-spend) and they provided a channel alias
+      val switchToZeroConf = d.commitments.params.localParams.isInitiator &&
+        remoteChannelReady.alias_opt.isDefined &&
+        !d.commitments.params.localParams.initFeatures.hasFeature(Features.ZeroConf)
+      if (switchToZeroConf) {
         log.info("this channel isn't zero-conf, but we are funder and they sent an early channel_ready with an alias: no need to wait for confirmations")
-        // NB: we will receive a WatchFundingConfirmedTriggered later that will simply be ignored
         blockchain ! WatchPublished(self, d.commitments.latest.fundingTxId)
       }
       log.debug("received their channel_ready, deferring message")

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

@@ -25,7 +25,7 @@ import fr.acinq.eclair.channel.LocalFundingStatus.DualFundedUnconfirmedFundingTx
 import fr.acinq.eclair.channel._
 import fr.acinq.eclair.channel.fsm.Channel.BITCOIN_FUNDING_DOUBLE_SPENT
 import fr.acinq.eclair.channel.fund.InteractiveTxBuilder._
-import fr.acinq.eclair.wire.protocol.Error
+import fr.acinq.eclair.wire.protocol.{ChannelReady, Error}
 
 import scala.concurrent.Future
 import scala.util.{Failure, Success}
@@ -59,6 +59,22 @@ trait DualFundingHandlers extends CommonFundingHandlers {
     }
   }
 
+  /** Return true if we should stop waiting for confirmations when receiving our peer's channel_ready. */
+  def switchToZeroConf(remoteChannelReady: ChannelReady, d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED): Boolean = d.latestFundingTx.fundingParams.minDepth_opt match {
+    case Some(_) =>
+      // We're not using zero-conf, but our peer decided to trust us anyway. We can skip waiting for confirmations if:
+      //  - they provided a channel alias
+      //  - there is a single version of the funding tx (otherwise we don't know which one to use)
+      //  - they didn't contribute to the funding transaction (and thus cannot double-spend it)
+      remoteChannelReady.alias_opt.isDefined &&
+        d.commitments.active.size == 1 &&
+        d.latestFundingTx.sharedTx.tx.remoteInputs.isEmpty
+    case None =>
+      // We're already using zero-conf, but our peer was very fast and we received their channel_ready before our
+      // watcher notification that the funding tx has been successfully published.
+      false
+  }
+
   def handleNewBlockDualFundingUnconfirmed(c: CurrentBlockHeight, d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED) = {
     // We regularly notify the node operator that they may want to RBF this channel.
     val blocksSinceOpen = c.blockHeight - d.waitingSince

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/c/WaitForDualFundingConfirmedStateSpec.scala

@@ -19,6 +19,7 @@ package fr.acinq.eclair.channel.states.c
 import akka.actor.Status
 import akka.actor.typed.scaladsl.adapter.actorRefAdapter
 import akka.testkit.{TestFSMRef, TestProbe}
+import com.softwaremill.quicklens.{ModifyPimp, QuicklensAt}
 import fr.acinq.bitcoin.scalacompat.{ByteVector32, SatoshiLong, Transaction}
 import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._
 import fr.acinq.eclair.blockchain.{CurrentBlockHeight, SingleKeyOnChainWallet}
@@ -521,6 +522,7 @@ class WaitForDualFundingConfirmedStateSpec extends TestKitBaseClass with Fixture
     assert(channelReady.alias_opt.isDefined)
     bob2alice.forward(alice)
     alice2bob.expectNoMessage(100 millis)
+    alice2blockchain.expectNoMessage(100 millis)
     awaitCond(alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].deferred.contains(channelReady))
     awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED)
     awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_READY)
@@ -544,6 +546,33 @@ class WaitForDualFundingConfirmedStateSpec extends TestKitBaseClass with Fixture
     awaitCond(alice.stateName == NORMAL)
   }
 
+  test("recv ChannelReady (initiator, no remote contribution, with remote inputs)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
+    import f._
+    // We test the following scenario:
+    //  - Bob doesn't contribute to the channel funding output
+    //  - But Bob adds inputs to the interactive-tx transaction
+    //  - And sends an early channel_ready to try to fool us into using zero-conf
+    // We don't have code to contribute to an interactive-tx without contributing to the funding output, so we tweak
+    // internal channel data to simulate it.
+    val aliceData = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED]
+    val fundingTx1 = aliceData.latestFundingTx.copy(fundingParams = aliceData.latestFundingTx.fundingParams.copy(remoteAmount = 0 sat))
+    val aliceData1 = aliceData
+      .modify(_.commitments.active.at(0).localFundingStatus)
+      .setTo(fundingTx1)
+    alice.setState(alice.stateName, aliceData1)
+    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].latestFundingTx.fundingParams.minDepth_opt.nonEmpty)
+    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].latestFundingTx.fundingParams.remoteAmount == 0.sat)
+    assert(alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].latestFundingTx.sharedTx.tx.remoteInputs.nonEmpty)
+    bob ! WatchFundingConfirmedTriggered(BlockHeight(42000), 42, fundingTx1.sharedTx.asInstanceOf[FullySignedSharedTransaction].signedTx)
+    val channelReady = bob2alice.expectMsgType[ChannelReady]
+    assert(channelReady.alias_opt.isDefined)
+    bob2alice.forward(alice)
+    alice2bob.expectNoMessage(100 millis)
+    alice2blockchain.expectNoMessage(100 millis)
+    awaitCond(alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].deferred.contains(channelReady))
+    awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED)
+  }
+
   test("recv ChannelReady (non-initiator)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
     import f._
     val fundingTx = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].latestFundingTx.sharedTx.asInstanceOf[FullySignedSharedTransaction].signedTx
@@ -552,6 +581,7 @@ class WaitForDualFundingConfirmedStateSpec extends TestKitBaseClass with Fixture
     assert(channelReady.alias_opt.isDefined)
     alice2bob.forward(bob)
     bob2alice.expectNoMessage(100 millis)
+    bob2blockchain.expectNoMessage(100 millis)
     awaitCond(bob.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED].deferred.contains(channelReady))
     awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED)
     awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_READY)