Do not update amount in db for incoming amount of pay-to-open origin (#242)

Setting the payment's amount in the database is the responsibility of the main 
payment handler.

Co-authored-by: Pierre-Marie Padiou <[email protected]>

Author: dpad85

src/commonMain/kotlin/fr/acinq/eclair/db/InMemoryPaymentsDb.kt

@@ -8,6 +8,7 @@ import fr.acinq.eclair.payment.FinalFailure
 import fr.acinq.eclair.payment.OutgoingPaymentFailure
 import fr.acinq.eclair.utils.Either
 import fr.acinq.eclair.utils.UUID
+import fr.acinq.eclair.utils.msat
 import fr.acinq.eclair.utils.toByteVector32
 import fr.acinq.eclair.wire.FailureMessage
 
@@ -27,7 +28,10 @@ class InMemoryPaymentsDb : PaymentsDb {
     override suspend fun receivePayment(paymentHash: ByteVector32, amount: MilliSatoshi, receivedWith: IncomingPayment.ReceivedWith, receivedAt: Long) {
         when (val payment = incoming[paymentHash]) {
             null -> Unit // no-op
-            else -> incoming[paymentHash] = payment.copy(received = IncomingPayment.Received(amount, receivedWith, receivedAt))
+            else -> incoming[paymentHash] = run {
+                val alreadyReceived = payment.received?.amount ?: 0.msat
+                payment.copy(received = IncomingPayment.Received(amount + alreadyReceived, receivedWith, receivedAt))
+            }
         }
     }
 

src/commonMain/kotlin/fr/acinq/eclair/io/Peer.kt

@@ -370,38 +370,8 @@ class Peer(
                     action.htlcs.forEach { db.channels.addHtlcInfo(actualChannelId, it.commitmentNumber, it.paymentHash, it.cltvExpiry) }
                 }
                 action is ChannelAction.Storage.StoreIncomingAmount -> {
-                    logger.info { "storing incoming amount=${action.amount} with origin=${action.origin}" }
-                    when (action.origin) {
-                        null -> {
-                            logger.warning { "n:$remoteNodeId c:$actualChannelId incoming amount with empty origin, store minimal information" }
-                            val fakePreimage = actualChannelId.sha256()
-                            db.payments.addAndReceivePayment(
-                                preimage = fakePreimage,
-                                origin = IncomingPayment.Origin.SwapIn(address = ""),
-                                amount = action.amount,
-                                receivedWith = IncomingPayment.ReceivedWith.NewChannel(fees = 0.msat, channelId = actualChannelId)
-                            )
-                        }
-                        is ChannelOrigin.PayToOpenOrigin -> {
-                            if (db.payments.getIncomingPayment(action.origin.paymentHash) != null) {
-                                db.payments.receivePayment(paymentHash = action.origin.paymentHash, amount = action.amount, receivedWith = IncomingPayment.ReceivedWith.NewChannel(
-                                    fees = action.origin.fee.toMilliSatoshi(),
-                                    channelId = actualChannelId
-                                ))
-                            } else {
-                                logger.warning { "n:$remoteNodeId c:$actualChannelId ignored pay-to-open storage, no payments in db for hash=${action.origin.paymentHash}" }
-                            }
-                        }
-                        is ChannelOrigin.SwapInOrigin -> {
-                            val fakePreimage = actualChannelId.sha256()
-                            db.payments.addAndReceivePayment(
-                                preimage = fakePreimage,
-                                origin = IncomingPayment.Origin.SwapIn(address = action.origin.bitcoinAddress),
-                                amount = action.amount,
-                                receivedWith = IncomingPayment.ReceivedWith.NewChannel(fees = action.origin.fee.toMilliSatoshi(), channelId = actualChannelId)
-                            )
-                        }
-                    }
+                    logger.info { "n:$remoteNodeId c:$channelId storing incoming amount=${action.amount} with origin=${action.origin}" }
+                    incomingPaymentHandler.process(actualChannelId, action)
                 }
                 action is ChannelAction.Storage.GetHtlcInfos -> {
                     val htlcInfos = db.channels.listHtlcInfos(actualChannelId, action.commitmentNumber).map { ChannelAction.Storage.HtlcInfo(actualChannelId, action.commitmentNumber, it.first, it.second) }

src/commonMain/kotlin/fr/acinq/eclair/payment/IncomingPaymentHandler.kt

@@ -93,6 +93,56 @@ class IncomingPaymentHandler(val nodeParams: NodeParams, val walletParams: Walle
         return pr
     }
 
+    /**
+     * Save the "received-with" details of an incoming amount.
+     *
+     * - for a pay-to-open origin, we only save the id of the channel that was created for this payment.
+     * - for a swap-in origin, a new incoming payment must be created. We use the channel id to generate the payment's preimage.
+     * - for unknown origin, the amount is handled as a swap-in coming from an unknown address.
+     */
+    suspend fun process(channelId: ByteVector32, action: ChannelAction.Storage.StoreIncomingAmount) {
+        when (action.origin) {
+            null -> {
+                // TODO: hacky, needs clean-up
+                logger.warning { "incoming amount with empty origin, we store only minimal information" }
+                val fakePreimage = channelId.sha256()
+                db.addAndReceivePayment(
+                    preimage = fakePreimage,
+                    origin = IncomingPayment.Origin.SwapIn(address = ""),
+                    amount = action.amount,
+                    receivedWith = IncomingPayment.ReceivedWith.NewChannel(fees = 0.msat, channelId = channelId)
+                )
+            }
+            is ChannelOrigin.PayToOpenOrigin -> {
+                if (db.getIncomingPayment(action.origin.paymentHash) != null) {
+                    db.receivePayment(
+                        paymentHash = action.origin.paymentHash,
+                        amount = 0.msat, // do not update the amount, it's already set by the main payment handler.
+                        receivedWith = IncomingPayment.ReceivedWith.NewChannel(
+                            fees = action.origin.fee.toMilliSatoshi(),
+                            channelId = channelId
+                        )
+                    )
+                } else {
+                    logger.warning { "ignored pay-to-open origin action, there are no payments in db for payment_hash=${action.origin.paymentHash}" }
+                }
+            }
+            is ChannelOrigin.SwapInOrigin -> {
+                // swap-ins are push payments made with an on-chain tx, there is no related preimage so we make up one so it fits in our model
+                val fakePreimage = channelId.sha256()
+                db.addAndReceivePayment(
+                    preimage = fakePreimage,
+                    origin = IncomingPayment.Origin.SwapIn(address = action.origin.bitcoinAddress),
+                    amount = action.amount,
+                    receivedWith = IncomingPayment.ReceivedWith.NewChannel(
+                        fees = action.origin.fee.toMilliSatoshi(),
+                        channelId = channelId
+                    )
+                )
+            }
+        }
+    }
+
     /**
      * Process an incoming htlc.
      * Before calling this, the htlc must be committed and ack-ed by both sides.

src/commonTest/kotlin/fr/acinq/eclair/payment/IncomingPaymentHandlerTestsCommon.kt

@@ -17,10 +17,7 @@ import fr.acinq.eclair.tests.utils.EclairTestSuite
 import fr.acinq.eclair.tests.utils.runSuspendTest
 import fr.acinq.eclair.utils.*
 import fr.acinq.eclair.wire.*
-import kotlin.test.Test
-import kotlin.test.assertEquals
-import kotlin.test.assertNull
-import kotlin.test.assertTrue
+import kotlin.test.*
 
 class IncomingPaymentHandlerTestsCommon : EclairTestSuite() {
 
@@ -314,6 +311,54 @@ class IncomingPaymentHandlerTestsCommon : EclairTestSuite() {
         assertEquals(setOf(expected), result.actions.toSet())
     }
 
+    @Test
+    fun `process incoming amount with unknown origin`() = runSuspendTest {
+        val channelId = randomBytes32()
+        val amountOrigin = ChannelAction.Storage.StoreIncomingAmount(amount = 15_000_000.msat, origin = null)
+        val handler = IncomingPaymentHandler(TestConstants.Bob.nodeParams, TestConstants.Bob.walletParams, InMemoryPaymentsDb())
+        handler.process(channelId, amountOrigin)
+        val dbPayment = handler.db.getIncomingPayment(channelId.sha256().sha256())
+        assertNotNull(dbPayment)
+        assertTrue { dbPayment.origin is IncomingPayment.Origin.SwapIn }
+        assertEquals(IncomingPayment.ReceivedWith.NewChannel(fees = 0.msat, channelId = channelId), dbPayment.received?.receivedWith)
+        assertEquals(15_000_000.msat, dbPayment.received?.amount)
+    }
+
+    @Test
+    fun `process incoming amount with pay-to-open origin`() = runSuspendTest {
+        val preimage = randomBytes32()
+        val channelId = randomBytes32()
+        val amountOrigin = ChannelAction.Storage.StoreIncomingAmount(
+            amount = 15_000_000.msat,
+            origin = ChannelOrigin.PayToOpenOrigin(paymentHash = preimage.sha256(), fee = 1000.sat))
+        val handler = IncomingPaymentHandler(TestConstants.Bob.nodeParams, TestConstants.Bob.walletParams, InMemoryPaymentsDb())
+        // simulate payment processed as a pay-to-open
+        handler.db.addIncomingPayment(preimage, IncomingPayment.Origin.KeySend)
+        handler.db.receivePayment(preimage.sha256(), 20_000_000.msat, receivedWith = IncomingPayment.ReceivedWith.NewChannel(0.msat, null))
+        // process the amount origin which must reconcile with the existing line in the database
+        handler.process(channelId, amountOrigin)
+        val dbPayment = handler.db.getIncomingPayment(preimage.sha256())
+        assertNotNull(dbPayment)
+        assertTrue { dbPayment.origin is IncomingPayment.Origin.KeySend }
+        assertEquals(IncomingPayment.ReceivedWith.NewChannel(fees = 1_000_000.msat, channelId = channelId), dbPayment.received?.receivedWith)
+        assertEquals(20_000_000.msat, dbPayment.received?.amount)
+    }
+
+    @Test
+    fun `process incoming amount with swap-in origin`() = runSuspendTest {
+        val channelId = randomBytes32()
+        val amountOrigin = ChannelAction.Storage.StoreIncomingAmount(
+            amount = 33_000_000.msat,
+            origin = ChannelOrigin.SwapInOrigin("tb1q97tpc0y4rvdnu9wm7nu354lmmzdm8du228u3g4", 1_200.sat))
+        val handler = IncomingPaymentHandler(TestConstants.Bob.nodeParams, TestConstants.Bob.walletParams, InMemoryPaymentsDb())
+        handler.process(channelId, amountOrigin)
+        val dbPayment = handler.db.getIncomingPayment(channelId.sha256().sha256())
+        assertNotNull(dbPayment)
+        assertTrue { dbPayment.origin is IncomingPayment.Origin.SwapIn }
+        assertEquals(IncomingPayment.ReceivedWith.NewChannel(fees = 1_200_000.msat, channelId = channelId), dbPayment.received?.receivedWith)
+        assertEquals(33_000_000.msat, dbPayment.received?.amount)
+    }
+
     @Test
     fun `receive multipart payment with multiple HTLCs via same channel`() = runSuspendTest {
         val channelId = randomBytes32()