Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update default path-finding weight ratios #1796

Merged
merged 2 commits into from
May 12, 2021
Merged

Update default path-finding weight ratios #1796

merged 2 commits into from
May 12, 2021

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented May 12, 2021

  • The two months window for channel age was too small for today's network (where most channels are at least 6 months old)
  • CLTV is less of an issue nowadays: there are fewer stuck payments and we're encouraging nodes to increase their CLTV because of the recent mempool congestions, so when your payment is stuck it's very painful regardless of the path you took

@t-bast
Update default path-finding weight ratios
74080a1 
* The two months window for channel age was too small for today's network
* CLTV is less of an issue nowadays: there are fewer stuck payments and
  we're encouraging nodes to increase their CLTV because of the recent
  mempool congestions
@t-bast t-bast requested review from thomash-acinq and pm47 May 12, 2021 12:57
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2021 •
edited
Loading

Codecov Report

Merging #1796 (5e668b2) into master (c641549) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1796      +/-   ##
==========================================
- Coverage   89.34%   89.30%   -0.04%     
==========================================
  Files         143      143              
  Lines       10932    10932              
  Branches      481      481              
==========================================
- Hits         9767     9763       -4     
- Misses       1165     1169       +4
Impacted Files Coverage Δ
.../src/main/scala/fr/acinq/eclair/router/Graph.scala 98.14% <100.00%> (ø)
...main/scala/fr/acinq/eclair/router/Validation.scala 90.76% <0.00%> (-1.54%) ⬇️

thomash-acinq
thomash-acinq reviewed May 12, 2021
View reviewed changes
Copy link
Member

@thomash-acinq thomash-acinq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that most channels are at least 6 months is not really relevant here. What matters is: Is a 6-month-old channel more reliable than a 2-month-old one? And I'm not convinced that's the case.

eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala Outdated Show resolved Hide resolved
thomash-acinq
thomash-acinq previously approved these changes May 12, 2021
View reviewed changes
@t-bast
Copy link
Member Author

t-bast commented May 12, 2021

What matters is: Is a 6-month-old channel more reliable than a 2-month-old one?

I believe that yes, the older a channel is, the more reliable it is, for the following reasons:

  • If an HTLC was ever stuck in that channel, the channel would have been force-closed. So the older a channel is, the less buggy it probably is
  • It shows that the channel is frequently used, otherwise its operator would have closed it to allocate its funds elsewhere. So it's frequently used and hasn't had a fatal bug, which is a good indication that it's reliable

The drawback is the barrier to entry for new channels / new nodes, maybe we should lower the ratio on channel age to offer a better opportunity for newcomers? But from the point of view of the sender, old is good if they don't care about the network as a market (and new channels can offer more competitive fees to make up for their young age).

Of course there's always the risk that a channel was very reliable for a long time, and then upgraded its software to a buggy version, but I don't think we should (or even can) account for that...

@thomash-acinq thomash-acinq merged commit 340fd29 into master May 12, 2021
@t-bast t-bast deleted the update-path-finding-default-ratios branch May 12, 2021 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomash-acinq thomash-acinq thomash-acinq approved these changes

@pm47 pm47 Awaiting requested review from pm47

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@t-bast @codecov-commenter @thomash-acinq