Skip to content

Commit ec79a63

Browse files
committed
Fix deadlock when handling bad calls to batch_funding.._generated
When handling calls to `batch_funding_transaction_generated` which were missing outputs for one of the batch channels, we'd previously deadlock when trying to clean up the now-closed channels. This fixes that and adds a new test case for it. Found by the full_stack_target fuzzer.
1 parent 5592378 commit ec79a63

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed

lightning/src/ln/channelmanager.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3984,6 +3984,7 @@ where
39843984
});
39853985
}
39863986
}
3987+
mem::drop(funding_batch_states);
39873988
for shutdown_result in shutdown_results.drain(..) {
39883989
self.finish_close_channel(shutdown_result);
39893990
}

lightning/src/ln/shutdown_tests.rs

+33-2
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@
77
// You may not use this file except in accordance with one or both of these
88
// licenses.
99

10-
//! Tests of our shutdown and closing_signed negotiation logic.
10+
//! Tests of our shutdown and closing_signed negotiation logic as well as some assorted force-close
11+
//! handling tests.
1112
1213
use crate::sign::{EntropySource, SignerProvider};
1314
use crate::chain::ChannelMonitorUpdateStatus;
1415
use crate::chain::transaction::OutPoint;
15-
use crate::events::{MessageSendEvent, HTLCDestination, MessageSendEventsProvider, ClosureReason};
16+
use crate::events::{Event, MessageSendEvent, HTLCDestination, MessageSendEventsProvider, ClosureReason};
1617
use crate::ln::channelmanager::{self, PaymentSendFailure, PaymentId, RecipientOnionFields, Retry, ChannelShutdownState, ChannelDetails};
1718
use crate::routing::router::{PaymentParameters, get_route, RouteParameters};
1819
use crate::ln::msgs;
@@ -25,6 +26,8 @@ use crate::util::errors::APIError;
2526
use crate::util::config::UserConfig;
2627
use crate::util::string::UntrustedString;
2728

29+
use bitcoin::{Transaction, TxOut};
30+
use bitcoin::blockdata::locktime::absolute::LockTime;
2831
use bitcoin::blockdata::script::Builder;
2932
use bitcoin::blockdata::opcodes;
3033
use bitcoin::network::constants::Network;
@@ -1375,3 +1378,31 @@ fn outbound_update_no_early_closing_signed() {
13751378
do_outbound_update_no_early_closing_signed(true);
13761379
do_outbound_update_no_early_closing_signed(false);
13771380
}
1381+
1382+
#[test]
1383+
fn batch_funding_failure() {
1384+
// Provides test coverage of batch funding failure, which previously deadlocked
1385+
let chanmon_cfgs = create_chanmon_cfgs(4);
1386+
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
1387+
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
1388+
let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
1389+
1390+
let chan_id_a = exchange_open_accept_chan(&nodes[0], &nodes[1], 1_000_000, 0);
1391+
let chan_id_a = exchange_open_accept_chan(&nodes[0], &nodes[2], 1_000_000, 0);
1392+
1393+
let events = nodes[0].node.get_and_clear_pending_events();
1394+
assert_eq!(events.len(), 2);
1395+
// Build a transaction which only has the output for one of the two channels we're trying to
1396+
// confirm. Previously this led to a deadlock in channel closure handling.
1397+
let mut tx = Transaction { version: 2, lock_time: LockTime::ZERO, input: Vec::new(), output: Vec::new() };
1398+
let mut chans = Vec::new();
1399+
for (idx, ev) in events.iter().enumerate() {
1400+
if let Event::FundingGenerationReady { temporary_channel_id, counterparty_node_id, output_script, .. } = ev {
1401+
if idx == 0 {
1402+
tx.output.push(TxOut { value: 1_000_000, script_pubkey: output_script.clone() });
1403+
}
1404+
chans.push((temporary_channel_id, counterparty_node_id));
1405+
} else { panic!(); }
1406+
}
1407+
nodes[0].node.batch_funding_transaction_generated(&chans, tx).unwrap_err();
1408+
}

0 commit comments

Comments
 (0)