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.
ACINQ · Nov 23, 2023 · c6b8908 · c6b8908
1 parent f1a09ce
commit c6b8908
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 4 deletions.
diff --git 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
@@ -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()