Skip to content

peer: only send send enable update for disabled chans#2017

Closed
halseth wants to merge 1 commit into
lightningnetwork:masterfrom
halseth:peer-no-enable-enabled-chans
Closed

peer: only send send enable update for disabled chans#2017
halseth wants to merge 1 commit into
lightningnetwork:masterfrom
halseth:peer-no-enable-enabled-chans

Conversation

@halseth
Copy link
Copy Markdown
Contributor

@halseth halseth commented Oct 8, 2018

To avoid spamming on each connection.

To avoid spamming on each connection.
Comment thread peer.go

// To ensure we can route through this channel now that the peer
// is back online, we'll attempt to send an update to enable it.
// If the channel is already enabled in the database, we won't
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Won't this already be done by the logic further into this execution pipeline? In that it will only send out a new update if this is a shift from the state of the past 20 mins?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, if we have already sent a disable+enable for this channel, the re-enabling will be ignored.

That means that this will only avoid the case where we first connect to a peer after startup, and send out a channel update for all our N channels (if all our peers are online). Maybe still good for the health of the graph not to send out N new updates after each restart?

@Roasbeef Roasbeef added p2p Code related to the peer-to-peer behaviour discovery Peer and route discovery / whisper protocol related issues/PRs bug fix needs review PR needs review by regular contributors needs testing PR hasn't yet been actively tested on testnet/mainnet labels Oct 18, 2018
@halseth
Copy link
Copy Markdown
Contributor Author

halseth commented Nov 2, 2018

Might not be needed after strict channel enabling and related PRs. Making P4 for now.

@halseth halseth added the P4 low prio label Nov 2, 2018
@halseth
Copy link
Copy Markdown
Contributor Author

halseth commented Dec 21, 2018

Replaced by #2080

@halseth halseth closed this Dec 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix discovery Peer and route discovery / whisper protocol related issues/PRs needs review PR needs review by regular contributors needs testing PR hasn't yet been actively tested on testnet/mainnet p2p Code related to the peer-to-peer behaviour P4 low prio

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants