Fix invalid_commit_sig issue when expecting tx_signatures

src/commonMain/kotlin/fr/acinq/lightning/channel/Commitments.kt

@@ -123,7 +123,7 @@ sealed class LocalFundingStatus {
         override val fee: Satoshi = sharedTx.tx.fees
     }
 
-    data class ConfirmedFundingTx(override val signedTx: Transaction, override val fee: Satoshi) : LocalFundingStatus() {
+    data class ConfirmedFundingTx(override val signedTx: Transaction, override val fee: Satoshi, val localSigs: TxSignatures) : LocalFundingStatus() {
         override val txId: ByteVector32 = signedTx.txid
     }
 }
@@ -832,7 +832,7 @@ data class Commitments(
                 when (c.localFundingStatus) {
                     is LocalFundingStatus.UnconfirmedFundingTx -> {
                         logger.debug { "setting localFundingStatus confirmed for fundingTxId=${fundingTx.txid}" }
-                        c.copy(localFundingStatus = LocalFundingStatus.ConfirmedFundingTx(fundingTx, c.localFundingStatus.sharedTx.tx.fees))
+                        c.copy(localFundingStatus = LocalFundingStatus.ConfirmedFundingTx(fundingTx, c.localFundingStatus.sharedTx.tx.fees, c.localFundingStatus.sharedTx.localSigs))
                     }
                     is LocalFundingStatus.ConfirmedFundingTx -> c
                 }

src/commonMain/kotlin/fr/acinq/lightning/channel/states/Channel.kt

@@ -144,7 +144,11 @@ sealed class ChannelState {
     }
 
     internal fun ChannelContext.unhandled(cmd: ChannelCommand): Pair<ChannelState, List<ChannelAction>> {
-        logger.warning { "unhandled command ${cmd::class.simpleName} in state ${this@ChannelState::class.simpleName}" }
+        when (cmd) {
+            is ChannelCommand.MessageReceived -> logger.warning { "unhandled message ${cmd.message::class.simpleName} in state ${this@ChannelState::class.simpleName}" }
+            is ChannelCommand.WatchReceived -> logger.warning { "unhandled watch event ${cmd.watch.event::class.simpleName} in state ${this@ChannelState::class.simpleName}" }
+            else -> logger.warning { "unhandled command ${cmd::class.simpleName} in state ${this@ChannelState::class.simpleName}" }
+        }
         return Pair(this@ChannelState, listOf())
     }
 
@@ -162,7 +166,11 @@ sealed class ChannelState {
     )
 
     internal fun ChannelContext.handleLocalError(cmd: ChannelCommand, t: Throwable): Pair<ChannelState, List<ChannelAction>> {
-        logger.error(t) { "error on command ${cmd::class.simpleName}" }
+        when (cmd) {
+            is ChannelCommand.MessageReceived -> logger.error(t) { "error on message ${cmd.message::class.simpleName}" }
+            is ChannelCommand.WatchReceived -> logger.error { "error on watch event ${cmd.watch.event::class.simpleName}" }
+            else -> logger.error(t) { "error on command ${cmd::class.simpleName}" }
+        }
 
         fun abort(channelId: ByteVector32?, state: ChannelState): Pair<ChannelState, List<ChannelAction>> {
             val actions = buildList {
@@ -180,7 +188,7 @@ sealed class ChannelState {
             return state.run { spendLocalCurrent().run { copy(second = second + ChannelAction.Message.Send(error)) } }
         }
 
-        return when(val state = this@ChannelState) {
+        return when (val state = this@ChannelState) {
             is WaitForInit -> abort(null, state)
             is WaitForOpenChannel -> abort(state.temporaryChannelId, state)
             is WaitForAcceptChannel -> abort(state.temporaryChannelId, state)
@@ -213,7 +221,7 @@ sealed class ChannelState {
                     // we already have published a mutual close tx, it's always better to use that
                     Pair(state, emptyList())
                 } else {
-                    state.localCommitPublished ?.let {
+                    state.localCommitPublished?.let {
                         // we're already trying to claim our commitment, there's nothing more we can do
                         Pair(state, emptyList())
                     } ?: state.run { spendLocalCurrent() }

src/commonMain/kotlin/fr/acinq/lightning/channel/states/Normal.kt

@@ -1,6 +1,7 @@
 package fr.acinq.lightning.channel.states
 
 import fr.acinq.bitcoin.ByteVector32
+import fr.acinq.bitcoin.SigHash
 import fr.acinq.lightning.Feature
 import fr.acinq.lightning.Features
 import fr.acinq.lightning.ShortChannelId
@@ -9,6 +10,7 @@ import fr.acinq.lightning.blockchain.WatchConfirmed
 import fr.acinq.lightning.blockchain.WatchEventConfirmed
 import fr.acinq.lightning.blockchain.WatchEventSpent
 import fr.acinq.lightning.channel.*
+import fr.acinq.lightning.transactions.Scripts
 import fr.acinq.lightning.transactions.Transactions
 import fr.acinq.lightning.utils.*
 import fr.acinq.lightning.wire.*
@@ -192,11 +194,10 @@ data class Normal(
                                 is InteractiveTxSigningSessionAction.SendTxSigs -> sendSpliceTxSigs(spliceStatus.origins, action, cmd.message.channelData)
                             }
                         }
-                        commitments.params.channelFeatures.hasFeature(Feature.DualFunding) && commitments.latest.localFundingStatus.signedTx == null && cmd.message.batchSize == 1 -> {
-                            // The latest funding transaction is unconfirmed and we're missing our peer's tx_signatures: any commit_sig that we receive before that should be ignored,
-                            // it's either a retransmission of a commit_sig we've already received or a bug that will eventually lead to a force-close anyway.
-                            // NB: we check the dual-funding feature, because legacy single-funding unconfirmed channels will have `localFundingStatus=UnconfirmedFundingTx`,
-                            // so we cannot just check on the absence of `signedTx`.
+                        commitments.params.channelFeatures.hasFeature(Feature.DualFunding) && ignoreRetransmittedCommitSig(cmd.message) -> {
+                            // We haven't received our peer's tx_signatures for the latest funding transaction and asked them to resend it on reconnection.
+                            // They also resend their corresponding commit_sig, but we have already received it so we should ignore it.
+                            // Note that the funding transaction may have confirmed while we were offline.
                             logger.info { "ignoring commit_sig, we're still waiting for tx_signatures" }
                             Pair(this@Normal, listOf())
                         }
@@ -733,6 +734,13 @@ data class Normal(
         return Pair(nextState, actions)
     }
 
+    /** This function should be used to ignore a commit_sig that we've already received. */
+    private fun ignoreRetransmittedCommitSig(commit: CommitSig): Boolean {
+        // If we already have a signed commitment transaction containing their signature, we must have previously received that commit_sig.
+        val commitTx = commitments.latest.localCommit.publishableTxs.commitTx.tx
+        return commit.batchSize == 1 && commitTx.txIn.first().witness.stack.contains(Scripts.der(commit.signature, SigHash.SIGHASH_ALL))
+    }
+
     /** If we haven't completed the signing steps of an interactive-tx session, we will ask our peer to retransmit signatures for the corresponding transaction. */
     fun getUnsignedFundingTxId(): ByteVector32? = when {
         spliceStatus is SpliceStatus.WaitingForSigs -> spliceStatus.session.fundingTx.txId

src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt

@@ -172,26 +172,29 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
                                         actions.add(ChannelAction.Message.Send(commitSig))
                                         state.spliceStatus
                                     } else if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) {
-                                        if (state.commitments.latest.localFundingStatus is LocalFundingStatus.UnconfirmedFundingTx) {
-                                            if (state.commitments.latest.localFundingStatus.sharedTx is 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
-                                                logger.info { "re-sending commit_sig for fundingTxIndex=${state.commitments.latest.fundingTxIndex} fundingTxId=${state.commitments.latest.fundingTxId}" }
-                                                val commitSig = state.commitments.latest.remoteCommit.sign(
-                                                    channelKeys(),
-                                                    state.commitments.params,
-                                                    fundingTxIndex = state.commitments.latest.fundingTxIndex,
-                                                    state.commitments.latest.remoteFundingPubkey,
-                                                    state.commitments.latest.commitInput
-                                                )
-                                                actions.add(ChannelAction.Message.Send(commitSig))
+                                        when (val localFundingStatus = state.commitments.latest.localFundingStatus) {
+                                            is LocalFundingStatus.UnconfirmedFundingTx -> {
+                                                if (localFundingStatus.sharedTx is 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
+                                                    logger.info { "re-sending commit_sig for fundingTxIndex=${state.commitments.latest.fundingTxIndex} fundingTxId=${state.commitments.latest.fundingTxId}" }
+                                                    val commitSig = state.commitments.latest.remoteCommit.sign(
+                                                        channelKeys(),
+                                                        state.commitments.params,
+                                                        fundingTxIndex = state.commitments.latest.fundingTxIndex,
+                                                        state.commitments.latest.remoteFundingPubkey,
+                                                        state.commitments.latest.commitInput
+                                                    )
+                                                    actions.add(ChannelAction.Message.Send(commitSig))
+                                                }
+                                                logger.info { "re-sending tx_signatures for fundingTxId=${cmd.message.nextFundingTxId}" }
+                                                actions.add(ChannelAction.Message.Send(localFundingStatus.sharedTx.localSigs))
+                                            }
+                                            is LocalFundingStatus.ConfirmedFundingTx -> {
+                                                // The funding tx is confirmed, and they have not received our tx_signatures, but they must have received our commit_sig, otherwise they
+                                                // would not have sent their tx_signatures and we would not have been able to publish the funding tx in the first place.
+                                                logger.info { "re-sending tx_signatures for fundingTxId=${cmd.message.nextFundingTxId}" }
+                                                actions.add(ChannelAction.Message.Send(localFundingStatus.localSigs))
                                             }
-                                            logger.info { "re-sending tx_signatures for fundingTxId=${cmd.message.nextFundingTxId}" }
-                                            actions.add(ChannelAction.Message.Send(state.commitments.latest.localFundingStatus.sharedTx.localSigs))
-                                        } else {
-                                            // The funding tx is confirmed, and they have not received our tx_signatures, but they must have received our commit_sig, otherwise they
-                                            // would not have sent their tx_signatures and we would not have been able to publish the funding tx in the first place. We could in theory
-                                            // recompute our tx_signatures, but instead we do nothing: they will shortly be notified that the funding tx has confirmed.
-                                            logger.warning { "cannot re-send tx_signatures for fundingTxId=${cmd.message.nextFundingTxId}, transaction is already confirmed" }
                                         }
                                         state.spliceStatus
                                     } else if (cmd.message.nextFundingTxId != null) {

src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt

@@ -936,7 +936,6 @@ class Peer(
                         }
                     }
                     is HasChannelId -> {
-                        logger.info { "received ${msg::class.simpleName} for channel ${msg.channelId}" }
                         if (msg is Error && msg.channelId == ByteVector32.Zeroes) {
                             logger.error { "connection error: ${msg.toAscii()}" }
                         } else {
@@ -952,7 +951,6 @@ class Peer(
                         }
                     }
                     is ChannelUpdate -> {
-                        logger.info { "received ${msg::class.simpleName} for channel ${msg.shortChannelId}" }
                         _channels.values.filterIsInstance<Normal>().find { it.shortChannelId == msg.shortChannelId }?.let { state ->
                             val event1 = ChannelCommand.MessageReceived(msg)
                             val (state1, actions) = state.process(event1)

src/commonMain/kotlin/fr/acinq/lightning/serialization/v4/Deserialization.kt

@@ -419,7 +419,16 @@ object Deserialization {
             )
             0x01 -> LocalFundingStatus.ConfirmedFundingTx(
                 signedTx = readTransaction(),
-                fee = readNumber().sat
+                fee = readNumber().sat,
+                // We previously didn't store the tx_signatures after the transaction was confirmed.
+                // It is only used to be retransmitted on reconnection if our peer had not received it.
+                // This happens very rarely in practice, so putting dummy values here shouldn't be an issue.
+                localSigs = TxSignatures(ByteVector32.Zeroes, ByteVector32.Zeroes, listOf())
+            )
+            0x02 -> LocalFundingStatus.ConfirmedFundingTx(
+                signedTx = readTransaction(),
+                fee = readNumber().sat,
+                localSigs = readLightningMessage() as TxSignatures
             )
             else -> error("unknown discriminator $discriminator for class ${LocalFundingStatus::class}")
         },

src/commonMain/kotlin/fr/acinq/lightning/serialization/v4/Serialization.kt

@@ -464,9 +464,10 @@ object Serialization {
                 writeNumber(localFundingStatus.createdAt)
             }
             is LocalFundingStatus.ConfirmedFundingTx -> {
-                write(0x01)
+                write(0x02)
                 writeBtcObject(localFundingStatus.signedTx)
                 writeNumber(localFundingStatus.fee.toLong())
+                writeLightningMessage(localFundingStatus.localSigs)
             }
         }
         when (remoteFundingStatus) {

src/commonTest/kotlin/fr/acinq/lightning/channel/CommitmentsTestsCommon.kt

@@ -26,6 +26,7 @@ import fr.acinq.lightning.transactions.Scripts
 import fr.acinq.lightning.transactions.Transactions
 import fr.acinq.lightning.utils.*
 import fr.acinq.lightning.wire.IncorrectOrUnknownPaymentDetails
+import fr.acinq.lightning.wire.TxSignatures
 import fr.acinq.lightning.wire.UpdateAddHtlc
 import org.kodein.log.LoggerFactory
 import org.kodein.log.newLogger
@@ -514,7 +515,7 @@ class CommitmentsTestsCommon : LightningTestSuite(), LoggingContext {
                     Commitment(
                         fundingTxIndex = 0,
                         remoteFundingPubkey = randomKey().publicKey(),
-                        LocalFundingStatus.ConfirmedFundingTx(dummyFundingTx, 500.sat),
+                        LocalFundingStatus.ConfirmedFundingTx(dummyFundingTx, 500.sat, TxSignatures(randomBytes32(), randomBytes32(), listOf())),
                         RemoteFundingStatus.Locked,
                         LocalCommit(0, CommitmentSpec(setOf(), feeRatePerKw, toLocal, toRemote), PublishableTxs(localCommitTx, listOf())),
                         RemoteCommit(0, CommitmentSpec(setOf(), feeRatePerKw, toRemote, toLocal), randomBytes32(), randomKey().publicKey()),
@@ -559,7 +560,7 @@ class CommitmentsTestsCommon : LightningTestSuite(), LoggingContext {
                     Commitment(
                         fundingTxIndex = 0,
                         remoteFundingPubkey = randomKey().publicKey(),
-                        LocalFundingStatus.ConfirmedFundingTx(dummyFundingTx, 500.sat),
+                        LocalFundingStatus.ConfirmedFundingTx(dummyFundingTx, 500.sat, TxSignatures(randomBytes32(), randomBytes32(), listOf())),
                         RemoteFundingStatus.Locked,
                         LocalCommit(0, CommitmentSpec(setOf(), FeeratePerKw(0.sat), toLocal, toRemote), PublishableTxs(localCommitTx, listOf())),
                         RemoteCommit(0, CommitmentSpec(setOf(), FeeratePerKw(0.sat), toRemote, toLocal), randomBytes32(), randomKey().publicKey()),