Fix nits/issues found by @t-bast

Also fixed two other issues:
 - add the splice channel to the routing graph as a new channel if the parent channel does not exist (could be pruned)
 - when a splice confirms, also send ChannelLost to remove the parent scid from the FrontRouter

Author: remyers

eclair-core/src/main/scala/fr/acinq/eclair/router/Router.scala

@@ -66,7 +66,6 @@ class Router(val nodeParams: NodeParams, watcher: typed.ActorRef[ZmqWatcher.Comm
   context.system.eventStream.subscribe(self, classOf[LocalChannelUpdate])
   context.system.eventStream.subscribe(self, classOf[LocalChannelDown])
   context.system.eventStream.subscribe(self, classOf[AvailableBalanceChanged])
-  context.system.eventStream.subscribe(self, classOf[CurrentBlockHeight])
   context.system.eventStream.publish(SubscriptionsComplete(this.getClass))
 
   startTimerWithFixedDelay(TickBroadcast.toString, TickBroadcast, nodeParams.routerConf.routerBroadcastInterval)
@@ -265,7 +264,7 @@ class Router(val nodeParams: NodeParams, watcher: typed.ActorRef[ZmqWatcher.Comm
 
     case Event(WatchExternalChannelSpentTriggered(shortChannelId, spendingTx), d) if d.channels.contains(shortChannelId) || d.prunedChannels.contains(shortChannelId) =>
       val txId = d.channels.getOrElse(shortChannelId, d.prunedChannels(shortChannelId)).fundingTxId
-      log.info("funding tx txId={} of channelId={} has been spent - delay removing it from the graph until {} blocks after the spend confirms", txId, shortChannelId, ANNOUNCEMENTS_MINCONF * 2)
+      log.info("funding tx txId={} of channelId={} has been spent by txId={}: waiting for the spending tx to have enough confirmations before removing the channel from the graph", txId, shortChannelId, spendingTx.txid)
       watcher ! WatchTxConfirmed(self, spendingTx.txid, ANNOUNCEMENTS_MINCONF * 2)
       stay() using d.copy(spentChannels = d.spentChannels + (spendingTx.txid -> shortChannelId))
 

eclair-core/src/main/scala/fr/acinq/eclair/router/Validation.scala

@@ -31,10 +31,9 @@ import fr.acinq.eclair.db.NetworkDb
 import fr.acinq.eclair.router.Graph.GraphStructure.GraphEdge
 import fr.acinq.eclair.router.Monitoring.Metrics
 import fr.acinq.eclair.router.Router._
-import fr.acinq.eclair.router.Validation.{addPublicChannel, splicePublicChannel}
 import fr.acinq.eclair.transactions.Scripts
 import fr.acinq.eclair.wire.protocol._
-import fr.acinq.eclair.{BlockHeight, Logs, MilliSatoshiLong, NodeParams, RealShortChannelId, ShortChannelId, TxCoordinates}
+import fr.acinq.eclair.{BlockHeight, Logs, MilliSatoshiLong, NodeParams, RealShortChannelId, ShortChannelId, TimestampSecond, TxCoordinates}
 
 object Validation {
 
@@ -115,7 +114,15 @@ object Validation {
             remoteOrigins.foreach(o => sendDecision(o.peerConnection, GossipDecision.Accepted(c)))
             val capacity = tx.txOut(outputIndex).amount
             d0.spentChannels.get(tx.txid) match {
-              case Some(parentScid) => Some(splicePublicChannel(d0, nodeParams, watcher, c, tx.txid, capacity, d0.channels(parentScid)))
+              case Some(parentScid) =>
+                d0.channels.get(parentScid) match {
+                  case Some(parentChannel) =>
+                    Some(updateSplicedPublicChannel(d0, nodeParams, watcher, c, tx.txid, capacity, parentChannel))
+                  case None =>
+                    log.error("spent parent channel shortChannelId={} not found for splice shortChannelId={}", parentScid, c.shortChannelId)
+                    val spentChannels1 = d0.spentChannels.filter(_._2 != parentScid)
+                    Some(addPublicChannel(d0, nodeParams, watcher, c, tx.txid, capacity, None).copy(spentChannels = spentChannels1))
+                }
               case None => Some(addPublicChannel(d0, nodeParams, watcher, c, tx.txid, capacity, None))
             }
           }
@@ -160,35 +167,40 @@ object Validation {
     }
   }
 
-  private def splicePublicChannel(d: Data, nodeParams: NodeParams, watcher: typed.ActorRef[ZmqWatcher.Command], ann: ChannelAnnouncement, spliceTxId: TxId, capacity: Satoshi, parentChannel: PublicChannel)(implicit ctx: ActorContext, log: DiagnosticLoggingAdapter): Data = {
+  private def updateSplicedPublicChannel(d: Data, nodeParams: NodeParams, watcher: typed.ActorRef[ZmqWatcher.Command], ann: ChannelAnnouncement, spliceTxId: TxId, capacity: Satoshi, parentChannel: PublicChannel)(implicit ctx: ActorContext, log: DiagnosticLoggingAdapter): Data = {
     implicit val sender: ActorRef = ctx.self // necessary to preserve origin when sending messages to other actors
     val fundingOutputIndex = outputIndex(ann.shortChannelId)
     watcher ! WatchExternalChannelSpent(ctx.self, spliceTxId, fundingOutputIndex, ann.shortChannelId)
+    // we notify front nodes that the channel has been replaced
     ctx.system.eventStream.publish(ChannelsDiscovered(SingleChannelDiscovered(ann, capacity, None, None) :: Nil))
+    ctx.system.eventStream.publish(ChannelLost(parentChannel.shortChannelId))
     nodeParams.db.network.addChannel(ann, spliceTxId, capacity)
     nodeParams.db.network.removeChannel(parentChannel.shortChannelId)
-    val pubChan = PublicChannel(
+    val newPubChan = parentChannel.copy(
       ann = ann,
       fundingTxId = spliceTxId,
       capacity = capacity,
-      update_1_opt = None,
-      update_2_opt = None,
-      meta_opt = parentChannel.meta_opt
+      // update the timestamps of the channel updates to ensure the spliced channel is not pruned
+      update_1_opt = parentChannel.update_1_opt.map(_.copy(shortChannelId = ann.shortChannelId, timestamp = TimestampSecond.now())),
+      update_2_opt = parentChannel.update_2_opt.map(_.copy(shortChannelId = ann.shortChannelId, timestamp = TimestampSecond.now())),
     )
-    log.debug("replacing parent channel scid={} with splice channel scid={}; splice channel={}", parentChannel.shortChannelId, ann.shortChannelId, pubChan)
+    log.debug("replacing parent channel scid={} with splice channel scid={}; splice channel={}", parentChannel.shortChannelId, ann.shortChannelId, newPubChan)
     // we need to update the graph because the edge identifiers and capacity change from the parent scid to the new splice scid
-    log.debug("updating the graph for shortChannelId={}", pubChan.shortChannelId)
+    log.debug("updating the graph for shortChannelId={}", newPubChan.shortChannelId)
     val graph1 = d.graphWithBalances.updateChannel(ChannelDesc(parentChannel.shortChannelId, parentChannel.nodeId1, parentChannel.nodeId2), ann.shortChannelId, capacity)
+    val spentChannels1 = d.spentChannels.filter(_._2 != parentChannel.shortChannelId)
     d.copy(
       // we also add the splice scid -> channelId and remove the parent scid -> channelId mappings
-      channels = d.channels + (pubChan.shortChannelId -> pubChan) - parentChannel.shortChannelId,
+      channels = d.channels + (newPubChan.shortChannelId -> newPubChan) - parentChannel.shortChannelId,
+      // remove the parent channel from the pruned channels
+      prunedChannels = d.prunedChannels - parentChannel.shortChannelId,
       // we also add the newly validated channels to the rebroadcast queue
       rebroadcast = d.rebroadcast.copy(
         // we rebroadcast the splice channel to our peers
-        channels = d.rebroadcast.channels + (pubChan.ann -> d.awaiting.getOrElse(pubChan.ann, if (pubChan.nodeId1 == nodeParams.nodeId || pubChan.nodeId2 == nodeParams.nodeId) Seq(LocalGossip) else Nil).toSet),
+        channels = d.rebroadcast.channels + (newPubChan.ann -> d.awaiting.getOrElse(newPubChan.ann, if (isRelatedTo(ann, nodeParams.nodeId)) Seq(LocalGossip) else Nil).toSet),
       ),
       graphWithBalances = graph1,
-      spentChannels = d.spentChannels.filter(_._2 != parentChannel.shortChannelId)
+      spentChannels = spentChannels1
     )
   }
 
@@ -272,7 +284,11 @@ object Validation {
         db.removeNode(nodeId)
         ctx.system.eventStream.publish(NodeLost(nodeId))
     }
-    d.copy(nodes = d.nodes -- lostNodes, channels = channels1, prunedChannels = prunedChannels1, graphWithBalances = graphWithBalances1, spentChannels = d.spentChannels.filter(_._2 != shortChannelId))
+    // we no longer need to track this or alternative transactions that spent the parent channel
+    // either this channel was really closed, or it was spliced and the announcement was not received in time
+    // we will re-add a spliced channel as a new channel later when we receive the announcement
+    val spentChannels1 = d.spentChannels.filter(_._2 != shortChannelId)
+    d.copy(nodes = d.nodes -- lostNodes, channels = channels1, prunedChannels = prunedChannels1, graphWithBalances = graphWithBalances1, spentChannels = spentChannels1)
   }
 
   def handleNodeAnnouncement(d: Data, db: NetworkDb, origins: Set[GossipOrigin], n: NodeAnnouncement, wasStashed: Boolean = false)(implicit ctx: ActorContext, log: LoggingAdapter): Data = {

eclair-core/src/test/scala/fr/acinq/eclair/router/RouterSpec.scala

@@ -1146,6 +1146,7 @@ class RouterSpec extends BaseRouterSpec {
     // The channel update for the splice is confirmed and the channel is not removed.
     router ! WatchTxConfirmedTriggered(BlockHeight(0), 0, spendingTx(funding_a, funding_b))
     eventListener.expectMsg(ChannelsDiscovered(SingleChannelDiscovered(spliceAnn, newCapacity1, None, None) :: Nil))
+    eventListener.expectMsg(ChannelLost(scid_ab))
     peerConnection.expectNoMessage(100 millis)
     eventListener.expectNoMessage(100 millis)