-
Notifications
You must be signed in to change notification settings - Fork 267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split MPP by maximizing expected delivered amount #2792
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,6 @@ import com.softwaremill.quicklens.ModifyPimp | |
import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey | ||
import fr.acinq.eclair.Logs.LogCategory | ||
import fr.acinq.eclair._ | ||
import fr.acinq.eclair.message.SendingMessage | ||
import fr.acinq.eclair.payment.send._ | ||
import fr.acinq.eclair.router.Graph.GraphStructure.DirectedGraph.graphEdgeToHop | ||
import fr.acinq.eclair.router.Graph.GraphStructure.{DirectedGraph, GraphEdge} | ||
|
@@ -420,7 +419,7 @@ object RouteCalculation { | |
// We want to ensure that the set of routes we find have enough capacity to allow sending the total amount, | ||
// without excluding routes with small capacity when the total amount is small. | ||
val minPartAmount = routeParams.mpp.minPartAmount.max(amount / numRoutes).min(amount) | ||
routeParams.copy(mpp = MultiPartParams(minPartAmount, numRoutes)) | ||
routeParams.copy(mpp = MultiPartParams(minPartAmount, numRoutes, routeParams.mpp.splittingStrategy)) | ||
} | ||
findRouteInternal(g, localNodeId, targetNodeId, routeParams1.mpp.minPartAmount, maxFee, routeParams1.mpp.maxParts, extraEdges, ignoredEdges, ignoredVertices, routeParams1, currentBlockHeight) match { | ||
case Right(routes) => | ||
|
@@ -446,16 +445,16 @@ object RouteCalculation { | |
// this route doesn't have enough capacity left: we remove it and continue. | ||
split(amount, paths, usedCapacity, routeParams, selectedRoutes) | ||
} else { | ||
val route = if (routeParams.randomize) { | ||
// randomly choose the amount to be between 20% and 100% of the available capacity. | ||
val randomizedAmount = candidate.amount * ((20d + Random.nextInt(81)) / 100) | ||
if (randomizedAmount < routeParams.mpp.minPartAmount) { | ||
candidate.copy(amount = routeParams.mpp.minPartAmount.min(amount)) | ||
} else { | ||
candidate.copy(amount = randomizedAmount.min(amount)) | ||
} | ||
} else { | ||
candidate.copy(amount = candidate.amount.min(amount)) | ||
val route = routeParams.mpp.splittingStrategy match { | ||
case MultiPartParams.Randomize => | ||
// randomly choose the amount to be between 20% and 100% of the available capacity. | ||
val randomizedAmount = candidate.amount * ((20d + Random.nextInt(81)) / 100) | ||
candidate.copy(amount = randomizedAmount.max(routeParams.mpp.minPartAmount).min(amount)) | ||
case MultiPartParams.MaxExpectedAmount => | ||
val bestAmount = optimizeExpectedValue(current.path, candidate.amount, usedCapacity) | ||
candidate.copy(amount = bestAmount.max(routeParams.mpp.minPartAmount).min(amount)) | ||
case MultiPartParams.FullCapacity => | ||
candidate.copy(amount = candidate.amount.min(amount)) | ||
} | ||
updateUsedCapacity(route, usedCapacity) | ||
// NB: we re-enqueue the current path, it may still have capacity for a second HTLC. | ||
|
@@ -464,6 +463,26 @@ object RouteCalculation { | |
} | ||
} | ||
|
||
private def optimizeExpectedValue(route: Seq[GraphEdge], capacity: MilliSatoshi, usedCapacity: mutable.Map[ShortChannelId, MilliSatoshi]): MilliSatoshi = { | ||
// We search the maximum value of a polynomial between its two smallest roots (0 and the minimum channel capacity on the path). | ||
// We use binary search to find where the derivative changes sign. | ||
var low = 1L | ||
var high = capacity.toLong | ||
while (high - low > 1L) { | ||
val mid = (high + low) / 2 | ||
val d = route.drop(1).foldLeft(1.0 / mid) { case (x, edge) => | ||
val availableCapacity = edge.capacity - usedCapacity.getOrElse(edge.desc.shortChannelId, 0 msat) | ||
x - 1.0 / (availableCapacity.toLong - mid) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For a uniform distribution between For two channels I computed that setting the derrivative of the expected value to 0 is equivalent to
assuming this generalizes to several channels (because of the product rule of derrivatives for the probability of the path this very much looks like this (I think I computed that before)). Thus I think it is indeed reasonable to look that one only has to check the sign of the term There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we consider the polynomial There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for laying out the proof. It's funny as I remember trying to find a formular during last summer to solve this analytically and I recognizedc the same structure. But I couldn't non numerically solve for x which is why I stopped there. However I still wonder how does the computation change if for example the channel |
||
} | ||
if (d > 0.0) { | ||
low = mid | ||
} else { | ||
high = mid | ||
} | ||
} | ||
MilliSatoshi(high) | ||
} | ||
|
||
/** Compute the maximum amount that we can send through the given route. */ | ||
private def computeRouteMaxAmount(route: Seq[GraphEdge], usedCapacity: mutable.Map[ShortChannelId, MilliSatoshi]): Route = { | ||
val firstHopMaxAmount = route.head.maxHtlcAmount(usedCapacity.getOrElse(route.head.desc.shortChannelId, 0 msat)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that using "randomize" by default wouldn't change the current behavior as the default sets "randomize-route-selection = true", however the code flips this "randomize" flag in several places. I need to think more about it to not change the default behavior, any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, when the default configuration fails, we fall back to a retry using the
randomize
flag set totrue
. IIRC we only override this to set it totrue
, is that correct or am I missing some cases? As long as the first attempts use the configured values, I think it's ok if eclair has a fallback on failure that uses the algorithm we think as being the best option.