Skip to content

Commit

Permalink
Treat local/remote commit index separately when splicing (#544)
Browse files Browse the repository at this point in the history
The assumption that local/remote index are equal when channel is idle is
wrong: they can diverge in some cases.

This bug led to "invalid sig" errors during splices.
  • Loading branch information
pm47 authored Sep 28, 2023
1 parent 8c241c9 commit 408d113
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 11 deletions.
9 changes: 5 additions & 4 deletions src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ object Helpers {
fundingAmount: Satoshi,
toLocal: MilliSatoshi,
toRemote: MilliSatoshi,
commitmentIndex: Long,
localCommitmentIndex: Long,
remoteCommitmentIndex: Long,
commitTxFeerate: FeeratePerKw,
fundingTxIndex: Long,
fundingTxHash: ByteVector32,
Expand All @@ -313,10 +314,10 @@ object Helpers {

val fundingPubKey = channelKeys.fundingPubKey(fundingTxIndex)
val commitmentInput = makeFundingInputInfo(fundingTxHash, fundingTxOutputIndex, fundingAmount, fundingPubKey, remoteFundingPubkey)
val localPerCommitmentPoint = channelKeys.commitmentPoint(commitmentIndex)
val localPerCommitmentPoint = channelKeys.commitmentPoint(localCommitmentIndex)
val localCommitTx = Commitments.makeLocalTxs(
channelKeys,
commitTxNumber = commitmentIndex,
commitTxNumber = localCommitmentIndex,
localParams,
remoteParams,
fundingTxIndex = fundingTxIndex,
Expand All @@ -327,7 +328,7 @@ object Helpers {
).first
val remoteCommitTx = Commitments.makeRemoteTxs(
channelKeys,
commitTxNumber = commitmentIndex,
commitTxNumber = remoteCommitmentIndex,
localParams,
remoteParams,
fundingTxIndex = fundingTxIndex,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,8 @@ data class InteractiveTxSigningSession(
sharedTx: SharedTransaction,
localPushAmount: MilliSatoshi,
remotePushAmount: MilliSatoshi,
commitmentIndex: Long,
localCommitmentIndex: Long,
remoteCommitmentIndex: Long,
commitTxFeerate: FeeratePerKw,
remotePerCommitmentPoint: PublicKey
): Either<ChannelException, Pair<InteractiveTxSigningSession, CommitSig>> {
Expand All @@ -839,16 +840,17 @@ data class InteractiveTxSigningSession(
fundingAmount = sharedTx.sharedOutput.amount,
toLocal = sharedTx.sharedOutput.localAmount - localPushAmount + remotePushAmount,
toRemote = sharedTx.sharedOutput.remoteAmount - remotePushAmount + localPushAmount,
commitmentIndex = commitmentIndex,
localCommitmentIndex = localCommitmentIndex,
remoteCommitmentIndex = remoteCommitmentIndex,
commitTxFeerate,
fundingTxIndex = fundingTxIndex, fundingTxHash = unsignedTx.hash, fundingTxOutputIndex = sharedOutputIndex,
remoteFundingPubkey = fundingParams.remoteFundingPubkey,
remotePerCommitmentPoint = remotePerCommitmentPoint
).map { firstCommitTx ->
val localSigOfRemoteTx = Transactions.sign(firstCommitTx.remoteCommitTx, channelKeys.fundingKey(fundingTxIndex))
val commitSig = CommitSig(channelParams.channelId, localSigOfRemoteTx, listOf())
val unsignedLocalCommit = UnsignedLocalCommit(commitmentIndex, firstCommitTx.localSpec, firstCommitTx.localCommitTx, listOf())
val remoteCommit = RemoteCommit(commitmentIndex, firstCommitTx.remoteSpec, firstCommitTx.remoteCommitTx.tx.txid, remotePerCommitmentPoint)
val unsignedLocalCommit = UnsignedLocalCommit(localCommitmentIndex, firstCommitTx.localSpec, firstCommitTx.localCommitTx, listOf())
val remoteCommit = RemoteCommit(remoteCommitmentIndex, firstCommitTx.remoteSpec, firstCommitTx.remoteCommitTx.tx.txid, remotePerCommitmentPoint)
val signedFundingTx = sharedTx.sign(keyManager, fundingParams, channelParams.localParams, channelParams.remoteParams.nodeId)
Pair(InteractiveTxSigningSession(fundingParams, fundingTxIndex, signedFundingTx, Either.Left(unsignedLocalCommit), remoteCommit), commitSig)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ data class Normal(
interactiveTxAction.sharedTx,
localPushAmount = spliceStatus.localPushAmount,
remotePushAmount = spliceStatus.remotePushAmount,
commitmentIndex = parentCommitment.localCommit.index, // localCommit.index == remoteCommit.index because the channel is idle
localCommitmentIndex = parentCommitment.localCommit.index,
remoteCommitmentIndex = parentCommitment.remoteCommit.index,
parentCommitment.localCommit.spec.feerate,
parentCommitment.remoteCommit.remotePerCommitmentPoint
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ data class WaitForFundingConfirmed(
interactiveTxAction.sharedTx,
localPushAmount,
remotePushAmount,
commitmentIndex = replacedCommitment.localCommit.index,
localCommitmentIndex = replacedCommitment.localCommit.index,
remoteCommitmentIndex = replacedCommitment.remoteCommit.index,
replacedCommitment.localCommit.spec.feerate,
replacedCommitment.remoteCommit.remotePerCommitmentPoint
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ data class WaitForFundingCreated(
interactiveTxAction.sharedTx,
localPushAmount,
remotePushAmount,
commitmentIndex = 0,
localCommitmentIndex = 0,
remoteCommitmentIndex = 0,
commitTxFeerate,
remoteFirstPerCommitmentPoint
)
Expand Down

0 comments on commit 408d113

Please sign in to comment.