-
Notifications
You must be signed in to change notification settings - Fork 265
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
Trampoline payments use per-channel fee and cltv #1853
Conversation
16118b6
to
42dd0db
Compare
Codecov Report
@@ Coverage Diff @@
## master #1853 +/- ##
==========================================
+ Coverage 88.94% 88.99% +0.04%
==========================================
Files 150 150
Lines 11239 11241 +2
Branches 447 460 +13
==========================================
+ Hits 9997 10004 +7
+ Misses 1242 1237 -5
|
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.
Thanks, this is indeed simple enough to include asap ;)
@@ -120,12 +120,13 @@ object NodeRelay { | |||
|
|||
/** Compute route params that honor our fee and cltv requirements. */ | |||
def computeRouteParams(nodeParams: NodeParams, amountIn: MilliSatoshi, expiryIn: CltvExpiry, amountOut: MilliSatoshi, expiryOut: CltvExpiry): RouteParams = { |
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.
NB: we still verify in validateRelay
above that the fee is at least our current default fee, so we ignore the fact that we may have channels that require lower fees and could relay this payment.
But I think it's the right behavior to have, trampoline is more computationally expensive than standard channel-relay so it makes sense to have a higher threshold than our cheapest channels.
eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala
Outdated
Show resolved
Hide resolved
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.
LGTM!
Make sure to choose the "Squash and merge" option when merging, and provide a good commit title and message.
Make sure trampoline users pay at least as much as the other users.