Fix potential loop in path-finding (#2111)

Use the amount WITHOUT the fees when computing the success probability of routing through an edge.
This fixes a problem where the computed probability could be negative, leading to a shorter path after adding an edge.

eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala

@@ -90,6 +90,8 @@ object Graph {
 
   case class InfiniteLoop(path: Seq[GraphEdge]) extends Exception
 
+  case class NegativeProbability(edge: GraphEdge, weight: RichWeight, heuristicsConstants: HeuristicsConstants) extends Exception
+
   /**
    * Yen's algorithm to find the k-shortest (loop-less) paths in a graph, uses dijkstra as search algo. Is guaranteed to
    * terminate finding at most @pathsToFind paths sorted by cost (the cheapest is in position 0).
@@ -325,7 +327,10 @@ object Graph {
         val riskCost = totalAmount.toLong * totalCltv.toInt * heuristicsConstants.lockedFundsRisk
         // If the edge was added by the invoice, it is assumed that it can route the payment.
         // If we know the balance of the channel, then we will check separately that it can relay the payment.
-        val successProbability = if (edge.update.chainHash == ByteVector32.Zeroes || edge.balance_opt.nonEmpty) 1.0 else 1.0 - totalAmount.toLong.toDouble / edge.capacity.toMilliSatoshi.toLong.toDouble
+        val successProbability = if (edge.update.chainHash == ByteVector32.Zeroes || edge.balance_opt.nonEmpty) 1.0 else 1.0 - prev.amount.toLong.toDouble / edge.capacity.toMilliSatoshi.toLong.toDouble
+        if (successProbability < 0) {
+          throw NegativeProbability(edge, prev, heuristicsConstants)
+        }
         val totalSuccessProbability = prev.successProbability * successProbability
         val failureCost = nodeFee(heuristicsConstants.failureCost.feeBase, heuristicsConstants.failureCost.feeProportionalMillionths, totalAmount)
         val weight = totalFees.toLong + totalHopsCost.toLong + riskCost + failureCost.toLong / totalSuccessProbability

eclair-core/src/main/scala/fr/acinq/eclair/router/RouteCalculation.scala

@@ -26,7 +26,7 @@ import fr.acinq.eclair._
 import fr.acinq.eclair.payment.PaymentRequest.ExtraHop
 import fr.acinq.eclair.router.Graph.GraphStructure.DirectedGraph.graphEdgeToHop
 import fr.acinq.eclair.router.Graph.GraphStructure.{DirectedGraph, GraphEdge}
-import fr.acinq.eclair.router.Graph.{InfiniteLoop, RichWeight, RoutingHeuristics}
+import fr.acinq.eclair.router.Graph.{InfiniteLoop, NegativeProbability, RichWeight, RoutingHeuristics}
 import fr.acinq.eclair.router.Monitoring.{Metrics, Tags}
 import fr.acinq.eclair.router.Router._
 import fr.acinq.eclair.wire.protocol.ChannelUpdate
@@ -130,10 +130,14 @@ object RouteCalculation {
             Metrics.RouteResults.withTags(tags).record(routes.length)
             routes.foreach(route => Metrics.RouteLength.withTags(tags).record(route.length))
             ctx.sender() ! RouteResponse(routes)
-          case Failure(InfiniteLoop(loop)) =>
-            log.error(s"found infinite loop ${loop.map(edge => edge.desc).mkString(" -> ")}")
+          case Failure(failure: InfiniteLoop) =>
+            log.error(s"found infinite loop ${failure.path.map(edge => edge.desc).mkString(" -> ")}")
             Metrics.FindRouteErrors.withTags(tags.withTag(Tags.Error, "InfiniteLoop")).increment()
-            ctx.sender() ! Status.Failure(InfiniteLoop(loop))
+            ctx.sender() ! Status.Failure(failure)
+          case Failure(failure: NegativeProbability) =>
+            log.error(s"computed negative probability $failure")
+            Metrics.FindRouteErrors.withTags(tags.withTag(Tags.Error, "NegativeProbability")).increment()
+            ctx.sender() ! Status.Failure(failure)
           case Failure(t) =>
             val failure = if (isNeighborBalanceTooLow(d.graph, r)) BalanceTooLow else t
             Metrics.FindRouteErrors.withTags(tags.withTag(Tags.Error, failure.getClass.getSimpleName)).increment()

eclair-core/src/test/scala/fr/acinq/eclair/router/GraphSpec.scala

@@ -17,17 +17,16 @@
 package fr.acinq.eclair.router
 
 import fr.acinq.bitcoin.Crypto.PublicKey
-import fr.acinq.bitcoin.{ByteVector32, SatoshiLong}
+import fr.acinq.bitcoin.SatoshiLong
+import fr.acinq.eclair.payment.relay.Relayer.RelayFees
 import fr.acinq.eclair.router.Graph.GraphStructure.{DirectedGraph, GraphEdge}
-import fr.acinq.eclair.router.Graph.RoutingHeuristics
+import fr.acinq.eclair.router.Graph.{HeuristicsConstants, yenKshortestPaths}
 import fr.acinq.eclair.router.RouteCalculationSpec._
-import fr.acinq.eclair.router.Router.{ChannelDesc, PublicChannel}
+import fr.acinq.eclair.router.Router.ChannelDesc
 import fr.acinq.eclair.{MilliSatoshiLong, ShortChannelId}
 import org.scalatest.funsuite.AnyFunSuite
 import scodec.bits._
 
-import scala.collection.immutable.SortedMap
-
 class GraphSpec extends AnyFunSuite {
 
   val (a, b, c, d, e, f, g) = (
@@ -239,4 +238,27 @@ class GraphSpec extends AnyFunSuite {
 
   def edgeFromNodes(shortChannelId: Long, a: PublicKey, b: PublicKey): GraphEdge = makeEdge(shortChannelId, a, b, 0 msat, 0)
 
+  test("amount with fees larger than channel capacity") {
+    /*
+    The channel C -> D is just large enough for the payment to go through but when adding the channel fee it becomes too big.
+    A --> B --> C <-> D
+                 \   /
+                  \ /
+                   E
+    This tests that the success probability for the channel C -> D is computed properly and is positive.
+    */
+    val edgeAB = makeEdge(1L, a, b, 10001 msat, 0, capacity = 200000 sat)
+    val edgeBC = makeEdge(2L, b, c, 10000 msat, 0, capacity = 200000 sat)
+    val edgeCD = makeEdge(3L, c, d, 20001 msat, 0, capacity = 100011 sat)
+    val edgeDC = makeEdge(4L, d, c, 1 msat, 0, capacity = 300000 sat)
+    val edgeCE = makeEdge(5L, c, e, 10 msat, 0, capacity = 200000 sat)
+    val edgeDE = makeEdge(6L, d, e, 9 msat, 0, capacity = 200000 sat)
+    val graph = DirectedGraph(Seq(edgeAB, edgeBC, edgeCD, edgeDC, edgeCE, edgeDE))
+
+    val path :: Nil = yenKshortestPaths(graph, a, e, 100000000 msat,
+      Set.empty, Set.empty, Set.empty, 1,
+      Right(HeuristicsConstants(1.0E-8, RelayFees(2000 msat, 500), RelayFees(50 msat, 20))),
+      714930, _ => true, includeLocalChannelCost = true)
+    assert(path.path == Seq(edgeAB, edgeBC, edgeCE))
+  }
 }