Remove spurious interactive-tx commit_sig retransmission

We fully implement lightning/bolts#1214 to stop
retransmitting `commit_sig` when our peer has already received it. We
also correctly set `next_commitment_number` to let our peer know whether
we have received their `commit_sig` or not.

Author: t-bast

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

@@ -2108,7 +2108,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
       val nextFundingTlv: Set[ChannelReestablishTlv] = Set(ChannelReestablishTlv.NextFundingTlv(d.signingSession.fundingTx.txId))
       val channelReestablish = ChannelReestablish(
         channelId = d.channelId,
-        nextLocalCommitmentNumber = 1,
+        nextLocalCommitmentNumber = d.signingSession.reconnectNextLocalCommitmentNumber,
         nextRemoteRevocationNumber = 0,
         yourLastPerCommitmentSecret = PrivateKey(ByteVector32.Zeroes),
         myCurrentPerCommitmentPoint = myFirstPerCommitmentPoint,
@@ -2123,6 +2123,19 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
       val yourLastPerCommitmentSecret = remotePerCommitmentSecrets.lastIndex.flatMap(remotePerCommitmentSecrets.getHash).getOrElse(ByteVector32.Zeroes)
       val channelKeyPath = keyManager.keyPath(d.commitments.params.localParams, d.commitments.params.channelConfig)
       val myCurrentPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, d.commitments.localCommitIndex)
+      // If we disconnected while signing a funding transaction, we may need our peer to retransmit their commit_sig.
+      val nextLocalCommitmentNumber = d match {
+        case d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED => d.status match {
+          case DualFundingStatus.RbfWaitingForSigs(status) => status.reconnectNextLocalCommitmentNumber
+          case _ => d.commitments.localCommitIndex + 1
+        }
+        case d: DATA_NORMAL => d.spliceStatus match {
+          case SpliceStatus.SpliceWaitingForSigs(status) => status.reconnectNextLocalCommitmentNumber
+          case _ => d.commitments.localCommitIndex + 1
+        }
+        case _ => d.commitments.localCommitIndex + 1
+      }
+      // If we disconnected while signing a funding transaction, we may need our peer to (re)transmit their tx_signatures.
       val rbfTlv: Set[ChannelReestablishTlv] = d match {
         case d: DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED => d.status match {
           case DualFundingStatus.RbfWaitingForSigs(status) => Set(ChannelReestablishTlv.NextFundingTlv(status.fundingTx.txId))
@@ -2142,7 +2155,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
       }
       val channelReestablish = ChannelReestablish(
         channelId = d.channelId,
-        nextLocalCommitmentNumber = d.commitments.localCommitIndex + 1,
+        nextLocalCommitmentNumber = nextLocalCommitmentNumber,
         nextRemoteRevocationNumber = d.commitments.remoteCommitIndex,
         yourLastPerCommitmentSecret = PrivateKey(yourLastPerCommitmentSecret),
         myCurrentPerCommitmentPoint = myCurrentPerCommitmentPoint,
@@ -2183,8 +2196,9 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
 
     case Event(channelReestablish: ChannelReestablish, d: DATA_WAIT_FOR_DUAL_FUNDING_SIGNED) =>
       channelReestablish.nextFundingTxId_opt match {
-        case Some(fundingTxId) if fundingTxId == d.signingSession.fundingTx.txId =>
-          // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig.
+        case Some(fundingTxId) if fundingTxId == d.signingSession.fundingTx.txId && channelReestablish.nextLocalCommitmentNumber == 0 =>
+          // They haven't received our commit_sig: we retransmit it, and will send our tx_signatures once we've received
+          // their commit_sig or their tx_signatures (depending on who must send tx_signatures first).
           val commitSig = d.signingSession.remoteCommit.sign(keyManager, d.channelParams, d.signingSession.fundingTxIndex, d.signingSession.fundingParams.remoteFundingPubKey, d.signingSession.commitInput)
           goto(WAIT_FOR_DUAL_FUNDING_SIGNED) sending commitSig
         case _ => goto(WAIT_FOR_DUAL_FUNDING_SIGNED)
@@ -2195,20 +2209,25 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
         case Some(fundingTxId) =>
           d.status match {
             case DualFundingStatus.RbfWaitingForSigs(signingSession) if signingSession.fundingTx.txId == fundingTxId =>
-              // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig.
-              val commitSig = signingSession.remoteCommit.sign(keyManager, d.commitments.params, signingSession.fundingTxIndex, signingSession.fundingParams.remoteFundingPubKey, signingSession.commitInput)
-              goto(WAIT_FOR_DUAL_FUNDING_CONFIRMED) sending commitSig
+              if (channelReestablish.nextLocalCommitmentNumber == 0) {
+                // They haven't received our commit_sig: we retransmit it.
+                // We're also waiting for signatures from them, and will send our tx_signatures once we receive them.
+                val commitSig = signingSession.remoteCommit.sign(keyManager, d.commitments.params, signingSession.fundingTxIndex, signingSession.fundingParams.remoteFundingPubKey, signingSession.commitInput)
+                goto(WAIT_FOR_DUAL_FUNDING_CONFIRMED) sending commitSig
+              } else {
+                // They have already received our commit_sig, but we were waiting for them to send either commit_sig or
+                // tx_signatures first. We wait for their message before sending our tx_signatures.
+                goto(WAIT_FOR_DUAL_FUNDING_CONFIRMED)
+              }
             case _ if d.latestFundingTx.sharedTx.txId == fundingTxId =>
-              val toSend = d.latestFundingTx.sharedTx match {
-                case fundingTx: InteractiveTxBuilder.PartiallySignedSharedTransaction =>
-                  // We have not received their tx_signatures: we retransmit our commit_sig because we don't know if they received it.
-                  val commitSig = d.commitments.latest.remoteCommit.sign(keyManager, d.commitments.params, d.commitments.latest.fundingTxIndex, d.commitments.latest.remoteFundingPubKey, d.commitments.latest.commitInput)
-                  Seq(commitSig, fundingTx.localSigs)
-                case fundingTx: InteractiveTxBuilder.FullySignedSharedTransaction =>
-                  // We've already received their tx_signatures, which means they've received and stored our commit_sig, we only need to retransmit our tx_signatures.
-                  Seq(fundingTx.localSigs)
+              // We've already received their commit_sig and sent our tx_signatures. We retransmit our tx_signatures
+              // and our commit_sig if they haven't received it already.
+              if (channelReestablish.nextLocalCommitmentNumber == 0) {
+                val commitSig = d.commitments.latest.remoteCommit.sign(keyManager, d.commitments.params, d.commitments.latest.fundingTxIndex, d.commitments.latest.remoteFundingPubKey, d.commitments.latest.commitInput)
+                goto(WAIT_FOR_DUAL_FUNDING_CONFIRMED) sending Seq(commitSig, d.latestFundingTx.sharedTx.localSigs)
+              } else {
+                goto(WAIT_FOR_DUAL_FUNDING_CONFIRMED) sending d.latestFundingTx.sharedTx.localSigs
               }
-              goto(WAIT_FOR_DUAL_FUNDING_CONFIRMED) sending toSend
             case _ =>
               // The fundingTxId must be for an RBF attempt that we didn't store (we got disconnected before receiving
               // their tx_complete): we tell them to abort that RBF attempt.
@@ -2250,23 +2269,26 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with
             case Some(fundingTxId) =>
               d.spliceStatus match {
                 case SpliceStatus.SpliceWaitingForSigs(signingSession) if signingSession.fundingTx.txId == fundingTxId =>
-                  // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig.
-                  log.info("re-sending commit_sig for splice attempt with fundingTxIndex={} fundingTxId={}", signingSession.fundingTxIndex, signingSession.fundingTx.txId)
-                  val commitSig = signingSession.remoteCommit.sign(keyManager, d.commitments.params, signingSession.fundingTxIndex, signingSession.fundingParams.remoteFundingPubKey, signingSession.commitInput)
-                  sendQueue = sendQueue :+ commitSig
+                  if (channelReestablish.nextLocalCommitmentNumber == d.commitments.remoteCommitIndex) {
+                    // They haven't received our commit_sig: we retransmit it.
+                    // We're also waiting for signatures from them, and will send our tx_signatures once we receive them.
+                    log.info("re-sending commit_sig for splice attempt with fundingTxIndex={} fundingTxId={}", signingSession.fundingTxIndex, signingSession.fundingTx.txId)
+                    val commitSig = signingSession.remoteCommit.sign(keyManager, d.commitments.params, signingSession.fundingTxIndex, signingSession.fundingParams.remoteFundingPubKey, signingSession.commitInput)
+                    sendQueue = sendQueue :+ commitSig
+                  }
                   d.spliceStatus
                 case _ if d.commitments.latest.fundingTxId == fundingTxId =>
                   d.commitments.latest.localFundingStatus match {
                     case dfu: LocalFundingStatus.DualFundedUnconfirmedFundingTx =>
-                      dfu.sharedTx match {
-                        case fundingTx: InteractiveTxBuilder.PartiallySignedSharedTransaction =>
-                          // If we have not received their tx_signatures, we can't tell whether they had received our commit_sig, so we need to retransmit it
-                          log.info("re-sending commit_sig and tx_signatures for fundingTxIndex={} fundingTxId={}", d.commitments.latest.fundingTxIndex, d.commitments.latest.fundingTxId)
-                          val commitSig = d.commitments.latest.remoteCommit.sign(keyManager, d.commitments.params, d.commitments.latest.fundingTxIndex, d.commitments.latest.remoteFundingPubKey, d.commitments.latest.commitInput)
-                          sendQueue = sendQueue :+ commitSig :+ fundingTx.localSigs
-                        case fundingTx: InteractiveTxBuilder.FullySignedSharedTransaction =>
-                          log.info("re-sending tx_signatures for fundingTxIndex={} fundingTxId={}", d.commitments.latest.fundingTxIndex, d.commitments.latest.fundingTxId)
-                          sendQueue = sendQueue :+ fundingTx.localSigs
+                      // We've already received their commit_sig and sent our tx_signatures. We retransmit our
+                      // tx_signatures and our commit_sig if they haven't received it already.
+                      if (channelReestablish.nextLocalCommitmentNumber == d.commitments.remoteCommitIndex) {
+                        log.info("re-sending commit_sig and tx_signatures for fundingTxIndex={} fundingTxId={}", d.commitments.latest.fundingTxIndex, d.commitments.latest.fundingTxId)
+                        val commitSig = d.commitments.latest.remoteCommit.sign(keyManager, d.commitments.params, d.commitments.latest.fundingTxIndex, d.commitments.latest.remoteFundingPubKey, d.commitments.latest.commitInput)
+                        sendQueue = sendQueue :+ commitSig :+ dfu.sharedTx.localSigs
+                      } else {
+                        log.info("re-sending tx_signatures for fundingTxIndex={} fundingTxId={}", d.commitments.latest.fundingTxIndex, d.commitments.latest.fundingTxId)
+                        sendQueue = sendQueue :+ dfu.sharedTx.localSigs
                       }
                     case fundingStatus =>
                       // They have not received our tx_signatures, but they must have received our commit_sig, otherwise we would be in the case above.

eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala

@@ -1082,6 +1082,11 @@ object InteractiveTxSigningSession {
                             liquidityPurchase_opt: Option[LiquidityAds.PurchaseBasicInfo]) extends InteractiveTxSigningSession {
     val commitInput: InputInfo = localCommit.fold(_.commitTx.input, _.commitTxAndRemoteSig.commitTx.input)
     val localCommitIndex: Long = localCommit.fold(_.index, _.index)
+    // This value tells our peer whether we need them to retransmit their commit_sig on reconnection or not.
+    val reconnectNextLocalCommitmentNumber: Long = localCommit match {
+      case Left(commit) => commit.index
+      case Right(commit) => commit.index + 1
+    }
 
     def receiveCommitSig(nodeParams: NodeParams, channelParams: ChannelParams, remoteCommitSig: CommitSig)(implicit log: LoggingAdapter): Either[ChannelException, InteractiveTxSigningSession] = {
       localCommit match {

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingSignedStateSpec.scala

@@ -375,15 +375,16 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
     bob ! INPUT_DISCONNECTED
     awaitCond(bob.stateName == OFFLINE)
 
-    reconnect(f, fundingTxId)
+    reconnect(f, fundingTxId, aliceExpectsCommitSig = true, bobExpectsCommitSig = true)
   }
 
-  test("recv INPUT_DISCONNECTED (commit_sig not received, next_commitment_number = 0)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
+  test("recv INPUT_DISCONNECTED (commit_sig received by Alice)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
     import f._
 
     val fundingTxId = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_SIGNED].signingSession.fundingTx.txId
+    bob2alice.expectMsgType[CommitSig]
+    bob2alice.forward(alice)
     alice2bob.expectMsgType[CommitSig] // Bob doesn't receive Alice's commit_sig
-    bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig
     awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED)
     awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED)
 
@@ -392,10 +393,10 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
     bob ! INPUT_DISCONNECTED
     awaitCond(bob.stateName == OFFLINE)
 
-    reconnect(f, fundingTxId, aliceCommitmentNumber = 0, bobCommitmentNumber = 0)
+    reconnect(f, fundingTxId, aliceExpectsCommitSig = false, bobExpectsCommitSig = true)
   }
 
-  test("recv INPUT_DISCONNECTED (commit_sig partially received)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
+  test("recv INPUT_DISCONNECTED (commit_sig received by Bob)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
     import f._
 
     val fundingTxId = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_SIGNED].signingSession.fundingTx.txId
@@ -411,26 +412,7 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
     bob ! INPUT_DISCONNECTED
     awaitCond(bob.stateName == OFFLINE)
 
-    reconnect(f, fundingTxId)
-  }
-
-  test("recv INPUT_DISCONNECTED (commit_sig partially received, next_commitment_number = 0)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
-    import f._
-
-    val fundingTxId = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_SIGNED].signingSession.fundingTx.txId
-    alice2bob.expectMsgType[CommitSig]
-    alice2bob.forward(bob)
-    bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig
-    bob2alice.expectMsgType[TxSignatures] // Alice doesn't receive Bob's tx_signatures
-    awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED)
-    awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED)
-
-    alice ! INPUT_DISCONNECTED
-    awaitCond(alice.stateName == OFFLINE)
-    bob ! INPUT_DISCONNECTED
-    awaitCond(bob.stateName == OFFLINE)
-
-    reconnect(f, fundingTxId, aliceCommitmentNumber = 0)
+    reconnect(f, fundingTxId, aliceExpectsCommitSig = true, bobExpectsCommitSig = false)
   }
 
   test("recv INPUT_DISCONNECTED (commit_sig received)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
@@ -450,7 +432,7 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
     bob ! INPUT_DISCONNECTED
     awaitCond(bob.stateName == OFFLINE)
 
-    reconnect(f, fundingTxId)
+    reconnect(f, fundingTxId, aliceExpectsCommitSig = false, bobExpectsCommitSig = false)
   }
 
   test("recv INPUT_DISCONNECTED (tx_signatures received)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
@@ -490,7 +472,7 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
     assert(listener.expectMsgType[TransactionPublished].tx.txid == fundingTxId)
   }
 
-  private def reconnect(f: FixtureParam, fundingTxId: TxId, aliceCommitmentNumber: Long = 1, bobCommitmentNumber: Long = 1): Unit = {
+  private def reconnect(f: FixtureParam, fundingTxId: TxId, aliceExpectsCommitSig: Boolean, bobExpectsCommitSig: Boolean): Unit = {
     import f._
 
     val listener = TestProbe()
@@ -501,17 +483,24 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
     alice ! INPUT_RECONNECTED(bob, aliceInit, bobInit)
     bob ! INPUT_RECONNECTED(alice, bobInit, aliceInit)
     val channelReestablishAlice = alice2bob.expectMsgType[ChannelReestablish]
+    val nextLocalCommitmentNumberAlice = if (aliceExpectsCommitSig) 0 else 1
     assert(channelReestablishAlice.nextFundingTxId_opt.contains(fundingTxId))
-    assert(channelReestablishAlice.nextLocalCommitmentNumber == 1)
-    alice2bob.forward(bob, channelReestablishAlice.copy(nextLocalCommitmentNumber = aliceCommitmentNumber))
+    assert(channelReestablishAlice.nextLocalCommitmentNumber == nextLocalCommitmentNumberAlice)
+    alice2bob.forward(bob, channelReestablishAlice)
     val channelReestablishBob = bob2alice.expectMsgType[ChannelReestablish]
+    val nextLocalCommitmentNumberBob = if (bobExpectsCommitSig) 0 else 1
     assert(channelReestablishBob.nextFundingTxId_opt.contains(fundingTxId))
-    assert(channelReestablishBob.nextLocalCommitmentNumber == 1)
-    bob2alice.forward(alice, channelReestablishBob.copy(nextLocalCommitmentNumber = bobCommitmentNumber))
-    bob2alice.expectMsgType[CommitSig]
-    bob2alice.forward(alice)
-    alice2bob.expectMsgType[CommitSig]
-    alice2bob.forward(bob)
+    assert(channelReestablishBob.nextLocalCommitmentNumber == nextLocalCommitmentNumberBob)
+    bob2alice.forward(alice, channelReestablishBob)
+
+    if (aliceExpectsCommitSig) {
+      bob2alice.expectMsgType[CommitSig]
+      bob2alice.forward(alice)
+    }
+    if (bobExpectsCommitSig) {
+      alice2bob.expectMsgType[CommitSig]
+      alice2bob.forward(bob)
+    }
     bob2alice.expectMsgType[TxSignatures]
     bob2alice.forward(alice)
     alice2bob.expectMsgType[TxSignatures]