Address review comments

- add a pubkey script to the SharedInput() class (we don't need the full TxOut which we can recreate) - remove aggregate nonce check ins FullySignedTx: code already handles transactions that are not properly signed - generate musig2 nonces when we send TxAddInput
ACINQ · Jan 8, 2024 · 3b231d8 · 3b231d8
1 parent ed984c0
commit 3b231d8
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 25 deletions.
diff --git a/RECOVERY.md b/RECOVERY.md
@@ -28,7 +28,7 @@ swap-in transactions to your wallet are indistinguishable from other p2tr transa
 
 The swap transaction's output can be spent using either:
 
-1. A aggregated musig2 signature built from a partial signature from the user's wallet and a partial signature from the remote node
+1. An aggregated musig2 signature built from a partial signature from the user's wallet and a partial signature from the remote node
 2. A signature from the user's wallet after a refund delay
 
 Funds can be recovered using the second option and [Bitcoin Core](https://github.com/bitcoin/bitcoin).
@@ -38,7 +38,7 @@ This process will become simpler once popular on-chain wallets (such as [electru
 
 ### Get your wallet descriptor
 
-lighting-kmp provides both a public descriptor and private descriptor for your swap-in wallet.
+lightning-kmp provides both a public descriptor and private descriptor for your swap-in wallet.
 The public descriptor can be used to create a watch-only wallet for your swap-in funds.
 The private descriptor can be used to recover your swap-in funds, after the refund delay has passed.
 :warning: Do not share this private descriptor with anyone ! 
@@ -62,13 +62,13 @@ tr(<extended_public_key>,and_v(v:pk(<master_key>/<derivation_path>),older(<refun
 For example, your public descriptor will look like this:
 
 ```txt
-tr(1fc559d9c96c5953895d3150e64ebf3dd696a0b08e758650b48ff6251d7e60d1,and_v(v:pk(tprv8h9x3k1njDX6to9q2G3aEvcic81MJk64SUVMXFc2Eo2YQqPGCBpQa8uJDkTz3DMHVXEmvhuwf4ShjLQ7YaVr34x9DFT3y43cPzVKGB94r1n/*),older(25920)))#7dne06j5
+tr(1fc559d9c96c5953895d3150e64ebf3dd696a0b08e758650b48ff6251d7e60d1,and_v(v:pk(tpubDDqzCA42sbCmnGBcuuiAeLGqB9XHU5Gy1n68omeKf4pwFKe2padzkdXAPsDMWMdee879oPYrGrTS8sioqyjv8b6TztunE526eo4Au9kTef3/*),older(25920)))#z6mq2a3u
 ```
 
 And your private descriptor will look like this: 
 
 ```
-tr(1fc559d9c96c5953895d3150e64ebf3dd696a0b08e758650b48ff6251d7e60d1,and_v(v:pk(tpubDDqzCA42sbCmnGBcuuiAeLGqB9XHU5Gy1n68omeKf4pwFKe2padzkdXAPsDMWMdee879oPYrGrTS8sioqyjv8b6TztunE526eo4Au9kTef3/*),older(25920)))#z6mq2a3u
+tr(1fc559d9c96c5953895d3150e64ebf3dd696a0b08e758650b48ff6251d7e60d1,and_v(v:pk(tprv8h9x3k1njDX6to9q2G3aEvcic81MJk64SUVMXFc2Eo2YQqPGCBpQa8uJDkTz3DMHVXEmvhuwf4ShjLQ7YaVr34x9DFT3y43cPzVKGB94r1n/*),older(25920)))#7dne06j5
 ```
 
 We can import our private descriptor into our recovery wallet:

diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
@@ -151,8 +151,16 @@ sealed class InteractiveTxInput {
     ) : Remote()
 
     /** The shared input can be added by us or by our peer, depending on who initiated the protocol. */
-    data class Shared(override val serialId: Long, override val outPoint: OutPoint, override val txOut: TxOut, override val sequence: UInt, val localAmount: MilliSatoshi, val remoteAmount: MilliSatoshi) : InteractiveTxInput(), Incoming,
-        Outgoing
+    data class Shared(
+        override val serialId: Long,
+        override val outPoint: OutPoint,
+        val publicKeyScript: ByteVector, override
+        val sequence: UInt,
+        val localAmount: MilliSatoshi,
+        val remoteAmount: MilliSatoshi
+    ) : InteractiveTxInput(), Incoming, Outgoing {
+        override val txOut: TxOut get() = TxOut((localAmount + remoteAmount).truncateToSatoshi(), publicKeyScript)
+    }
 }
 
 sealed class InteractiveTxOutput {
@@ -266,7 +274,7 @@ data class FundingContributions(val inputs: List<InteractiveTxInput.Outgoing>, v
                     }
                 }
             }
-            val sharedInput = sharedUtxo?.let { (i, balances) -> listOf(InteractiveTxInput.Shared(0, i.info.outPoint, i.info.txOut, 0xfffffffdU, balances.toLocal, balances.toRemote)) } ?: listOf()
+            val sharedInput = sharedUtxo?.let { (i, balances) -> listOf(InteractiveTxInput.Shared(0, i.info.outPoint, i.info.txOut.publicKeyScript, 0xfffffffdU, balances.toLocal, balances.toRemote)) } ?: listOf()
             val localInputs = walletInputs.map { i ->
                 when {
                     Script.isPay2wsh(i.previousTx.txOut[i.outputIndex].publicKeyScript.toByteArray()) ->
@@ -525,7 +533,6 @@ data class FullySignedSharedTransaction(override val tx: SharedTransaction, over
         val localSwapTxInMusig2 = tx.localInputs.filterIsInstance<InteractiveTxInput.LocalSwapIn>().sortedBy { i -> i.serialId }.zip(localSigs.swapInUserPartialSigs.zip(remoteSigs.swapInServerPartialSigs)).map { (i, sigs) ->
             val (userSig, serverSig) = sigs
             val swapInProtocol = SwapInProtocol(i.swapInParams)
-            require(userSig.aggregatedPublicNonce == serverSig.aggregatedPublicNonce) { "aggregated public nonces mismatch for local input ${i.serialId}" }
             val commonNonce = userSig.aggregatedPublicNonce
             val unsignedTx = tx.buildUnsignedTx()
             val ctx = swapInProtocol.session(unsignedTx, unsignedTx.txIn.indexOfFirst { it.outPoint == i.outPoint }, unsignedTx.txIn.map { tx.spentOutputs[it.outPoint]!! }, commonNonce)
@@ -544,7 +551,6 @@ data class FullySignedSharedTransaction(override val tx: SharedTransaction, over
         val remoteSwapTxInMusig2 = tx.remoteInputs.filterIsInstance<InteractiveTxInput.RemoteSwapIn>().sortedBy { i -> i.serialId }.zip(remoteSigs.swapInUserPartialSigs.zip(localSigs.swapInServerPartialSigs)).map { (i, sigs) ->
             val (userSig, serverSig) = sigs
             val swapInProtocol = SwapInProtocol(i.swapInParams)
-            require(userSig.aggregatedPublicNonce == serverSig.aggregatedPublicNonce) { "aggregated public nonces mismatch for remote input ${i.serialId}" }
             val commonNonce = userSig.aggregatedPublicNonce
             val unsignedTx = tx.buildUnsignedTx()
             val ctx = swapInProtocol.session(unsignedTx, unsignedTx.txIn.indexOfFirst { it.outPoint == i.outPoint }, unsignedTx.txIn.map { tx.spentOutputs[it.outPoint]!! }, commonNonce)
@@ -646,19 +652,12 @@ data class InteractiveTxSession(
     fun send(): Pair<InteractiveTxSession, InteractiveTxSessionAction> {
         return when (val msg = toSend.firstOrNull()) {
             null -> {
-                // generate a new secret nonce for each musig2 new swapin every time we send TxComplete
                 val localMusig2SwapIns = localInputs.filterIsInstance<InteractiveTxInput.LocalSwapIn>()
-                val secretNonces1 = localMusig2SwapIns.fold(secretNonces) { nonces, i ->
-                    nonces + (i.serialId to (nonces[i.serialId] ?: SecretNonce.generate(randomBytes32(), swapInKeys.userPrivateKey, swapInKeys.userPublicKey, null, null, null)))
-                }
                 val remoteMusig2SwapIns = remoteInputs.filterIsInstance<InteractiveTxInput.RemoteSwapIn>()
-                val secretNonces2 = remoteMusig2SwapIns.fold(secretNonces1) { nonces, i ->
-                    nonces + (i.serialId to (nonces[i.serialId] ?: SecretNonce.generate(randomBytes32(),null, i.swapInParams.serverKey, null, null, null)))
-                }
                 val serialIds = (localMusig2SwapIns.map { it.serialId } + remoteMusig2SwapIns.map { it.serialId }).sorted()
-                val nonces = serialIds.map { secretNonces2[it]?.second }.filterNotNull()
+                val nonces = serialIds.map { secretNonces[it]?.second }.filterNotNull()
                 val txComplete = TxComplete(fundingParams.channelId, nonces)
-                val next = copy(secretNonces = secretNonces2, txCompleteSent = txComplete)
+                val next = copy(txCompleteSent = txComplete)
                 if (next.isComplete) {
                     Pair(next, next.validateTx(txComplete))
                 } else {
@@ -667,7 +666,6 @@ data class InteractiveTxSession(
             }
 
             is Either.Left -> {
-                val next = copy(toSend = toSend.tail(), localInputs = localInputs + msg.value, txCompleteSent = null)
                 val txAddInput = when (msg.value) {
                     is InteractiveTxInput.LocalOnly -> TxAddInput(fundingParams.channelId, msg.value.serialId, msg.value.previousTx, msg.value.previousTxOutput, msg.value.sequence)
                     is InteractiveTxInput.LocalLegacySwapIn -> {
@@ -682,7 +680,16 @@ data class InteractiveTxSession(
 
                     is InteractiveTxInput.Shared -> TxAddInput(fundingParams.channelId, msg.value.serialId, msg.value.outPoint, msg.value.sequence)
                 }
-                Pair(next, InteractiveTxSessionAction.SendMessage(txAddInput))
+                val next = copy(toSend = toSend.tail(), localInputs = localInputs + msg.value, txCompleteSent = null)
+                val next1 = when (msg.value) {
+                    is InteractiveTxInput.LocalSwapIn -> {
+                        // generate a secret nonce for this input if we don't already have one
+                        val secretNonce = next.secretNonces[msg.value.serialId] ?: SecretNonce.generate(randomBytes32(), swapInKeys.userPrivateKey, swapInKeys.userPublicKey, null, null, null)
+                        next.copy(secretNonces = next.secretNonces + (msg.value.serialId to secretNonce))
+                    }
+                    else -> next
+                }
+                Pair(next1, InteractiveTxSessionAction.SendMessage(txAddInput))
             }
 
             is Either.Right -> {
@@ -709,7 +716,7 @@ data class InteractiveTxSession(
                 val expectedSharedOutpoint = fundingParams.sharedInput?.info?.outPoint ?: return Either.Left(InteractiveTxSessionAction.PreviousTxMissing(message.channelId, message.serialId))
                 val receivedSharedOutpoint = message.sharedInput ?: return Either.Left(InteractiveTxSessionAction.PreviousTxMissing(message.channelId, message.serialId))
                 if (expectedSharedOutpoint != receivedSharedOutpoint) return Either.Left(InteractiveTxSessionAction.PreviousTxMissing(message.channelId, message.serialId))
-                InteractiveTxInput.Shared(message.serialId, receivedSharedOutpoint, fundingParams.sharedInput.info.txOut, message.sequence, previousFunding.toLocal, previousFunding.toRemote)
+                InteractiveTxInput.Shared(message.serialId, receivedSharedOutpoint, fundingParams.sharedInput.info.txOut.publicKeyScript, message.sequence, previousFunding.toLocal, previousFunding.toRemote)
             }
 
             else -> {

diff --git a/src/commonMain/kotlin/fr/acinq/lightning/serialization/v4/Deserialization.kt b/src/commonMain/kotlin/fr/acinq/lightning/serialization/v4/Deserialization.kt
@@ -212,15 +212,15 @@ object Deserialization {
         0x01 -> InteractiveTxInput.Shared(
             serialId = readNumber(),
             outPoint = readOutPoint(),
-            txOut = TxOut(Satoshi(0), ByteVector.empty),
+            publicKeyScript = ByteVector.empty,
             sequence = readNumber().toUInt(),
             localAmount = readNumber().msat,
             remoteAmount = readNumber().msat,
         )
         0x02 -> InteractiveTxInput.Shared(
             serialId = readNumber(),
             outPoint = readOutPoint(),
-            txOut = readTxOut(),
+            publicKeyScript = readDelimitedByteArray().byteVector(),
             sequence = readNumber().toUInt(),
             localAmount = readNumber().msat,
             remoteAmount = readNumber().msat,

diff --git a/src/commonMain/kotlin/fr/acinq/lightning/serialization/v4/Serialization.kt b/src/commonMain/kotlin/fr/acinq/lightning/serialization/v4/Serialization.kt
@@ -258,7 +258,7 @@ object Serialization {
         write(0x02)
         writeNumber(serialId)
         writeBtcObject(outPoint)
-        writeBtcObject(txOut)
+        writeDelimited(publicKeyScript.toByteArray())
         writeNumber(sequence.toLong())
         writeNumber(localAmount.toLong())
         writeNumber(remoteAmount.toLong())

diff --git a/src/commonMain/kotlin/fr/acinq/lightning/transactions/Transactions.kt b/src/commonMain/kotlin/fr/acinq/lightning/transactions/Transactions.kt
@@ -159,7 +159,7 @@ object Transactions {
      *     - [[ClaimDelayedOutputPenaltyTx]] spends [[HtlcTimeoutTx]] using the revocation secret (published by local)
      *   - [[HtlcPenaltyTx]] spends competes with [[HtlcSuccessTx]] and [[HtlcTimeoutTx]] for the same outputs (published by local)
      */
-    // legacy swap-in. witness is 2 signatures (73 bytes) + redeem script (77 bytes))
+    // legacy swap-in. witness is 2 signatures (73 bytes) + redeem script (77 bytes)
     const val swapInputWeightLegacy = 392
     // musig2 swap-in. witness is a single Schnorr signature (64 bytes)
     const val swapInputWeight = 233