-
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
Add firstHopChannel/lastHopChannel parameters to findroute* API calls #1950
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1950 +/- ##
==========================================
+ Coverage 87.63% 87.66% +0.02%
==========================================
Files 159 159
Lines 12438 12448 +10
Branches 519 521 +2
==========================================
+ Hits 10900 10912 +12
+ Misses 1538 1536 -2
|
Completely unrelated to this PR, but while we have you, can you please fix this regression: #1947 |
firstHopChannelId: Option[ShortChannelId], | ||
lastHopChannelId: Option[ShortChannelId]): Seq[GraphEdge] = { |
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.
Please don't touch dijkstraShortestPath
, there is no need. Instead search for a path from the end of firstHopChannelId
to the start of lastHopChannelId
and add our nodeId
to ignoredVertices
.
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.
It turns out there's no need in changing Router
at all. All above can be done over the API parameters (see #1969). I'm closing this PR.
eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala
Outdated
Show resolved
Hide resolved
@@ -122,9 +122,9 @@ trait Eclair { | |||
|
|||
def sendOnChain(address: String, amount: Satoshi, confirmationTarget: Long): Future[ByteVector32] | |||
|
|||
def findRoute(targetNodeId: PublicKey, amount: MilliSatoshi, pathFindingExperimentName_opt: Option[String], assistedRoutes: Seq[Seq[PaymentRequest.ExtraHop]] = Seq.empty)(implicit timeout: Timeout): Future[RouteResponse] | |||
def findRoute(targetNodeId: PublicKey, amount: MilliSatoshi, pathFindingExperimentName_opt: Option[String], assistedRoutes: Seq[Seq[PaymentRequest.ExtraHop]] = Seq.empty, firstHopChannelId_opt: Option[ShortChannelId], lastHopChannelId_opt: Option[ShortChannelId])(implicit timeout: Timeout): Future[RouteResponse] |
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.
Do you really need to change both findRoute
and findRouteBetween
?
It seems to me that findRouteBetween
is sufficient and is the API that better matches rebalancing needs, isn't it?
We just merged a change to continually edit release notes as PRs are accepted (#1951). |
does this work now, with the new APIs? |
The rationale behind this PR is repurposing https://github.com/C-Otto/rebalance-lnd for Eclair to allow the Eclair’s users to automatically re-balance their channels.
This PR allows the users to specify which channels exactly they want to re-balance by providing their short channel ID's using
firstHopChannel
and/orlastHopChannel
parameters.This is one PR in the upcoming series of PR's changing the router. Since the router is a one of the most critical parts, I’d like to keep them rather small.