From 27a68a489886048b85935f5abf55c06fbec4b2fa Mon Sep 17 00:00:00 2001
From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
Date: Wed, 11 Dec 2019 13:53:53 +0100
Subject: [PATCH] MPP: don't retry if failure comes from final recipient
 (#1246)

---
 .../payment/send/MultiPartPaymentLifecycle.scala    |  7 ++-----
 .../acinq/eclair/payment/MultiPartHandlerSpec.scala |  7 +++++--
 .../payment/MultiPartPaymentLifecycleSpec.scala     | 13 +++++++++++++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala
index 0eee188e4..0082a7a05 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala
@@ -249,11 +249,8 @@ class MultiPartPaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig,
   }
 
   def handleChildFailure(pf: PaymentFailed, d: PaymentProgress): Option[PaymentAborted] = {
-    val paymentTimedOut = pf.failures.exists {
-      case f: RemoteFailure => f.e.failureMessage == PaymentTimeout
-      case _ => false
-    }
-    if (paymentTimedOut) {
+    val isFromFinalRecipient = pf.failures.collectFirst { case f: RemoteFailure if f.e.originNode == d.request.targetNodeId => true }.isDefined
+    if (isFromFinalRecipient) {
       Some(PaymentAborted(d.sender, d.request, d.failures ++ pf.failures, d.pending.keySet - pf.id))
     } else if (d.remainingAttempts == 0) {
       val failure = LocalFailure(RetryExhausted)
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartHandlerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartHandlerSpec.scala
index 8f7aac610..9ac7a3a34 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartHandlerSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartHandlerSpec.scala
@@ -334,8 +334,11 @@ class MultiPartHandlerSpec extends TestKit(ActorSystem("test")) with fixture.Fun
     f.sender.send(handler, GetPendingPayments)
     assert(f.sender.expectMsgType[PendingPayments].paymentHashes.nonEmpty)
 
-    f.commandBuffer.expectMsg(CommandBuffer.CommandSend(ByteVector32.One, 0, CMD_FAIL_HTLC(0, Right(PaymentTimeout), commit = true)))
-    f.commandBuffer.expectMsg(CommandBuffer.CommandSend(ByteVector32.One, 1, CMD_FAIL_HTLC(1, Right(PaymentTimeout), commit = true)))
+    val commands = f.commandBuffer.expectMsgType[CommandBuffer.CommandSend] :: f.commandBuffer.expectMsgType[CommandBuffer.CommandSend] :: Nil
+    assert(commands.toSet === Set(
+      CommandBuffer.CommandSend(ByteVector32.One, 0, CMD_FAIL_HTLC(0, Right(PaymentTimeout), commit = true)),
+      CommandBuffer.CommandSend(ByteVector32.One, 1, CMD_FAIL_HTLC(1, Right(PaymentTimeout), commit = true))
+    ))
     awaitCond({
       f.sender.send(handler, GetPendingPayments)
       f.sender.expectMsgType[PendingPayments].paymentHashes.isEmpty
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala
index 34c7c06ac..65beb0f76 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala
@@ -403,6 +403,19 @@ class MultiPartPaymentLifecycleSpec extends TestKit(ActorSystem("test")) with fi
     awaitCond(payFsm.stateName === PAYMENT_ABORTED)
   }
 
+  test("failure received from final recipient") { f =>
+    import f._
+    val payment = SendMultiPartPayment(paymentHash, randomBytes32, e, 3000 * 1000 msat, expiry, 5)
+    initPayment(f, payment, emptyStats.copy(capacity = Stats(Seq(1000), d => Satoshi(d.toLong))), localChannels())
+    waitUntilAmountSent(f, payment.totalAmount)
+    val (childId1, _) = payFsm.stateData.asInstanceOf[PaymentProgress].pending.head
+
+    // If we receive a failure from the final node, we directly abort the payment instead of retrying.
+    childPayFsm.send(payFsm, PaymentFailed(childId1, paymentHash, RemoteFailure(Nil, Sphinx.DecryptedFailurePacket(e, IncorrectOrUnknownPaymentDetails(3000 * 1000 msat, 42))) :: Nil))
+    relayer.expectNoMsg(50 millis)
+    awaitCond(payFsm.stateName === PAYMENT_ABORTED)
+  }
+
   test("fail after too many attempts") { f =>
     import f._
     val payment = SendMultiPartPayment(paymentHash, randomBytes32, e, 3000 * 1000 msat, expiry, 2)