From c6b8908b74eb22a47b1438356687739f06ffb38a Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Thu, 23 Nov 2023 14:49:43 +0100 Subject: [PATCH] Fix `tx_signatures` ordering for splices (#554) We were incorrectly splitting the shared input amount based on each peer's balance, but for the signing order the peer that adds the shared input takes credit for the whole amount of that input. --- .../fr/acinq/lightning/channel/InteractiveTx.kt | 11 +++++++---- .../lightning/channel/states/SpliceTestsCommon.kt | 6 ++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt index af65d8a4b..ec3b68efa 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt @@ -785,7 +785,7 @@ data class InteractiveTxSigningSession( } is Try.Success -> { val signedLocalCommit = LocalCommit(localCommit.value.index, localCommit.value.spec, PublishableTxs(signedLocalCommitTx, listOf())) - if (shouldSignFirst(channelParams, fundingTx.tx)) { + if (shouldSignFirst(fundingParams.isInitiator, channelParams, fundingTx.tx)) { val fundingStatus = LocalFundingStatus.UnconfirmedFundingTx(fundingTx, fundingParams, currentBlockHeight) val commitment = Commitment(fundingTxIndex, fundingParams.remoteFundingPubkey, fundingStatus, RemoteFundingStatus.NotLocked, signedLocalCommit, remoteCommit, nextRemoteCommit = null) val action = InteractiveTxSigningSessionAction.SendTxSigs(fundingStatus, commitment, fundingTx.localSigs) @@ -873,13 +873,16 @@ data class InteractiveTxSigningSession( } } - fun shouldSignFirst(channelParams: ChannelParams, tx: SharedTransaction): Boolean { - return if (tx.localAmountIn == tx.remoteAmountIn) { + fun shouldSignFirst(isInitiator: Boolean, channelParams: ChannelParams, tx: SharedTransaction): Boolean { + val sharedAmountIn = tx.sharedInput?.let { it.localAmount + it.remoteAmount } ?: 0.msat + val localAmountIn = tx.localInputs.map { it.txOut.amount }.sum().toMilliSatoshi() + if (isInitiator) sharedAmountIn else 0.msat + val remoteAmountIn = tx.remoteInputs.map { it.txOut.amount }.sum().toMilliSatoshi() + if (isInitiator) 0.msat else sharedAmountIn + return if (localAmountIn == remoteAmountIn) { // When both peers contribute the same amount, the peer with the lowest pubkey must transmit its `tx_signatures` first. LexicographicalOrdering.isLessThan(channelParams.localParams.nodeId, channelParams.remoteParams.nodeId) } else { // Otherwise, the peer with the lowest total of input amount must transmit its `tx_signatures` first. - tx.localAmountIn < tx.remoteAmountIn + localAmountIn < remoteAmountIn } } } diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt index 3b6110b4e..88f8ddfac 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt +++ b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/SpliceTestsCommon.kt @@ -37,6 +37,12 @@ class SpliceTestsCommon : LightningTestSuite() { spliceIn(alice, bob, listOf(50_000.sat)) } + @Test + fun `splice funds in -- non-initiator`() { + val (alice, bob) = reachNormal() + spliceIn(bob, alice, listOf(50_000.sat)) + } + @Test fun `splice funds in -- many utxos`() { val (alice, bob) = reachNormal()