Don't send splice_locked before tx_signatures (#2741)

When reconnecting in the middle of signing a splice, we must ensure that
splice_locked is sent *after* tx_signatures. Otherwise when using 0-conf
we may retransmit splice_locked before tx_signatures, which our peer will
ignore because they don't have a corresponding fully signed commitment.

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

@@ -1873,26 +1873,14 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
           var sendQueue = Queue.empty[LightningMessage]
           // normal case, our data is up-to-date
 
-          // re-send channel_ready or splice_locked
+          // re-send channel_ready if necessary
           if (d.commitments.latest.fundingTxIndex == 0 && channelReestablish.nextLocalCommitmentNumber == 1 && d.commitments.localCommitIndex == 0) {
             // 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
             log.debug("re-sending channelReady")
             val channelKeyPath = keyManager.keyPath(d.commitments.params.localParams, d.commitments.params.channelConfig)
             val nextPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, 1)
             val channelReady = ChannelReady(d.commitments.channelId, nextPerCommitmentPoint)
             sendQueue = sendQueue :+ channelReady
-          } else {
-            // NB: there is a key difference between channel_ready and splice_confirmed:
-            // - channel_ready: a non-zero commitment index implies that both sides have seen the channel_ready
-            // - splice_confirmed: the commitment index can be updated as long as it is compatible with all splices, so
-            //   we must keep sending our most recent splice_locked at each reconnection
-            val spliceLocked = d.commitments.active
-              .filter(c => c.fundingTxIndex > 0) // only consider splice txs
-              .collectFirst { case c if c.localFundingStatus.isInstanceOf[LocalFundingStatus.Locked] =>
-                log.debug(s"re-sending splice_locked for fundingTxId=${c.fundingTxId}")
-                SpliceLocked(d.channelId, c.fundingTxId.reverse)
-              }
-            sendQueue = sendQueue ++ spliceLocked
           }
 
           // resume splice signing session if any
@@ -1935,6 +1923,19 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
             case None => d.spliceStatus
           }
 
+          // re-send splice_locked (must come *after* potentially retransmitting tx_signatures)
+          // NB: there is a key difference between channel_ready and splice_confirmed:
+          // - channel_ready: a non-zero commitment index implies that both sides have seen the channel_ready
+          // - splice_confirmed: the commitment index can be updated as long as it is compatible with all splices, so
+          //   we must keep sending our most recent splice_locked at each reconnection
+          val spliceLocked = d.commitments.active
+            .filter(c => c.fundingTxIndex > 0) // only consider splice txs
+            .collectFirst { case c if c.localFundingStatus.isInstanceOf[LocalFundingStatus.Locked] =>
+              log.debug(s"re-sending splice_locked for fundingTxId=${c.fundingTxId}")
+              SpliceLocked(d.channelId, c.fundingTxId.reverse)
+            }
+          sendQueue = sendQueue ++ spliceLocked
+
           // we may need to retransmit updates and/or commit_sig and/or revocation
           sendQueue = sendQueue ++ syncSuccess.retransmit
 

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala

@@ -948,6 +948,45 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik
     awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.active.size == 1)
   }
 
+  test("disconnect (tx_signatures received by alice, zero-conf", Tag(ChannelStateTestsTags.ZeroConf), Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
+    import f._
+
+    initiateSpliceWithoutSigs(f, spliceOut_opt = Some(SpliceOut(20_000 sat, defaultSpliceOutScriptPubKey)))
+    alice2bob.expectMsgType[CommitSig]
+    alice2bob.forward(bob)
+    bob2alice.expectMsgType[CommitSig]
+    bob2alice.forward(alice)
+    bob2alice.expectMsgType[TxSignatures]
+    bob2alice.forward(alice)
+    awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice)
+    alice2bob.expectMsgType[TxSignatures] // Bob doesn't receive Alice's tx_signatures
+    alice2blockchain.expectMsgType[WatchPublished]
+    awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus == SpliceStatus.NoSplice)
+    val spliceTx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.signedTx_opt.get
+    alice ! WatchPublishedTriggered(spliceTx)
+    assert(alice2bob.expectMsgType[SpliceLocked].fundingTxid == spliceTx.txid) // Bob doesn't receive Alice's splice_locked
+
+    disconnect(f)
+    val (channelReestablishAlice, channelReestablishBob) = reconnect(f, interceptFundingDeeplyBuried = false)
+    assert(channelReestablishAlice.nextFundingTxId_opt.isEmpty)
+    assert(channelReestablishBob.nextFundingTxId_opt.contains(spliceTx.txid))
+    alice2blockchain.expectWatchFundingConfirmed(spliceTx.txid)
+    bob2blockchain.expectWatchPublished(spliceTx.txid)
+    bob2blockchain.expectMsgType[WatchFundingDeeplyBuried]
+
+    // Alice doesn't retransmit her tx_signatures because the funding transaction has already been published.
+    assert(alice2bob.expectMsgType[SpliceLocked].fundingTxid == spliceTx.txid)
+    alice2bob.forward(bob)
+    // Bob cannot publish the transaction, but it will eventually confirm because it was published by Alice.
+    bob2blockchain.expectNoMessage(100 millis)
+    bob2alice.expectNoMessage(100 millis)
+    bob ! WatchFundingConfirmedTriggered(BlockHeight(42), 0, spliceTx)
+    bob2alice.expectMsgType[SpliceLocked]
+    bob2alice.forward(alice)
+    awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.active.size == 1)
+    awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.active.size == 1)
+  }
+
   test("don't resend splice_locked when zero-conf channel confirms", Tag(ChannelStateTestsTags.ZeroConf), Tag(ChannelStateTestsTags.AnchorOutputsZeroFeeHtlcTxs)) { f =>
     import f._