Ignore non-relayed incoming HTLCs when closing (#2672)

If we force-close with HTLCs that have just been signed by our peer but
for which we haven't received their revocation, we should ignore them.
We have not relayed those HTLCs so they can't be fulfilled. It is our
peer's responsibility to claim them on-chain (using their HTLC-timeout),
but if for some reason they don't claim it, we don't want the channel to
be stuck in the closing state.

Fixes #2669

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

@@ -783,6 +783,8 @@ object Helpers {
           case u: UpdateFailHtlc => u.id
           case u: UpdateFailMalformedHtlc => u.id
         }.toSet
+        // these htlcs have been signed by our peer, but we haven't received their revocation and relayed them yet
+        val nonRelayedIncomingHtlcs: Set[Long] = commitment.changes.remoteChanges.all.collect { case add: UpdateAddHtlc => add.id }.toSet
 
         commitment.localCommit.htlcTxsAndRemoteSigs.collect {
           case HtlcTxAndRemoteSig(txInfo@HtlcSuccessTx(_, _, paymentHash, _, _), remoteSig) =>
@@ -795,6 +797,9 @@ object Helpers {
             } else if (failedIncomingHtlcs.contains(txInfo.htlcId)) {
               // incoming htlc that we know for sure will never be fulfilled downstream: we can safely discard it
               None
+            } else if (nonRelayedIncomingHtlcs.contains(txInfo.htlcId)) {
+              // incoming htlc that we haven't relayed yet: we can safely discard it, our peer will claim it once it times out
+              None
             } else {
               // incoming htlc for which we don't have the preimage: we can't spend it immediately, but we may learn the
               // preimage later, otherwise it will eventually timeout and they will get their funds back

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala

@@ -511,6 +511,29 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
     alice2relayer.expectNoMessage(100 millis)
   }
 
+  test("recv WatchTxConfirmedTriggered (local commit with htlcs only signed by remote)") { f =>
+    import f._
+    // Bob sends an htlc and signs it.
+    addHtlc(75_000_000 msat, bob, alice, bob2alice, alice2bob)
+    bob ! CMD_SIGN()
+    bob2alice.expectMsgType[CommitSig]
+    bob2alice.forward(alice)
+    alice2bob.expectMsgType[RevokeAndAck]
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localCommit.htlcTxsAndRemoteSigs.size == 1)
+    val aliceCommitTx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localCommit.commitTxAndRemoteSig.commitTx.tx
+    // Note that alice has not signed the htlc yet!
+    // We make her unilaterally close the channel.
+    val closingState = localClose(alice, alice2blockchain)
+
+    channelUpdateListener.expectMsgType[LocalChannelDown]
+    assert(closingState.htlcTxs.isEmpty && closingState.claimHtlcDelayedTxs.isEmpty)
+    // Alice should ignore the htlc (she hasn't relayed it yet): it is Bob's responsibility to claim it.
+    // Once the commit tx and her main output are confirmed, she can consider the channel closed.
+    alice ! WatchTxConfirmedTriggered(BlockHeight(0), 0, aliceCommitTx)
+    closingState.claimMainDelayedOutputTx.foreach(claimMain => alice ! WatchTxConfirmedTriggered(BlockHeight(0), 0, claimMain.tx))
+    awaitCond(alice.stateName == CLOSED)
+  }
+
   test("recv WatchTxConfirmedTriggered (local commit with fulfill only signed by local)") { f =>
     import f._
     // bob sends an htlc