From 6c6446c18fe22dee06145df07f6ca8bd6e06e055 Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Wed, 13 Sep 2023 15:48:18 +0200 Subject: [PATCH] Don't send splice_locked before tx_signatures (#528) 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. --- .../acinq/lightning/channel/states/Syncing.kt | 29 ++++----- .../channel/states/SpliceTestsCommon.kt | 61 +++++++++++++++++++ 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt index 3b78e1a49..09c9bf5d8 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt @@ -155,26 +155,13 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: // normal case, our data is up-to-date val actions = ArrayList() - // re-send channel_ready or splice_locked + // re-send channel_ready if necessary if (state.commitments.latest.fundingTxIndex == 0L && cmd.message.nextLocalCommitmentNumber == 1L && state.commitments.localCommitIndex == 0L) { // 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 logger.debug { "re-sending channel_ready" } val nextPerCommitmentPoint = channelKeys().commitmentPoint(1) val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint) actions.add(ChannelAction.Message.Send(channelReady)) - } else { - // NB: there is a key difference between channel_ready and splice_locked: - // - channel_ready: a non-zero commitment index implies that both sides have seen the channel_ready - // - splice_locked: 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 - state.commitments.active - .filter { it.fundingTxIndex > 0L } // only consider splice txs - .firstOrNull { staticParams.useZeroConf || it.localFundingStatus is LocalFundingStatus.ConfirmedFundingTx } - ?.let { - logger.debug { "re-sending splice_locked for fundingTxId=${it.fundingTxId}" } - val spliceLocked = SpliceLocked(channelId, it.fundingTxId.reversed()) - actions.add(ChannelAction.Message.Send(spliceLocked)) - } } // resume splice signing session if any @@ -216,6 +203,20 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: state.spliceStatus } + // Re-send splice_locked (must come *after* potentially retransmitting tx_signatures). + // NB: there is a key difference between channel_ready and splice_locked: + // - channel_ready: a non-zero commitment index implies that both sides have seen the channel_ready + // - splice_locked: 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 + state.commitments.active + .filter { it.fundingTxIndex > 0L } // only consider splice txs + .firstOrNull { staticParams.useZeroConf || it.localFundingStatus is LocalFundingStatus.ConfirmedFundingTx } + ?.let { + logger.debug { "re-sending splice_locked for fundingTxId=${it.fundingTxId}" } + val spliceLocked = SpliceLocked(channelId, it.fundingTxId.reversed()) + actions.add(ChannelAction.Message.Send(spliceLocked)) + } + try { val (commitments1, sendQueue1) = handleSync(cmd.message, state) actions.addAll(sendQueue1) diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt index 4690d9bfb..df94114b7 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt @@ -445,6 +445,67 @@ class SpliceTestsCommon : LightningTestSuite() { actionsBob6.has() } + @Test + fun `disconnect -- tx_signatures sent by bob -- zero-conf`() { + val (alice, bob) = reachNormalWithConfirmedFundingTx(zeroConf = true) + val (alice1, commitSigAlice1, bob1, _) = spliceOutWithoutSigs(alice, bob, 20_000.sat) + val (bob2, actionsBob2) = bob1.process(ChannelCommand.MessageReceived(commitSigAlice1)) + assertIs>(bob2) + val spliceTxId = actionsBob2.hasOutgoingMessage().txId + actionsBob2.hasOutgoingMessage() + assertEquals(bob2.state.spliceStatus, SpliceStatus.None) + + val (alice2, bob3, channelReestablishAlice) = disconnect(alice1, bob2) + assertEquals(channelReestablishAlice.nextFundingTxId, spliceTxId) + val (bob4, actionsBob4) = bob3.process(ChannelCommand.MessageReceived(channelReestablishAlice)) + assertEquals(actionsBob4.size, 4) + val channelReestablishBob = actionsBob4.findOutgoingMessage() + val commitSigBob2 = actionsBob4.findOutgoingMessage() + val txSigsBob = actionsBob4.findOutgoingMessage() + // splice_locked must always be sent *after* tx_signatures + assertIs(actionsBob4.filterIsInstance().last().message) + val spliceLockedBob = actionsBob4.findOutgoingMessage() + assertEquals(channelReestablishBob.nextFundingTxId, spliceTxId) + val (alice3, actionsAlice3) = alice2.process(ChannelCommand.MessageReceived(channelReestablishBob)) + assertEquals(actionsAlice3.size, 1) + val commitSigAlice2 = actionsAlice3.findOutgoingMessage() + + val (alice4, actionsAlice4) = alice3.process(ChannelCommand.MessageReceived(commitSigBob2)) + assertTrue(actionsAlice4.isEmpty()) + val (alice5, actionsAlice5) = alice4.process(ChannelCommand.MessageReceived(txSigsBob)) + assertIs>(alice5) + assertEquals(alice5.state.commitments.active.size, 2) + assertEquals(actionsAlice5.size, 6) + assertEquals(actionsAlice5.hasPublishTx(ChannelAction.Blockchain.PublishTx.Type.FundingTx).txid, spliceTxId) + actionsAlice5.hasWatchConfirmed(spliceTxId) + actionsAlice5.has() + actionsAlice5.has() + val txSigsAlice = actionsAlice5.findOutgoingMessage() + assertIs(actionsAlice5.filterIsInstance().last().message) + val spliceLockedAlice = actionsAlice5.findOutgoingMessage() + val (alice6, actionsAlice6) = alice5.process(ChannelCommand.MessageReceived(spliceLockedBob)) + assertIs>(alice6) + assertEquals(alice6.state.commitments.active.size, 1) + assertEquals(actionsAlice6.size, 2) + actionsAlice6.find().also { assertEquals(it.txId, spliceTxId) } + actionsAlice6.has() + + val (bob5, actionsBob5) = bob4.process(ChannelCommand.MessageReceived(commitSigAlice2)) + assertTrue(actionsBob5.isEmpty()) + val (bob6, actionsBob6) = bob5.process(ChannelCommand.MessageReceived(txSigsAlice)) + assertIs>(bob6) + assertEquals(bob6.state.commitments.active.size, 2) + assertEquals(actionsBob6.size, 2) + assertEquals(actionsBob6.hasPublishTx(ChannelAction.Blockchain.PublishTx.Type.FundingTx).txid, spliceTxId) + actionsBob6.has() + val (bob7, actionsBob7) = bob6.process(ChannelCommand.MessageReceived(spliceLockedAlice)) + assertIs>(bob7) + assertEquals(bob7.state.commitments.active.size, 1) + assertEquals(actionsBob7.size, 2) + actionsBob7.find().also { assertEquals(it.txId, spliceTxId) } + actionsBob7.has() + } + @Test fun `disconnect -- tx_signatures received by alice`() { val (alice, bob) = reachNormalWithConfirmedFundingTx()