From 45ae872e238a69d7334af1b094c6a0d1add00f6a Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 12 Sep 2023 11:35:41 +0200 Subject: [PATCH] Don't send splice_locked before tx_signatures 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. --- .../fr/acinq/eclair/channel/fsm/Channel.scala | 27 ++++++------- .../states/e/NormalSplicesStateSpec.scala | 39 +++++++++++++++++++ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala index 65af30ce1f..fafccae0a9 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala @@ -1873,7 +1873,7 @@ 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") @@ -1881,18 +1881,6 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder with 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 diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala index 4f6882cfdc..7c4973cc3f 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala +++ b/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._