Broadcast channel_update due to peer_disconnection after timer expiration #396
Conversation
ariard
commented
Nov 18, 2019
ariard
commented
- process_channels_liveness design pattern: IMO kind of the equivalent of process_pending_htlc_forwards to manage gossip messages generation, I'm planning to reuse it for other todos : "Do some kind of timer to set the channel as !is_live() as we don't really wants..." and "Broadcast channel update for closed channels"
- time_broadcastable duration? see MIN_DELAY_ANNOUNCE_USELESS_CHAN_SECS
- process_channels_liveness name?
|
Let me know on the high-level approach with generating a PendingChannelUpdate and will update tests in consenquence to make them pass. |
|
Hmm, awesome functionality, though I kinda prefer we go a similar route as for #382 - just a generic "timer_tick_60_s()" function (and maybe trait?) which will check for disabled channels and generate BroadcastChannelUpdate events from there. |
|
Hmmmm, shouldn't change that much and avoid too much test update. Mostly just to reconvert process_channel_liveness as timer_tick_60_s. And better because not have to update other tests due to new event generated |
ariard
force-pushed
the
2019-11-pending-disabled-chan
branch
2 times, most recently
from
81b1af7 to
7138c12
Compare
|
Updated 7138c12, I think right now making it a trait doesn't give any flexibility to API clients due to global lock on channel state? |
|
Dropped the new event, but the counter-part is now we need to iterate over all channels to verify their state ? Or maybe let the caller pass a set of chans to verify ? |
lightning/src/ln/channelmanager.rs
Outdated
| let channel_state = channel_state_lock.borrow_parts(); | ||
| for (_, chan) in channel_state.by_id { | ||
| // Check if channel is still disabled | ||
| if !chan.is_live() { |
There was a problem hiding this comment.
This may have happened one second ago, though? The channel needs to have been disabled at the last call to this fn.
There was a problem hiding this comment.
Right so you need to track pending_disabled_chan in ChannelManager between the calls but it may imply we may worst-case take up to ~120 seconds to announce a ChannelUpdate?
There was a problem hiding this comment.
Yep. I think thats totally fine, but if you are worried about it you can also halve the time and make the user call every 30 seconds and then its worst-case 1.5 minutes (but I dont think it matters).
ariard
force-pushed
the
2019-11-pending-disabled-chan
branch
from
7138c12 to
4db90e5
Compare
|
Okay added a UpdateStatus field in channel with following logic, at peer_disconnected, fresh -> dirty, at timer_chan_freshness_every_min tick, dirty -> staged, at timer_chan_freshness_every_min tick (again), staged -> fresh (+ BroadcastChannelUpdate gen). Staged is assumed to be a disable update now, it may be extended in the future to stage live update. As say above, we may turn timer_chan_fresness_every_min as a trait in the future to enable more fancy gossip management like batching/pruning, ... |
lightning/src/ln/channelmanager.rs
Outdated
| /// about the uselessness of this channels. | ||
| pub fn timer_chan_freshness_every_min(&self) { | ||
| let _ = self.total_consistency_lock.read().unwrap(); | ||
| { |
There was a problem hiding this comment.
This scope seems unnecessary now?
| /// best-effort, which means we may filter out some status transitions to avoid spam. | ||
| /// See further timer_chan_freshness_every_min. | ||
| #[derive(PartialEq)] | ||
| enum UpdateStatus { |
There was a problem hiding this comment.
I find the constant names here confusing, can we use like "Live", "DisabledUnannounced", and "DisabledAnnounced" or so, mostly cause I dont want to look up the constant docs to figure out what to_dirty() and to_fresh() means.
Added enum and method are only used in next commit.
Latency/peer disconnection may trigger us to mark as disabled some of our channels. After some time, if channels are still disabled we need to broadcast ChannelUpdate to inform other network peers about the uselessness of these channels.
ariard
force-pushed
the
2019-11-pending-disabled-chan
branch
from
4db90e5 to
da94bac
Compare
|
Will take as #407. |
#396 with a few english fixes
