Skip to content

Commit 5231799

Browse files
committed
Send peers error messages for failures due to invalid batch funding
If we fail to fund a batch open we'll force-close all channels in the batch, however would previously fail to send error messages to our peers for any channels we were due to test after the one that failed. This commit fixes that issue, sending the required error messages to allow our peers to clean up their state.
1 parent 804aec9 commit 5231799

File tree

3 files changed

+28
-15
lines changed

3 files changed

+28
-15
lines changed

lightning/src/ln/channelmanager.rs

+11-2
Original file line numberDiff line numberDiff line change
@@ -4726,11 +4726,20 @@ where
47264726
for (channel_id, counterparty_node_id) in channels_to_remove {
47274727
per_peer_state.get(&counterparty_node_id)
47284728
.map(|peer_state_mutex| peer_state_mutex.lock().unwrap())
4729-
.and_then(|mut peer_state| peer_state.channel_by_id.remove(&channel_id))
4730-
.map(|mut chan| {
4729+
.and_then(|mut peer_state| peer_state.channel_by_id.remove(&channel_id).map(|chan| (chan, peer_state)))
4730+
.map(|(mut chan, mut peer_state)| {
47314731
update_maps_on_chan_removal!(self, &chan.context());
47324732
let closure_reason = ClosureReason::ProcessingError { err: e.clone() };
47334733
shutdown_results.push(chan.context_mut().force_shutdown(false, closure_reason));
4734+
peer_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
4735+
node_id: counterparty_node_id,
4736+
action: msgs::ErrorAction::SendErrorMessage {
4737+
msg: msgs::ErrorMessage {
4738+
channel_id,
4739+
data: "Failed to fund channel".to_owned(),
4740+
}
4741+
},
4742+
});
47344743
});
47354744
}
47364745
}

lightning/src/ln/functional_tests.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -10110,14 +10110,9 @@ fn test_non_final_funding_tx() {
1011010110
},
1011110111
_ => panic!()
1011210112
}
10113-
let events = nodes[0].node.get_and_clear_pending_events();
10114-
assert_eq!(events.len(), 1);
10115-
match events[0] {
10116-
Event::ChannelClosed { channel_id, .. } => {
10117-
assert_eq!(channel_id, temp_channel_id);
10118-
},
10119-
_ => panic!("Unexpected event"),
10120-
}
10113+
let err = "Error in transaction funding: Misuse error: Funding transaction absolute timelock is non-final".to_owned();
10114+
check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(temp_channel_id, false, ClosureReason::ProcessingError { err })]);
10115+
assert_eq!(get_err_msg(&nodes[0], &nodes[1].node.get_our_node_id()).data, "Failed to fund channel");
1012110116
}
1012210117

1012310118
#[test]

lightning/src/ln/shutdown_tests.rs

+14-5
Original file line numberDiff line numberDiff line change
@@ -1430,23 +1430,32 @@ fn batch_funding_failure() {
14301430
nodes[0].node.batch_funding_transaction_generated(&chans, tx).unwrap_err();
14311431

14321432
let msgs = nodes[0].node.get_and_clear_pending_msg_events();
1433-
assert_eq!(msgs.len(), 2);
1433+
assert_eq!(msgs.len(), 3);
14341434
// We currently spuriously send `FundingCreated` for the first channel and then immediately
14351435
// fail both channels, which isn't ideal but should be fine.
14361436
assert!(msgs.iter().any(|msg| {
14371437
if let MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage {
14381438
msg: msgs::ErrorMessage { channel_id, .. }, ..
14391439
}, .. } = msg {
1440-
assert_eq!(*channel_id, temp_chan_id_b);
1441-
true
1440+
*channel_id == temp_chan_id_b
14421441
} else { false }
14431442
}));
1444-
assert!(msgs.iter().any(|msg| {
1443+
let funding_created_pos = msgs.iter().position(|msg| {
14451444
if let MessageSendEvent::SendFundingCreated { msg: msgs::FundingCreated { temporary_channel_id, .. }, .. } = msg {
14461445
assert_eq!(*temporary_channel_id, temp_chan_id_a);
14471446
true
14481447
} else { false }
1449-
}));
1448+
}).unwrap();
1449+
let funded_channel_close_pos = msgs.iter().position(|msg| {
1450+
if let MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage {
1451+
msg: msgs::ErrorMessage { channel_id, .. }, ..
1452+
}, .. } = msg {
1453+
*channel_id == post_funding_chan_id_a
1454+
} else { false }
1455+
}).unwrap();
1456+
1457+
// The error message uses the funded channel_id so must come after the funding_created
1458+
assert!(funded_channel_close_pos > funding_created_pos);
14501459

14511460
check_closed_events(&nodes[0], &close);
14521461
assert_eq!(nodes[0].node.list_channels().len(), 0);

0 commit comments

Comments
 (0)