From f690fd59461103e171a8b08829ffae04a8e7a06d Mon Sep 17 00:00:00 2001 From: pm47 Date: Tue, 16 Apr 2024 15:36:14 +0200 Subject: [PATCH 1/5] handle reestablish in state `ShuttingDown` We use a full pattern matching on the `when` to prevent future omissions. --- .../acinq/lightning/channel/states/Syncing.kt | 86 +++++++++++-------- .../channel/states/ShutdownTestsCommon.kt | 15 ++++ 2 files changed, 64 insertions(+), 37 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 c95250824..26bc77fff 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt @@ -21,11 +21,11 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: return when (cmd) { is ChannelCommand.MessageReceived -> when (cmd.message) { is ChannelReestablish -> { - val (nextState, actions) = when { - state is LegacyWaitForFundingConfirmed -> { + val (nextState, actions) = when (state) { + is LegacyWaitForFundingConfirmed -> { Pair(state, listOf()) } - state is WaitForFundingSigned -> { + is WaitForFundingSigned -> { when (cmd.message.nextFundingTxId) { // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig. state.signingSession.fundingTx.txId -> { @@ -35,7 +35,7 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: else -> Pair(state, listOf()) } } - state is WaitForFundingConfirmed -> { + is WaitForFundingConfirmed -> { when (cmd.message.nextFundingTxId) { null -> Pair(state, listOf()) else -> { @@ -72,7 +72,7 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: } } } - state is WaitForChannelReady -> { + is WaitForChannelReady -> { val actions = ArrayList() if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) { @@ -106,14 +106,14 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: Pair(state, actions) } - state is LegacyWaitForFundingLocked -> { + is LegacyWaitForFundingLocked -> { logger.debug { "re-sending channel_ready" } val nextPerCommitmentPoint = channelKeys().commitmentPoint(1) val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint) val actions = listOf(ChannelAction.Message.Send(channelReady)) Pair(state, actions) } - state is Normal -> { + is Normal -> { when { !Helpers.checkLocalCommit(state.commitments, cmd.message.nextRemoteRevocationNumber) -> { // if next_remote_revocation_number is greater than our local commitment index, it means that either we are using an outdated commitment, or they are lying @@ -235,38 +235,50 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: } } } - // BOLT 2: A node if it has sent a previous shutdown MUST retransmit shutdown. + is ShuttingDown -> { + try { + val (commitments1, sendQueue1) = handleSync(cmd.message, state) + val actions = buildList { + addAll(sendQueue1) + add(ChannelAction.Message.Send(state.localShutdown)) + } + Pair(state.copy(commitments = commitments1), actions) + } catch (e: RevocationSyncError) { + val error = Error(channelId, e.message) + state.run { spendLocalCurrent() }.run { copy(second = second + ChannelAction.Message.Send(error)) } + } + } // negotiation restarts from the beginning, and is initialized by the initiator // note: in any case we still need to keep all previously sent closing_signed, because they may publish one of them - state is Negotiating && state.commitments.params.localParams.isInitiator -> { - // we could use the last closing_signed we sent, but network fees may have changed while we were offline so it is better to restart from scratch - val (closingTx, closingSigned) = Helpers.Closing.makeFirstClosingTx( - channelKeys(), - state.commitments.latest, - state.localShutdown.scriptPubKey.toByteArray(), - state.remoteShutdown.scriptPubKey.toByteArray(), - state.closingFeerates ?: ClosingFeerates(currentOnChainFeerates.mutualCloseFeerate) - ) - val closingTxProposed1 = state.closingTxProposed + listOf(listOf(ClosingTxProposed(closingTx, closingSigned))) - val nextState = state.copy(closingTxProposed = closingTxProposed1) - val actions = listOf( - ChannelAction.Storage.StoreState(nextState), - ChannelAction.Message.Send(state.localShutdown), - ChannelAction.Message.Send(closingSigned) - ) - return Pair(nextState, actions) - } - state is Negotiating -> { - // we start a new round of negotiation - val closingTxProposed1 = if (state.closingTxProposed.last().isEmpty()) state.closingTxProposed else state.closingTxProposed + listOf(listOf()) - val nextState = state.copy(closingTxProposed = closingTxProposed1) - val actions = listOf( - ChannelAction.Storage.StoreState(nextState), - ChannelAction.Message.Send(state.localShutdown) - ) - return Pair(nextState, actions) - } - else -> unhandled(cmd) + is Negotiating -> + if (state.commitments.params.localParams.isInitiator) { + // we could use the last closing_signed we sent, but network fees may have changed while we were offline so it is better to restart from scratch + val (closingTx, closingSigned) = Helpers.Closing.makeFirstClosingTx( + channelKeys(), + state.commitments.latest, + state.localShutdown.scriptPubKey.toByteArray(), + state.remoteShutdown.scriptPubKey.toByteArray(), + state.closingFeerates ?: ClosingFeerates(currentOnChainFeerates.mutualCloseFeerate) + ) + val closingTxProposed1 = state.closingTxProposed + listOf(listOf(ClosingTxProposed(closingTx, closingSigned))) + val nextState = state.copy(closingTxProposed = closingTxProposed1) + val actions = listOf( + ChannelAction.Storage.StoreState(nextState), + ChannelAction.Message.Send(state.localShutdown), + ChannelAction.Message.Send(closingSigned) + ) + return Pair(nextState, actions) + } else { + // we start a new round of negotiation + val closingTxProposed1 = if (state.closingTxProposed.last().isEmpty()) state.closingTxProposed else state.closingTxProposed + listOf(listOf()) + val nextState = state.copy(closingTxProposed = closingTxProposed1) + val actions = listOf( + ChannelAction.Storage.StoreState(nextState), + ChannelAction.Message.Send(state.localShutdown) + ) + return Pair(nextState, actions) + } + is Closing, is Closed, is WaitForRemotePublishFutureCommitment -> unhandled(cmd) } Pair(nextState, buildList { if (!channelReestablishSent) { diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt index 50b4685ff..41376d445 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt @@ -528,6 +528,19 @@ class ShutdownTestsCommon : LightningTestSuite() { testLocalForceClose(alice1, actions1) } + @Test + fun `basic disconnection and reconnection`() { + val (alice0, bob0) = init(bobFeatures = TestConstants.Bob.nodeParams.features.remove(Feature.ChannelBackupClient)) + val (alice1, bob1, reestablishes) = SyncingTestsCommon.disconnect(alice0, bob0) + val (aliceReestablish, bobReestablish) = reestablishes + val (alice2, actionsAlice2) = alice1.process(ChannelCommand.MessageReceived(aliceReestablish)) + assertIs(alice2.state) + actionsAlice2.hasOutgoingMessage() + val (bob2, actionsBob2) = bob1.process(ChannelCommand.MessageReceived(bobReestablish)) + assertIs(bob2.state) + actionsBob2.hasOutgoingMessage() + } + companion object { val r1 = randomBytes32() val r2 = randomBytes32() @@ -566,6 +579,8 @@ class ShutdownTestsCommon : LightningTestSuite() { val (alice2, _) = alice1.process(ChannelCommand.MessageReceived(shutdown1)) assertIs>(alice2) assertIs>(bob1) + assertIs(alice2.state) + assertIs(bob1.state) if (alice2.state.commitments.params.channelFeatures.hasFeature(Feature.ChannelBackupClient)) assertFalse(shutdown.channelData.isEmpty()) if (bob1.state.commitments.params.channelFeatures.hasFeature(Feature.ChannelBackupClient)) assertFalse(shutdown1.channelData.isEmpty()) return Pair(alice2, bob1) From dedd445e9e9a053be1cfea485168452c71964998 Mon Sep 17 00:00:00 2001 From: pm47 Date: Tue, 16 Apr 2024 16:20:47 +0200 Subject: [PATCH 2/5] minor fixup Use exhaustive match. --- .../kotlin/fr/acinq/lightning/channel/Helpers.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt index c37a6a3e0..d63f182de 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt @@ -200,8 +200,8 @@ object Helpers { * - false if we are behind */ fun checkRemoteCommit(commitments: Commitments, nextLocalCommitmentNumber: Long): Boolean { - return when { - commitments.remoteNextCommitInfo.isLeft -> + return when (commitments.remoteNextCommitInfo) { + is Either.Left -> when { // we just sent a new commit_sig but they didn't receive it nextLocalCommitmentNumber == commitments.nextRemoteCommitIndex -> true @@ -211,7 +211,7 @@ object Helpers { nextLocalCommitmentNumber < commitments.nextRemoteCommitIndex -> true else -> false } - commitments.remoteNextCommitInfo.isRight -> + is Either.Right -> when { // they have acknowledged the last commit_sig we sent nextLocalCommitmentNumber == (commitments.remoteCommitIndex + 1) -> true @@ -219,7 +219,6 @@ object Helpers { nextLocalCommitmentNumber < (commitments.remoteCommitIndex + 1) -> true else -> false } - else -> false } } From f3b1dc4380451b81be969a7c52d728a06182dd4a Mon Sep 17 00:00:00 2001 From: pm47 Date: Tue, 16 Apr 2024 17:06:59 +0200 Subject: [PATCH 3/5] factor sync logic between Normal/ShuttingDown This is inspired from eclair, but changes are kept minimal. We don't support the `RemoteLate` case. Eclair had a full rework in https://github.com/ACINQ/eclair/pull/2036, that separated concerns better and got completely rid of exceptions. --- .../acinq/lightning/channel/states/Syncing.kt | 273 +++++++++++------- 1 file changed, 162 insertions(+), 111 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 26bc77fff..f37be9f9b 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt @@ -1,5 +1,6 @@ package fr.acinq.lightning.channel.states +import fr.acinq.bitcoin.PrivateKey import fr.acinq.bitcoin.utils.Either import fr.acinq.lightning.ShortChannelId import fr.acinq.lightning.blockchain.* @@ -114,135 +115,106 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: Pair(state, actions) } is Normal -> { - when { - !Helpers.checkLocalCommit(state.commitments, cmd.message.nextRemoteRevocationNumber) -> { - // if next_remote_revocation_number is greater than our local commitment index, it means that either we are using an outdated commitment, or they are lying - // but first we need to make sure that the last per_commitment_secret that they claim to have received from us is correct for that next_remote_revocation_number minus 1 - if (channelKeys().commitmentSecret(cmd.message.nextRemoteRevocationNumber - 1) == cmd.message.yourLastCommitmentSecret) { - // their data checks out, we indeed seem to be using an old revoked commitment, and must absolutely *NOT* publish it, because that would be a cheating attempt and they - // would punish us by taking all the funds in the channel - logger.warning { "counterparty proved that we have an outdated (revoked) local commitment!!! ourCommitmentNumber=${state.commitments.localCommitIndex} theirCommitmentNumber=${cmd.message.nextRemoteRevocationNumber}" } - } else { - // they are deliberately trying to fool us into thinking we have a late commitment, but we cannot risk publishing it ourselves, because it may really be revoked! - logger.warning { "counterparty claims that we have an outdated commitment, but they sent an invalid proof, so our commitment may or may not be revoked: ourLocalCommitmentNumber=${state.commitments.localCommitIndex} theirRemoteCommitmentNumber=${cmd.message.nextRemoteRevocationNumber}" } - } - val exc = PleasePublishYourCommitment(channelId) - val error = Error(channelId, exc.message.encodeToByteArray().toByteVector()) - val nextState = WaitForRemotePublishFutureCommitment(state.commitments, cmd.message) - val actions = listOf( - ChannelAction.Storage.StoreState(nextState), - ChannelAction.Message.Send(error) - ) - Pair(nextState, actions) - } - !Helpers.checkRemoteCommit(state.commitments, cmd.message.nextLocalCommitmentNumber) -> { - // if next_local_commit_number is more than one more our remote commitment index, it means that either we are using an outdated commitment, or they are lying - logger.warning { "counterparty says that they have a more recent commitment than the one we know of!!! ourCommitmentNumber=${state.commitments.latest.nextRemoteCommit?.commit?.index ?: state.commitments.latest.remoteCommit.index} theirCommitmentNumber=${cmd.message.nextLocalCommitmentNumber}" } - // there is no way to make sure that they are saying the truth, the best thing to do is ask them to publish their commitment right now - // maybe they will publish their commitment, in that case we need to remember their commitment point in order to be able to claim our outputs - // not that if they don't comply, we could publish our own commitment (it is not stale, otherwise we would be in the case above) - val exc = PleasePublishYourCommitment(channelId) - val error = Error(channelId, exc.message.encodeToByteArray().toByteVector()) - val nextState = WaitForRemotePublishFutureCommitment(state.commitments, cmd.message) - val actions = listOf( - ChannelAction.Storage.StoreState(nextState), - ChannelAction.Message.Send(error) - ) - Pair(nextState, actions) - } - else -> { - // normal case, our data is up-to-date - val actions = ArrayList() + try { + when (val syncResult = checkSync(state, cmd.message)) { + is SyncResult.Failure -> handleSyncFailure(cmd.message, syncResult, state.commitments) + is SyncResult.Success -> { + // normal case, our data is up-to-date + val actions = ArrayList() - // 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)) - } + // 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)) + } - // resume splice signing session if any - val spliceStatus1 = if (state.spliceStatus is SpliceStatus.WaitingForSigs && state.spliceStatus.session.fundingTx.txId == cmd.message.nextFundingTxId) { - // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig. - logger.info { "re-sending commit_sig for splice attempt with fundingTxIndex=${state.spliceStatus.session.fundingTxIndex} fundingTxId=${state.spliceStatus.session.fundingTx.txId}" } - val commitSig = state.spliceStatus.session.remoteCommit.sign(channelKeys(), state.commitments.params, state.spliceStatus.session) - actions.add(ChannelAction.Message.Send(commitSig)) - state.spliceStatus - } else if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) { - 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)) + // resume splice signing session if any + val spliceStatus1 = if (state.spliceStatus is SpliceStatus.WaitingForSigs && state.spliceStatus.session.fundingTx.txId == cmd.message.nextFundingTxId) { + // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig. + logger.info { "re-sending commit_sig for splice attempt with fundingTxIndex=${state.spliceStatus.session.fundingTxIndex} fundingTxId=${state.spliceStatus.session.fundingTx.txId}" } + val commitSig = state.spliceStatus.session.remoteCommit.sign(channelKeys(), state.commitments.params, state.spliceStatus.session) + actions.add(ChannelAction.Message.Send(commitSig)) + state.spliceStatus + } else if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) { + 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(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)) } + state.spliceStatus + } else if (cmd.message.nextFundingTxId != null) { + // The fundingTxId must be for a splice attempt that we didn't store (we got disconnected before receiving their tx_complete) + logger.info { "aborting obsolete splice attempt for fundingTxId=${cmd.message.nextFundingTxId}" } + actions.add(ChannelAction.Message.Send(TxAbort(state.channelId, SpliceAborted(state.channelId).message))) + SpliceStatus.Aborted + } else { + state.spliceStatus } - state.spliceStatus - } else if (cmd.message.nextFundingTxId != null) { - // The fundingTxId must be for a splice attempt that we didn't store (we got disconnected before receiving their tx_complete) - logger.info { "aborting obsolete splice attempt for fundingTxId=${cmd.message.nextFundingTxId}" } - actions.add(ChannelAction.Message.Send(TxAbort(state.channelId, SpliceAborted(state.channelId).message))) - SpliceStatus.Aborted - } else { - 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) - actions.add(ChannelAction.Message.Send(spliceLocked)) - } + // 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) + actions.add(ChannelAction.Message.Send(spliceLocked)) + } - try { - val (commitments1, sendQueue1) = handleSync(cmd.message, state) - actions.addAll(sendQueue1) + val commitments1 = syncResult.commitments + actions.addAll(syncResult.actions) // BOLT 2: A node if it has sent a previous shutdown MUST retransmit shutdown. state.localShutdown?.let { logger.debug { "re-sending local shutdown" } actions.add(ChannelAction.Message.Send(it)) } Pair(state.copy(commitments = commitments1, spliceStatus = spliceStatus1), actions) - } catch (e: RevocationSyncError) { - val error = Error(channelId, e.message) - state.run { spendLocalCurrent() }.run { copy(second = second + ChannelAction.Message.Send(error)) } } } + } catch (e: RevocationSyncError) { + val error = Error(channelId, e.message) + state.run { spendLocalCurrent() }.run { copy(second = second + ChannelAction.Message.Send(error)) } } } is ShuttingDown -> { try { - val (commitments1, sendQueue1) = handleSync(cmd.message, state) - val actions = buildList { - addAll(sendQueue1) - add(ChannelAction.Message.Send(state.localShutdown)) + when (val syncResult = checkSync(state, cmd.message)) { + is SyncResult.Failure -> handleSyncFailure(cmd.message, syncResult, state.commitments) + is SyncResult.Success -> { + val commitments1 = syncResult.commitments + val actions = buildList { + addAll(syncResult.actions) + add(ChannelAction.Message.Send(state.localShutdown)) + } + Pair(state.copy(commitments = commitments1), actions) + } } - Pair(state.copy(commitments = commitments1), actions) } catch (e: RevocationSyncError) { val error = Error(channelId, e.message) state.run { spendLocalCurrent() }.run { copy(second = second + ChannelAction.Message.Send(error)) } @@ -369,7 +341,20 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: } companion object { - private fun ChannelContext.handleSync(channelReestablish: ChannelReestablish, d: ChannelStateWithCommitments): Pair> { + + // @formatter:off + sealed class SyncResult { + data class Success(val commitments: Commitments, val actions: List) : SyncResult() + sealed class Failure : SyncResult() { + data class LocalLateProven(val ourLocalCommitmentNumber: Long, val theirRemoteCommitmentNumber: Long) : Failure() + data class LocalLateUnproven(val ourRemoteCommitmentNumber: Long, val theirLocalCommitmentNumber: Long) : Failure() + data class RemoteLying(val ourLocalCommitmentNumber: Long, val theirRemoteCommitmentNumber: Long, val invalidPerCommitmentSecret: PrivateKey) : Failure() + //data object RemoteLate : Failure() + } + } + // @formatter:on + + private fun ChannelContext.handleSync(channelReestablish: ChannelReestablish, d: ChannelStateWithCommitments): SyncResult.Success { val sendQueue = ArrayList() // first we clean up unacknowledged updates logger.debug { "discarding proposed OUT: ${d.commitments.changes.localChanges.proposed}" } @@ -443,7 +428,73 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: logger.info { "re-processing signed IN: ${htlcsToReprocess.map { it.add.id }.joinToString()}" } sendQueue.addAll(htlcsToReprocess) - return Pair(commitments1, sendQueue) + return SyncResult.Success(commitments1, sendQueue) + } + + private fun ChannelContext.checkSync(d: ChannelStateWithCommitments, remoteChannelReestablish: ChannelReestablish): SyncResult { + val commitments = d.commitments + val channelKeys = keyManager.channelKeys(commitments.params.localParams.fundingKeyPath) + return when { + !Helpers.checkLocalCommit(commitments, remoteChannelReestablish.nextRemoteRevocationNumber) -> { + // if next_remote_revocation_number is greater than our local commitment index, it means that either we are using an outdated commitment, or they are lying + // but first we need to make sure that the last per_commitment_secret that they claim to have received from us is correct for that next_remote_revocation_number minus 1 + if (channelKeys.commitmentSecret(remoteChannelReestablish.nextRemoteRevocationNumber - 1) == remoteChannelReestablish.yourLastCommitmentSecret) { + SyncResult.Failure.LocalLateProven(ourLocalCommitmentNumber = commitments.localCommitIndex, theirRemoteCommitmentNumber = remoteChannelReestablish.nextRemoteRevocationNumber) + } else { + SyncResult.Failure.RemoteLying( + ourLocalCommitmentNumber = commitments.localCommitIndex, + theirRemoteCommitmentNumber = remoteChannelReestablish.nextRemoteRevocationNumber, + invalidPerCommitmentSecret = remoteChannelReestablish.yourLastCommitmentSecret + ) + } + } + !Helpers.checkRemoteCommit(commitments, remoteChannelReestablish.nextLocalCommitmentNumber) -> { + // if next_local_commit_number is more than one more our remote commitment index, it means that either we are using an outdated commitment, or they are lying + SyncResult.Failure.LocalLateUnproven( + ourRemoteCommitmentNumber = commitments.latest.nextRemoteCommit?.commit?.index ?: commitments.latest.remoteCommit.index, + theirLocalCommitmentNumber = remoteChannelReestablish.nextLocalCommitmentNumber + ) + } + else -> handleSync(remoteChannelReestablish, d) + } + } + + private fun handleOutdatedCommitment(remoteChannelReestablish: ChannelReestablish, commitments: Commitments): Pair> { + val exc = PleasePublishYourCommitment(commitments.channelId) + val error = Error(commitments.channelId, exc.message.encodeToByteArray().toByteVector()) + val nextState = WaitForRemotePublishFutureCommitment(commitments, remoteChannelReestablish) + val actions = listOf( + ChannelAction.Storage.StoreState(nextState), + ChannelAction.Message.Send(error) + ) + return Pair(nextState, actions) + } + + private fun ChannelContext.handleSyncFailure(remoteChannelReestablish: ChannelReestablish, syncFailure: SyncResult.Failure, commitments: Commitments): Pair> { + return when (syncFailure) { + is SyncResult.Failure.LocalLateProven -> { + // their data checks out, we indeed seem to be using an old revoked commitment, and must absolutely *NOT* publish it, because that would be a cheating attempt and they + // would punish us by taking all the funds in the channel + logger.warning { "counterparty proved that we have an outdated (revoked) local commitment!!! ourCommitmentNumber=${commitments.localCommitIndex} theirCommitmentNumber=${remoteChannelReestablish.nextRemoteRevocationNumber}" } + handleOutdatedCommitment(remoteChannelReestablish, commitments) + } + is SyncResult.Failure.LocalLateUnproven -> { + // there is no way to make sure that they are saying the truth, the best thing to do is ask them to publish their commitment right now + // maybe they will publish their commitment, in that case we need to remember their commitment point in order to be able to claim our outputs + // not that if they don't comply, we could publish our own commitment (it is not stale, otherwise we would be in the case above) + logger.warning { "counterparty says that they have a more recent commitment than the one we know of!!! ourCommitmentNumber=${commitments.latest.nextRemoteCommit?.commit?.index ?: commitments.latest.remoteCommit.index} theirCommitmentNumber=${remoteChannelReestablish.nextLocalCommitmentNumber}" } + handleOutdatedCommitment(remoteChannelReestablish, commitments) + } + is SyncResult.Failure.RemoteLying -> { + // they are deliberately trying to fool us into thinking we have a late commitment, but we cannot risk publishing it ourselves, because it may really be revoked! + logger.warning { "counterparty claims that we have an outdated commitment, but they sent an invalid proof, so our commitment may or may not be revoked: ourLocalCommitmentNumber=${commitments.localCommitIndex} theirRemoteCommitmentNumber=${remoteChannelReestablish.nextRemoteRevocationNumber}" } + handleOutdatedCommitment(remoteChannelReestablish, commitments) + } +// is SyncResult.Failure.RemoteLate -> { +// logger.error { "counterparty appears to be using an outdated commitment, they may request a force-close, standing by..." } +// stay() +// } + } } } From 45f9a48ca126586f120aeecc6d14470929895bb7 Mon Sep 17 00:00:00 2001 From: pm47 Date: Tue, 16 Apr 2024 18:19:47 +0200 Subject: [PATCH 4/5] =?UTF-8?q?full=20factorization=20=C3=A0=20la=20eclair?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the equivalent of https://github.com/ACINQ/eclair/pull/2036. Notably we remove the `RevocationSyncError` exception, and in case the peer has an outdated commitment we stay in stand by, waiting for their `error` before force-closing. --- .../lightning/channel/ChannelException.kt | 1 - .../fr/acinq/lightning/channel/Helpers.kt | 50 -- .../acinq/lightning/channel/states/Syncing.kt | 455 ++++++++++-------- .../channel/states/ClosingTestsCommon.kt | 20 +- .../channel/states/OfflineTestsCommon.kt | 19 +- 5 files changed, 265 insertions(+), 280 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt index 3f08d7cc2..ee4724cec 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt @@ -84,7 +84,6 @@ data class CannotSignWithoutChanges (override val channelId: Byte data class CannotSignBeforeRevocation (override val channelId: ByteVector32) : ChannelException(channelId, "cannot sign until next revocation hash is received") data class UnexpectedRevocation (override val channelId: ByteVector32) : ChannelException(channelId, "received unexpected RevokeAndAck message") data class InvalidRevocation (override val channelId: ByteVector32) : ChannelException(channelId, "invalid revocation") -data class RevocationSyncError (override val channelId: ByteVector32) : ChannelException(channelId, "revocation sync error") data class InvalidFailureCode (override val channelId: ByteVector32) : ChannelException(channelId, "UpdateFailMalformedHtlc message doesn't have BADONION bit set") data class PleasePublishYourCommitment (override val channelId: ByteVector32) : ChannelException(channelId, "please publish your local commitment") data class CommandUnavailableInThisState (override val channelId: ByteVector32, val state: String) : ChannelException(channelId, "cannot execute command in state=$state") diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt index d63f182de..c37a4e884 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt @@ -172,56 +172,6 @@ object Helpers { toRemote > it.localChannelReserve(commitments.params) } - /** - * Tells whether or not their expected next remote commitment number matches with our data - * - * @return - * - true if parties are in sync or remote is behind - * - false if we are behind - */ - fun checkLocalCommit(commitments: Commitments, nextRemoteRevocationNumber: Long): Boolean { - return when { - // they just sent a new commit_sig, we have received it but they didn't receive our revocation - commitments.localCommitIndex == nextRemoteRevocationNumber -> true - // we are in sync - commitments.localCommitIndex == nextRemoteRevocationNumber + 1 -> true - // remote is behind: we return true because things are fine on our side - commitments.localCommitIndex > nextRemoteRevocationNumber + 1 -> true - // we are behind - else -> false - } - } - - /** - * Tells whether or not their expected next local commitment number matches with our data - * - * @return - * - true if parties are in sync or remote is behind - * - false if we are behind - */ - fun checkRemoteCommit(commitments: Commitments, nextLocalCommitmentNumber: Long): Boolean { - return when (commitments.remoteNextCommitInfo) { - is Either.Left -> - when { - // we just sent a new commit_sig but they didn't receive it - nextLocalCommitmentNumber == commitments.nextRemoteCommitIndex -> true - // we just sent a new commit_sig, they have received it but we haven't received their revocation - nextLocalCommitmentNumber == (commitments.nextRemoteCommitIndex + 1) -> true - // they are behind - nextLocalCommitmentNumber < commitments.nextRemoteCommitIndex -> true - else -> false - } - is Either.Right -> - when { - // they have acknowledged the last commit_sig we sent - nextLocalCommitmentNumber == (commitments.remoteCommitIndex + 1) -> true - // they are behind - nextLocalCommitmentNumber < (commitments.remoteCommitIndex + 1) -> true - else -> false - } - } - } - /** This helper method will publish txs only if they haven't yet reached minDepth. */ fun LoggingContext.publishIfNeeded(txs: List, irrevocablySpent: Map): List { val (skip, process) = txs.partition { it.tx.inputsAlreadySpent(irrevocablySpent) } 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 f37be9f9b..2b51aab4f 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt @@ -115,109 +115,114 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: Pair(state, actions) } is Normal -> { - try { - when (val syncResult = checkSync(state, cmd.message)) { - is SyncResult.Failure -> handleSyncFailure(cmd.message, syncResult, state.commitments) - is SyncResult.Success -> { - // normal case, our data is up-to-date - val actions = ArrayList() + when (val syncResult = handleSync(state.commitments, cmd.message)) { + is SyncResult.Failure -> handleSyncFailure(state.commitments, cmd.message, syncResult) + is SyncResult.Success -> { + // normal case, our data is up-to-date + val actions = ArrayList() - // 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)) - } + // 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)) + } - // resume splice signing session if any - val spliceStatus1 = if (state.spliceStatus is SpliceStatus.WaitingForSigs && state.spliceStatus.session.fundingTx.txId == cmd.message.nextFundingTxId) { - // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig. - logger.info { "re-sending commit_sig for splice attempt with fundingTxIndex=${state.spliceStatus.session.fundingTxIndex} fundingTxId=${state.spliceStatus.session.fundingTx.txId}" } - val commitSig = state.spliceStatus.session.remoteCommit.sign(channelKeys(), state.commitments.params, state.spliceStatus.session) - actions.add(ChannelAction.Message.Send(commitSig)) - state.spliceStatus - } else if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) { - 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)) + // resume splice signing session if any + val spliceStatus1 = if (state.spliceStatus is SpliceStatus.WaitingForSigs && state.spliceStatus.session.fundingTx.txId == cmd.message.nextFundingTxId) { + // We retransmit our commit_sig, and will send our tx_signatures once we've received their commit_sig. + logger.info { "re-sending commit_sig for splice attempt with fundingTxIndex=${state.spliceStatus.session.fundingTxIndex} fundingTxId=${state.spliceStatus.session.fundingTx.txId}" } + val commitSig = state.spliceStatus.session.remoteCommit.sign(channelKeys(), state.commitments.params, state.spliceStatus.session) + actions.add(ChannelAction.Message.Send(commitSig)) + state.spliceStatus + } else if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) { + 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)) } - state.spliceStatus - } else if (cmd.message.nextFundingTxId != null) { - // The fundingTxId must be for a splice attempt that we didn't store (we got disconnected before receiving their tx_complete) - logger.info { "aborting obsolete splice attempt for fundingTxId=${cmd.message.nextFundingTxId}" } - actions.add(ChannelAction.Message.Send(TxAbort(state.channelId, SpliceAborted(state.channelId).message))) - SpliceStatus.Aborted - } else { - 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) - actions.add(ChannelAction.Message.Send(spliceLocked)) + 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)) } + } + state.spliceStatus + } else if (cmd.message.nextFundingTxId != null) { + // The fundingTxId must be for a splice attempt that we didn't store (we got disconnected before receiving their tx_complete) + logger.info { "aborting obsolete splice attempt for fundingTxId=${cmd.message.nextFundingTxId}" } + actions.add(ChannelAction.Message.Send(TxAbort(state.channelId, SpliceAborted(state.channelId).message))) + SpliceStatus.Aborted + } else { + state.spliceStatus + } - val commitments1 = syncResult.commitments - actions.addAll(syncResult.actions) - // BOLT 2: A node if it has sent a previous shutdown MUST retransmit shutdown. - state.localShutdown?.let { - logger.debug { "re-sending local shutdown" } - actions.add(ChannelAction.Message.Send(it)) + // 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) + actions.add(ChannelAction.Message.Send(spliceLocked)) } - Pair(state.copy(commitments = commitments1, spliceStatus = spliceStatus1), actions) + + // we may need to retransmit updates and/or commit_sig and/or revocation + actions.addAll(syncResult.retransmit.map { ChannelAction.Message.Send(it) }) + + // then we clean up unsigned updates + val commitments1 = discardUnsignedUpdates(state.commitments) + + if (commitments1.changes.localHasChanges()) { + actions.add(ChannelAction.Message.SendToSelf(ChannelCommand.Commitment.Sign)) } + + // When a channel is reestablished after a wallet restarts, we need to reprocess incoming HTLCs that may have been only partially processed + // (either because they didn't reach the payment handler, or because the payment handler response didn't reach the channel). + // Otherwise these HTLCs will stay in our commitment until they timeout and our peer closes the channel. + val htlcsToReprocess = commitments1.reprocessIncomingHtlcs() + logger.info { "re-processing signed IN: ${htlcsToReprocess.map { it.add.id }.joinToString()}" } + actions.addAll(htlcsToReprocess) + + // BOLT 2: A node if it has sent a previous shutdown MUST retransmit shutdown. + state.localShutdown?.let { + logger.debug { "re-sending local shutdown" } + actions.add(ChannelAction.Message.Send(it)) + } + Pair(state.copy(commitments = commitments1, spliceStatus = spliceStatus1), actions) } - } catch (e: RevocationSyncError) { - val error = Error(channelId, e.message) - state.run { spendLocalCurrent() }.run { copy(second = second + ChannelAction.Message.Send(error)) } } } is ShuttingDown -> { - try { - when (val syncResult = checkSync(state, cmd.message)) { - is SyncResult.Failure -> handleSyncFailure(cmd.message, syncResult, state.commitments) - is SyncResult.Success -> { - val commitments1 = syncResult.commitments - val actions = buildList { - addAll(syncResult.actions) - add(ChannelAction.Message.Send(state.localShutdown)) - } - Pair(state.copy(commitments = commitments1), actions) - } + when (val syncResult = handleSync(state.commitments, cmd.message)) { + is SyncResult.Failure -> handleSyncFailure(state.commitments, cmd.message, syncResult) + is SyncResult.Success -> { + val commitments1 = discardUnsignedUpdates(state.commitments) + val actions = buildList { + addAll(syncResult.retransmit) + add(state.localShutdown) + }.map { ChannelAction.Message.Send(it) } + Pair(state.copy(commitments = commitments1), actions) } - } catch (e: RevocationSyncError) { - val error = Error(channelId, e.message) - state.run { spendLocalCurrent() }.run { copy(second = second + ChannelAction.Message.Send(error)) } } } // negotiation restarts from the beginning, and is initialized by the initiator @@ -340,123 +345,176 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: } } + private fun ChannelContext.handleSyncFailure(commitments: Commitments, remoteChannelReestablish: ChannelReestablish, syncFailure: SyncResult.Failure): Pair> { + return when (syncFailure) { + is SyncResult.Failure.LocalLateProven -> { + // their data checks out, we indeed seem to be using an old revoked commitment, and must absolutely *NOT* publish it, because that would be a cheating attempt and they + // would punish us by taking all the funds in the channel + logger.warning { "counterparty proved that we have an outdated (revoked) local commitment!!! ourCommitmentNumber=${commitments.localCommitIndex} theirCommitmentNumber=${remoteChannelReestablish.nextRemoteRevocationNumber}" } + handleOutdatedCommitment(remoteChannelReestablish, commitments) + } + is SyncResult.Failure.LocalLateUnproven -> { + // there is no way to make sure that they are saying the truth, the best thing to do is ask them to publish their commitment right now + // maybe they will publish their commitment, in that case we need to remember their commitment point in order to be able to claim our outputs + // not that if they don't comply, we could publish our own commitment (it is not stale, otherwise we would be in the case above) + logger.warning { "counterparty says that they have a more recent commitment than the one we know of!!! ourCommitmentNumber=${commitments.latest.nextRemoteCommit?.commit?.index ?: commitments.latest.remoteCommit.index} theirCommitmentNumber=${remoteChannelReestablish.nextLocalCommitmentNumber}" } + handleOutdatedCommitment(remoteChannelReestablish, commitments) + } + is SyncResult.Failure.RemoteLying -> { + // they are deliberately trying to fool us into thinking we have a late commitment, but we cannot risk publishing it ourselves, because it may really be revoked! + logger.warning { "counterparty claims that we have an outdated commitment, but they sent an invalid proof, so our commitment may or may not be revoked: ourLocalCommitmentNumber=${commitments.localCommitIndex} theirRemoteCommitmentNumber=${remoteChannelReestablish.nextRemoteRevocationNumber}" } + handleOutdatedCommitment(remoteChannelReestablish, commitments) + } + is SyncResult.Failure.RemoteLate -> { + logger.error { "counterparty appears to be using an outdated commitment, they may request a force-close, standing by..." } + Pair(this@Syncing, listOf()) + } + } + } + companion object { // @formatter:off sealed class SyncResult { - data class Success(val commitments: Commitments, val actions: List) : SyncResult() + data class Success(val retransmit: List) : SyncResult() sealed class Failure : SyncResult() { data class LocalLateProven(val ourLocalCommitmentNumber: Long, val theirRemoteCommitmentNumber: Long) : Failure() data class LocalLateUnproven(val ourRemoteCommitmentNumber: Long, val theirLocalCommitmentNumber: Long) : Failure() data class RemoteLying(val ourLocalCommitmentNumber: Long, val theirRemoteCommitmentNumber: Long, val invalidPerCommitmentSecret: PrivateKey) : Failure() - //data object RemoteLate : Failure() + data object RemoteLate : Failure() } } // @formatter:on - private fun ChannelContext.handleSync(channelReestablish: ChannelReestablish, d: ChannelStateWithCommitments): SyncResult.Success { - val sendQueue = ArrayList() - // first we clean up unacknowledged updates - logger.debug { "discarding proposed OUT: ${d.commitments.changes.localChanges.proposed}" } - logger.debug { "discarding proposed IN: ${d.commitments.changes.remoteChanges.proposed}" } - val commitments1 = d.commitments.copy( - changes = d.commitments.changes.copy( - localChanges = d.commitments.changes.localChanges.copy(proposed = emptyList()), - remoteChanges = d.commitments.changes.remoteChanges.copy(proposed = emptyList()), - localNextHtlcId = d.commitments.changes.localNextHtlcId - d.commitments.changes.localChanges.proposed.filterIsInstance().size, - remoteNextHtlcId = d.commitments.changes.remoteNextHtlcId - d.commitments.changes.remoteChanges.proposed.filterIsInstance().size - ) - ) - logger.debug { "localNextHtlcId=${d.commitments.changes.localNextHtlcId}->${commitments1.changes.localNextHtlcId}" } - logger.debug { "remoteNextHtlcId=${d.commitments.changes.remoteNextHtlcId}->${commitments1.changes.remoteNextHtlcId}" } + /** + * Check whether we are in sync with our peer. + */ + fun ChannelContext.handleSync(commitments: Commitments, remoteChannelReestablish: ChannelReestablish): SyncResult { + val channelKeys = keyManager.channelKeys(commitments.params.localParams.fundingKeyPath) + // This is done in two steps: + // - step 1: we check our local commitment + // - step 2: we check the remote commitment + // step 2 depends on step 1 because we need to preserve ordering between commit_sig and revocation - fun resendRevocation() { - // let's see the state of remote sigs - when (commitments1.localCommitIndex) { - channelReestablish.nextRemoteRevocationNumber -> { - // nothing to do + // step 2: we check the remote commitment + fun checkRemoteCommit(remoteChannelReestablish: ChannelReestablish, retransmitRevocation: RevokeAndAck?): SyncResult { + return when (val rnci = commitments.remoteNextCommitInfo) { + is Either.Left -> { + when { + remoteChannelReestablish.nextLocalCommitmentNumber == commitments.nextRemoteCommitIndex -> { + // we just sent a new commit_sig but they didn't receive it + // we resend the same updates and the same sig, and preserve the same ordering + val signedUpdates = commitments.changes.localChanges.signed + val commitSigs = commitments.active.map { it.nextRemoteCommit }.filterIsInstance().map { it.sig } + SyncResult.Success(retransmit = when (retransmitRevocation) { + null -> buildList { + addAll(signedUpdates) + addAll(commitSigs) + } + else -> if (commitments.localCommitIndex > rnci.value.sentAfterLocalCommitIndex) { + buildList { + addAll(signedUpdates) + addAll(commitSigs) + add(retransmitRevocation) + } + } else { + buildList { + add(retransmitRevocation) + addAll(signedUpdates) + addAll(commitSigs) + } + } + }) + } + remoteChannelReestablish.nextLocalCommitmentNumber == (commitments.nextRemoteCommitIndex + 1) -> { + // we just sent a new commit_sig, they have received it but we haven't received their revocation + SyncResult.Success(retransmit = listOfNotNull(retransmitRevocation)) + } + remoteChannelReestablish.nextLocalCommitmentNumber < commitments.nextRemoteCommitIndex -> { + // they are behind + SyncResult.Failure.RemoteLate + } + else -> { + // we are behind + SyncResult.Failure.LocalLateUnproven( + ourRemoteCommitmentNumber = commitments.nextRemoteCommitIndex, + theirLocalCommitmentNumber = remoteChannelReestablish.nextLocalCommitmentNumber - 1 + ) + } + } } - channelReestablish.nextRemoteRevocationNumber + 1 -> { - // our last revocation got lost, let's resend it - logger.debug { "re-sending last revocation" } - val channelKeys = keyManager.channelKeys(d.commitments.params.localParams.fundingKeyPath) - val localPerCommitmentSecret = channelKeys.commitmentSecret(d.commitments.localCommitIndex - 1) - val localNextPerCommitmentPoint = channelKeys.commitmentPoint(d.commitments.localCommitIndex + 1) - val revocation = RevokeAndAck(commitments1.channelId, localPerCommitmentSecret, localNextPerCommitmentPoint) - sendQueue.add(ChannelAction.Message.Send(revocation)) + is Either.Right -> { + when { + remoteChannelReestablish.nextLocalCommitmentNumber == (commitments.remoteCommitIndex + 1) -> { + // they have acknowledged the last commit_sig we sent + SyncResult.Success(retransmit = listOfNotNull(retransmitRevocation)) + } + remoteChannelReestablish.nextLocalCommitmentNumber < (commitments.remoteCommitIndex + 1) -> { + // they are behind + SyncResult.Failure.RemoteLate + } + else -> { + // we are behind + SyncResult.Failure.LocalLateUnproven( + ourRemoteCommitmentNumber = commitments.remoteCommitIndex, + theirLocalCommitmentNumber = remoteChannelReestablish.nextLocalCommitmentNumber - 1 + ) + } + } } - else -> throw RevocationSyncError(d.channelId) } } - when { - commitments1.remoteNextCommitInfo.isLeft && commitments1.nextRemoteCommitIndex + 1 == channelReestablish.nextLocalCommitmentNumber -> { - // we had sent a new sig and were waiting for their revocation - // they had received the new sig but their revocation was lost during the disconnection - // they will send us the revocation, nothing to do here - logger.debug { "waiting for them to re-send their last revocation" } - resendRevocation() - } - commitments1.remoteNextCommitInfo.isLeft && commitments1.nextRemoteCommitIndex == channelReestablish.nextLocalCommitmentNumber -> { - // we had sent a new sig and were waiting for their revocation - // they didn't receive the new sig because of the disconnection - // we just resend the same updates and the same sig - val revWasSentLast = commitments1.localCommitIndex > commitments1.remoteNextCommitInfo.left!!.sentAfterLocalCommitIndex - if (!revWasSentLast) resendRevocation() - - logger.debug { "re-sending previously local signed changes: ${commitments1.changes.localChanges.signed}" } - commitments1.changes.localChanges.signed.forEach { sendQueue.add(ChannelAction.Message.Send(it)) } - logger.debug { "re-sending the exact same previous sig" } - commitments1.active.forEach { sendQueue.add(ChannelAction.Message.Send(it.nextRemoteCommit!!.sig)) } - if (revWasSentLast) resendRevocation() - } - commitments1.remoteNextCommitInfo.isRight && commitments1.remoteCommitIndex + 1 == channelReestablish.nextLocalCommitmentNumber -> { - // there wasn't any sig in-flight when the disconnection occurred - resendRevocation() + // step 1: we check our local commitment + return if (commitments.localCommitIndex == remoteChannelReestablish.nextRemoteRevocationNumber) { + // our local commitment is in sync, let's check the remote commitment + checkRemoteCommit(remoteChannelReestablish, retransmitRevocation = null) + } else if (commitments.localCommitIndex == remoteChannelReestablish.nextRemoteRevocationNumber + 1) { + // they just sent a new commit_sig, we have received it but they didn't receive our revocation + val localPerCommitmentSecret = channelKeys.commitmentSecret(commitments.localCommitIndex - 1) + val localNextPerCommitmentPoint = channelKeys.commitmentPoint(commitments.localCommitIndex + 1) + val revocation = RevokeAndAck( + channelId = commitments.channelId, + perCommitmentSecret = localPerCommitmentSecret, + nextPerCommitmentPoint = localNextPerCommitmentPoint + ) + checkRemoteCommit(remoteChannelReestablish, retransmitRevocation = revocation) + } else if (commitments.localCommitIndex > remoteChannelReestablish.nextRemoteRevocationNumber + 1) { + SyncResult.Failure.RemoteLate + } else { + // if next_remote_revocation_number is greater than our local commitment index, it means that either we are using an outdated commitment, or they are lying + // but first we need to make sure that the last per_commitment_secret that they claim to have received from us is correct for that next_remote_revocation_number minus 1 + if (channelKeys.commitmentSecret(remoteChannelReestablish.nextRemoteRevocationNumber - 1) == remoteChannelReestablish.yourLastCommitmentSecret) { + SyncResult.Failure.LocalLateProven( + ourLocalCommitmentNumber = commitments.localCommitIndex, + theirRemoteCommitmentNumber = remoteChannelReestablish.nextRemoteRevocationNumber + ) + } else { + // they lied! the last per_commitment_secret they claimed to have received from us is invalid + SyncResult.Failure.RemoteLying( + ourLocalCommitmentNumber = commitments.localCommitIndex, + theirRemoteCommitmentNumber = remoteChannelReestablish.nextRemoteRevocationNumber, + invalidPerCommitmentSecret = remoteChannelReestablish.yourLastCommitmentSecret + ) } - else -> throw RevocationSyncError(d.channelId) } - - if (commitments1.changes.localHasChanges()) { - sendQueue.add(ChannelAction.Message.SendToSelf(ChannelCommand.Commitment.Sign)) - } - - // When a channel is reestablished after a wallet restarts, we need to reprocess incoming HTLCs that may have been only partially processed - // (either because they didn't reach the payment handler, or because the payment handler response didn't reach the channel). - // Otherwise these HTLCs will stay in our commitment until they timeout and our peer closes the channel. - val htlcsToReprocess = commitments1.reprocessIncomingHtlcs() - logger.info { "re-processing signed IN: ${htlcsToReprocess.map { it.add.id }.joinToString()}" } - sendQueue.addAll(htlcsToReprocess) - - return SyncResult.Success(commitments1, sendQueue) } - private fun ChannelContext.checkSync(d: ChannelStateWithCommitments, remoteChannelReestablish: ChannelReestablish): SyncResult { - val commitments = d.commitments - val channelKeys = keyManager.channelKeys(commitments.params.localParams.fundingKeyPath) - return when { - !Helpers.checkLocalCommit(commitments, remoteChannelReestablish.nextRemoteRevocationNumber) -> { - // if next_remote_revocation_number is greater than our local commitment index, it means that either we are using an outdated commitment, or they are lying - // but first we need to make sure that the last per_commitment_secret that they claim to have received from us is correct for that next_remote_revocation_number minus 1 - if (channelKeys.commitmentSecret(remoteChannelReestablish.nextRemoteRevocationNumber - 1) == remoteChannelReestablish.yourLastCommitmentSecret) { - SyncResult.Failure.LocalLateProven(ourLocalCommitmentNumber = commitments.localCommitIndex, theirRemoteCommitmentNumber = remoteChannelReestablish.nextRemoteRevocationNumber) - } else { - SyncResult.Failure.RemoteLying( - ourLocalCommitmentNumber = commitments.localCommitIndex, - theirRemoteCommitmentNumber = remoteChannelReestablish.nextRemoteRevocationNumber, - invalidPerCommitmentSecret = remoteChannelReestablish.yourLastCommitmentSecret - ) - } - } - !Helpers.checkRemoteCommit(commitments, remoteChannelReestablish.nextLocalCommitmentNumber) -> { - // if next_local_commit_number is more than one more our remote commitment index, it means that either we are using an outdated commitment, or they are lying - SyncResult.Failure.LocalLateUnproven( - ourRemoteCommitmentNumber = commitments.latest.nextRemoteCommit?.commit?.index ?: commitments.latest.remoteCommit.index, - theirLocalCommitmentNumber = remoteChannelReestablish.nextLocalCommitmentNumber - ) - } - else -> handleSync(remoteChannelReestablish, d) - } + /** When reconnecting, we drop all unsigned changes. */ + fun ChannelContext.discardUnsignedUpdates(commitments: Commitments): Commitments { + logger.debug { "discarding proposed OUT: ${commitments.changes.localChanges.proposed}" } + logger.debug { "discarding proposed IN: ${commitments.changes.remoteChanges.proposed}" } + val commitments1 = commitments.copy( + changes = commitments.changes.copy( + localChanges = commitments.changes.localChanges.copy(proposed = emptyList()), + remoteChanges = commitments.changes.remoteChanges.copy(proposed = emptyList()), + localNextHtlcId = commitments.changes.localNextHtlcId - commitments.changes.localChanges.proposed.filterIsInstance().size, + remoteNextHtlcId = commitments.changes.remoteNextHtlcId - commitments.changes.remoteChanges.proposed.filterIsInstance().size + ) + ) + logger.debug { "localNextHtlcId=${commitments.changes.localNextHtlcId}->${commitments1.changes.localNextHtlcId}" } + logger.debug { "remoteNextHtlcId=${commitments.changes.remoteNextHtlcId}->${commitments1.changes.remoteNextHtlcId}" } + return commitments1 } private fun handleOutdatedCommitment(remoteChannelReestablish: ChannelReestablish, commitments: Commitments): Pair> { @@ -469,33 +527,6 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent: ) return Pair(nextState, actions) } - - private fun ChannelContext.handleSyncFailure(remoteChannelReestablish: ChannelReestablish, syncFailure: SyncResult.Failure, commitments: Commitments): Pair> { - return when (syncFailure) { - is SyncResult.Failure.LocalLateProven -> { - // their data checks out, we indeed seem to be using an old revoked commitment, and must absolutely *NOT* publish it, because that would be a cheating attempt and they - // would punish us by taking all the funds in the channel - logger.warning { "counterparty proved that we have an outdated (revoked) local commitment!!! ourCommitmentNumber=${commitments.localCommitIndex} theirCommitmentNumber=${remoteChannelReestablish.nextRemoteRevocationNumber}" } - handleOutdatedCommitment(remoteChannelReestablish, commitments) - } - is SyncResult.Failure.LocalLateUnproven -> { - // there is no way to make sure that they are saying the truth, the best thing to do is ask them to publish their commitment right now - // maybe they will publish their commitment, in that case we need to remember their commitment point in order to be able to claim our outputs - // not that if they don't comply, we could publish our own commitment (it is not stale, otherwise we would be in the case above) - logger.warning { "counterparty says that they have a more recent commitment than the one we know of!!! ourCommitmentNumber=${commitments.latest.nextRemoteCommit?.commit?.index ?: commitments.latest.remoteCommit.index} theirCommitmentNumber=${remoteChannelReestablish.nextLocalCommitmentNumber}" } - handleOutdatedCommitment(remoteChannelReestablish, commitments) - } - is SyncResult.Failure.RemoteLying -> { - // they are deliberately trying to fool us into thinking we have a late commitment, but we cannot risk publishing it ourselves, because it may really be revoked! - logger.warning { "counterparty claims that we have an outdated commitment, but they sent an invalid proof, so our commitment may or may not be revoked: ourLocalCommitmentNumber=${commitments.localCommitIndex} theirRemoteCommitmentNumber=${remoteChannelReestablish.nextRemoteRevocationNumber}" } - handleOutdatedCommitment(remoteChannelReestablish, commitments) - } -// is SyncResult.Failure.RemoteLate -> { -// logger.error { "counterparty appears to be using an outdated commitment, they may request a force-close, standing by..." } -// stay() -// } - } - } } } diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ClosingTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ClosingTestsCommon.kt index 63af10dcb..29b7f026e 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ClosingTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ClosingTestsCommon.kt @@ -1180,25 +1180,25 @@ class ClosingTestsCommon : LightningTestSuite() { // alice realizes it has an old state and asks bob to publish its current commitment val (alice2, aliceActions2) = alice1.process(ChannelCommand.MessageReceived(channelReestablishB)) assertIs(alice2.state) - val error = aliceActions2.findOutgoingMessage() - assertEquals(PleasePublishYourCommitment(alice2.channelId).message, error.toAscii()) + val errorA = aliceActions2.findOutgoingMessage() + assertEquals(PleasePublishYourCommitment(alice2.channelId).message, errorA.toAscii()) - // bob is nice and publishes its commitment as soon as it detects that alice has an outdated commitment + // bob detects that alice has an outdated commitment and stands by val (bob2, bobActions2) = bob1.process(ChannelCommand.MessageReceived(channelReestablishA)) - assertIs(bob2.state) - bobActions2.hasOutgoingMessage() + assertIs(bob2.state) + assertTrue { bobActions2.isEmpty() } + + // bob receives the error from alice and publishes his local commitment tx + val (bob3, bobActions3) = bob2.process(ChannelCommand.MessageReceived(errorA)) + assertIs(bob3.state) val bobCommitTx = bob2.commitments.latest.localCommit.publishableTxs.commitTx.tx assertEquals(6, bobCommitTx.txOut.size) // 2 main outputs + 2 anchors + 2 HTLCs - bobActions2.find().also { + bobActions3.find().also { assertEquals(bobCommitTx.txid, it.txId) assertEquals(ChannelClosingType.Local, it.closingType) assertTrue(it.isSentToDefaultAddress) } - val (bob3, bobActions3) = bob2.process(ChannelCommand.MessageReceived(error)) - assertEquals(bob2, bob3) - assertTrue(bobActions3.isEmpty()) - val (alice3, aliceActions3) = alice2.process(ChannelCommand.WatchReceived(WatchEventSpent(alice0.channelId, BITCOIN_FUNDING_SPENT, bobCommitTx))) assertIs(alice3.state) aliceActions3.find().also { diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/OfflineTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/OfflineTestsCommon.kt index 50e41c0f5..e1810a8fd 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/OfflineTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/OfflineTestsCommon.kt @@ -320,16 +320,21 @@ class OfflineTestsCommon : LightningTestSuite() { // alice realizes she has an old state... val (alice3, actionsAlice3) = alice2.process(ChannelCommand.MessageReceived(channelReestablishB)) // ...and asks bob to publish its current commitment - val error = actionsAlice3.findOutgoingMessage() - assertEquals(error.toAscii(), PleasePublishYourCommitment(aliceOld.channelId).message) + val errorA = actionsAlice3.findOutgoingMessage() + assertEquals(errorA.toAscii(), PleasePublishYourCommitment(aliceOld.channelId).message) assertIs(alice3.state) - // bob is nice and publishes its commitment as soon as it detects that alice has an outdated commitment + // bob detects that alice has an outdated commitment and stands by val (bob3, actionsBob3) = bob2.process(ChannelCommand.MessageReceived(channelReestablishA)) - assertIs(bob3.state) - assertNotNull(bob3.state.localCommitPublished) - val bobCommitTx = bob3.state.localCommitPublished!!.commitTx - actionsBob3.hasPublishTx(bobCommitTx) + assertIs(bob3.state) + assertTrue { actionsBob3.isEmpty() } + + // bob receives the error from alice and publishes his local commitment tx + val (bob4, actionsBob4) = bob3.process(ChannelCommand.MessageReceived(errorA)) + assertIs(bob4.state) + assertNotNull(bob4.state.localCommitPublished) + val bobCommitTx = bob4.state.localCommitPublished!!.commitTx + actionsBob4.hasPublishTx(bobCommitTx) // alice is able to claim her main output val (alice4, actionsAlice4) = alice3.process(ChannelCommand.WatchReceived(WatchEventSpent(aliceOld.channelId, BITCOIN_FUNDING_SPENT, bobCommitTx))) From 938facb8e1df2fd6ac720d15a03fc09bac9ee721 Mon Sep 17 00:00:00 2001 From: pm47 Date: Wed, 17 Apr 2024 17:21:10 +0200 Subject: [PATCH 5/5] fixup! handle reestablish in state `ShuttingDown` --- .../fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt index 41376d445..407147edb 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ShutdownTestsCommon.kt @@ -533,10 +533,10 @@ class ShutdownTestsCommon : LightningTestSuite() { val (alice0, bob0) = init(bobFeatures = TestConstants.Bob.nodeParams.features.remove(Feature.ChannelBackupClient)) val (alice1, bob1, reestablishes) = SyncingTestsCommon.disconnect(alice0, bob0) val (aliceReestablish, bobReestablish) = reestablishes - val (alice2, actionsAlice2) = alice1.process(ChannelCommand.MessageReceived(aliceReestablish)) + val (alice2, actionsAlice2) = alice1.process(ChannelCommand.MessageReceived(bobReestablish)) assertIs(alice2.state) actionsAlice2.hasOutgoingMessage() - val (bob2, actionsBob2) = bob1.process(ChannelCommand.MessageReceived(bobReestablish)) + val (bob2, actionsBob2) = bob1.process(ChannelCommand.MessageReceived(aliceReestablish)) assertIs(bob2.state) actionsBob2.hasOutgoingMessage() }