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

Relay to channel with lowest possible balance #784

Merged
merged 3 commits into from Jan 21, 2019

Conversation

Projects
None yet
3 participants
@pm47
Copy link
Member

pm47 commented Dec 18, 2018

Our current channel selection is very simplistic: we relay to the
channel with the largest balance. As time goes by, this leads to all
channels having the same balance.

A better strategy is to relay to the channel which has the smallest
balance but has enough to process the payment. This way we save larger
channels for larger payments, and also on average channels get depleted
one after the other.

@pm47 pm47 changed the title [WIP] Relay to channel with lowest possible balance Relay to channel with lowest possible balance Jan 3, 2019

@pm47 pm47 requested a review from sstone Jan 3, 2019

@sstone

This comment has been minimized.

Copy link
Member

sstone commented Jan 4, 2019

There is no specific test for this (but there was no specific test to check that we used the channel with the maximum capacity either).

relay to channel with lowest possible balance
Our current channel selection is very simplistic: we relay to the
channel with the largest balance. As time goes by, this leads to all
channels having the same balance.

A better strategy is to relay to the channel which has the smallest
balance but has enough to process the payment. This way we save larger
channels for larger payments, and also on average channels get depleted
one after the other.

@pm47 pm47 force-pushed the relay-to-smaller-channel branch from 2e83397 to 405759f Jan 5, 2019

@pm47

This comment has been minimized.

Copy link
Member Author

pm47 commented Jan 5, 2019

rebased

added tests...
...and found bugs!

Note that there is something fishy in BOLT 4, filed a PR:
lightningnetwork/lightning-rfc#538

Also, first try of softwaremill's quicklens lib (in scope test for now)

@pm47 pm47 force-pushed the relay-to-smaller-channel branch from fa93767 to c750829 Jan 5, 2019

@pm47

This comment has been minimized.

Copy link
Member Author

pm47 commented Jan 8, 2019

Let's wait for more feedbacks on the lightning-rfc's pull requests before merging this.

@btcontract

This comment has been minimized.

Copy link
Contributor

btcontract commented Jan 14, 2019

Should comment here be "lowest balance"?

* Select a channel to the same node to the relay the payment to, that has the highest balance and is compatible in

@pm47

This comment has been minimized.

Copy link
Member Author

pm47 commented Jan 14, 2019

@btcontract good catch, that's a left over from previous strategy.

@btcontract

This comment has been minimized.

Copy link
Contributor

btcontract commented Jan 14, 2019

Does selectPreferredChannel check that new channel it deviates to is not the same channel it got a payment from? (lightningnetwork/lnd#2470)

@pm47

This comment has been minimized.

Copy link
Member Author

pm47 commented Jan 15, 2019

I'm not sure tbh, will write a test. If we don't I don't think that's necessarily a problem.

@btcontract

This comment has been minimized.

Copy link
Contributor

btcontract commented Jan 15, 2019

It's not strictly a bug but seems like it's almost never what a sender would want.

@btcontract

This comment has been minimized.

Copy link
Contributor

btcontract commented Jan 15, 2019

Actually I think I'm wrong here and allowing this is correct: lightningnetwork/lnd#2470 (comment)

@sstone

sstone approved these changes Jan 18, 2019

@pm47

This comment has been minimized.

Copy link
Member Author

pm47 commented Jan 18, 2019

NB: still waiting for lightningnetwork/lightning-rfc#538 before merging this.

@pm47 pm47 merged commit 3aa5754 into master Jan 21, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@pm47 pm47 deleted the relay-to-smaller-channel branch Jan 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment