Skip to content

Commit 3524e70

Browse files
t-bastt-bast
committed
Reestablish partially signed splice with current remote_commitment_number
As pointed out in lightning/bolts#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.
1 parent 61af10a commit 3524e70

File tree

3 files changed

+144
-6
lines changed

3 files changed

+144
-6
lines changed

‎eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,10 @@ object Helpers {
507507
case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber == (commitments.remoteCommitIndex + 1) =>
508508
// they have acknowledged the last commit_sig we sent
509509
SyncResult.Success(retransmit = retransmitRevocation_opt.toSeq)
510+
case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber == commitments.remoteCommitIndex && remoteChannelReestablish.nextFundingTxId_opt.nonEmpty =>
511+
// they haven't received the commit_sig we sent as part of signing a splice transaction
512+
// we will retransmit it before exchanging tx_signatures
513+
SyncResult.Success(retransmit = retransmitRevocation_opt.toSeq)
510514
case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber < (commitments.remoteCommitIndex + 1) =>
511515
// they are behind
512516
SyncResult.RemoteLate

‎eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingSignedStateSpec.scala

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,23 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
378378
reconnect(f, fundingTxId)
379379
}
380380

381+
test("recv INPUT_DISCONNECTED (commit_sig not received, next_commitment_number = 0)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
382+
import f._
383+
384+
val fundingTxId = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_SIGNED].signingSession.fundingTx.txId
385+
alice2bob.expectMsgType[CommitSig] // Bob doesn't receive Alice's commit_sig
386+
bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig
387+
awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED)
388+
awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED)
389+
390+
alice ! INPUT_DISCONNECTED
391+
awaitCond(alice.stateName == OFFLINE)
392+
bob ! INPUT_DISCONNECTED
393+
awaitCond(bob.stateName == OFFLINE)
394+
395+
reconnect(f, fundingTxId, aliceCommitmentNumber = 0, bobCommitmentNumber = 0)
396+
}
397+
381398
test("recv INPUT_DISCONNECTED (commit_sig partially received)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
382399
import f._
383400

@@ -397,6 +414,25 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
397414
reconnect(f, fundingTxId)
398415
}
399416

417+
test("recv INPUT_DISCONNECTED (commit_sig partially received, next_commitment_number = 0)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
418+
import f._
419+
420+
val fundingTxId = alice.stateData.asInstanceOf[DATA_WAIT_FOR_DUAL_FUNDING_SIGNED].signingSession.fundingTx.txId
421+
alice2bob.expectMsgType[CommitSig]
422+
alice2bob.forward(bob)
423+
bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig
424+
bob2alice.expectMsgType[TxSignatures] // Alice doesn't receive Bob's tx_signatures
425+
awaitCond(alice.stateName == WAIT_FOR_DUAL_FUNDING_SIGNED)
426+
awaitCond(bob.stateName == WAIT_FOR_DUAL_FUNDING_CONFIRMED)
427+
428+
alice ! INPUT_DISCONNECTED
429+
awaitCond(alice.stateName == OFFLINE)
430+
bob ! INPUT_DISCONNECTED
431+
awaitCond(bob.stateName == OFFLINE)
432+
433+
reconnect(f, fundingTxId, aliceCommitmentNumber = 0)
434+
}
435+
400436
test("recv INPUT_DISCONNECTED (commit_sig received)", Tag(ChannelStateTestsTags.DualFunding)) { f =>
401437
import f._
402438

@@ -454,7 +490,7 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
454490
assert(listener.expectMsgType[TransactionPublished].tx.txid == fundingTxId)
455491
}
456492

457-
private def reconnect(f: FixtureParam, fundingTxId: TxId): Unit = {
493+
private def reconnect(f: FixtureParam, fundingTxId: TxId, aliceCommitmentNumber: Long = 1, bobCommitmentNumber: Long = 1): Unit = {
458494
import f._
459495

460496
val listener = TestProbe()
@@ -467,11 +503,11 @@ class WaitForDualFundingSignedStateSpec extends TestKitBaseClass with FixtureAny
467503
val channelReestablishAlice = alice2bob.expectMsgType[ChannelReestablish]
468504
assert(channelReestablishAlice.nextFundingTxId_opt.contains(fundingTxId))
469505
assert(channelReestablishAlice.nextLocalCommitmentNumber == 1)
470-
alice2bob.forward(bob)
506+
alice2bob.forward(bob, channelReestablishAlice.copy(nextLocalCommitmentNumber = aliceCommitmentNumber))
471507
val channelReestablishBob = bob2alice.expectMsgType[ChannelReestablish]
472508
assert(channelReestablishBob.nextFundingTxId_opt.contains(fundingTxId))
473509
assert(channelReestablishBob.nextLocalCommitmentNumber == 1)
474-
bob2alice.forward(alice)
510+
bob2alice.forward(alice, channelReestablishBob.copy(nextLocalCommitmentNumber = bobCommitmentNumber))
475511
bob2alice.expectMsgType[CommitSig]
476512
bob2alice.forward(alice)
477513
alice2bob.expectMsgType[CommitSig]

‎eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalSplicesStateSpec.scala

Lines changed: 101 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,17 +1560,17 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik
15601560
awaitCond(bob.stateName == OFFLINE)
15611561
}
15621562

1563-
private def reconnect(f: FixtureParam): (ChannelReestablish, ChannelReestablish) = {
1563+
private def reconnect(f: FixtureParam, sendReestablish: Boolean = true): (ChannelReestablish, ChannelReestablish) = {
15641564
import f._
15651565

15661566
val aliceInit = Init(alice.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.params.localParams.initFeatures)
15671567
val bobInit = Init(bob.stateData.asInstanceOf[ChannelDataWithCommitments].commitments.params.localParams.initFeatures)
15681568
alice ! INPUT_RECONNECTED(alice2bob.ref, aliceInit, bobInit)
15691569
bob ! INPUT_RECONNECTED(bob2alice.ref, bobInit, aliceInit)
15701570
val channelReestablishAlice = alice2bob.expectMsgType[ChannelReestablish]
1571-
alice2bob.forward(bob)
1571+
if (sendReestablish) alice2bob.forward(bob)
15721572
val channelReestablishBob = bob2alice.expectMsgType[ChannelReestablish]
1573-
bob2alice.forward(alice)
1573+
if (sendReestablish) bob2alice.forward(alice)
15741574
(channelReestablishAlice, channelReestablishBob)
15751575
}
15761576

@@ -1638,6 +1638,54 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik
16381638
resolveHtlcs(f, htlcs)
16391639
}
16401640

1641+
test("disconnect (commit_sig not received, reestablish with previous commitment_number)") { f =>
1642+
import f._
1643+
1644+
val htlcs = setupHtlcs(f)
1645+
val aliceCommitIndex = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex
1646+
val bobCommitIndex = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex
1647+
1648+
val sender = initiateSpliceWithoutSigs(f, spliceIn_opt = Some(SpliceIn(500_000 sat)), spliceOut_opt = Some(SpliceOut(100_000 sat, defaultSpliceOutScriptPubKey)))
1649+
alice2bob.expectMsgType[CommitSig] // Bob doesn't receive Alice's commit_sig
1650+
bob2alice.expectMsgType[CommitSig] // Alice doesn't receive Bob's commit_sig
1651+
awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceWaitingForSigs])
1652+
val spliceStatus = alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.asInstanceOf[SpliceStatus.SpliceWaitingForSigs]
1653+
1654+
disconnect(f)
1655+
val (channelReestablishAlice, channelReestablishBob) = reconnect(f, sendReestablish = false)
1656+
assert(channelReestablishAlice.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId))
1657+
assert(channelReestablishAlice.nextLocalCommitmentNumber == aliceCommitIndex + 1)
1658+
alice2bob.forward(bob, channelReestablishAlice.copy(nextLocalCommitmentNumber = aliceCommitIndex))
1659+
assert(channelReestablishBob.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId))
1660+
assert(channelReestablishBob.nextLocalCommitmentNumber == bobCommitIndex + 1)
1661+
bob2alice.forward(alice, channelReestablishBob.copy(nextLocalCommitmentNumber = bobCommitIndex))
1662+
1663+
// Alice and Bob retransmit commit_sig and tx_signatures.
1664+
alice2bob.expectMsgType[CommitSig]
1665+
alice2bob.forward(bob)
1666+
bob2alice.expectMsgType[CommitSig]
1667+
bob2alice.forward(alice)
1668+
bob2alice.expectMsgType[TxSignatures]
1669+
bob2alice.forward(alice)
1670+
alice2bob.expectMsgType[TxSignatures]
1671+
alice2bob.forward(bob)
1672+
sender.expectMsgType[RES_SPLICE]
1673+
1674+
val spliceTx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.signedTx_opt.get
1675+
alice2blockchain.expectWatchFundingConfirmed(spliceTx.txid)
1676+
bob2blockchain.expectWatchFundingConfirmed(spliceTx.txid)
1677+
alice ! WatchFundingConfirmedTriggered(BlockHeight(42), 0, spliceTx)
1678+
alice2bob.expectMsgType[SpliceLocked]
1679+
alice2bob.forward(bob)
1680+
bob ! WatchFundingConfirmedTriggered(BlockHeight(42), 0, spliceTx)
1681+
bob2alice.expectMsgType[SpliceLocked]
1682+
bob2alice.forward(alice)
1683+
awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.active.size == 1)
1684+
awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.active.size == 1)
1685+
1686+
resolveHtlcs(f, htlcs)
1687+
}
1688+
16411689
test("disconnect (commit_sig received by alice)") { f =>
16421690
import f._
16431691

@@ -1686,6 +1734,56 @@ class NormalSplicesStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLik
16861734
resolveHtlcs(f, htlcs)
16871735
}
16881736

1737+
test("disconnect (commit_sig received by alice, reestablish with previous commitment_number)") { f =>
1738+
import f._
1739+
1740+
val htlcs = setupHtlcs(f)
1741+
val aliceCommitIndex = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex
1742+
val bobCommitIndex = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommitIndex
1743+
assert(aliceCommitIndex != bobCommitIndex)
1744+
1745+
val sender = initiateSpliceWithoutSigs(f, spliceIn_opt = Some(SpliceIn(500_000 sat)), spliceOut_opt = Some(SpliceOut(100_000 sat, defaultSpliceOutScriptPubKey)))
1746+
alice2bob.expectMsgType[CommitSig] // Bob doesn't receive Alice's commit_sig
1747+
bob2alice.expectMsgType[CommitSig]
1748+
bob2alice.forward(alice)
1749+
awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.isInstanceOf[SpliceStatus.SpliceWaitingForSigs])
1750+
val spliceStatus = alice.stateData.asInstanceOf[DATA_NORMAL].spliceStatus.asInstanceOf[SpliceStatus.SpliceWaitingForSigs]
1751+
1752+
disconnect(f)
1753+
val (channelReestablishAlice, channelReestablishBob) = reconnect(f, sendReestablish = false)
1754+
assert(channelReestablishAlice.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId))
1755+
assert(channelReestablishAlice.nextLocalCommitmentNumber == aliceCommitIndex + 1)
1756+
alice2bob.forward(bob, channelReestablishAlice)
1757+
assert(channelReestablishBob.nextFundingTxId_opt.contains(spliceStatus.signingSession.fundingTx.txId))
1758+
assert(channelReestablishBob.nextLocalCommitmentNumber == bobCommitIndex + 1)
1759+
bob2alice.forward(alice, channelReestablishBob.copy(nextLocalCommitmentNumber = bobCommitIndex))
1760+
1761+
// Alice and Bob retransmit commit_sig and tx_signatures.
1762+
alice2bob.expectMsgType[CommitSig]
1763+
alice2bob.forward(bob)
1764+
bob2alice.expectMsgType[CommitSig]
1765+
bob2alice.forward(alice)
1766+
bob2alice.expectMsgType[TxSignatures]
1767+
bob2alice.forward(alice)
1768+
alice2bob.expectMsgType[TxSignatures]
1769+
alice2bob.forward(bob)
1770+
sender.expectMsgType[RES_SPLICE]
1771+
1772+
val spliceTx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.latest.localFundingStatus.signedTx_opt.get
1773+
alice2blockchain.expectWatchFundingConfirmed(spliceTx.txid)
1774+
bob2blockchain.expectWatchFundingConfirmed(spliceTx.txid)
1775+
alice ! WatchFundingConfirmedTriggered(BlockHeight(42), 0, spliceTx)
1776+
alice2bob.expectMsgType[SpliceLocked]
1777+
alice2bob.forward(bob)
1778+
bob ! WatchFundingConfirmedTriggered(BlockHeight(42), 0, spliceTx)
1779+
bob2alice.expectMsgType[SpliceLocked]
1780+
bob2alice.forward(alice)
1781+
awaitCond(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.active.size == 1)
1782+
awaitCond(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.active.size == 1)
1783+
1784+
resolveHtlcs(f, htlcs)
1785+
}
1786+
16891787
test("disconnect (tx_signatures sent by bob)") { f =>
16901788
import f._
16911789

0 commit comments

Comments
 (0)