Skip to content

Commit 938722c

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 8be300c commit 938722c

File tree

12 files changed

+237
-95
lines changed

12 files changed

+237
-95
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
@@ -608,6 +608,9 @@ data class Commitments(
608608
// We always use the last commitment that was created, to make sure we never go back in time.
609609
val latest = active.first().let { c -> FullCommitment(params, changes, c.fundingTxIndex, c.remoteFundingPubkey, c.localFundingStatus, c.remoteFundingStatus, c.localCommit, c.remoteCommit, c.nextRemoteCommit) }
610610

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

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import fr.acinq.lightning.CltvExpiryDelta
66
import fr.acinq.lightning.Feature
77
import fr.acinq.lightning.NodeParams
88
import fr.acinq.lightning.SensitiveTaskEvents
9-
import fr.acinq.lightning.blockchain.*
9+
import fr.acinq.lightning.blockchain.WatchConfirmed
10+
import fr.acinq.lightning.blockchain.WatchConfirmedTriggered
11+
import fr.acinq.lightning.blockchain.WatchSpent
12+
import fr.acinq.lightning.blockchain.WatchSpentTriggered
1013
import fr.acinq.lightning.blockchain.fee.OnChainFeerates
1114
import fr.acinq.lightning.channel.*
1215
import fr.acinq.lightning.channel.Helpers.Closing.claimCurrentLocalCommitTxOutputs
@@ -336,14 +339,18 @@ sealed class PersistedChannelState : ChannelState() {
336339
is Normal -> state.getUnsignedFundingTxId()
337340
else -> null
338341
}
339-
val tlvs: TlvStream<ChannelReestablishTlv> = unsignedFundingTxId?.let { TlvStream(ChannelReestablishTlv.NextFunding(it)) } ?: TlvStream.empty()
342+
val tlvs: Set<ChannelReestablishTlv> = setOfNotNull(
343+
unsignedFundingTxId?.let { ChannelReestablishTlv.NextFunding(it) },
344+
state.commitments.lastRemoteLocked?.let { ChannelReestablishTlv.YourLastFundingLocked(it.fundingTxId) },
345+
state.commitments.run { lastLocalLocked() }?.let { ChannelReestablishTlv.MyCurrentFundingLocked(it.fundingTxId) },
346+
)
340347
ChannelReestablish(
341348
channelId = channelId,
342349
nextLocalCommitmentNumber = nextLocalCommitmentNumber,
343350
nextRemoteRevocationNumber = state.commitments.remoteCommitIndex,
344351
yourLastCommitmentSecret = PrivateKey(yourLastPerCommitmentSecret),
345352
myCurrentPerCommitmentPoint = myCurrentPerCommitmentPoint,
346-
tlvStream = tlvs
353+
tlvStream = TlvStream(tlvs),
347354
).withChannelData(state.commitments.remoteChannelData, logger)
348355
}
349356
}

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

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
7777
}
7878
is WaitForChannelReady -> {
7979
val actions = ArrayList<ChannelAction>()
80-
80+
// If they haven't received our signatures for the channel funding transaction, we retransmit them.
8181
if (state.commitments.latest.fundingTxId == cmd.message.nextFundingTxId) {
8282
if (state.commitments.latest.localFundingStatus is LocalFundingStatus.UnconfirmedFundingTx) {
8383
if (state.commitments.latest.localFundingStatus.sharedTx is PartiallySignedSharedTransaction) {
@@ -101,12 +101,13 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
101101
logger.warning { "cannot re-send tx_signatures for fundingTxId=${cmd.message.nextFundingTxId}, transaction is already confirmed" }
102102
}
103103
}
104-
105-
logger.debug { "re-sending channel_ready" }
106-
val nextPerCommitmentPoint = channelKeys().commitmentPoint(1)
107-
val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint)
108-
actions.add(ChannelAction.Message.Send(channelReady))
109-
104+
// If they haven't received our channel_ready, we retransmit it.
105+
if (cmd.message.yourLastFundingLocked == null) {
106+
logger.debug { "re-sending channel_ready" }
107+
val nextPerCommitmentPoint = channelKeys().commitmentPoint(1)
108+
val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint)
109+
actions.add(ChannelAction.Message.Send(channelReady))
110+
}
110111
Pair(state, actions)
111112
}
112113
is LegacyWaitForFundingLocked -> {
@@ -124,8 +125,7 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
124125
val actions = ArrayList<ChannelAction>()
125126

126127
// re-send channel_ready if necessary
127-
if (state.commitments.latest.fundingTxIndex == 0L && cmd.message.nextLocalCommitmentNumber == 1L && state.commitments.localCommitIndex == 0L) {
128-
// 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
128+
if (cmd.message.yourLastFundingLocked == null && state.commitments.latest.fundingTxIndex == 0L) {
129129
logger.debug { "re-sending channel_ready" }
130130
val nextPerCommitmentPoint = channelKeys().commitmentPoint(1)
131131
val channelReady = ChannelReady(state.commitments.channelId, nextPerCommitmentPoint)
@@ -174,25 +174,36 @@ data class Syncing(val state: PersistedChannelState, val channelReestablishSent:
174174
state.spliceStatus
175175
}
176176

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

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

194-
// then we clean up unsigned updates
195-
val commitments1 = discardUnsignedUpdates(state.commitments)
191+
val commitments1 = run {
192+
// Prune previous funding transactions if we already sent splice_locked for the last funding transaction
193+
// that is also locked by our counterparty; we either missed their splice_locked or it confirmed while disconnected.
194+
val withRemoteLocked = when (val remoteTxId = cmd.message.myCurrentFundingLocked) {
195+
null -> state.commitments
196+
else -> when (val commitments1 = state.commitments.run { updateRemoteFundingStatus(remoteTxId) }) {
197+
is Either.Left -> state.commitments
198+
is Either.Right -> {
199+
state.run { newlyLocked(state.commitments, commitments1.value.first) }.forEach { actions.add(ChannelAction.Storage.SetLocked(it.fundingTxId)) }
200+
commitments1.value.first
201+
}
202+
}
203+
}
204+
// Then we clean up unsigned updates.
205+
discardUnsignedUpdates(withRemoteLocked)
206+
}
196207

197208
if (commitments1.changes.localHasChanges()) {
198209
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.TlvStreamSerializer::class,
8787
JsonSerializers.ShutdownTlvSerializer::class,
8888
JsonSerializers.ClosingSignedTlvSerializer::class,
89+
JsonSerializers.MyCurrentFundingLockedTlvSerializer::class,
90+
JsonSerializers.YourLastFundingLockedTlvSerializer::class,
8991
JsonSerializers.ChannelReestablishTlvSerializer::class,
9092
JsonSerializers.ChannelReadyTlvSerializer::class,
9193
JsonSerializers.CommitSigTlvAlternativeFeerateSigSerializer::class,
@@ -204,6 +206,8 @@ object JsonSerializers {
204206
subclass(UpdateFee::class, UpdateFeeSerializer)
205207
}
206208
polymorphic(Tlv::class) {
209+
subclass(ChannelReestablishTlv.MyCurrentFundingLocked::class, MyCurrentFundingLockedTlvSerializer)
210+
subclass(ChannelReestablishTlv.YourLastFundingLocked::class, YourLastFundingLockedTlvSerializer)
207211
subclass(ChannelReadyTlv.ShortChannelIdTlv::class, ChannelReadyTlvShortChannelIdTlvSerializer)
208212
subclass(CommitSigTlv.AlternativeFeerateSigs::class, CommitSigTlvAlternativeFeerateSigsSerializer)
209213
subclass(CommitSigTlv.Batch::class, CommitSigTlvBatchSerializer)
@@ -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
@@ -203,6 +203,28 @@ sealed class ChannelReestablishTlv : Tlv {
203203
}
204204
}
205205

206+
/** Latest splice_locked or channel_ready received before disconnecting. */
207+
data class YourLastFundingLocked(val txId: TxId) : ChannelReestablishTlv() {
208+
override val tag: Long get() = YourLastFundingLocked.tag
209+
override fun write(out: Output) = LightningCodecs.writeTxHash(TxHash(txId), out)
210+
211+
companion object : TlvValueReader<YourLastFundingLocked> {
212+
const val tag: Long = 1
213+
override fun read(input: Input): YourLastFundingLocked = YourLastFundingLocked(TxId(LightningCodecs.txHash(input)))
214+
}
215+
}
216+
217+
/** Latest splice_locked or channel_ready that we're ready to send. */
218+
data class MyCurrentFundingLocked(val txId: TxId) : ChannelReestablishTlv() {
219+
override val tag: Long get() = MyCurrentFundingLocked.tag
220+
override fun write(out: Output) = LightningCodecs.writeTxHash(TxHash(txId), out)
221+
222+
companion object : TlvValueReader<MyCurrentFundingLocked> {
223+
const val tag: Long = 3
224+
override fun read(input: Input): MyCurrentFundingLocked = MyCurrentFundingLocked(TxId(LightningCodecs.txHash(input)))
225+
}
226+
}
227+
206228
data class ChannelData(val ecb: EncryptedChannelData) : ChannelReestablishTlv() {
207229
override val tag: Long get() = ChannelData.tag
208230
override fun write(out: Output) = LightningCodecs.writeBytes(ecb.data, out)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,9 @@ data class ChannelReestablish(
13401340
override val type: Long get() = ChannelReestablish.type
13411341

13421342
val nextFundingTxId: TxId? = tlvStream.get<ChannelReestablishTlv.NextFunding>()?.txId
1343+
val yourLastFundingLocked: TxId? = tlvStream.get<ChannelReestablishTlv.YourLastFundingLocked>()?.txId
1344+
val myCurrentFundingLocked: TxId? = tlvStream.get<ChannelReestablishTlv.MyCurrentFundingLocked>()?.txId
1345+
13431346
override val channelData: EncryptedChannelData get() = tlvStream.get<ChannelReestablishTlv.ChannelData>()?.ecb ?: EncryptedChannelData.empty
13441347
override fun withNonEmptyChannelData(ecd: EncryptedChannelData): ChannelReestablish = copy(tlvStream = tlvStream.addOrUpdate(ChannelReestablishTlv.ChannelData(ecd)))
13451348

@@ -1359,6 +1362,8 @@ data class ChannelReestablish(
13591362
val readers = mapOf(
13601363
ChannelReestablishTlv.ChannelData.tag to ChannelReestablishTlv.ChannelData.Companion as TlvValueReader<ChannelReestablishTlv>,
13611364
ChannelReestablishTlv.NextFunding.tag to ChannelReestablishTlv.NextFunding.Companion as TlvValueReader<ChannelReestablishTlv>,
1365+
ChannelReestablishTlv.YourLastFundingLocked.tag to ChannelReestablishTlv.YourLastFundingLocked.Companion as TlvValueReader<ChannelReestablishTlv>,
1366+
ChannelReestablishTlv.MyCurrentFundingLocked.tag to ChannelReestablishTlv.MyCurrentFundingLocked.Companion as TlvValueReader<ChannelReestablishTlv>,
13621367
)
13631368

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

0 commit comments

Comments
 (0)