Skip to content

Broadcast channel_update due to peer_disconnection after timer expiration #396

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,20 @@ const MULTI_STATE_FLAGS: u32 = (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::PeerDis

const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;

/// Liveness is called to fluctuate given peer disconnecton/monitor failures/closing.
/// If channel is public, network should have a liveness view announced by us on a
/// 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 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

/// Status has been gossiped.
Fresh,
/// Status has been changed.
DisabledMarked,
/// Status has been marked to be gossiped at next flush
DisabledStaged,
}

// TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
// has been completed, and then turn into a Channel to get compiler-time enforcement of things like
// calling channel_id() before we're set up or things like get_outbound_funding_signed on an
Expand Down Expand Up @@ -340,6 +354,8 @@ pub(super) struct Channel {

channel_monitor: ChannelMonitor,

network_sync: UpdateStatus,

logger: Arc<Logger>,
}

Expand Down Expand Up @@ -517,6 +533,8 @@ impl Channel {

channel_monitor: channel_monitor,

network_sync: UpdateStatus::Fresh,

logger,
})
}
Expand Down Expand Up @@ -734,6 +752,8 @@ impl Channel {

channel_monitor: channel_monitor,

network_sync: UpdateStatus::Fresh,

logger,
};

Expand Down Expand Up @@ -2993,6 +3013,26 @@ impl Channel {
} else { false }
}

pub fn to_disabled_staged(&mut self) {
self.network_sync = UpdateStatus::DisabledStaged;
}

pub fn to_disabled_marked(&mut self) {
self.network_sync = UpdateStatus::DisabledMarked;
}

pub fn to_fresh(&mut self) {
self.network_sync = UpdateStatus::Fresh;
}

pub fn is_disabled_staged(&self) -> bool {
self.network_sync == UpdateStatus::DisabledStaged
}

pub fn is_disabled_marked(&self) -> bool {
self.network_sync == UpdateStatus::DisabledMarked
}

/// Called by channelmanager based on chain blocks being connected.
/// Note that we only need to use this to detect funding_signed, anything else is handled by
/// the channel_monitor.
Expand Down Expand Up @@ -4091,6 +4131,8 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {

channel_monitor,

network_sync: UpdateStatus::Fresh,

logger,
})
}
Expand Down
33 changes: 32 additions & 1 deletion lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,13 @@ const ERR: () = "You need at least 32 bit pointers (well, usize, but we'll assum
/// the "reorg path" (ie call block_disconnected() until you get to a common block and then call
/// block_connected() to step towards your best block) upon deserialization before using the
/// object!
///
/// Note that ChannelManager is responsible of tracking liveness of its channels and by so
/// to generate ChannelUpdate messages intended to be broadcast on the gossip layer. To avoid
/// spam due to quick connection/reconnection, updates should be first stagged then after a period
/// of 1 min flushed to the network through call to timer_chan_freshness_every_min. You may
/// delay or anticipate call to this method to suit your announcements requirements different than
/// the 1 min period.
pub struct ChannelManager<'a> {
default_configuration: UserConfig,
genesis_hash: Sha256dHash,
Expand Down Expand Up @@ -1487,6 +1494,30 @@ impl<'a> ChannelManager<'a> {
events.append(&mut new_events);
}

/// 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 this channels.
pub fn timer_chan_freshness_every_min(&self) {
let _ = self.total_consistency_lock.read().unwrap();
let mut channel_state_lock = self.channel_state.lock().unwrap();
let channel_state = channel_state_lock.borrow_parts();
for (_, chan) in channel_state.by_id {
if chan.is_disabled_staged() && !chan.is_live() {
if let Ok(update) = self.get_channel_update(&chan) {
channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
msg: update
});
}
chan.to_fresh();
} else if chan.is_disabled_staged() && chan.is_live() {
chan.to_fresh();
} else if chan.is_disabled_marked() {
chan.to_disabled_staged();
}
}
}

/// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect
/// after a PaymentReceived event, failing the HTLC back to its origin and freeing resources
/// along the path (including in our own channel on which we received it).
Expand Down Expand Up @@ -2795,8 +2826,8 @@ impl<'a> ChannelMessageHandler for ChannelManager<'a> {
log_debug!(self, "Marking channels with {} disconnected and generating channel_updates", log_pubkey!(their_node_id));
channel_state.by_id.retain(|_, chan| {
if chan.get_their_node_id() == *their_node_id {
//TODO: mark channel disabled (and maybe announce such after a timeout).
let failed_adds = chan.remove_uncommitted_htlcs_and_mark_paused();
chan.to_disabled_marked();
if !failed_adds.is_empty() {
let chan_update = self.get_channel_update(&chan).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe
failed_payments.push((chan_update, failed_adds));
Expand Down
60 changes: 60 additions & 0 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6256,3 +6256,63 @@ fn test_check_htlc_underpaying() {
}
nodes[1].node.get_and_clear_pending_events();
}

#[test]
fn test_announce_disable_channels() {
// Create 2 channels between A and B. Disconnect B. Call timer_chan_freshness_every_min and check for generated
// ChannelUpdate. Reconnect B, reestablish and check there is non-generated ChannelUpdate.

let nodes = create_network(2, &[None, None]);

let short_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1, LocalFeatures::new(), LocalFeatures::new()).0.contents.short_channel_id;
let short_id_2 = create_announced_chan_between_nodes(&nodes, 1, 0, LocalFeatures::new(), LocalFeatures::new()).0.contents.short_channel_id;
let short_id_3 = create_announced_chan_between_nodes(&nodes, 0, 1, LocalFeatures::new(), LocalFeatures::new()).0.contents.short_channel_id;

// Disconnect peers
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

nodes[0].node.timer_chan_freshness_every_min(); // dirty -> stagged
nodes[0].node.timer_chan_freshness_every_min(); // staged -> fresh
let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(msg_events.len(), 3);
for e in msg_events {
match e {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
let short_id = msg.contents.short_channel_id;
// Check generated channel_update match list in PendingChannelUpdate
if short_id != short_id_1 && short_id != short_id_2 && short_id != short_id_3 {
panic!("Generated ChannelUpdate for wrong chan!");
}
},
_ => panic!("Unexpected event"),
}
}
// Reconnect peers
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id());
let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
assert_eq!(reestablish_1.len(), 3);
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
assert_eq!(reestablish_2.len(), 3);

// Reestablish chan_1
nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
// Reestablish chan_2
nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[1]).unwrap();
handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[1]).unwrap();
handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
// Reestablish chan_3
nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[2]).unwrap();
handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[2]).unwrap();
handle_chan_reestablish_msgs!(nodes[1], nodes[0]);

nodes[0].node.timer_chan_freshness_every_min();
let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(msg_events.len(), 0);
}