-
Notifications
You must be signed in to change notification settings - Fork 268
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
Use correct cost comparison when evaluating candidate channels #1090
Conversation
…ng is within th range htlcMinimumMsat/htlcMaximumMsat of the candidate channel
Codecov Report
@@ Coverage Diff @@
## master #1090 +/- ##
==========================================
+ Coverage 82.64% 82.79% +0.14%
==========================================
Files 101 101
Lines 7652 7653 +1
Branches 312 312
==========================================
+ Hits 6324 6336 +12
+ Misses 1328 1317 -11
|
@@ -230,8 +230,8 @@ object Graph { | |||
val newMinimumKnownWeight = edgeWeight(edge, currentWeight, initialWeight.length == 0 && neighbor == sourceNode, currentBlockHeight, wr) | |||
|
|||
// test for ignored edges | |||
if (edge.update.htlcMaximumMsat.forall(newMinimumKnownWeight.cost + amountMsat <= _) && | |||
newMinimumKnownWeight.cost + amountMsat >= edge.update.htlcMinimumMsat && | |||
if (edge.update.htlcMaximumMsat.forall(newMinimumKnownWeight.cost <= _) && |
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.
If amountMsat
is not used here then it's not used at all in dijkstraShortestPath
and should be removed from the parameter list. We then need to make sure that
val spurPath = dijkstraShortestPath(graph, spurEdge.desc.a, targetNode, amountMsat, ignoredEdges ++ edgesToIgnore.toSet ++ returningEdges.toSet, extraEdges, rootPathWeight, boundaries, currentBlockHeight, wr) |
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.
With this change dijkstraShortestPath
will use only the cost found in the initialWeight
parameter which is RichWeight(amountMsat, 0, 0, 0)
for the initial step and when calculating the 'spurPath' for an alternative path this is the result of pathWeight
so it's the cost to go from the starting node to this intermediate node (from here the algorithm will search for a different path). I will remove amountMsat
from the parameter list.
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.
f62f472 removes the unused parameter and also extracts the initialWeight
parameter in the first invocation of dijkstraShortestPath
, for more readability.
…eight in the first step of 'yenKshortestPaths'
This PR fixes bug ACINQ/eclair-mobile#207 which was caused by an incorrect channel cost evaluation against its
htlcMaximumMsat
. The fix is to consider the correct channel cost.