lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.58k stars 2.06k forks source link

discovery - [feature]: implement bandwidth based, daemon wide gossip rate limiting. #8902

Open Roasbeef opened 2 weeks ago

Roasbeef commented 2 weeks ago

Is your feature request related to a problem? Please describe.

Today we rate limit based on the the number of messages a peer sends, within the syncer: https://github.com/lightningnetwork/lnd/blob/3526f82b5d9c7a2ac9201d6b7f32ac78f2ae9202/discovery/syncer.go#L392-L396.

With this, we're able to rate limit the messages sent during the interactive sync/reconciliation protocol, but not also what we'll send/recv from peers as a part of the normal gossip trickle.

We also rate limit based on the number of channel updates sent/recv'd: https://github.com/lightningnetwork/lnd/blob/3526f82b5d9c7a2ac9201d6b7f32ac78f2ae9202/discovery/gossiper.go#L484-L491.

These values were set long ago, and not properly tuned before settling in a value. These values also don't give users precise control w.r.t exactly how much bandwidth we'll dedicate to gossip data. By using more precise, yet global rate limiting, we'd also be able to give node operators first-class telemetry w.r.t how much bandwidth is actually being used for gossip.

This is a brainstorming issue to explore switching all the gossip related rate limiting to a stricter/simpler bandwidth based approach.

Describe the solution you'd like

Simplify and unify rate limiting, using Go's token bucket rate limiter. In other areas we set the limit based on the number of messages. Instead, I propose we set the limit based on a bytes/second target.

Implementation wise, we may want to have this closer to the peer, occupying the role of the current msg queue into the gossiper. we'll want to take care that we avoid unnecessary blocking and also compromising the uptime of an existing channel based on bandwidth based rate limiting.

morehouse commented 2 weeks ago

Implementation wise, we may want to have this closer to the peer, occupying the role of the current msg queue into the gossiper.

Sounds good for incoming messages.

Might be a little tricky for outgoing messages. We don't want the outgoing rate limit at the peer level to bottleneck the syncer/gossiper, causing outgoing messages to get queued up and waste resources.