Skip to content

Commit 29d8ae8

Browse files
t-bastt-bast
committed
Add channel_reestablish TLVs for retransmission
With splicing, we introduce new TLVs to `channel_reestablish` to let our peer know: - the latest `splice_locked` (or `channel_ready`) we received - the latest `splice_locked` (or `channel_ready`) we're ready to send This lets us clean-up retransmission of those messages and remove an edge case around concurrent locking and payments.
1 parent 075b3b9 commit 29d8ae8

File tree

12 files changed

+231
-93
lines changed

12 files changed

+231
-93
lines changed

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/Commitments.kt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,9 @@ data class Commitments(
603603
// We always use the last commitment that was created, to make sure we never go back in time.
604604
val latest = active.first().let { c -> FullCommitment(params, changes, c.fundingTxIndex, c.remoteFundingPubkey, c.localFundingStatus, c.remoteFundingStatus, c.localCommit, c.remoteCommit, c.nextRemoteCommit) }
605605

606+
fun ChannelContext.lastLocalLocked(): Commitment? = active.find { staticParams.useZeroConf || it.localFundingStatus is LocalFundingStatus.ConfirmedFundingTx }
607+
val lastRemoteLocked: Commitment? = active.find { it.remoteFundingStatus == RemoteFundingStatus.Locked }
608+
606609
val all = buildList {
607610
addAll(active)
608611
addAll(inactive)
@@ -952,18 +955,17 @@ data class Commitments(
952955
// This ensures that we only have to send splice_locked for the latest commitment instead of sending it for every commitment.
953956
// A side-effect is that previous commitments that are implicitly locked don't necessarily have their status correctly set.
954957
// That's why we look at locked commitments separately and then select the one with the oldest fundingTxIndex.
955-
val lastLocalLocked = active.find { staticParams.useZeroConf || it.localFundingStatus is LocalFundingStatus.ConfirmedFundingTx }
956-
val lastRemoteLocked = active.find { it.remoteFundingStatus == RemoteFundingStatus.Locked }
958+
val lastLocal = lastLocalLocked()
957959
return when {
958960
// We select the locked commitment with the smaller value for fundingTxIndex, but both have to be defined.
959961
// If both have the same fundingTxIndex, they must actually be the same commitment, because:
960962
// - we only allow RBF attempts when we're not using zero-conf
961963
// - transactions with the same fundingTxIndex double-spend each other, so only one of them can confirm
962964
// - we don't allow creating a splice on top of an unconfirmed transaction that has RBF attempts (because it
963965
// would become invalid if another of the RBF attempts end up being confirmed)
964-
lastLocalLocked != null && lastRemoteLocked != null -> listOf(lastLocalLocked, lastRemoteLocked).minByOrNull { it.fundingTxIndex }
966+
lastLocal != null && lastRemoteLocked != null -> listOf(lastLocal, lastRemoteLocked).minByOrNull { it.fundingTxIndex }
965967
// Special case for the initial funding tx, we only require a local lock because channel_ready doesn't explicitly reference a funding tx.
966-
lastLocalLocked != null && lastLocalLocked.fundingTxIndex == 0L -> lastLocalLocked
968+
lastLocal != null && lastLocal.fundingTxIndex == 0L -> lastLocal
967969
else -> null
968970
}
969971
}

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Channel.kt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,14 +323,18 @@ sealed class PersistedChannelState : ChannelState() {
323323
is Normal -> state.getUnsignedFundingTxId()
324324
else -> null
325325
}
326-
val tlvs: TlvStream<ChannelReestablishTlv> = unsignedFundingTxId?.let { TlvStream(ChannelReestablishTlv.NextFunding(it)) } ?: TlvStream.empty()
326+
val tlvs: Set<ChannelReestablishTlv> = setOfNotNull(
327+
unsignedFundingTxId?.let { ChannelReestablishTlv.NextFunding(it) },
328+
state.commitments.lastRemoteLocked?.let { ChannelReestablishTlv.YourLastFundingLocked(it.fundingTxId) },
329+
state.commitments.run { lastLocalLocked() }?.let { ChannelReestablishTlv.MyCurrentFundingLocked(it.fundingTxId) },
330+
)
327331
ChannelReestablish(
328332
channelId = channelId,
329333
nextLocalCommitmentNumber = nextLocalCommitmentNumber,
330334
nextRemoteRevocationNumber = state.commitments.remoteCommitIndex,
331335
yourLastCommitmentSecret = PrivateKey(yourLastPerCommitmentSecret),
332336
myCurrentPerCommitmentPoint = myCurrentPerCommitmentPoint,
333-
tlvStream = tlvs
337+
tlvStream = TlvStream(tlvs)
334338
)
335339
}
336340
}

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Syncing.kt‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
8585
}
8686
is WaitForChannelReady -> {
8787
val actions = ArrayList<ChannelAction>()
88-
// We've already received their commit_sig and sent our tx_signatures. We retransmit our tx_signatures
89-
// and our commit_sig if they haven't received it already.
88+
// If they haven't received our signatures for the channel funding transaction, we retransmit them.
9089
if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) {
9190
if (state.commitments.latest.localFundingStatus is LocalFundingStatus.UnconfirmedFundingTx) {
9291
if (cmd.message.nextLocalCommitmentNumber == 0L) {
@@ -109,10 +108,13 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
109108
logger.warning { "cannot re-send tx_signatures for fundingTxId=${cmd.message.nextFundingTxId}, transaction is already confirmed" }
110109
}
111110
}
112-
logger.debug { "re-sending channel_ready" }
113-
val nextPerCommitmentPoint = channelKeys().commitmentPoint(1)
114-
val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint)
115-
actions.add(ChannelAction.Message.Send(channelReady))
111+
// If they haven't received our channel_ready, we retransmit it.
112+
if (cmd.message.yourLastFundingLocked == null) {
113+
logger.debug { "re-sending channel_ready" }
114+
val nextPerCommitmentPoint = channelKeys().commitmentPoint(1)
115+
val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint)
116+
actions.add(ChannelAction.Message.Send(channelReady))
117+
}
116118
Pair(state, actions)
117119
}
118120
is LegacyWaitForFundingLocked -> {
@@ -130,8 +132,7 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
130132
val actions = ArrayList<ChannelAction>()
131133

132134
// re-send channel_ready if necessary
133-
if (state.commitments.latest.fundingTxIndex == 0L && cmd.message.nextLocalCommitmentNumber == 1L && state.commitments.localCommitIndex == 0L) {
134-
// 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
135+
if (cmd.message.yourLastFundingLocked == null && state.commitments.latest.fundingTxIndex == 0L) {
135136
logger.debug { "re-sending channel_ready" }
136137
val nextPerCommitmentPoint = channelKeys().commitmentPoint(1)
137138
val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint)
@@ -184,25 +185,36 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
184185
state.spliceStatus
185186
}
186187

187-
// Re-send splice_locked (must come *after* potentially retransmitting tx_signatures).
188-
// NB: there is a key difference between channel_ready and splice_locked:
189-
// - channel_ready: a non-zero commitment index implies that both sides have seen the channel_ready
190-
// - splice_locked: the commitment index can be updated as long as it is compatible with all splices, so
191-
// we must keep sending our most recent splice_locked at each reconnection
192-
state.commitments.active
193-
.filter { it.fundingTxIndex > 0L } // only consider splice txs
194-
.firstOrNull { staticParams.useZeroConf || it.localFundingStatus is LocalFundingStatus.ConfirmedFundingTx }
195-
?.let {
196-
logger.debug { "re-sending splice_locked for fundingTxId=${it.fundingTxId}" }
197-
val spliceLocked = SpliceLocked(channelId, it.fundingTxId)
188+
// Re-send splice_locked if necessary (must come *after* potentially retransmitting tx_signatures).
189+
state.commitments.run { lastLocalLocked() }?.let { lastLocked ->
190+
val isSplice = lastLocked.fundingTxIndex > 0
191+
val notReceivedByRemote = cmd.message.yourLastFundingLocked != lastLocked.fundingTxId
192+
if (isSplice && notReceivedByRemote) {
193+
logger.debug { "re-sending splice_locked for fundingTxId=${lastLocked.fundingTxId}" }
194+
val spliceLocked = SpliceLocked(channelId, lastLocked.fundingTxId)
198195
actions.add(ChannelAction.Message.Send(spliceLocked))
199196
}
197+
}
200198

201199
// we may need to retransmit updates and/or commit_sig and/or revocation
202200
actions.addAll(syncResult.retransmit.map { ChannelAction.Message.Send(it) })
203201

204-
// then we clean up unsigned updates
205-
val commitments1 = discardUnsignedUpdates(state.commitments)
202+
val commitments1 = run {
203+
// Prune previous funding transactions if we already sent splice_locked for the last funding transaction
204+
// that is also locked by our counterparty; we either missed their splice_locked or it confirmed while disconnected.
205+
val withRemoteLocked = when (val remoteTxId = cmd.message.myCurrentFundingLocked) {
206+
null -> state.commitments
207+
else -> when (val commitments1 = state.commitments.run { updateRemoteFundingStatus(remoteTxId) }) {
208+
is Either.Left -> state.commitments
209+
is Either.Right -> {
210+
state.run { newlyLocked(state.commitments, commitments1.value.first) }.forEach { actions.add(ChannelAction.Storage.SetLocked(it.fundingTxId)) }
211+
commitments1.value.first
212+
}
213+
}
214+
}
215+
// Then we clean up unsigned updates.
216+
discardUnsignedUpdates(withRemoteLocked)
217+
}
206218

207219
if (commitments1.changes.localHasChanges()) {
208220
actions.add(ChannelAction.Message.SendToSelf(ChannelCommand.Commitment.Sign))

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/states/WaitForChannelReady.kt‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,21 +59,23 @@ data class WaitForChannelReady(
5959
Pair(this@WaitForChannelReady, listOf(ChannelAction.Message.Send(TxAbort(channelId, InvalidRbfTxConfirmed(channelId, commitments.latest.fundingTxId).message))))
6060
}
6161
is ChannelReady -> {
62+
// We mark the funding transaction as locked by our peer: we won't ask them to retransmit channel_ready.
63+
val commitments1 = commitments.run { updateRemoteFundingStatus(commitments.latest.fundingTxId) }.fold({ it }, { it.first })
6264
// we create a channel_update early so that we can use it to send payments through this channel, but it won't be propagated to other nodes since the channel is not yet announced
6365
val initialChannelUpdate = Announcements.makeChannelUpdate(
6466
staticParams.nodeParams.chainHash,
6567
staticParams.nodeParams.nodePrivateKey,
6668
staticParams.remoteNodeId,
6769
shortChannelId,
6870
staticParams.nodeParams.expiryDeltaBlocks,
69-
commitments.params.remoteParams.htlcMinimum,
71+
commitments1.params.remoteParams.htlcMinimum,
7072
staticParams.nodeParams.feeBase,
7173
staticParams.nodeParams.feeProportionalMillionths.toLong(),
72-
commitments.latest.fundingAmount.toMilliSatoshi(),
73-
enable = Helpers.aboveReserve(commitments)
74+
commitments1.latest.fundingAmount.toMilliSatoshi(),
75+
enable = Helpers.aboveReserve(commitments1)
7476
)
7577
val nextState = Normal(
76-
commitments,
78+
commitments1,
7779
shortChannelId,
7880
initialChannelUpdate,
7981
null,
@@ -84,7 +86,7 @@ data class WaitForChannelReady(
8486
)
8587
val actions = listOf(
8688
ChannelAction.Storage.StoreState(nextState),
87-
ChannelAction.Storage.SetLocked(commitments.latest.fundingTxId),
89+
ChannelAction.Storage.SetLocked(commitments1.latest.fundingTxId),
8890
ChannelAction.EmitEvent(ChannelEvents.Confirmed(nextState)),
8991
)
9092
Pair(nextState, actions)

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/json/JsonSerializers.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@
8686
JsonSerializers.ShutdownTlvSerializer::class,
8787
JsonSerializers.ClosingCompleteTlvSerializer::class,
8888
JsonSerializers.ClosingSigTlvSerializer::class,
89+
JsonSerializers.MyCurrentFundingLockedTlvSerializer::class,
90+
JsonSerializers.YourLastFundingLockedTlvSerializer::class,
8991
JsonSerializers.ChannelReestablishTlvSerializer::class,
9092
JsonSerializers.ChannelReadyTlvSerializer::class,
9193
JsonSerializers.CommitSigTlvAlternativeFeerateSigSerializer::class,
@@ -201,6 +203,8 @@ object JsonSerializers {
201203
subclass(UpdateFee::class, UpdateFeeSerializer)
202204
}
203205
polymorphic(Tlv::class) {
206+
subclass(ChannelReestablishTlv.MyCurrentFundingLocked::class, MyCurrentFundingLockedTlvSerializer)
207+
subclass(ChannelReestablishTlv.YourLastFundingLocked::class, YourLastFundingLockedTlvSerializer)
204208
subclass(ChannelReadyTlv.ShortChannelIdTlv::class, ChannelReadyTlvShortChannelIdTlvSerializer)
205209
subclass(CommitSigTlv.AlternativeFeerateSigs::class, CommitSigTlvAlternativeFeerateSigsSerializer)
206210
subclass(CommitSigTlv.FundingTx::class, CommitSigTlvFundingTxSerializer)
@@ -551,6 +555,12 @@ object JsonSerializers {
551555
@Serializer(forClass = ChannelReadyTlv::class)
552556
object ChannelReadyTlvSerializer
553557

558+
@Serializer(forClass = ChannelReestablishTlv.MyCurrentFundingLocked::class)
559+
object MyCurrentFundingLockedTlvSerializer
560+
561+
@Serializer(forClass = ChannelReestablishTlv.YourLastFundingLocked::class)
562+
object YourLastFundingLockedTlvSerializer
563+
554564
@Serializer(forClass = ChannelReestablishTlv::class)
555565
object ChannelReestablishTlvSerializer
556566

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/wire/ChannelTlv.kt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,28 @@ sealed class ChannelReestablishTlv : Tlv {
180180
}
181181
}
182182

183+
/** Latest splice_locked or channel_ready received before disconnecting. */
184+
data class YourLastFundingLocked(val txId: TxId) : ChannelReestablishTlv() {
185+
override val tag: Long get() = YourLastFundingLocked.tag
186+
override fun write(out: Output) = LightningCodecs.writeTxHash(TxHash(txId), out)
187+
188+
companion object : TlvValueReader<YourLastFundingLocked> {
189+
const val tag: Long = 1
190+
override fun read(input: Input): YourLastFundingLocked = YourLastFundingLocked(TxId(LightningCodecs.txHash(input)))
191+
}
192+
}
193+
194+
/** Latest splice_locked or channel_ready that we're ready to send. */
195+
data class MyCurrentFundingLocked(val txId: TxId) : ChannelReestablishTlv() {
196+
override val tag: Long get() = MyCurrentFundingLocked.tag
197+
override fun write(out: Output) = LightningCodecs.writeTxHash(TxHash(txId), out)
198+
199+
companion object : TlvValueReader<MyCurrentFundingLocked> {
200+
const val tag: Long = 3
201+
override fun read(input: Input): MyCurrentFundingLocked = MyCurrentFundingLocked(TxId(LightningCodecs.txHash(input)))
202+
}
203+
}
204+
183205
// Legacy TLV needed to deserialize old backups
184206
data class ChannelData(val ecb: EncryptedChannelData) : ChannelReestablishTlv() {
185207
override val tag: Long get() = ChannelData.tag

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/wire/LightningMessages.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,8 @@ data class ChannelReestablish(
13771377
override val type: Long get() = ChannelReestablish.type
13781378

13791379
val nextFundingTxId: TxId? = tlvStream.get<ChannelReestablishTlv.NextFunding>()?.txId
1380+
val yourLastFundingLocked: TxId? = tlvStream.get<ChannelReestablishTlv.YourLastFundingLocked>()?.txId
1381+
val myCurrentFundingLocked: TxId? = tlvStream.get<ChannelReestablishTlv.MyCurrentFundingLocked>()?.txId
13801382
// Legacy channel backup present only on old inactive channels, will be replaced by peer storage next time this channel data is updated.
13811383
val legacyChannelData: EncryptedChannelData get() = tlvStream.get<ChannelReestablishTlv.ChannelData>()?.ecb ?: EncryptedChannelData.empty
13821384

@@ -1396,6 +1398,8 @@ data class ChannelReestablish(
13961398
val readers = mapOf(
13971399
ChannelReestablishTlv.ChannelData.tag to ChannelReestablishTlv.ChannelData.Companion as TlvValueReader<ChannelReestablishTlv>,
13981400
ChannelReestablishTlv.NextFunding.tag to ChannelReestablishTlv.NextFunding.Companion as TlvValueReader<ChannelReestablishTlv>,
1401+
ChannelReestablishTlv.YourLastFundingLocked.tag to ChannelReestablishTlv.YourLastFundingLocked.Companion as TlvValueReader<ChannelReestablishTlv>,
1402+
ChannelReestablishTlv.MyCurrentFundingLocked.tag to ChannelReestablishTlv.MyCurrentFundingLocked.Companion as TlvValueReader<ChannelReestablishTlv>,
13991403
)
14001404

14011405
override fun read(input: Input): ChannelReestablish {

0 commit comments

Comments
 (0)