From 3524e70c53b0f3e98c3dd5e0f16d094fcf941348 Mon Sep 17 00:00:00 2001 From: t-bast Date: Thu, 12 Dec 2024 18:28:19 +0100 Subject: [PATCH] Reestablish partially signed splice with current `remote_commitment_number` As pointed out in https://github.com/lightning/bolts/pull/1214, when reconnecting a partially signed `interactive-tx` session, we should set `next_commitment_number` to the current commitment number if we haven't received our peer's `commit_sig`, which tells them they need to retransmit it. That's not what we're currently doing: we're currently setting this value to the next commitment number, regardless of whether or not we have received our peer's `commit_sig`. And we always retransmit our `commit_sig` if our peer is setting `next_funding_txid`, even if they have already received it. More importantly, if our peer behaves correctly and sends us the current commitment number, we will think that they're late and will halt, waiting for them to send `error`. This commit fixes that by allowing our peers to use the current commitment number when they set `next_funding_txid`. Note that this doesn't yet make us spec-compliant, but in order to guarantee backwards-compatibility, we must first deploy that change before we can start removing spurious `commit_sig` retransmissions. --- .../fr/acinq/eclair/channel/Helpers.scala | 4 + .../b/WaitForDualFundingSignedStateSpec.scala | 42 ++++++- .../states/e/NormalSplicesStateSpec.scala | 104 +++++++++++++++++- 3 files changed, 144 insertions(+), 6 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala index 78db6d35d0..09402d1cdb 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala @@ -507,6 +507,10 @@ object Helpers { case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber == (commitments.remoteCommitIndex + 1) => // they have acknowledged the last commit_sig we sent SyncResult.Success(retransmit = retransmitRevocation_opt.toSeq) + case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber == commitments.remoteCommitIndex && remoteChannelReestablish.nextFundingTxId_opt.nonEmpty => + // they haven't received the commit_sig we sent as part of signing a splice transaction + // we will retransmit it before exchanging tx_signatures + SyncResult.Success(retransmit = retransmitRevocation_opt.toSeq) case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber < (commitments.remoteCommitIndex + 1) => // they are behind SyncResult.RemoteLate diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingSignedStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingSignedStateSpec.scala index 517d704f60..ceb7082d21 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingSignedStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingSignedStateSpec.scala @@ -378,6 +378,23 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny reconnect(f, fundingTxId) } + test("recv INPUT_DISCONNECTED (commit_sig not received, next_commitment_number = 0)", Tag(ChannelStateTestsTags.DualFunding)) { f => + import f._ + + val fundingTxId = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_SIGNED].signingSession.fundingTx.txId + alice2bob.expectMsgType[CommitSig] // Bob doesn't receive Alice's commit_sig + bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig + awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED) + awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED) + + alice ! INPUT_DISCONNECTED + awaitCond(alice.stateName == OFFLINE) + bob ! INPUT_DISCONNECTED + awaitCond(bob.stateName == OFFLINE) + + reconnect(f, fundingTxId, aliceCommitmentNumber = 0, bobCommitmentNumber = 0) + } + test("recv INPUT_DISCONNECTED (commit_sig partially received)", Tag(ChannelStateTestsTags.DualFunding)) { f => import f._ @@ -397,6 +414,25 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny reconnect(f, fundingTxId) } + test("recv INPUT_DISCONNECTED (commit_sig partially received, next_commitment_number = 0)", Tag(ChannelStateTestsTags.DualFunding)) { f => + import f._ + + val fundingTxId = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_SIGNED].signingSession.fundingTx.txId + alice2bob.expectMsgType[CommitSig] + alice2bob.forward(bob) + bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig + bob2alice.expectMsgType[TxSignatures] // Alice doesn't receive Bob's tx_signatures + awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED) + awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED) + + alice ! INPUT_DISCONNECTED + awaitCond(alice.stateName == OFFLINE) + bob ! INPUT_DISCONNECTED + awaitCond(bob.stateName == OFFLINE) + + reconnect(f, fundingTxId, aliceCommitmentNumber = 0) + } + test("recv INPUT_DISCONNECTED (commit_sig received)", Tag(ChannelStateTestsTags.DualFunding)) { f => import f._ @@ -454,7 +490,7 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny assert(listener.expectMsgType[TransactionPublished].tx.txid == fundingTxId) } - private def reconnect(f: FixtureParam, fundingTxId: TxId): Unit = { + private def reconnect(f: FixtureParam, fundingTxId: TxId, aliceCommitmentNumber: Long = 1, bobCommitmentNumber: Long = 1): Unit = { import f._ val listener = TestProbe() @@ -467,11 +503,11 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny val channelReestablishAlice = alice2bob.expectMsgType[ChannelReestablish] assert(channelReestablishAlice.nextFundingTxId_opt.contains(fundingTxId)) assert(channelReestablishAlice.nextLocalCommitmentNumber == 1) - alice2bob.forward(bob) + alice2bob.forward(bob, channelReestablishAlice.copy(nextLocalCommitmentNumber = aliceCommitmentNumber)) val channelReestablishBob = bob2alice.expectMsgType[ChannelReestablish] assert(channelReestablishBob.nextFundingTxId_opt.contains(fundingTxId)) assert(channelReestablishBob.nextLocalCommitmentNumber == 1) - bob2alice.forward(alice) + bob2alice.forward(alice, channelReestablishBob.copy(nextLocalCommitmentNumber = bobCommitmentNumber)) bob2alice.expectMsgType[CommitSig] bob2alice.forward(alice) alice2bob.expectMsgType[CommitSig] 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 03cc0a1430..2a722d2f03 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 @@ -1560,7 +1560,7 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik awaitCond(bob.stateName == OFFLINE) } - private def reconnect(f: FixtureParam): (ChannelReestablish, ChannelReestablish) = { + private def reconnect(f: FixtureParam, sendReestablish: Boolean = true): (ChannelReestablish, ChannelReestablish) = { import f._ val aliceInit = Init(alice.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.params.localParams.initFeatures) @@ -1568,9 +1568,9 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik alice ! INPUT_RECONNECTED(alice2bob.ref, aliceInit, bobInit) bob ! INPUT_RECONNECTED(bob2alice.ref, bobInit, aliceInit) val channelReestablishAlice = alice2bob.expectMsgType[ChannelReestablish] - alice2bob.forward(bob) + if (sendReestablish) alice2bob.forward(bob) val channelReestablishBob = bob2alice.expectMsgType[ChannelReestablish] - bob2alice.forward(alice) + if (sendReestablish) bob2alice.forward(alice) (channelReestablishAlice, channelReestablishBob) } @@ -1638,6 +1638,54 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik resolveHtlcs(f, htlcs) } + test("disconnect (commit_sig not received, reestablish with previous commitment_number)") { f => + import f._ + + val htlcs = setupHtlcs(f) + val aliceCommitIndex = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex + val bobCommitIndex = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex + + val sender = initiateSpliceWithoutSigs(f, spliceIn_opt = Some(SpliceIn(500_000 sat)), spliceOut_opt = Some(SpliceOut(100_000 sat, defaultSpliceOutScriptPubKey))) + alice2bob.expectMsgType[CommitSig] // Bob doesn't receive Alice's commit_sig + bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig + awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceWaitingForSigs]) + val spliceStatus = alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.asInstanceOf[SpliceStatus.SpliceWaitingForSigs] + + disconnect(f) + val (channelReestablishAlice, channelReestablishBob) = reconnect(f, sendReestablish = false) + assert(channelReestablishAlice.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId)) + assert(channelReestablishAlice.nextLocalCommitmentNumber == aliceCommitIndex + 1) + alice2bob.forward(bob, channelReestablishAlice.copy(nextLocalCommitmentNumber = aliceCommitIndex)) + assert(channelReestablishBob.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId)) + assert(channelReestablishBob.nextLocalCommitmentNumber == bobCommitIndex + 1) + bob2alice.forward(alice, channelReestablishBob.copy(nextLocalCommitmentNumber = bobCommitIndex)) + + // Alice and Bob retransmit commit_sig and tx_signatures. + alice2bob.expectMsgType[CommitSig] + alice2bob.forward(bob) + bob2alice.expectMsgType[CommitSig] + bob2alice.forward(alice) + bob2alice.expectMsgType[TxSignatures] + bob2alice.forward(alice) + alice2bob.expectMsgType[TxSignatures] + alice2bob.forward(bob) + sender.expectMsgType[RES_SPLICE] + + val spliceTx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.signedTx_opt.get + alice2blockchain.expectWatchFundingConfirmed(spliceTx.txid) + bob2blockchain.expectWatchFundingConfirmed(spliceTx.txid) + alice ! WatchFundingConfirmedTriggered(BlockHeight(42), 0, spliceTx) + alice2bob.expectMsgType[SpliceLocked] + alice2bob.forward(bob) + 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) + + resolveHtlcs(f, htlcs) + } + test("disconnect (commit_sig received by alice)") { f => import f._ @@ -1686,6 +1734,56 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik resolveHtlcs(f, htlcs) } + test("disconnect (commit_sig received by alice, reestablish with previous commitment_number)") { f => + import f._ + + val htlcs = setupHtlcs(f) + val aliceCommitIndex = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex + val bobCommitIndex = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex + assert(aliceCommitIndex != bobCommitIndex) + + val sender = initiateSpliceWithoutSigs(f, spliceIn_opt = Some(SpliceIn(500_000 sat)), spliceOut_opt = Some(SpliceOut(100_000 sat, defaultSpliceOutScriptPubKey))) + alice2bob.expectMsgType[CommitSig] // Bob doesn't receive Alice's commit_sig + bob2alice.expectMsgType[CommitSig] + bob2alice.forward(alice) + awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceWaitingForSigs]) + val spliceStatus = alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.asInstanceOf[SpliceStatus.SpliceWaitingForSigs] + + disconnect(f) + val (channelReestablishAlice, channelReestablishBob) = reconnect(f, sendReestablish = false) + assert(channelReestablishAlice.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId)) + assert(channelReestablishAlice.nextLocalCommitmentNumber == aliceCommitIndex + 1) + alice2bob.forward(bob, channelReestablishAlice) + assert(channelReestablishBob.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId)) + assert(channelReestablishBob.nextLocalCommitmentNumber == bobCommitIndex + 1) + bob2alice.forward(alice, channelReestablishBob.copy(nextLocalCommitmentNumber = bobCommitIndex)) + + // Alice and Bob retransmit commit_sig and tx_signatures. + alice2bob.expectMsgType[CommitSig] + alice2bob.forward(bob) + bob2alice.expectMsgType[CommitSig] + bob2alice.forward(alice) + bob2alice.expectMsgType[TxSignatures] + bob2alice.forward(alice) + alice2bob.expectMsgType[TxSignatures] + alice2bob.forward(bob) + sender.expectMsgType[RES_SPLICE] + + val spliceTx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.signedTx_opt.get + alice2blockchain.expectWatchFundingConfirmed(spliceTx.txid) + bob2blockchain.expectWatchFundingConfirmed(spliceTx.txid) + alice ! WatchFundingConfirmedTriggered(BlockHeight(42), 0, spliceTx) + alice2bob.expectMsgType[SpliceLocked] + alice2bob.forward(bob) + 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) + + resolveHtlcs(f, htlcs) + } + test("disconnect (tx_signatures sent by bob)") { f => import f._