Re-work my_current_funding_locked reestablish

We introduce a `retransmit_flags` field to ask our peer to retransmit
`announcement_signatures` if we're expecting them: this way we don't
need to retransmit `splice_locked` on reconnection to trigger the
exchange of `announcement_signatures`. We don't need to retransmit it
to let our peer know that we've seen enough confirmations for the splice
either, since `my_current_funding_locked` implies that.

This allows us to completely remove retransmission of `splice_locked` on
reconnection, and also get rid of the `your_last_funding_locked` TLV,
which greatly simplifies the reconnection logic.

This will work with taproot channels since we'll simply provide nonces
in `channel_reestablish` when we need our peer to send announcement
signatures.

Note that we use a different TLV type to more easily allow migrating
existing users of the previous versions of the spec to the latest one.

Author: t-bast

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

@@ -236,8 +236,6 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
   var announcementSigsStash = Map.empty[RealShortChannelId, AnnouncementSignatures]
   // we record the announcement_signatures messages we already sent to avoid unnecessary retransmission
   var announcementSigsSent = Set.empty[RealShortChannelId]
-  // we keep track of the splice_locked we sent after channel_reestablish and it's funding tx index to avoid sending it again
-  private var spliceLockedSent = Map.empty[TxId, Long]
 
   private def trimAnnouncementSigsStashIfNeeded(): Unit = {
     if (announcementSigsStash.size >= 10) {
@@ -249,17 +247,6 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
     }
   }
 
-  private def trimSpliceLockedSentIfNeeded(): Unit = {
-    if (spliceLockedSent.size >= 10) {
-      // We shouldn't store an unbounded number of splice_locked: on long-lived connections where we do a lot of splice
-      // transactions, we only need to keep track of the most recent ones.
-      val oldestFundingTxId = spliceLockedSent.toSeq
-        .sortBy { case (_, fundingTxIndex) => fundingTxIndex }
-        .map { case (fundingTxId, _) => fundingTxId }.head
-      spliceLockedSent -= oldestFundingTxId
-    }
-  }
-
   val txPublisher = txPublisherFactory.spawnTxPublisher(context, remoteNodeId)
 
   // this will be used to detect htlc timeouts
@@ -1427,8 +1414,6 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
           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)
-          spliceLockedSent += (commitment.fundingTxId -> commitment.fundingTxIndex)
-          trimSpliceLockedSentIfNeeded()
           stay() using d.copy(commitments = commitments1) storing() sending SpliceLocked(d.channelId, w.tx.txid)
         case Left(_) => stay()
       }
@@ -1439,11 +1424,7 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
           // We check if this commitment was already locked before receiving the event (which happens when using 0-conf
           // or for the initial funding transaction). If it was previously not locked, we must send splice_locked now.
           val previouslyNotLocked = d.commitments.all.exists(c => c.fundingTxId == commitment.fundingTxId && c.localFundingStatus.isInstanceOf[LocalFundingStatus.NotLocked])
-          val spliceLocked_opt = if (previouslyNotLocked) {
-            spliceLockedSent += (commitment.fundingTxId -> commitment.fundingTxIndex)
-            trimSpliceLockedSentIfNeeded()
-            Some(SpliceLocked(d.channelId, w.tx.txid))
-          } else None
+          val spliceLocked_opt = if (previouslyNotLocked) Some(SpliceLocked(d.channelId, w.tx.txid)) else None
           // If the channel is public and we've received the remote splice_locked, we send our announcement_signatures
           // in order to generate the channel_announcement.
           val remoteLocked = commitment.fundingTxIndex == 0 || d.commitments.all.exists(c => c.fundingTxId == commitment.fundingTxId && c.remoteFundingStatus == RemoteFundingStatus.Locked)
@@ -1466,21 +1447,6 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
     case Event(msg: SpliceLocked, d: DATA_NORMAL) =>
       d.commitments.updateRemoteFundingStatus(msg.fundingTxId, d.lastAnnouncedFundingTxId_opt) match {
         case Right((commitments1, commitment)) =>
-          // If we have both already sent splice_locked for this commitment, then we are receiving splice_locked
-          // again after a reconnection and must retransmit our splice_locked and new announcement_signatures. Nodes
-          // retransmit splice_locked after a reconnection when they have received splice_locked but NOT matching signatures
-          // before the last disconnect. If a matching splice_locked has already been sent since reconnecting, then do not
-          // retransmit splice_locked to avoid a loop.
-          // NB: It is important both nodes retransmit splice_locked after reconnecting to ensure new Taproot nonces
-          // are exchanged for channel announcements.
-          val isLatestLocked = d.commitments.lastLocalLocked_opt.exists(_.fundingTxId == msg.fundingTxId) && d.commitments.lastRemoteLocked_opt.exists(_.fundingTxId == msg.fundingTxId)
-          val spliceLocked_opt = if (d.commitments.announceChannel && isLatestLocked && !spliceLockedSent.contains(commitment.fundingTxId)) {
-            spliceLockedSent += (commitment.fundingTxId -> commitment.fundingTxIndex)
-            trimSpliceLockedSentIfNeeded()
-            Some(SpliceLocked(d.channelId, commitment.fundingTxId))
-          } else {
-            None
-          }
           // If the commitment is confirmed, we were waiting to receive the remote splice_locked before sending our announcement_signatures.
           val localAnnSigs_opt = commitment.signAnnouncement(nodeParams, commitments1.channelParams, channelKeys.fundingKey(commitment.fundingTxIndex)) match {
             case Some(localAnnSigs) if !announcementSigsSent.contains(localAnnSigs.shortChannelId) =>
@@ -1493,7 +1459,7 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
           }
           maybeEmitEventsPostSplice(d.aliases, d.commitments, commitments1, d.lastAnnouncement_opt)
           maybeUpdateMaxHtlcAmount(d.channelUpdate.htlcMaximumMsat, commitments1)
-          stay() using d.copy(commitments = commitments1) storing() sending spliceLocked_opt.toSeq ++ localAnnSigs_opt.toSeq
+          stay() using d.copy(commitments = commitments1) storing() sending localAnnSigs_opt.toSeq
         case Left(_) => stay()
       }
 
@@ -2394,8 +2360,14 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
       }
       val remoteFeatures = d.commitments.remoteChannelParams.initFeatures
       val lastFundingLockedTlvs: Set[ChannelReestablishTlv] = if (remoteFeatures.hasFeature(Features.Splicing) || remoteFeatures.unknown.contains(UnknownFeature(154)) || remoteFeatures.unknown.contains(UnknownFeature(155))) {
-        d.commitments.lastLocalLocked_opt.map(c => ChannelReestablishTlv.MyCurrentFundingLockedTlv(c.fundingTxId)).toSet ++
-          d.commitments.lastRemoteLocked_opt.map(c => ChannelReestablishTlv.YourLastFundingLockedTlv(c.fundingTxId)).toSet
+        d.commitments.lastLocalLocked_opt.map(c => {
+          // We ask our peer to retransmit their announcement_signatures if we haven't already announced that splice.
+          val retransmitAnnSigs = d match {
+            case d: DATA_NORMAL if d.commitments.announceChannel => !d.lastAnnouncedFundingTxId_opt.contains(c.fundingTxId)
+            case _ => false
+          }
+          ChannelReestablishTlv.MyCurrentFundingLockedTlv(c.fundingTxId, retransmitAnnSigs)
+        }).toSet
       } else Set.empty
 
       val channelReestablish = ChannelReestablish(
@@ -2522,22 +2494,18 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
             // We only send channel_ready for initial funding transactions.
             case Some(c) if c.fundingTxIndex != 0 => ()
             case Some(c) =>
-              val remoteFeatures = d.commitments.remoteChannelParams.initFeatures
-              val remoteSpliceSupport = remoteFeatures.hasFeature(Features.Splicing) || remoteFeatures.unknown.contains(UnknownFeature(154)) || remoteFeatures.unknown.contains(UnknownFeature(155))
-              // If our peer has not received our channel_ready, we retransmit it.
-              val notReceivedByRemote = remoteSpliceSupport && channelReestablish.yourLastFundingLocked_opt.isEmpty
               // If next_local_commitment_number is 1 in both the channel_reestablish it sent and received, then the node
               // MUST retransmit channel_ready, otherwise it MUST NOT
-              val notReceivedByRemoteLegacy = !remoteSpliceSupport && channelReestablish.nextLocalCommitmentNumber == 1 && c.localCommit.index == 0
+              val notReceivedByRemote = channelReestablish.nextLocalCommitmentNumber == 1 && c.localCommit.index == 0
               // If this is a public channel and we haven't announced the channel, we retransmit our channel_ready and
               // will also send announcement_signatures.
               val notAnnouncedYet = d.commitments.announceChannel && c.shortChannelId_opt.nonEmpty && d.lastAnnouncement_opt.isEmpty
-              if (notAnnouncedYet || notReceivedByRemote || notReceivedByRemoteLegacy) {
+              if (notAnnouncedYet || notReceivedByRemote) {
                 log.debug("re-sending channel_ready")
                 val nextPerCommitmentPoint = channelKeys.commitmentPoint(1)
                 sendQueue = sendQueue :+ ChannelReady(d.commitments.channelId, nextPerCommitmentPoint)
               }
-              if (notAnnouncedYet) {
+              if (notAnnouncedYet || channelReestablish.retransmitAnnSigs) {
                 // The funding transaction is confirmed, so we've already sent our announcement_signatures.
                 // We haven't announced the channel yet, which means we haven't received our peer's announcement_signatures.
                 // We retransmit our announcement_signatures to let our peer know that we're ready to announce the channel.
@@ -2600,23 +2568,19 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
             // We then clean up unsigned updates that haven't been received before the disconnection.
             .discardUnsignedUpdates()
 
+          // We retransmit our latest announcement_signatures if our peer requests it.
           commitments1.lastLocalLocked_opt match {
             case None => ()
-            // We only send splice_locked for splice transactions.
+            // This retransmit mechanism is only available for splice transactions.
             case Some(c) if c.fundingTxIndex == 0 => ()
             case Some(c) =>
-              // If our peer has not received our splice_locked, we retransmit it.
-              val notReceivedByRemote = !channelReestablish.yourLastFundingLocked_opt.contains(c.fundingTxId)
-              // If this is a public channel and we haven't announced the splice, we retransmit our splice_locked and
-              // will exchange announcement_signatures afterwards.
-              val notAnnouncedYet = commitments1.announceChannel && d.lastAnnouncement_opt.forall(ann => !c.shortChannelId_opt.contains(ann.shortChannelId))
-              if (notReceivedByRemote || notAnnouncedYet) {
-                // Retransmission of local announcement_signatures for splices are done when receiving splice_locked, no need
-                // to retransmit here.
-                log.debug("re-sending splice_locked for fundingTxId={}", c.fundingTxId)
-                spliceLockedSent += (c.fundingTxId -> c.fundingTxIndex)
-                trimSpliceLockedSentIfNeeded()
-                sendQueue = sendQueue :+ SpliceLocked(d.channelId, c.fundingTxId)
+              if (channelReestablish.retransmitAnnSigs && d.commitments.announceChannel) {
+                val localAnnSigs = c.signAnnouncement(nodeParams, d.commitments.channelParams, channelKeys.fundingKey(c.fundingTxIndex))
+                localAnnSigs.foreach(annSigs => {
+                  log.debug("re-sending announcement_signatures for fundingTxId={}", c.fundingTxId)
+                  announcementSigsSent += annSigs.shortChannelId
+                  sendQueue = sendQueue :+ annSigs
+                })
               }
           }
 
@@ -3108,7 +3072,6 @@ class Channel(val nodeParams: NodeParams, val channelKeys: ChannelKeys, val wall
     case _ -> OFFLINE =>
       announcementSigsStash = Map.empty
       announcementSigsSent = Set.empty
-      spliceLockedSent = Map.empty[TxId, Long]
   }
 
   /*

eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala

@@ -436,13 +436,13 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A
             }
           // If our peer is using the experimental splice version, we convert splice messages.
           case msg: ExperimentalSpliceInit =>
-            d.peer ! msg.toSpliceInit()
+            d.peer ! msg.toSpliceInit
             stay()
           case msg: ExperimentalSpliceAck =>
-            d.peer ! msg.toSpliceAck()
+            d.peer ! msg.toSpliceAck
             stay()
           case msg: ExperimentalSpliceLocked =>
-            d.peer ! msg.toSpliceLocked()
+            d.peer ! msg.toSpliceLocked
             stay()
           case _ =>
             d.peer ! msg

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/ChannelTlv.scala

@@ -240,28 +240,23 @@ object ChannelReestablishTlv {
    */
   case class NextFundingTlv(txId: TxId) extends ChannelReestablishTlv
 
-  /** The txid of the last [[ChannelReady]] or [[SpliceLocked]] message received before disconnecting, if any. */
-  case class YourLastFundingLockedTlv(txId: TxId) extends ChannelReestablishTlv
-
-  /** The txid of our latest outgoing [[ChannelReady]] or [[SpliceLocked]] for this channel. */
-  case class MyCurrentFundingLockedTlv(txId: TxId) extends ChannelReestablishTlv
+  /**
+   * @param txId              the txid of our latest outgoing [[ChannelReady]] or [[SpliceLocked]] for this channel.
+   * @param retransmitAnnSigs true if [[AnnouncementSignatures]] must be retransmitted.
+   */
+  case class MyCurrentFundingLockedTlv(txId: TxId, retransmitAnnSigs: Boolean) extends ChannelReestablishTlv
 
   object NextFundingTlv {
     val codec: Codec[NextFundingTlv] = tlvField(txIdAsHash)
   }
 
-  object YourLastFundingLockedTlv {
-    val codec: Codec[YourLastFundingLockedTlv] = tlvField("your_last_funding_locked_txid" | txIdAsHash)
-  }
-
   object MyCurrentFundingLockedTlv {
-    val codec: Codec[MyCurrentFundingLockedTlv] = tlvField("my_current_funding_locked_txid" | txIdAsHash)
+    val codec: Codec[MyCurrentFundingLockedTlv] = tlvField(("my_current_funding_locked_txid" | txIdAsHash) :: ("retransmit_flags" | (ignore(7) :: bool)))
   }
 
   val channelReestablishTlvCodec: Codec[TlvStream[ChannelReestablishTlv]] = tlvStream(discriminated[ChannelReestablishTlv].by(varint)
     .typecase(UInt64(0), NextFundingTlv.codec)
-    .typecase(UInt64(1), YourLastFundingLockedTlv.codec)
-    .typecase(UInt64(3), MyCurrentFundingLockedTlv.codec)
+    .typecase(UInt64(5), MyCurrentFundingLockedTlv.codec)
   )
 }
 

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala

@@ -196,7 +196,7 @@ case class ChannelReestablish(channelId: ByteVector32,
                               tlvStream: TlvStream[ChannelReestablishTlv] = TlvStream.empty) extends ChannelMessage with HasChannelId {
   val nextFundingTxId_opt: Option[TxId] = tlvStream.get[ChannelReestablishTlv.NextFundingTlv].map(_.txId)
   val myCurrentFundingLocked_opt: Option[TxId] = tlvStream.get[ChannelReestablishTlv.MyCurrentFundingLockedTlv].map(_.txId)
-  val yourLastFundingLocked_opt: Option[TxId] = tlvStream.get[ChannelReestablishTlv.YourLastFundingLockedTlv].map(_.txId)
+  val retransmitAnnSigs: Boolean = tlvStream.get[ChannelReestablishTlv.MyCurrentFundingLockedTlv].exists(_.retransmitAnnSigs)
 }
 
 case class OpenChannel(chainHash: BlockHash,
@@ -343,7 +343,7 @@ case class ExperimentalSpliceInit(channelId: ByteVector32,
                                   lockTime: Long,
                                   fundingPubKey: PublicKey,
                                   tlvStream: TlvStream[SpliceInitTlv] = TlvStream.empty) extends ChannelMessage with HasChannelId {
-  def toSpliceInit(): SpliceInit = SpliceInit(channelId, fundingContribution, feerate, lockTime, fundingPubKey, tlvStream)
+  def toSpliceInit: SpliceInit = SpliceInit(channelId, fundingContribution, feerate, lockTime, fundingPubKey, tlvStream)
 }
 
 object ExperimentalSpliceInit {
@@ -375,7 +375,7 @@ case class ExperimentalSpliceAck(channelId: ByteVector32,
                                  fundingContribution: Satoshi,
                                  fundingPubKey: PublicKey,
                                  tlvStream: TlvStream[SpliceAckTlv] = TlvStream.empty) extends ChannelMessage with HasChannelId {
-  def toSpliceAck(): SpliceAck = SpliceAck(channelId, fundingContribution, fundingPubKey, tlvStream)
+  def toSpliceAck: SpliceAck = SpliceAck(channelId, fundingContribution, fundingPubKey, tlvStream)
 }
 
 object ExperimentalSpliceAck {
@@ -390,7 +390,7 @@ case class SpliceLocked(channelId: ByteVector32,
 case class ExperimentalSpliceLocked(channelId: ByteVector32,
                                     fundingTxId: TxId,
                                     tlvStream: TlvStream[SpliceLockedTlv] = TlvStream.empty) extends ChannelMessage with HasChannelId {
-  def toSpliceLocked(): SpliceLocked = SpliceLocked(channelId, fundingTxId, tlvStream)
+  def toSpliceLocked: SpliceLocked = SpliceLocked(channelId, fundingTxId, tlvStream)
 }
 
 object ExperimentalSpliceLocked {

eclair-core/src/test/scala/fr/acinq/eclair/channel/RestoreSpec.scala

@@ -12,7 +12,7 @@ import fr.acinq.eclair.channel.fsm.Channel
 import fr.acinq.eclair.channel.states.ChannelStateTestsBase.FakeTxPublisherFactory
 import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags}
 import fr.acinq.eclair.router.Announcements
-import fr.acinq.eclair.wire.protocol.{ChannelReestablish, ChannelUpdate, CommitSig, Error, Init, RevokeAndAck}
+import fr.acinq.eclair.wire.protocol.{ChannelReady, ChannelReestablish, ChannelUpdate, CommitSig, Error, Init, RevokeAndAck}
 import fr.acinq.eclair.{TestKitBaseClass, _}
 import org.scalatest.funsuite.FixtureAnyFunSuiteLike
 import org.scalatest.{Outcome, Tag}
@@ -197,14 +197,15 @@ class RestoreSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with Chan
       assert(u1.channelUpdate.feeProportionalMillionths == newConfig.relayParams.privateChannelFees.feeProportionalMillionths)
       assert(u1.channelUpdate.cltvExpiryDelta == newConfig.channelConf.expiryDelta)
 
+      alice2bob.ignoreMsg { case _: ChannelUpdate | _: ChannelReady => true }
+      bob2alice.ignoreMsg { case _: ChannelUpdate | _: ChannelReady => true }
+
       newAlice ! INPUT_RECONNECTED(alice2bob.ref, aliceInit, bobInit)
       bob ! INPUT_RECONNECTED(bob2alice.ref, bobInit, aliceInit)
       alice2bob.expectMsgType[ChannelReestablish]
       bob2alice.expectMsgType[ChannelReestablish]
       alice2bob.forward(bob)
       bob2alice.forward(newAlice)
-      alice2bob.expectMsgType[ChannelUpdate]
-      bob2alice.expectMsgType[ChannelUpdate]
       alice2bob.expectNoMessage()
       bob2alice.expectNoMessage()
 

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

@@ -238,18 +238,16 @@ class WaitForDualFundingReadyStateSpec extends TestKitBaseClass with FixtureAnyF
     val bobInit = Init(TestConstants.Bob.nodeParams.features.initFeatures())
     alice ! INPUT_RECONNECTED(alice2bob.ref, aliceInit, bobInit)
     bob ! INPUT_RECONNECTED(bob2alice.ref, bobInit, aliceInit)
-    alice2bob.expectMsgType[ChannelReestablish]
+    assert(alice2bob.expectMsgType[ChannelReestablish].retransmitAnnSigs)
     alice2bob.forward(bob)
-    bob2alice.expectMsgType[ChannelReestablish]
+    assert(!bob2alice.expectMsgType[ChannelReestablish].retransmitAnnSigs)
     bob2alice.forward(alice)
-    // Bob does not retransmit channel_ready and announcement_signatures because he has already received both of them from Alice.
-    bob2alice.expectNoMessage(100 millis)
-    // Alice has already received Bob's channel_ready, but not its announcement_signatures.
-    // She retransmits channel_ready and Bob will retransmit its announcement_signatures in response.
     alice2bob.expectMsgType[ChannelReady]
     alice2bob.forward(bob)
     alice2bob.expectMsgType[AnnouncementSignatures]
     alice2bob.forward(bob)
+    bob2alice.expectMsgType[ChannelReady]
+    bob2alice.forward(alice)
     bob2alice.expectMsgType[AnnouncementSignatures]
     bob2alice.forward(alice)
     awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].lastAnnouncement_opt.nonEmpty)