Skip to content

Commit ae1fe19

Browse files
author
Antoine Riard
committed
Add ChannelClosed generation at cooperative/force-close/error processing
When we detect a channel `is_shutdown()` or call on it `force_shutdown()`, we notify the user with a Event::ChannelClosed informing about the id and closure reason.
1 parent f52db3f commit ae1fe19

File tree

9 files changed

+325
-34
lines changed

9 files changed

+325
-34
lines changed

lightning-background-processor/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,7 @@ mod tests {
614614
.expect("SpendableOutputs not handled within deadline");
615615
match event {
616616
Event::SpendableOutputs { .. } => {},
617+
Event::ChannelClosed { .. } => {},
617618
_ => panic!("Unexpected event: {:?}", event),
618619
}
619620

lightning-persister/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,11 @@ mod tests {
182182
use bitcoin::Txid;
183183
use lightning::chain::channelmonitor::{Persist, ChannelMonitorUpdateErr};
184184
use lightning::chain::transaction::OutPoint;
185-
use lightning::{check_closed_broadcast, check_added_monitors};
185+
use lightning::{check_closed_broadcast, check_closed_event, check_added_monitors};
186186
use lightning::ln::features::InitFeatures;
187187
use lightning::ln::functional_test_utils::*;
188188
use lightning::ln::msgs::ErrorAction;
189-
use lightning::util::events::{MessageSendEventsProvider, MessageSendEvent};
189+
use lightning::util::events::{ClosureReason, Event, MessageSendEventsProvider, MessageSendEvent};
190190
use lightning::util::test_utils;
191191
use std::fs;
192192
#[cfg(target_os = "windows")]
@@ -259,6 +259,7 @@ mod tests {
259259
// Force close because cooperative close doesn't result in any persisted
260260
// updates.
261261
nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id).unwrap();
262+
check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
262263
check_closed_broadcast!(nodes[0], true);
263264
check_added_monitors!(nodes[0], 1);
264265

@@ -268,6 +269,7 @@ mod tests {
268269
let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
269270
connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[0].clone()]});
270271
check_closed_broadcast!(nodes[1], true);
272+
check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
271273
check_added_monitors!(nodes[1], 1);
272274

273275
// Make sure everything is persisted as expected after close.
@@ -291,6 +293,7 @@ mod tests {
291293
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
292294
let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
293295
nodes[1].node.force_close_channel(&chan.2).unwrap();
296+
check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed);
294297
let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
295298

296299
// Set the persister's directory to read-only, which should result in

lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use ln::msgs::{ChannelMessageHandler, ErrorAction, RoutingMessageHandler};
2828
use routing::router::get_route;
2929
use util::config::UserConfig;
3030
use util::enforcing_trait_impls::EnforcingSigner;
31-
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose};
31+
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason};
3232
use util::errors::APIError;
3333
use util::ser::{ReadableArgs, Writeable};
3434
use util::test_utils::TestBroadcaster;
@@ -81,6 +81,7 @@ fn do_test_simple_monitor_permanent_update_fail(persister_fail: bool) {
8181
// PaymentFailed event
8282

8383
assert_eq!(nodes[0].node.list_channels().len(), 0);
84+
check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
8485
}
8586

8687
#[test]
@@ -269,6 +270,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail
269270
// PaymentFailed event
270271

271272
assert_eq!(nodes[0].node.list_channels().len(), 0);
273+
check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
272274
}
273275

274276
#[test]
@@ -1985,6 +1987,8 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
19851987

19861988
send_payment(&nodes[0], &[&nodes[1]], 8000000);
19871989
close_channel(&nodes[0], &nodes[1], &channel_id, funding_tx, true);
1990+
check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure);
1991+
check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure);
19881992
}
19891993

19901994
#[test]
@@ -2610,6 +2614,8 @@ fn test_temporary_error_during_shutdown() {
26102614
assert_eq!(txn_a, txn_b);
26112615
assert_eq!(txn_a.len(), 1);
26122616
check_spends!(txn_a[0], funding_tx);
2617+
check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure);
2618+
check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure);
26132619
}
26142620

26152621
#[test]
@@ -2630,6 +2636,7 @@ fn test_permanent_error_during_sending_shutdown() {
26302636
assert!(nodes[0].node.close_channel(&channel_id).is_ok());
26312637
check_closed_broadcast!(nodes[0], true);
26322638
check_added_monitors!(nodes[0], 2);
2639+
check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
26332640
}
26342641

26352642
#[test]
@@ -2652,6 +2659,7 @@ fn test_permanent_error_during_handling_shutdown() {
26522659
nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &InitFeatures::known(), &shutdown);
26532660
check_closed_broadcast!(nodes[1], true);
26542661
check_added_monitors!(nodes[1], 2);
2662+
check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
26552663
}
26562664

26572665
#[test]

lightning/src/ln/channelmanager.rs

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ use ln::onion_utils;
5252
use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, OptionalField};
5353
use chain::keysinterface::{Sign, KeysInterface, KeysManager, InMemorySigner};
5454
use util::config::UserConfig;
55-
use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
55+
use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
5656
use util::{byte_utils, events};
5757
use util::ser::{Readable, ReadableArgs, MaybeReadable, Writeable, Writer};
5858
use util::chacha20::{ChaCha20, ChaChaReader};
@@ -835,6 +835,9 @@ macro_rules! handle_error {
835835
msg: update
836836
});
837837
}
838+
if let Some(channel_id) = chan_id {
839+
$self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id, reason: ClosureReason::ProcessingError { err: err.err.clone() } });
840+
}
838841
}
839842

840843
log_error!($self.logger, "{}", err.err);
@@ -1368,6 +1371,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
13681371
msg: channel_update
13691372
});
13701373
}
1374+
if let Ok(mut pending_events_lock) = self.pending_events.lock() {
1375+
pending_events_lock.push(events::Event::ChannelClosed {
1376+
channel_id: *channel_id,
1377+
reason: ClosureReason::HolderForceClosed
1378+
});
1379+
}
13711380
}
13721381
break Ok(());
13731382
},
@@ -1443,7 +1452,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
14431452
}
14441453
}
14451454

1446-
fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: Option<&PublicKey>) -> Result<PublicKey, APIError> {
1455+
/// `peer_node_id` should be set when we receive a message from a peer, but not set when the
1456+
/// user closes, which will be re-exposed as the `ChannelClosed` reason.
1457+
fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: Option<&PublicKey>, peer_msg: Option<&String>) -> Result<PublicKey, APIError> {
14471458
let mut chan = {
14481459
let mut channel_state_lock = self.channel_state.lock().unwrap();
14491460
let channel_state = &mut *channel_state_lock;
@@ -1456,6 +1467,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
14561467
if let Some(short_id) = chan.get().get_short_channel_id() {
14571468
channel_state.short_to_id.remove(&short_id);
14581469
}
1470+
let mut pending_events_lock = self.pending_events.lock().unwrap();
1471+
if peer_node_id.is_some() {
1472+
if let Some(peer_msg) = peer_msg {
1473+
pending_events_lock.push(events::Event::ChannelClosed { channel_id: *channel_id, reason: ClosureReason::CounterpartyForceClosed { peer_msg: peer_msg.to_string() } });
1474+
}
1475+
} else {
1476+
pending_events_lock.push(events::Event::ChannelClosed { channel_id: *channel_id, reason: ClosureReason::HolderForceClosed });
1477+
}
14591478
chan.remove_entry().1
14601479
} else {
14611480
return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
@@ -1477,7 +1496,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
14771496
/// the chain and rejecting new HTLCs on the given channel. Fails if channel_id is unknown to the manager.
14781497
pub fn force_close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
14791498
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
1480-
match self.force_close_channel_with_peer(channel_id, None) {
1499+
match self.force_close_channel_with_peer(channel_id, None, None) {
14811500
Ok(counterparty_node_id) => {
14821501
self.channel_state.lock().unwrap().pending_msg_events.push(
14831502
events::MessageSendEvent::HandleError {
@@ -2421,6 +2440,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
24212440
if let Some(short_id) = channel.get_short_channel_id() {
24222441
channel_state.short_to_id.remove(&short_id);
24232442
}
2443+
// ChannelClosed event is generated by handle_error for us.
24242444
Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
24252445
},
24262446
ChannelError::CloseDelayBroadcast(_) => { panic!("Wait is only generated on receipt of channel_reestablish, which is handled by try_chan_entry, we don't bother to support it here"); }
@@ -3550,6 +3570,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
35503570
msg: update
35513571
});
35523572
}
3573+
self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: msg.channel_id, reason: ClosureReason::CooperativeClosure });
35533574
}
35543575
Ok(())
35553576
}
@@ -3961,6 +3982,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
39613982
msg: update
39623983
});
39633984
}
3985+
self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), reason: ClosureReason::CommitmentTxConfirmed });
39643986
pending_msg_events.push(events::MessageSendEvent::HandleError {
39653987
node_id: chan.get_counterparty_node_id(),
39663988
action: msgs::ErrorAction::SendErrorMessage {
@@ -4022,6 +4044,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
40224044
Err(e) => {
40234045
let (close_channel, res) = convert_chan_err!(self, e, short_to_id, chan, channel_id);
40244046
handle_errors.push((chan.get_counterparty_node_id(), Err(res)));
4047+
// ChannelClosed event is generated by handle_error for us
40254048
!close_channel
40264049
}
40274050
}
@@ -4075,6 +4098,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
40754098
});
40764099
}
40774100

4101+
if let Ok(mut pending_events_lock) = self.pending_events.lock() {
4102+
pending_events_lock.push(events::Event::ChannelClosed {
4103+
channel_id: *channel_id,
4104+
reason: ClosureReason::CooperativeClosure
4105+
});
4106+
}
4107+
40784108
log_info!(self.logger, "Broadcasting {}", log_tx!(tx));
40794109
self.tx_broadcaster.broadcast_transaction(&tx);
40804110
false
@@ -4495,6 +4525,7 @@ where
44954525
msg: update
44964526
});
44974527
}
4528+
self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(), reason: ClosureReason::CommitmentTxConfirmed });
44984529
pending_msg_events.push(events::MessageSendEvent::HandleError {
44994530
node_id: channel.get_counterparty_node_id(),
45004531
action: msgs::ErrorAction::SendErrorMessage { msg: e },
@@ -4685,6 +4716,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
46854716
msg: update
46864717
});
46874718
}
4719+
self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), reason: ClosureReason::DisconnectedPeer });
46884720
false
46894721
} else {
46904722
true
@@ -4699,6 +4731,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
46994731
if let Some(short_id) = chan.get_short_channel_id() {
47004732
short_to_id.remove(&short_id);
47014733
}
4734+
self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), reason: ClosureReason::DisconnectedPeer });
47024735
return false;
47034736
} else {
47044737
no_channels_remain = false;
@@ -4789,12 +4822,12 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
47894822
for chan in self.list_channels() {
47904823
if chan.counterparty.node_id == *counterparty_node_id {
47914824
// Untrusted messages from peer, we throw away the error if id points to a non-existent channel
4792-
let _ = self.force_close_channel_with_peer(&chan.channel_id, Some(counterparty_node_id));
4825+
let _ = self.force_close_channel_with_peer(&chan.channel_id, Some(counterparty_node_id), Some(&msg.data));
47934826
}
47944827
}
47954828
} else {
47964829
// Untrusted messages from peer, we throw away the error if id points to a non-existent channel
4797-
let _ = self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id));
4830+
let _ = self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id), Some(&msg.data));
47984831
}
47994832
}
48004833
}
@@ -5295,6 +5328,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
52955328
let mut funding_txo_set = HashSet::with_capacity(cmp::min(channel_count as usize, 128));
52965329
let mut by_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
52975330
let mut short_to_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
5331+
let mut channel_closures = Vec::new();
52985332
for _ in 0..channel_count {
52995333
let mut channel: Channel<Signer> = Channel::read(reader, &args.keys_manager)?;
53005334
let funding_txo = channel.get_funding_txo().ok_or(DecodeError::InvalidValue)?;
@@ -5325,6 +5359,10 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
53255359
let (_, mut new_failed_htlcs) = channel.force_shutdown(true);
53265360
failed_htlcs.append(&mut new_failed_htlcs);
53275361
monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
5362+
channel_closures.push(events::Event::ChannelClosed {
5363+
channel_id: channel.channel_id(),
5364+
reason: ClosureReason::OutdatedChannelManager
5365+
});
53285366
} else {
53295367
if let Some(short_channel_id) = channel.get_short_channel_id() {
53305368
short_to_id.insert(short_channel_id, channel.channel_id());
@@ -5432,6 +5470,10 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
54325470
let mut secp_ctx = Secp256k1::new();
54335471
secp_ctx.seeded_randomize(&args.keys_manager.get_secure_random_bytes());
54345472

5473+
if !channel_closures.is_empty() {
5474+
pending_events_read.append(&mut channel_closures);
5475+
}
5476+
54355477
let channel_manager = ChannelManager {
54365478
genesis_hash,
54375479
fee_estimator: args.fee_estimator,

lightning/src/ln/functional_test_utils.rs

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -743,16 +743,16 @@ macro_rules! get_closing_signed_broadcast {
743743
#[macro_export]
744744
macro_rules! check_closed_broadcast {
745745
($node: expr, $with_error_msg: expr) => {{
746-
let events = $node.node.get_and_clear_pending_msg_events();
747-
assert_eq!(events.len(), if $with_error_msg { 2 } else { 1 });
748-
match events[0] {
746+
let msg_events = $node.node.get_and_clear_pending_msg_events();
747+
assert_eq!(msg_events.len(), if $with_error_msg { 2 } else { 1 });
748+
match msg_events[0] {
749749
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
750750
assert_eq!(msg.contents.flags & 2, 2);
751751
},
752752
_ => panic!("Unexpected event"),
753753
}
754754
if $with_error_msg {
755-
match events[1] {
755+
match msg_events[1] {
756756
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id: _ } => {
757757
// TODO: Check node_id
758758
Some(msg.clone())
@@ -763,6 +763,24 @@ macro_rules! check_closed_broadcast {
763763
}}
764764
}
765765

766+
/// Check that a channel's closing channel event has been issued
767+
#[macro_export]
768+
macro_rules! check_closed_event {
769+
($node: expr, $events: expr, $reason: expr) => {{
770+
let events = $node.node.get_and_clear_pending_events();
771+
assert_eq!(events.len(), $events);
772+
let expected_reason = $reason;
773+
for event in events {
774+
match event {
775+
Event::ChannelClosed { ref reason, .. } => {
776+
assert_eq!(*reason, expected_reason);
777+
},
778+
_ => panic!("Unexpected event"),
779+
}
780+
}
781+
}}
782+
}
783+
766784
pub fn close_channel<'a, 'b, 'c>(outbound_node: &Node<'a, 'b, 'c>, inbound_node: &Node<'a, 'b, 'c>, channel_id: &[u8; 32], funding_tx: Transaction, close_inbound_first: bool) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, Transaction) {
767785
let (node_a, broadcaster_a, struct_a) = if close_inbound_first { (&inbound_node.node, &inbound_node.tx_broadcaster, inbound_node) } else { (&outbound_node.node, &outbound_node.tx_broadcaster, outbound_node) };
768786
let (node_b, broadcaster_b, struct_b) = if close_inbound_first { (&outbound_node.node, &outbound_node.tx_broadcaster, outbound_node) } else { (&inbound_node.node, &inbound_node.tx_broadcaster, inbound_node) };
@@ -986,6 +1004,20 @@ macro_rules! expect_pending_htlcs_forwardable {
9861004
}}
9871005
}
9881006

1007+
#[cfg(test)]
1008+
macro_rules! expect_pending_htlcs_forwardable_from_events {
1009+
($node: expr, $events: expr, $ignore: expr) => {{
1010+
assert_eq!($events.len(), 1);
1011+
match $events[0] {
1012+
Event::PendingHTLCsForwardable { .. } => { },
1013+
_ => panic!("Unexpected event"),
1014+
};
1015+
if $ignore {
1016+
$node.node.process_pending_htlc_forwards();
1017+
}
1018+
}}
1019+
}
1020+
9891021
#[cfg(any(test, feature = "unstable"))]
9901022
macro_rules! expect_payment_received {
9911023
($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr) => {

0 commit comments

Comments
 (0)