Remove watch-confirmed with depth 0 in zero-conf (#2310)

Instead of using a watch with minDepth=0, we can directly skip the
wait_for_funding_confirmed state when using 0-conf, which is less hacky.

Co-authored-by: pm47 <pm.padiou@gmail.com>

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

@@ -39,7 +39,7 @@ case class UnspecifiedShortChannelId(private val id: Long) extends ShortChannelI
   override def toLong: Long = id
   override def toString: String = toCoordinatesString // for backwards compatibility, because ChannelUpdate have an unspecified scid
 }
-case class RealShortChannelId private (private val id: Long) extends ShortChannelId {
+case class RealShortChannelId private(private val id: Long) extends ShortChannelId {
   override def toLong: Long = id
   override def toString: String = toCoordinatesString
   def blockHeight: BlockHeight = ShortChannelId.blockHeight(this)

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

@@ -20,18 +20,17 @@ import akka.actor.typed.eventstream.EventStream
 import akka.actor.typed.scaladsl.{ActorContext, Behaviors, TimerScheduler}
 import akka.actor.typed.{ActorRef, Behavior, SupervisorStrategy}
 import fr.acinq.bitcoin.scalacompat._
-import fr.acinq.eclair.RealShortChannelId
 import fr.acinq.eclair.blockchain.Monitoring.Metrics
 import fr.acinq.eclair.blockchain._
 import fr.acinq.eclair.blockchain.bitcoind.rpc.BitcoinCoreClient
 import fr.acinq.eclair.blockchain.watchdogs.BlockchainWatchdog
 import fr.acinq.eclair.wire.protocol.ChannelAnnouncement
-import fr.acinq.eclair.{BlockHeight, KamonExt, NodeParams, ShortChannelId, TimestampSecond}
+import fr.acinq.eclair.{BlockHeight, KamonExt, NodeParams, RealShortChannelId, TimestampSecond}
 
 import java.util.concurrent.atomic.AtomicLong
 import scala.concurrent.duration._
 import scala.concurrent.{ExecutionContext, Future}
-import scala.util.{Failure, Random, Success}
+import scala.util.{Failure, Success}
 
 /**
  * Created by PM on 21/02/2016.
@@ -237,11 +236,6 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
             case _: WatchConfirmed[_] => // nothing to do
             case _: WatchFundingLost => // nothing to do
           }
-        watches
-          .collect {
-            case w: WatchFundingConfirmed if w.minDepth == 0 && w.txId == tx.txid =>
-              checkConfirmed(w)
-          }
         Behaviors.same
 
       case ProcessNewBlock(blockHash) =>
@@ -412,21 +406,13 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client
     client.getTxConfirmations(w.txId).flatMap {
       case Some(confirmations) if confirmations >= w.minDepth =>
         client.getTransaction(w.txId).flatMap { tx =>
-          w match {
-            case w: WatchFundingConfirmed if confirmations == 0 =>
-              // if the tx doesn't have confirmations but we don't require any, we reply with a fake block index
-              // otherwise, we get the real short id
-              context.self ! TriggerEvent(w.replyTo, w, WatchFundingConfirmedTriggered(BlockHeight(0), 0, tx))
-              Future.successful((): Unit)
-            case _ =>
-              client.getTransactionShortId(w.txId).map {
-                case (height, index) => w match {
-                  case w: WatchFundingConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchFundingConfirmedTriggered(height, index, tx))
-                  case w: WatchFundingDeeplyBuried => context.self ! TriggerEvent(w.replyTo, w, WatchFundingDeeplyBuriedTriggered(height, index, tx))
-                  case w: WatchTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchTxConfirmedTriggered(height, index, tx))
-                  case w: WatchParentTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchParentTxConfirmedTriggered(height, index, tx))
-                }
-              }
+          client.getTransactionShortId(w.txId).map {
+            case (height, index) => w match {
+              case w: WatchFundingConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchFundingConfirmedTriggered(height, index, tx))
+              case w: WatchFundingDeeplyBuried => context.self ! TriggerEvent(w.replyTo, w, WatchFundingDeeplyBuriedTriggered(height, index, tx))
+              case w: WatchTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchTxConfirmedTriggered(height, index, tx))
+              case w: WatchParentTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchParentTxConfirmedTriggered(height, index, tx))
+            }
           }
         }
       case _ => Future.successful((): Unit)

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

@@ -429,10 +429,14 @@ final case class DATA_WAIT_FOR_FUNDING_CONFIRMED(commitments: Commitments,
 final case class DATA_WAIT_FOR_CHANNEL_READY(commitments: Commitments,
                                              shortIds: ShortIds,
                                              lastSent: ChannelReady) extends PersistentChannelData
+
 sealed trait RealScidStatus { def toOption: Option[RealShortChannelId] }
 object RealScidStatus {
+  /** The funding transaction has been confirmed but hasn't reached min_depth, we must be ready for a reorg. */
   case class Temporary(realScid: RealShortChannelId) extends RealScidStatus { override def toOption: Option[RealShortChannelId] = Some(realScid) }
+  /** The funding transaction has been deeply confirmed. */
   case class Final(realScid: RealShortChannelId) extends RealScidStatus { override def toOption: Option[RealShortChannelId] = Some(realScid) }
+  /** We don't know the status of the funding transaction. */
   case object Unknown extends RealScidStatus { override def toOption: Option[RealShortChannelId] = None }
 }
 
@@ -441,7 +445,7 @@ object RealScidStatus {
  *
  * @param real            the real scid, it may change if a reorg happens before the channel reaches 6 conf
  * @param localAlias      we must remember the alias that we sent to our peer because we use it to:
- *                          - identify incoming [[ChannelUpdate]]
+ *                          - identify incoming [[ChannelUpdate]] at the connection level
  *                          - route outgoing payments to that channel
  * @param remoteAlias_opt we only remember the last alias received from our peer, we use this to generate
  *                        routing hints in [[fr.acinq.eclair.payment.Bolt11Invoice]]

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

@@ -19,10 +19,10 @@ package fr.acinq.eclair.channel
 import akka.actor.ActorRef
 import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
 import fr.acinq.bitcoin.scalacompat.{ByteVector32, Satoshi, Transaction}
-import fr.acinq.eclair.{BlockHeight, Features, Alias, RealShortChannelId, ShortChannelId}
 import fr.acinq.eclair.blockchain.fee.FeeratePerKw
 import fr.acinq.eclair.channel.Helpers.Closing.ClosingType
 import fr.acinq.eclair.wire.protocol.{ChannelAnnouncement, ChannelUpdate}
+import fr.acinq.eclair.{BlockHeight, Features, ShortChannelId}
 
 /**
  * Created by PM on 17/08/2016.
@@ -51,15 +51,18 @@ case class ShortChannelIdAssigned(channel: ActorRef, channelId: ByteVector32, sh
 
 case class LocalChannelUpdate(channel: ActorRef, channelId: ByteVector32, shortIds: ShortIds, remoteNodeId: PublicKey, channelAnnouncement_opt: Option[ChannelAnnouncement], channelUpdate: ChannelUpdate, commitments: AbstractCommitments) extends ChannelEvent {
   /**
-   * We always map the local alias because we must always be able to route based on it
-   * However we only map the real scid if option_scid_alias (TODO: rename to option_scid_privacy) is disabled
+   * We always include the local alias because we must always be able to route based on it.
+   * However we only include the real scid if option_scid_alias is disabled, because we otherwise want to hide it.
    */
   def scidsForRouting: Seq[ShortChannelId] = {
-    commitments match {
-      case c: Commitments =>
-        val realScid_opt = if (c.channelFeatures.hasFeature(Features.ScidAlias)) None else shortIds.real.toOption
-        realScid_opt.toSeq :+ shortIds.localAlias
-      case _ => Seq(shortIds.localAlias) // TODO: ugly
+    val canUseRealScid = commitments match {
+      case c: Commitments => !c.channelFeatures.hasFeature(Features.ScidAlias)
+      case _ => false
+    }
+    if (canUseRealScid) {
+      shortIds.real.toOption.toSeq :+ shortIds.localAlias
+    } else {
+      Seq(shortIds.localAlias)
     }
   }
 }

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

@@ -115,7 +115,7 @@ object ChannelTypes {
     ).flatten
     override def paysDirectlyToWallet: Boolean = false
     override def commitmentFormat: CommitmentFormat = ZeroFeeHtlcTxAnchorOutputsCommitmentFormat
-    override def toString: String = s"anchor_outputs_zero_fee_htlc_tx${if (scidAlias) "+scid_alias" else ""}${if (zeroConf) "+zeroconf" else ""}"
+    override def toString: String = s"anchor_outputs_zero_fee_htlc_tx${if (scidAlias) "+scid_alias" else ""}${if (zeroConf) "+zero_conf" else ""}"
   }
   case class UnsupportedChannelType(featureBits: Features[InitFeature]) extends ChannelType {
     override def features: Set[InitFeature] = featureBits.activated.keySet

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

@@ -21,6 +21,7 @@ import fr.acinq.bitcoin.ScriptFlags
 import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey, sha256}
 import fr.acinq.bitcoin.scalacompat.Script._
 import fr.acinq.bitcoin.scalacompat._
+import fr.acinq.eclair._
 import fr.acinq.eclair.blockchain.OnChainAddressGenerator
 import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeTargets, FeeratePerKw}
 import fr.acinq.eclair.channel.fsm.Channel
@@ -35,7 +36,6 @@ import fr.acinq.eclair.transactions.Scripts._
 import fr.acinq.eclair.transactions.Transactions._
 import fr.acinq.eclair.transactions._
 import fr.acinq.eclair.wire.protocol._
-import fr.acinq.eclair.{RealShortChannelId, _}
 import scodec.bits.ByteVector
 
 import scala.concurrent.duration._
@@ -196,17 +196,17 @@ object Helpers {
    *    - before channel announcement: use remote_alias
    *    - after channel announcement: use real scid
    *  - no remote_alias from peer
-   *    - min_depth > 0 : use real scid (may change if reorg between min_depth and 6 conf)
-   *    - min_depth = 0 (zero-conf) : unsupported
+   *    - min_depth > 0: use real scid (may change if reorg between min_depth and 6 conf)
+   *    - min_depth = 0 (zero-conf): spec violation, our peer MUST send an alias when using zero-conf
    */
-  def scidForChannelUpdate(channelAnnouncement_opt: Option[ChannelAnnouncement], shortIds: ShortIds)(implicit log: DiagnosticLoggingAdapter): ShortChannelId = {
+  def scidForChannelUpdate(channelAnnouncement_opt: Option[ChannelAnnouncement], shortIds: ShortIds): ShortChannelId = {
     channelAnnouncement_opt.map(_.shortChannelId) // we use the real "final" scid when it is publicly announced
       .orElse(shortIds.remoteAlias_opt) // otherwise the remote alias
       .orElse(shortIds.real.toOption) // if we don't have a remote alias, we use the real scid (which could change because the funding tx possibly has less than 6 confs here)
       .getOrElse(throw new RuntimeException("this is a zero-conf channel and no alias was provided in channel_ready")) // if we don't have a real scid, it means this is a zero-conf channel and our peer must have sent an alias
   }
 
-  def scidForChannelUpdate(d: DATA_NORMAL)(implicit log: DiagnosticLoggingAdapter): ShortChannelId = scidForChannelUpdate(d.channelAnnouncement, d.shortIds)
+  def scidForChannelUpdate(d: DATA_NORMAL): ShortChannelId = scidForChannelUpdate(d.channelAnnouncement, d.shortIds)
 
   /**
    * Compute the delay until we need to refresh the channel_update for our channel not to be considered stale by
@@ -291,11 +291,11 @@ object Helpers {
      * wait for one conf, except if the channel has the zero-conf feature (because presumably the peer will send an
      * alias in that case).
      */
-    def minDepthFunder(channelFeatures: ChannelFeatures): Long = {
+    def minDepthFunder(channelFeatures: ChannelFeatures): Option[Long] = {
       if (channelFeatures.hasFeature(Features.ZeroConf)) {
-        0
+        None
       } else {
-        1
+        Some(1)
       }
     }
 
@@ -304,16 +304,16 @@ object Helpers {
      * we make sure the cumulative block reward largely exceeds the channel size.
      *
      * @param fundingSatoshis funding amount of the channel
-     * @return number of confirmations needed
+     * @return number of confirmations needed, if any
      */
-    def minDepthFundee(channelConf: ChannelConf, channelFeatures: ChannelFeatures, fundingSatoshis: Satoshi): Long = fundingSatoshis match {
-      case _ if channelFeatures.hasFeature(Features.ZeroConf) => 0 // zero-conf stay zero-conf, whatever the funding amount is
-      case funding if funding <= Channel.MAX_FUNDING => channelConf.minDepthBlocks
+    def minDepthFundee(channelConf: ChannelConf, channelFeatures: ChannelFeatures, fundingSatoshis: Satoshi): Option[Long] = fundingSatoshis match {
+      case _ if channelFeatures.hasFeature(Features.ZeroConf) => None // zero-conf stay zero-conf, whatever the funding amount is
+      case funding if funding <= Channel.MAX_FUNDING => Some(channelConf.minDepthBlocks)
       case funding =>
         val blockReward = 6.25 // this is true as of ~May 2020, but will be too large after 2024
         val scalingFactor = 15
         val blocksToReachFunding = (((scalingFactor * funding.toBtc.toDouble) / blockReward).ceil + 1).toInt
-        channelConf.minDepthBlocks.max(blocksToReachFunding)
+        Some(channelConf.minDepthBlocks.max(blocksToReachFunding))
     }
 
     def makeFundingInputInfo(fundingTxId: ByteVector32, fundingTxOutputIndex: Int, fundingSatoshis: Satoshi, fundingPubkey1: PublicKey, fundingPubkey2: PublicKey): InputInfo = {

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

@@ -49,7 +49,7 @@ class Register extends Actor with ActorLogging {
 
     case scidAssigned: ShortChannelIdAssigned =>
       // We map all known scids (real or alias) to the channel_id. The relayer is in charge of deciding whether a real
-      // scid can be used or not for routing (see option_scid_privacy), but the register is neutral.
+      // scid can be used or not for routing (see option_scid_alias), but the register is neutral.
       val m = (scidAssigned.shortIds.real.toOption.toSeq :+ scidAssigned.shortIds.localAlias).map(_ -> scidAssigned.channelId).toMap
       context become main(channels, shortIds ++ m, channelsTo)