Skip to content

Commit f508529

Browse files
authored
Remove amount-based confirmation scaling (#770)
We previously scaled the number of confirmations based on the channel capacity: however, this doesn't really work with splicing, as shown by the following attack scenario: - a malicious node opens a very large channel - we thus wait for more confirmations to protect against reorgs - they then send lightning payments until all of the balance is on our side of the channel - we splice-out to shrink the channel to a minimal size - if we thus us a smaller number of confirmations (because the channel is now small), the malicious node can perform this small reorg and publish one of their revoked commitment There are more involved variations of this attack, which show that the real amount at stake is potentially much more than a single channel's current capacity. Instead of trying to find a complex solution that would likely have subtle edge cases, it's a lot simpler to always use a static number of confirmations that is large enough to protect against reorgs entirely.
1 parent 7fae0fa commit f508529

File tree

19 files changed

+101
-129
lines changed

19 files changed

+101
-129
lines changed

modules/core/src/commonMain/kotlin/fr/acinq/lightning/NodeParams.kt

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ data class RecipientCltvExpiryParams(val min: CltvExpiryDelta, val max: CltvExpi
116116
* @param htlcMinimum minimum accepted HTLC value.
117117
* @param toRemoteDelayBlocks number of blocks our peer will have to wait before they get their main output back in case they force-close a channel.
118118
* @param maxToLocalDelayBlocks maximum number of blocks we will have to wait before we get our main output back in case we force-close a channel.
119-
* @param minDepthBlocks minimum depth of a transaction before we consider it safely confirmed: note that it will be scaled based on the amount at stake.
119+
* @param minDepthBlocks minimum depth of a transaction before we consider it safe from reorgs.
120120
* @param feeBase base fee used in our channel_update: since our channels are private and we don't relay payments, this will be basically ignored.
121121
* @param feeProportionalMillionths proportional fee used in our channel_update: since our channels are private and we don't relay payments, this will be basically ignored.
122122
* @param pingInterval delay between ping messages.
@@ -236,7 +236,7 @@ data class NodeParams(
236236
bolt11InvoiceExpiry = 24.hours,
237237
bolt12InvoiceExpiry = 24.hours,
238238
htlcMinimum = 1000.msat,
239-
minDepthBlocks = 3,
239+
minDepthBlocks = 8,
240240
toRemoteDelayBlocks = CltvExpiryDelta(2016),
241241
maxToLocalDelayBlocks = CltvExpiryDelta(1008),
242242
feeBase = 1000.msat,
@@ -260,22 +260,6 @@ data class NodeParams(
260260
),
261261
)
262262

263-
/**
264-
* Returns the number of confirmations needed to consider a transaction irrevocably confirmed
265-
* and thus safe from reorgs. We make sure the cumulative block reward largely exceeds the
266-
* transaction amount.
267-
*
268-
* @param amount amount at stake in this transaction.
269-
* @return number of confirmations needed.
270-
*/
271-
fun minDepth(amount: Satoshi): Long {
272-
val blockReward = 3.125f // this will be too large after the halving in 2028
273-
val scalingFactor = 10
274-
val btc = amount.toLong().toDouble() / 100_000_000L
275-
val blocksToReachFunding = (((scalingFactor * btc) / blockReward) + 1).toLong()
276-
return max(minDepthBlocks.toLong(), blocksToReachFunding)
277-
}
278-
279263
/**
280264
* We generate a default, deterministic Bolt 12 offer based on the node's seed and its trampoline node.
281265
* This offer will stay valid after restoring the seed on a different device.

modules/core/src/commonMain/kotlin/fr/acinq/lightning/blockchain/WatcherTypes.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ data class WatchConfirmed(
1717
val txId: TxId,
1818
// We need a public key script to use electrum apis.
1919
val publicKeyScript: ByteVector,
20-
val minDepth: Long,
20+
val minDepth: Int,
2121
val event: OnChainEvent,
2222
) : Watch() {
2323
// If we have the entire transaction, we can get the redeemScript from the witness, and re-compute the publicKeyScript.
2424
// We support both p2pkh and p2wpkh scripts.
25-
constructor(channelId: ByteVector32, tx: Transaction, minDepth: Long, event: OnChainEvent) : this(
25+
constructor(channelId: ByteVector32, tx: Transaction, minDepth: Int, event: OnChainEvent) : this(
2626
channelId,
2727
tx.txid,
2828
if (tx.txOut.isEmpty()) ByteVector.empty else tx.txOut[0].publicKeyScript,

modules/core/src/commonMain/kotlin/fr/acinq/lightning/blockchain/electrum/ElectrumWatcher.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ class ElectrumWatcher(val client: IElectrumClient, val scope: CoroutineScope, lo
247247
val parentTxid = tx.txIn[0].outPoint.txid
248248
logger.info { "txid=${tx.txid} has a relative timeout of $csvTimeout blocks, watching parenttxid=$parentTxid tx=$tx" }
249249
val parentPublicKeyScript = WatchConfirmed.extractPublicKeyScript(tx.txIn.first().witness)
250-
addWatch(WatchConfirmed(ByteVector32.Zeroes, parentTxid, parentPublicKeyScript, csvTimeout, WatchConfirmed.ParentTxConfirmed(tx)))
250+
addWatch(WatchConfirmed(ByteVector32.Zeroes, parentTxid, parentPublicKeyScript, csvTimeout.toInt(), WatchConfirmed.ParentTxConfirmed(tx)))
251251
}
252252

253253
cltvTimeout > blockCount -> {

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -586,10 +586,6 @@ data class Commitments(
586586
val remoteCommitIndex = active.first().remoteCommit.index
587587
val nextRemoteCommitIndex = remoteCommitIndex + 1
588588

589-
// While we have multiple active commitments, we use the min or max depending on the scenario.
590-
val capacityMin = active.minOf { it.fundingAmount }
591-
val capacityMax = active.maxOf { it.fundingAmount }
592-
593589
fun availableBalanceForSend(): MilliSatoshi = active.minOf { it.availableBalanceForSend(params, changes) }
594590
fun availableBalanceForReceive(): MilliSatoshi = active.minOf { it.availableBalanceForReceive(params, changes) }
595591

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,7 @@ object Helpers {
157157
fun LoggingContext.watchConfirmedIfNeeded(nodeParams: NodeParams, channelId: ByteVector32, txs: List<Transaction>, irrevocablySpent: Map<OutPoint, Transaction>): List<ChannelAction.Blockchain.SendWatch> {
158158
val (skip, process) = txs.partition { it.inputsAlreadySpent(irrevocablySpent) }
159159
skip.forEach { tx -> logger.info { "no need to watch txid=${tx.txid}, it has already been confirmed" } }
160-
return process.map { tx ->
161-
// Those are channel force-close transactions, which don't include a change output: every output is potentially at stake.
162-
val minDepth = nodeParams.minDepth(tx.txOut.map { it.amount }.sum())
163-
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, tx, minDepth, WatchConfirmed.ClosingTxConfirmed))
164-
}
160+
return process.map { tx -> ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, tx, nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed)) }
165161
}
166162

167163
/** This helper method will watch txs only if the utxo they spend hasn't already been irrevocably spent. */

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ sealed class ChannelState {
192192

193193
internal fun ChannelContext.doPublish(tx: ClosingTx, channelId: ByteVector32): List<ChannelAction.Blockchain> = listOf(
194194
ChannelAction.Blockchain.PublishTx(tx),
195-
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, tx.tx, staticParams.nodeParams.minDepth(tx.amountIn), WatchConfirmed.ClosingTxConfirmed))
195+
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, tx.tx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed))
196196
)
197197

198198
internal suspend fun ChannelContext.handleLocalError(cmd: ChannelCommand, t: Throwable): Pair<ChannelState, List<ChannelAction>> {
@@ -458,7 +458,7 @@ sealed class ChannelStateWithCommitments : PersistedChannelState() {
458458
is Commitment -> {
459459
logger.warning { "a commit tx for an older commitment has been published fundingTxId=${commitment.fundingTxId} fundingTxIndex=${commitment.fundingTxIndex}" }
460460
// We try spending our latest commitment but we also watch their commitment: if it confirms, we will react by spending our corresponding outputs.
461-
val watch = ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, w.spendingTx, staticParams.nodeParams.minDepth(commitments.capacityMax), WatchConfirmed.AlternativeCommitTxConfirmed))
461+
val watch = ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, w.spendingTx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.AlternativeCommitTxConfirmed))
462462
spendLocalCurrent().run { copy(second = second + watch) }
463463
}
464464
else -> {

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ data class Closing(
229229
else -> when (val commitment = commitments.resolveCommitment(watch.spendingTx)) {
230230
is Commitment -> {
231231
logger.warning { "a commit tx for an older commitment has been published fundingTxId=${commitment.fundingTxId} fundingTxIndex=${commitment.fundingTxIndex}" }
232-
Pair(this@Closing, listOf(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepth(commitments.capacityMax), WatchConfirmed.AlternativeCommitTxConfirmed))))
232+
Pair(this@Closing, listOf(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.AlternativeCommitTxConfirmed))))
233233
}
234234
else -> {
235235
logger.warning { "unrecognized tx=${watch.spendingTx.txid}" }
@@ -270,7 +270,7 @@ data class Closing(
270270
add(ChannelAction.Storage.StoreState(nextState))
271271
// one of the outputs of the local/remote/revoked commit was spent
272272
// we just put a watch to be notified when it is confirmed
273-
add(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepth(commitments.capacityMax), WatchConfirmed.ClosingTxConfirmed)))
273+
add(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed)))
274274
addAll(revokedCommitPublishActions)
275275
addAll(htlcSettledActions)
276276
}
@@ -367,25 +367,25 @@ data class Closing(
367367
*/
368368
private fun isClosed(additionalConfirmedTx: Transaction?): ClosingType? {
369369
return when {
370-
additionalConfirmedTx?.let { tx -> mutualClosePublished.any { it.tx.txid == tx.txid } } ?: false -> {
371-
val closingTx = mutualClosePublished.first { it.tx.txid == additionalConfirmedTx!!.txid }.copy(tx = additionalConfirmedTx!!)
370+
additionalConfirmedTx?.let { tx -> mutualClosePublished.any { it.tx.txid == tx.txid } } == true -> {
371+
val closingTx = mutualClosePublished.first { it.tx.txid == additionalConfirmedTx.txid }.copy(tx = additionalConfirmedTx)
372372
MutualClose(closingTx)
373373
}
374-
localCommitPublished?.isDone() ?: false -> LocalClose(commitments.latest.localCommit, localCommitPublished!!)
375-
remoteCommitPublished?.isDone() ?: false -> CurrentRemoteClose(commitments.latest.remoteCommit, remoteCommitPublished!!)
376-
nextRemoteCommitPublished?.isDone() ?: false -> NextRemoteClose(commitments.latest.nextRemoteCommit!!.commit, nextRemoteCommitPublished!!)
377-
futureRemoteCommitPublished?.isDone() ?: false -> RecoveryClose(futureRemoteCommitPublished!!)
374+
localCommitPublished?.isDone() == true -> LocalClose(commitments.latest.localCommit, localCommitPublished)
375+
remoteCommitPublished?.isDone() == true -> CurrentRemoteClose(commitments.latest.remoteCommit, remoteCommitPublished)
376+
nextRemoteCommitPublished?.isDone() == true -> NextRemoteClose(commitments.latest.nextRemoteCommit!!.commit, nextRemoteCommitPublished)
377+
futureRemoteCommitPublished?.isDone() == true -> RecoveryClose(futureRemoteCommitPublished)
378378
revokedCommitPublished.any { it.isDone() } -> RevokedClose(revokedCommitPublished.first { it.isDone() })
379379
else -> null
380380
}
381381
}
382382

383383
fun closingTypeAlreadyKnown(): ClosingType? {
384384
return when {
385-
localCommitPublished?.isConfirmed() ?: false -> LocalClose(commitments.latest.localCommit, localCommitPublished!!)
386-
remoteCommitPublished?.isConfirmed() ?: false -> CurrentRemoteClose(commitments.latest.remoteCommit, remoteCommitPublished!!)
387-
nextRemoteCommitPublished?.isConfirmed() ?: false -> NextRemoteClose(commitments.latest.nextRemoteCommit!!.commit, nextRemoteCommitPublished!!)
388-
futureRemoteCommitPublished?.isConfirmed() ?: false -> RecoveryClose(futureRemoteCommitPublished!!)
385+
localCommitPublished?.isConfirmed() == true -> LocalClose(commitments.latest.localCommit, localCommitPublished)
386+
remoteCommitPublished?.isConfirmed() == true -> CurrentRemoteClose(commitments.latest.remoteCommit, remoteCommitPublished)
387+
nextRemoteCommitPublished?.isConfirmed() == true -> NextRemoteClose(commitments.latest.nextRemoteCommit!!.commit, nextRemoteCommitPublished)
388+
futureRemoteCommitPublished?.isConfirmed() == true -> RecoveryClose(futureRemoteCommitPublished)
389389
revokedCommitPublished.any { it.isConfirmed() } -> RevokedClose(revokedCommitPublished.first { it.isConfirmed() })
390390
else -> null
391391
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ data class Negotiating(
6161
val actions = listOf(
6262
ChannelAction.Storage.StoreState(nextState),
6363
ChannelAction.Blockchain.PublishTx(signedClosingTx),
64-
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, signedClosingTx.tx, staticParams.nodeParams.minDepth(signedClosingTx.amountIn), WatchConfirmed.ClosingTxConfirmed)),
64+
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, signedClosingTx.tx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed)),
6565
ChannelAction.Message.Send(closingSig)
6666
)
6767
Pair(nextState, actions)
@@ -83,7 +83,7 @@ data class Negotiating(
8383
val actions = listOf(
8484
ChannelAction.Storage.StoreState(nextState),
8585
ChannelAction.Blockchain.PublishTx(signedClosingTx),
86-
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, signedClosingTx.tx, staticParams.nodeParams.minDepth(signedClosingTx.amountIn), WatchConfirmed.ClosingTxConfirmed)),
86+
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, signedClosingTx.tx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed)),
8787
)
8888
Pair(nextState, actions)
8989
}
@@ -111,8 +111,7 @@ data class Negotiating(
111111
is WatchSpent.ChannelSpent -> when {
112112
publishedClosingTxs.any { it.tx.txid == watch.spendingTx.txid } -> {
113113
// This is one of the transactions we already published, we watch for confirmations.
114-
val closingTx = publishedClosingTxs.first { it.tx.txid == watch.spendingTx.txid }
115-
val actions = listOf(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepth(closingTx.amountIn), WatchConfirmed.ClosingTxConfirmed)))
114+
val actions = listOf(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed)))
116115
Pair(this@Negotiating, actions)
117116
}
118117
proposedClosingTxs.flatMap { it.all }.any { it.tx.txid == watch.spendingTx.txid } -> {
@@ -122,7 +121,7 @@ data class Negotiating(
122121
val actions = listOf(
123122
ChannelAction.Storage.StoreState(nextState),
124123
ChannelAction.Blockchain.PublishTx(closingTx),
125-
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepth(closingTx.amountIn), WatchConfirmed.ClosingTxConfirmed))
124+
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed))
126125
)
127126
Pair(nextState, actions)
128127
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -841,8 +841,7 @@ data class Normal(
841841
): Pair<Normal, List<ChannelAction>> {
842842
logger.info { "sending tx_sigs" }
843843
// We watch for confirmation in all cases, to allow pruning outdated commitments when transactions confirm.
844-
val fundingMinDepth = staticParams.nodeParams.minDepth(action.fundingTx.fundingParams.fundingAmount)
845-
val watchConfirmed = WatchConfirmed(channelId, action.commitment.fundingTxId, action.commitment.commitInput.txOut.publicKeyScript, fundingMinDepth, WatchConfirmed.ChannelFundingDepthOk)
844+
val watchConfirmed = WatchConfirmed(channelId, action.commitment.fundingTxId, action.commitment.commitInput.txOut.publicKeyScript, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ChannelFundingDepthOk)
846845
val commitments = commitments.add(action.commitment).copy(remoteChannelData = remoteChannelData)
847846
val nextState = this@Normal.copy(commitments = commitments, spliceStatus = SpliceStatus.None)
848847
val actions = buildList {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ data class Offline(val state: PersistedChannelState) : ChannelState() {
9797
is WatchSpentTriggered -> when {
9898
state is Negotiating && state.publishedClosingTxs.any { it.tx.txid == watch.spendingTx.txid } -> {
9999
// This is one of the transactions we already published, we watch for confirmations.
100-
val actions = listOf(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepth(state.commitments.capacityMax), WatchConfirmed.ClosingTxConfirmed)))
100+
val actions = listOf(ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed)))
101101
Pair(this@Offline, actions)
102102
}
103103
state is Negotiating && state.proposedClosingTxs.flatMap { it.all }.any { it.tx.txid == watch.spendingTx.txid } -> {
@@ -107,7 +107,7 @@ data class Offline(val state: PersistedChannelState) : ChannelState() {
107107
val actions = listOf(
108108
ChannelAction.Storage.StoreState(nextState),
109109
ChannelAction.Blockchain.PublishTx(closingTx),
110-
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepth(state.commitments.capacityMax), WatchConfirmed.ClosingTxConfirmed))
110+
ChannelAction.Blockchain.SendWatch(WatchConfirmed(channelId, watch.spendingTx, staticParams.nodeParams.minDepthBlocks, WatchConfirmed.ClosingTxConfirmed))
111111
)
112112
Pair(Offline(nextState), actions)
113113
}

0 commit comments

Comments
 (0)