-
Notifications
You must be signed in to change notification settings - Fork 421
Add max_no_channel_peers to UserConfig #3382
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
base: main
Are you sure you want to change the base?
Add max_no_channel_peers to UserConfig #3382
Conversation
The MAX_NO_CHANNEL_PEERS constant provides helpful protection from an excessive number of peer connections, but as a constant, isn't user modifiable. The desired number of non-channel peers may vary depending on the application, so here we make it configurable.
|
How many do y'all want? I'm of half a mind to just 10x it and call it a day. |
|
Today, 2500 -- tomorrow, THE WORLD! We currently run a fork with 2500. This is presently plenty of headroom, but I might want to tweak it down... |
|
right. depending on the node's purpose and planned usage, a node operator might want to configure this to be higher or lower. Given how this impacts resource usage, it makes sense for this to be a tunable parameter. |
| /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment | ||
| pub manually_handle_bolt12_invoices: bool, | ||
| /// The maximum number of non-channel peers that we will accept. Once this number of non-channel | ||
| /// peers is reached, we will not accept any new non-channel peer connectionss. This could mean |
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.
| /// peers is reached, we will not accept any new non-channel peer connectionss. This could mean | |
| /// peers is reached, we will not accept any new non-channel peer connections. This could mean |
I'm not entirely sure that it does impact resource usage that much. The maximum number of pre-funded channels definitely does, but no-channel peers are incredibly cheap (in |
Well, would no limit at all be better? The options I can think of are (1) no limit, (2) a one-size-fits-all limit, or (3) a customizable limit with a sensible default. |
I mean, there is some cost, just not a hell of a lot. Not sure that actually no limit is the right answer given there is some non-zero cost, even if its tiny. |
|
Maybe leave the current limit as default, make it configurable, and then just add to the doc comment there is a low cost to no-channel peers, but may lead to a higher cost at gossip/bandwidth and use responsibly etc. |
SGTM, of course. I think this PR will get you that, minus the doc comment. |
|
Hey, I wanted to bump this. I'm happy to rebase it or whatever is necessary. I think the bigger blocker is consensus that this should be configurable. The main thing we don't seem to know/agree on above is the "cost" of a peer. I'd love to bring some data on the cost of a peer so that we can decide on a number, but it's not fixed. Two things can vary:
|
The MAX_NO_CHANNEL_PEERS constant provides helpful protection from an excessive number of peer connections, but as a constant, isn't user modifiable. The desired number of non-channel peers may vary depending on the application, so here we make it configurable.