Skip to content

Commit d8dc97c

Browse files
committed
Force-close if finish closing_signed negotiation takes a full minute
1 parent 0069a3a commit d8dc97c

File tree

2 files changed

+74
-36
lines changed

2 files changed

+74
-36
lines changed

lightning/src/ln/channel.rs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,6 @@ enum ChannelState {
244244
RemoteShutdownSent = 1 << 10,
245245
/// Flag which is set on ChannelFunded or FundingSent after sending a shutdown message. At this
246246
/// point, we may not add any new HTLCs to the channel.
247-
/// TODO: Investigate some kind of timeout mechanism by which point the remote end must provide
248-
/// us their shutdown.
249247
LocalShutdownSent = 1 << 11,
250248
/// We've successfully negotiated a closing_signed dance. At this point ChannelManager is about
251249
/// to drop us, but we store this anyway.
@@ -473,6 +471,11 @@ pub(super) struct Channel<Signer: Sign> {
473471
commitment_secrets: CounterpartyCommitmentSecrets,
474472

475473
channel_update_status: ChannelUpdateStatus,
474+
/// Once we reach `closing_negotiation_ready`, we set this, indicating if closing_signed does
475+
/// not complete within a single timer tick (one minte), we should force-close the channel.
476+
/// Note that this field is reset to false on deserialization to give us a chance to connect to
477+
/// our peer and start the closing_signed negotiation fresh.
478+
closing_signed_in_flight: bool,
476479

477480
/// Our counterparty's channel_announcement signatures provided in announcement_signatures.
478481
/// This can be used to rebroadcast the channel_announcement message later.
@@ -699,6 +702,7 @@ impl<Signer: Sign> Channel<Signer> {
699702
commitment_secrets: CounterpartyCommitmentSecrets::new(),
700703

701704
channel_update_status: ChannelUpdateStatus::Enabled,
705+
closing_signed_in_flight: false,
702706

703707
announcement_sigs: None,
704708

@@ -949,6 +953,7 @@ impl<Signer: Sign> Channel<Signer> {
949953
commitment_secrets: CounterpartyCommitmentSecrets::new(),
950954

951955
channel_update_status: ChannelUpdateStatus::Enabled,
956+
closing_signed_in_flight: false,
952957

953958
announcement_sigs: None,
954959

@@ -3275,16 +3280,38 @@ impl<Signer: Sign> Channel<Signer> {
32753280
self.closing_fee_limits = Some((proposed_total_fee_satoshis, proposed_max_total_fee_satoshis));
32763281
}
32773282

3283+
/// Returns true if we're ready to commence the closing_signed negotiation phase. This is true
3284+
/// after both sides have exchanged a `shutdown` message and all HTLCs have been drained. At
3285+
/// this point if we're the funder we should send the initial closing_signed, and in any case
3286+
/// shutdown should complete within a reasonable timeframe.
3287+
fn closing_negotiation_ready(&self) -> bool {
3288+
self.pending_inbound_htlcs.is_empty() && self.pending_outbound_htlcs.is_empty() &&
3289+
self.channel_state &
3290+
(BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32 |
3291+
ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)
3292+
== BOTH_SIDES_SHUTDOWN_MASK &&
3293+
self.pending_update_fee.is_none()
3294+
}
3295+
3296+
/// Checks if the closing_signed negotiation is making appropriate progress, possibly returning
3297+
/// an Err if no progress is being made and the channel should be force-closed instead.
3298+
/// Should be called on a one-minute timer.
3299+
pub fn timer_check_closing_negotiation_progress(&mut self) -> Result<(), ChannelError> {
3300+
if self.closing_negotiation_ready() {
3301+
if self.closing_signed_in_flight {
3302+
return Err(ChannelError::Close("closing_signed negotiation failed to finish within one minute".to_owned()));
3303+
} else {
3304+
self.closing_signed_in_flight = true;
3305+
}
3306+
}
3307+
Ok(())
3308+
}
3309+
32783310
pub fn maybe_propose_first_closing_signed<F: Deref, L: Deref>(&mut self, fee_estimator: &F, logger: &L)
32793311
-> Result<Option<msgs::ClosingSigned>, ChannelError>
32803312
where F::Target: FeeEstimator, L::Target: Logger
32813313
{
3282-
if !self.is_outbound() || !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() ||
3283-
self.channel_state &
3284-
(BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32 |
3285-
ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32)
3286-
!= BOTH_SIDES_SHUTDOWN_MASK ||
3287-
self.last_sent_closing_fee.is_some() || self.pending_update_fee.is_some() {
3314+
if !self.is_outbound() || self.last_sent_closing_fee.is_some() || !self.closing_negotiation_ready() {
32883315
return Ok(None);
32893316
}
32903317

@@ -5228,6 +5255,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
52285255
commitment_secrets,
52295256

52305257
channel_update_status,
5258+
closing_signed_in_flight: false,
52315259

52325260
announcement_sigs,
52335261

lightning/src/ln/channelmanager.rs

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2634,46 +2634,56 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
26342634
let pending_msg_events = &mut channel_state.pending_msg_events;
26352635
let short_to_id = &mut channel_state.short_to_id;
26362636
channel_state.by_id.retain(|chan_id, chan| {
2637-
match chan.channel_update_status() {
2638-
ChannelUpdateStatus::Enabled if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::DisabledStaged),
2639-
ChannelUpdateStatus::Disabled if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::EnabledStaged),
2640-
ChannelUpdateStatus::DisabledStaged if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Enabled),
2641-
ChannelUpdateStatus::EnabledStaged if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Disabled),
2642-
ChannelUpdateStatus::DisabledStaged if !chan.is_live() => {
2643-
if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
2644-
pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
2645-
msg: update
2646-
});
2647-
}
2648-
should_persist = NotifyOption::DoPersist;
2649-
chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
2650-
},
2651-
ChannelUpdateStatus::EnabledStaged if chan.is_live() => {
2652-
if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
2653-
pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
2654-
msg: update
2655-
});
2656-
}
2657-
should_persist = NotifyOption::DoPersist;
2658-
chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
2659-
},
2660-
_ => {},
2661-
}
2662-
26632637
let counterparty_node_id = chan.get_counterparty_node_id();
2664-
let (retain_channel, chan_needs_persist, err) = self.update_channel_fee(short_to_id, pending_msg_events, chan_id, chan, new_feerate);
2638+
let (mut retain_channel, chan_needs_persist, err) = self.update_channel_fee(short_to_id, pending_msg_events, chan_id, chan, new_feerate);
26652639
if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; }
26662640
if err.is_err() {
26672641
handle_errors.push((err, counterparty_node_id));
26682642
}
2643+
2644+
if retain_channel {
2645+
if let Err(e) = chan.timer_check_closing_negotiation_progress() {
2646+
let (needs_close, err) = convert_chan_err!(self, e, short_to_id, chan, chan_id);
2647+
handle_errors.push((Err(err), chan.get_counterparty_node_id()));
2648+
if needs_close { retain_channel = false; }
2649+
}
2650+
}
2651+
2652+
if retain_channel {
2653+
match chan.channel_update_status() {
2654+
ChannelUpdateStatus::Enabled if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::DisabledStaged),
2655+
ChannelUpdateStatus::Disabled if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::EnabledStaged),
2656+
ChannelUpdateStatus::DisabledStaged if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Enabled),
2657+
ChannelUpdateStatus::EnabledStaged if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Disabled),
2658+
ChannelUpdateStatus::DisabledStaged if !chan.is_live() => {
2659+
if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
2660+
pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
2661+
msg: update
2662+
});
2663+
}
2664+
should_persist = NotifyOption::DoPersist;
2665+
chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
2666+
},
2667+
ChannelUpdateStatus::EnabledStaged if chan.is_live() => {
2668+
if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
2669+
pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
2670+
msg: update
2671+
});
2672+
}
2673+
should_persist = NotifyOption::DoPersist;
2674+
chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
2675+
},
2676+
_ => {},
2677+
}
2678+
}
2679+
26692680
retain_channel
26702681
});
26712682
}
26722683

26732684
for (err, counterparty_node_id) in handle_errors.drain(..) {
26742685
let _ = handle_error!(self, err, counterparty_node_id);
26752686
}
2676-
26772687
should_persist
26782688
});
26792689

0 commit comments

Comments
 (0)