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.

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

Filtering route hints included in lightning-invoice::utils-constructed invoicse #1279

Closed
TheBlueMatt opened this issue Jan 25, 2022 · 5 comments
Labels
good first issue Good for newcomers

Comments

@TheBlueMatt
Copy link
Collaborator

Currently in create_invoice_from_channelmanager (and in create_phantom_invoice after #1199), we just blindly include hints about all the channels we have. This is usually not the right behavior. There's a few ways we should filter these:

  1. If we have multiple channels with the same counterparty node, only one needs to be included (see Use substitude channel for forwarding if we have another with the same peer #1278).
  2. If we have a mix of public and private channels, we probably shouldn't include any route hints, letting the sender look at our public channels (in general if we have public and private channels we probably dont want to dox our private channel peers, there's no need to).

Tagging good first issue, but if someone does take this up, probably base it on #1199 or wait until that is merged.

@TheBlueMatt TheBlueMatt added the good first issue Good for newcomers label Jan 25, 2022
@valentinewallace valentinewallace changed the title Filtering invoices included in lightning-invoice::utils-constructed invoicse Filtering route hints included in lightning-invoice::utils-constructed invoicse Feb 1, 2022
@ViktorTigerstrom
Copy link
Contributor

ViktorTigerstrom commented Feb 16, 2022

I'd like to work on this one if possible! Just a few questions in regards to the filtering criterias:

  1. If we have multiple channels with the same counterparty node, would we like choose which of the channels we pick to include in the hints based on some specific logic? Such as, based on the linked issue, making sure always to pick a channel with enough inbound_capacity to cover the amount if such a channel exists. If we want to do that, would we like to try make it harder in any way for the those requesting the invoices to try to determine the liquidity on the different ends for the different channels with the peer?

Just brainstorming a little, but maybe do something like this:
Randomize which channel we pick of the channels that has enough inbound_capacity. If no channel channel exists with enough inbound_capacity, pick a random channel.

Or perhaps always picking the one with the highest inbound_capacity give away even less, or the first channel we find with enough inbound_capacity..

  1. Just ensuring that I understand correctly. If we have ANY private channel with any peer, we never want include routing hints for any of our channels? That also includes hints for channels with peers where we only have public channel(s).

@TheBlueMatt
Copy link
Collaborator Author

  1. yea, maybe pick the highest capacity? It shouldn't matter, though, so whatever generates cleanest code. I agree we should stick to channels that have sufficient capacity, though.
  2. I believe that is the correct default behavior, yes. Users who wish to have other behavior can do so with their own invoice creation.

@ViktorTigerstrom
Copy link
Contributor

Ok thanks for the clarifications!

@ViktorTigerstrom
Copy link
Contributor

ViktorTigerstrom commented May 26, 2022

This should have been closed by #1325.

Looking back at this, I noticed that I forgot to flag the PR with a "closes" comment. Sorry about that!

@TheBlueMatt
Copy link
Collaborator Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants