Provide a set of trusted txids for swap-in (#497)

Instead of a `isMigrationFromLegacyApp` boolean, we provide a set of txids that can be considered safe for swap-in even if they are not confirmed. This is not more complicated than relying on a boolean to implement zeroconf migration, and much more robust.
ACINQ · Jul 3, 2023 · 0ca3d2e · 0ca3d2e
1 parent 74f26ae
commit 0ca3d2e
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 21 deletions.
diff --git a/src/commonMain/kotlin/fr/acinq/lightning/blockchain/electrum/SwapInManager.kt b/src/commonMain/kotlin/fr/acinq/lightning/blockchain/electrum/SwapInManager.kt
@@ -1,5 +1,6 @@
 package fr.acinq.lightning.blockchain.electrum
 
+import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.bitcoin.OutPoint
 import fr.acinq.lightning.Lightning
 import fr.acinq.lightning.channel.LocalFundingStatus
@@ -12,7 +13,7 @@ import fr.acinq.lightning.utils.MDCLogger
 import fr.acinq.lightning.utils.sat
 
 internal sealed class SwapInCommand {
-    data class TrySwapIn(val currentBlockHeight: Int, val wallet: WalletState, val swapInConfirmations: Int, val isMigrationFromLegacyApp: Boolean) : SwapInCommand()
+    data class TrySwapIn(val currentBlockHeight: Int, val wallet: WalletState, val swapInConfirmations: Int, val trustedTxs: Set<ByteVector32>) : SwapInCommand()
     data class UnlockWalletInputs(val inputs: Set<OutPoint>) : SwapInCommand()
 }
 
@@ -32,12 +33,12 @@ class SwapInManager(private var reservedUtxos: Set<OutPoint>, private val logger
     internal fun process(cmd: SwapInCommand): RequestChannelOpen? = when (cmd) {
         is SwapInCommand.TrySwapIn -> {
             val availableWallet = cmd.wallet.withoutReservedUtxos(reservedUtxos).withConfirmations(cmd.currentBlockHeight, cmd.swapInConfirmations)
-            logger.info { "swap-in wallet balance (migration=${cmd.isMigrationFromLegacyApp}): deeplyConfirmed=${availableWallet.deeplyConfirmed.balance}, weaklyConfirmed=${availableWallet.weaklyConfirmed.balance}, unconfirmed=${availableWallet.unconfirmed.balance}" }
-            val utxos = when {
-                // When migrating from the legacy android app, we use all utxos, even unconfirmed ones.
-                cmd.isMigrationFromLegacyApp -> availableWallet.all
-                else -> availableWallet.deeplyConfirmed
-            }
+            logger.info { "swap-in wallet balance: deeplyConfirmed=${availableWallet.deeplyConfirmed.balance}, weaklyConfirmed=${availableWallet.weaklyConfirmed.balance}, unconfirmed=${availableWallet.unconfirmed.balance}" }
+            val utxos = buildSet {
+                // some utxos may be used for swap-in even if they are not confirmed, for example when migrating from the legacy phoenix android app
+                addAll(availableWallet.all.filter { cmd.trustedTxs.contains(it.outPoint.txid) })
+                addAll(availableWallet.deeplyConfirmed)
+            }.toList()
             if (utxos.balance > 0.sat) {
                 logger.info { "swap-in wallet: requesting channel using ${utxos.size} utxos with balance=${utxos.balance}" }
                 reservedUtxos = reservedUtxos.union(utxos.map { it.outPoint })

diff --git a/src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt b/src/commonMain/kotlin/fr/acinq/lightning/io/Peer.kt
@@ -86,7 +86,7 @@ data class PhoenixAndroidLegacyInfoEvent(val info: PhoenixAndroidLegacyInfo) : P
  * @param watcher Watches events from the Electrum client and publishes transactions and events.
  * @param db Wraps the various databases persisting the channels and payments data related to the Peer.
  * @param socketBuilder Builds the TCP socket used to connect to the Peer.
- * @param isMigrationFromLegacyApp true if we're migrating from the legacy phoenix android app.
+ * @param trustedSwapInTxs a set of txids that can be used for swap-in even if they are zeroconf (useful when migrating from the legacy phoenix android app).
  * @param initTlvStream Optional stream of TLV for the [Init] message we send to this Peer after connection. Empty by default.
  */
 @OptIn(ExperimentalStdlibApi::class)
@@ -97,7 +97,7 @@ class Peer(
     val db: Databases,
     socketBuilder: TcpSocket.Builder?,
     scope: CoroutineScope,
-    private val isMigrationFromLegacyApp: Boolean = false,
+    private val trustedSwapInTxs: Set<ByteVector32> = emptySet(),
     private val initTlvStream: TlvStream<InitTlv> = TlvStream.empty()
 ) : CoroutineScope by scope {
     companion object {
@@ -394,7 +394,7 @@ class Peer(
             .filter { it.consistent }
             .collect {
                 val currentBlockHeight = currentTipFlow.filterNotNull().first().first
-                swapInCommands.send(SwapInCommand.TrySwapIn(currentBlockHeight, it, walletParams.swapInConfirmations, isMigrationFromLegacyApp))
+                swapInCommands.send(SwapInCommand.TrySwapIn(currentBlockHeight, it, walletParams.swapInConfirmations, trustedSwapInTxs))
             }
     }
 

diff --git a/src/commonTest/kotlin/fr/acinq/lightning/blockchain/electrum/SwapInManagerTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/blockchain/electrum/SwapInManagerTestsCommon.kt
@@ -40,7 +40,7 @@ class SwapInManagerTestsCommon : LightningTestSuite() {
             )
             WalletState(mapOf(dummyAddress to unspent), parentTxs.associateBy { it.txid })
         }
-        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 3, isMigrationFromLegacyApp = false)
+        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 3, trustedTxs = emptySet())
         mgr.process(cmd).also { result ->
             assertNotNull(result)
             assertEquals(result.walletInputs.map { it.amount }.toSet(), setOf(50_000.sat, 75_000.sat))
@@ -61,27 +61,27 @@ class SwapInManagerTestsCommon : LightningTestSuite() {
             )
             WalletState(mapOf(dummyAddress to unspent), parentTxs.associateBy { it.txid })
         }
-        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 101, wallet = wallet, swapInConfirmations = 3, isMigrationFromLegacyApp = false)
+        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 101, wallet = wallet, swapInConfirmations = 3, trustedTxs = emptySet())
         mgr.process(cmd).also { assertNull(it) }
     }
 
     @Test
     fun `swap funds -- allow unconfirmed in migration`() {
         val mgr = SwapInManager(listOf(), logger)
+        val parentTxs = listOf(
+            Transaction(2, listOf(TxIn(OutPoint(randomBytes32(), 1), 0)), listOf(TxOut(75_000.sat, dummyScript)), 0),
+            Transaction(2, listOf(TxIn(OutPoint(randomBytes32(), 2), 0)), listOf(TxOut(50_000.sat, dummyScript)), 0),
+            Transaction(2, listOf(TxIn(OutPoint(randomBytes32(), 0), 0)), listOf(TxOut(25_000.sat, dummyScript)), 0)
+        )
         val wallet = run {
-            val parentTxs = listOf(
-                Transaction(2, listOf(TxIn(OutPoint(randomBytes32(), 1), 0)), listOf(TxOut(75_000.sat, dummyScript)), 0),
-                Transaction(2, listOf(TxIn(OutPoint(randomBytes32(), 2), 0)), listOf(TxOut(50_000.sat, dummyScript)), 0),
-                Transaction(2, listOf(TxIn(OutPoint(randomBytes32(), 0), 0)), listOf(TxOut(25_000.sat, dummyScript)), 0)
-            )
             val unspent = listOf(
                 UnspentItem(parentTxs[0].txid, 0, 75_000, 100), // deeply confirmed
                 UnspentItem(parentTxs[1].txid, 0, 50_000, 150), // recently confirmed
                 UnspentItem(parentTxs[2].txid, 0, 25_000, 0), // unconfirmed
             )
             WalletState(mapOf(dummyAddress to unspent), parentTxs.associateBy { it.txid })
         }
-        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, isMigrationFromLegacyApp = true)
+        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, trustedTxs = parentTxs.map { it.txid }.toSet())
         mgr.process(cmd).also { result ->
             assertNotNull(result)
             assertEquals(result.walletInputs.map { it.amount }.toSet(), setOf(25_000.sat, 50_000.sat, 75_000.sat))
@@ -96,7 +96,7 @@ class SwapInManagerTestsCommon : LightningTestSuite() {
             val unspent = UnspentItem(parentTx.txid, 0, 75_000, 100)
             WalletState(mapOf(dummyAddress to listOf(unspent)), mapOf(parentTx.txid to parentTx))
         }
-        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, isMigrationFromLegacyApp = false)
+        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, trustedTxs = emptySet())
         mgr.process(cmd).also { assertNotNull(it) }
 
         // We cannot reuse the same inputs.
@@ -119,7 +119,7 @@ class SwapInManagerTestsCommon : LightningTestSuite() {
             WalletState(mapOf(dummyAddress to unspent), parentTxs.associateBy { it.txid })
         }
         val mgr = SwapInManager(listOf(waitForFundingSigned.state), logger)
-        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, isMigrationFromLegacyApp = false)
+        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, trustedTxs = emptySet())
         mgr.process(cmd).also { assertNull(it) }
 
         // The pending channel is aborted: we can reuse those inputs.
@@ -140,7 +140,7 @@ class SwapInManagerTestsCommon : LightningTestSuite() {
             WalletState(mapOf(dummyAddress to unspent), parentTxs.associateBy { it.txid })
         }
         val mgr = SwapInManager(listOf(alice1.state), logger)
-        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, isMigrationFromLegacyApp = false)
+        val cmd = SwapInCommand.TrySwapIn(currentBlockHeight = 150, wallet = wallet, swapInConfirmations = 5, trustedTxs = emptySet())
         mgr.process(cmd).also { assertNull(it) }
 
         // The channel is aborted: we can reuse those inputs.