Skip to content

Commit e2831cf

Browse files
TheBlueMattTheBlueMatt
committed
Rewrite first manual-broadcast test to test several more cases
This rewrites `test_manual_broadcast_skips_commitment_until_funding` to test manual-broadcast channel closing for both forced-broadcast and close-due-to-HTLC-expiry closes as well as 0-conf and non-0-conf channels.
1 parent a979c33 commit e2831cf

File tree

2 files changed

+150
-52
lines changed

2 files changed

+150
-52
lines changed

‎lightning/src/ln/functional_test_utils.rs‎

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,14 +1576,10 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(
15761576
open_zero_conf_channel_with_value(initiator, receiver, initiator_config, 100_000, 10_001)
15771577
}
15781578

1579-
// Receiver must have been initialized with manually_accept_inbound_channels set to true.
1580-
pub fn open_zero_conf_channel_with_value<'a, 'b, 'c, 'd>(
1579+
pub fn exchange_open_accept_zero_conf_chan<'a, 'b, 'c, 'd>(
15811580
initiator: &'a Node<'b, 'c, 'd>, receiver: &'a Node<'b, 'c, 'd>,
15821581
initiator_config: Option<UserConfig>, channel_value_sat: u64, push_msat: u64,
1583-
) -> (bitcoin::Transaction, ChannelId) {
1584-
let initiator_channels = initiator.node.list_usable_channels().len();
1585-
let receiver_channels = receiver.node.list_usable_channels().len();
1586-
1582+
) -> ChannelId {
15871583
let receiver_node_id = receiver.node.get_our_node_id();
15881584
let initiator_node_id = initiator.node.get_our_node_id();
15891585

@@ -1617,6 +1613,28 @@ pub fn open_zero_conf_channel_with_value<'a, 'b, 'c, 'd>(
16171613
assert_eq!(accept_channel.common_fields.minimum_depth, 0);
16181614
initiator.node.handle_accept_channel(receiver_node_id, &accept_channel);
16191615

1616+
accept_channel.common_fields.temporary_channel_id
1617+
}
1618+
1619+
// Receiver must have been initialized with manually_accept_inbound_channels set to true.
1620+
pub fn open_zero_conf_channel_with_value<'a, 'b, 'c, 'd>(
1621+
initiator: &'a Node<'b, 'c, 'd>, receiver: &'a Node<'b, 'c, 'd>,
1622+
initiator_config: Option<UserConfig>, channel_value_sat: u64, push_msat: u64,
1623+
) -> (bitcoin::Transaction, ChannelId) {
1624+
let initiator_channels = initiator.node.list_usable_channels().len();
1625+
let receiver_channels = receiver.node.list_usable_channels().len();
1626+
1627+
let receiver_node_id = receiver.node.get_our_node_id();
1628+
let initiator_node_id = initiator.node.get_our_node_id();
1629+
1630+
exchange_open_accept_zero_conf_chan(
1631+
initiator,
1632+
receiver,
1633+
initiator_config,
1634+
channel_value_sat,
1635+
push_msat,
1636+
);
1637+
16201638
let (temporary_channel_id, tx, _) =
16211639
create_funding_transaction(&initiator, &receiver_node_id, channel_value_sat, 42);
16221640
initiator
@@ -1806,14 +1824,18 @@ pub fn create_chan_between_nodes_with_value_a<'a, 'b, 'c: 'd, 'd>(
18061824

18071825
pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>(
18081826
nodes: &'a Vec<Node<'b, 'c, 'd>>, initiator: usize, counterparty: usize, channel_value: u64,
1809-
push_msat: u64,
1827+
push_msat: u64, zero_conf: bool,
18101828
) -> (ChannelId, Transaction, OutPoint) {
18111829
let node_a = &nodes[initiator];
18121830
let node_b = &nodes[counterparty];
18131831
let node_a_id = node_a.node.get_our_node_id();
18141832
let node_b_id = node_b.node.get_our_node_id();
18151833

1816-
let temp_channel_id = exchange_open_accept_chan(node_a, node_b, channel_value, push_msat);
1834+
let temp_channel_id = if zero_conf {
1835+
exchange_open_accept_zero_conf_chan(node_a, node_b, None, channel_value, push_msat)
1836+
} else {
1837+
exchange_open_accept_chan(node_a, node_b, channel_value, push_msat)
1838+
};
18171839

18181840
let (funding_temp_id, funding_tx, funding_outpoint) =
18191841
create_funding_transaction(node_a, &node_b_id, channel_value, 42);
@@ -1834,12 +1856,39 @@ pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>(
18341856
check_added_monitors!(node_b, 1);
18351857
let channel_id_b = expect_channel_pending_event(node_b, &node_a_id);
18361858

1837-
let funding_signed = get_event_msg!(node_b, MessageSendEvent::SendFundingSigned, node_a_id);
1838-
node_a.node.handle_funding_signed(node_b_id, &funding_signed);
1839-
check_added_monitors!(node_a, 1);
1859+
if zero_conf {
1860+
let bs_signed_locked = node_b.node.get_and_clear_pending_msg_events();
1861+
assert_eq!(bs_signed_locked.len(), 2);
1862+
match &bs_signed_locked[0] {
1863+
MessageSendEvent::SendFundingSigned { node_id, msg } => {
1864+
assert_eq!(*node_id, node_a_id);
1865+
node_a.node.handle_funding_signed(node_b_id, &msg);
1866+
check_added_monitors(node_a, 1);
1867+
1868+
assert!(node_a.tx_broadcaster.txn_broadcast().is_empty());
1869+
1870+
let as_channel_ready =
1871+
get_event_msg!(node_a, MessageSendEvent::SendChannelReady, node_b_id);
1872+
node_b.node.handle_channel_ready(node_a_id, &as_channel_ready);
1873+
expect_channel_ready_event(node_b, &node_a_id);
1874+
},
1875+
_ => panic!("Unexpected event"),
1876+
}
1877+
match &bs_signed_locked[1] {
1878+
MessageSendEvent::SendChannelReady { node_id, msg } => {
1879+
assert_eq!(*node_id, node_a_id);
1880+
node_a.node.handle_channel_ready(node_b_id, &msg);
1881+
},
1882+
_ => panic!("Unexpected event"),
1883+
}
1884+
} else {
1885+
let funding_signed = get_event_msg!(node_b, MessageSendEvent::SendFundingSigned, node_a_id);
1886+
node_a.node.handle_funding_signed(node_b_id, &funding_signed);
1887+
check_added_monitors(node_a, 1)
1888+
}
18401889

18411890
let events = node_a.node.get_and_clear_pending_events();
1842-
assert_eq!(events.len(), 2);
1891+
assert_eq!(events.len(), if zero_conf { 3 } else { 2 });
18431892
let funding_txid = funding_tx.compute_txid();
18441893
let mut channel_id = None;
18451894
for event in events {
@@ -1853,6 +1902,10 @@ pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>(
18531902
assert_eq!(counterparty_node_id, node_b_id);
18541903
channel_id = Some(pending_id);
18551904
},
1905+
Event::ChannelReady { channel_id: pending_id, counterparty_node_id, .. } => {
1906+
assert_eq!(counterparty_node_id, node_b_id);
1907+
channel_id = Some(pending_id);
1908+
},
18561909
_ => panic!("Unexpected event"),
18571910
}
18581911
}
@@ -1861,6 +1914,16 @@ pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>(
18611914

18621915
assert!(node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty());
18631916

1917+
if zero_conf {
1918+
let as_channel_update =
1919+
get_event_msg!(node_a, MessageSendEvent::SendChannelUpdate, node_b_id);
1920+
let bs_channel_update =
1921+
get_event_msg!(node_b, MessageSendEvent::SendChannelUpdate, node_a_id);
1922+
1923+
node_a.node.handle_channel_update(node_b_id, &bs_channel_update);
1924+
node_b.node.handle_channel_update(node_a_id, &as_channel_update);
1925+
}
1926+
18641927
(channel_id, funding_tx, funding_outpoint)
18651928
}
18661929

‎lightning/src/ln/functional_tests.rs‎

Lines changed: 75 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9629,64 +9629,79 @@ pub fn test_remove_expired_inbound_unfunded_channels() {
96299629
check_closed_event(&nodes[1], 1, reason, false, &[node_a_id], 100000);
96309630
}
96319631

9632-
#[test]
9633-
fn test_manual_broadcast_skips_commitment_until_funding_seen() {
9632+
fn do_test_manual_broadcast_skips_commitment_until_funding(
9633+
force_broadcast: bool, close_by_timeout: bool, zero_conf_open: bool,
9634+
) {
9635+
// Checks that commitment (and HTLC) transactions will not be broadcast for manual-funded
9636+
// channels until either the funding transaction is seen on-chain or the channel is manually
9637+
// forced to broadcast using `ChannelMonitor::broadcast_latest_holder_commitment_txn`.
96349638
let chanmon_cfgs = create_chanmon_cfgs(2);
96359639
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
9636-
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
9640+
let mut chan_config = test_default_channel_config();
9641+
if zero_conf_open {
9642+
chan_config.manually_accept_inbound_channels = true;
9643+
}
9644+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]);
96379645
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
96389646

9647+
let node_a_id = nodes[0].node.get_our_node_id();
96399648
let node_b_id = nodes[1].node.get_our_node_id();
96409649

96419650
let (channel_id, funding_tx, funding_outpoint) =
9642-
create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000);
9651+
create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000, zero_conf_open);
96439652

9644-
nodes[0]
9645-
.node
9646-
.force_close_broadcasting_latest_txn(&channel_id, &node_b_id, "manual close".to_owned())
9647-
.unwrap();
9648-
check_added_monitors!(&nodes[0], 1);
9649-
check_added_monitors!(&nodes[1], 0);
9650-
9651-
assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty());
9652-
nodes[0].node.get_and_clear_pending_msg_events();
9653-
nodes[1].node.get_and_clear_pending_msg_events();
9654-
let events = nodes[0].node.get_and_clear_pending_events();
9655-
assert_eq!(events.len(), 1);
9656-
match &events[0] {
9657-
Event::ChannelClosed {
9658-
reason: ClosureReason::HolderForceClosed { broadcasted_latest_txn, message },
9659-
counterparty_node_id: Some(id),
9660-
..
9661-
} => {
9662-
assert_eq!(*broadcasted_latest_txn, Some(true));
9663-
assert_eq!(message, "manual close");
9664-
assert_eq!(id, &node_b_id);
9665-
},
9666-
_ => panic!("Unexpected event"),
9653+
if close_by_timeout {
9654+
if !zero_conf_open {
9655+
panic!("Cant send a payment if we didn't open 0-conf");
9656+
}
9657+
let (_payment_preimage, payment_hash, _payment_secret, _payment_id) =
9658+
route_payment(&nodes[0], &[&nodes[1]], 10_000_000);
9659+
nodes[1].node.get_and_clear_pending_events();
9660+
9661+
connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
9662+
let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
9663+
check_closed_event(&nodes[0], 1, reason, false, &[node_b_id], 100_000);
9664+
9665+
// On timeout, B will try to fail the HTLC back, but its too late - A has already FC'd.
9666+
connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
9667+
let failure = HTLCHandlingFailureType::Receive { payment_hash };
9668+
expect_and_process_pending_htlcs_and_htlc_handling_failed(&nodes[1], &[failure]);
9669+
get_htlc_update_msgs(&nodes[1], &node_a_id);
9670+
check_added_monitors(&nodes[1], 1);
9671+
} else {
9672+
let msg = "manual close".to_owned();
9673+
nodes[0]
9674+
.node
9675+
.force_close_broadcasting_latest_txn(&channel_id, &node_b_id, msg.clone())
9676+
.unwrap();
9677+
let reason =
9678+
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message: msg };
9679+
check_closed_event(&nodes[0], 1, reason, false, &[node_b_id], 100_000);
96679680
}
9668-
nodes[1].node.get_and_clear_pending_events();
9681+
check_added_monitors(&nodes[0], 1);
9682+
assert_eq!(get_err_msg(&nodes[0], &node_b_id).channel_id, channel_id);
9683+
assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty());
96699684

96709685
let monitor_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
96719686
assert!(monitor_events.is_empty());
96729687

9673-
confirm_transaction(&nodes[0], &funding_tx);
9674-
confirm_transaction(&nodes[1], &funding_tx);
9675-
nodes[0].node.get_and_clear_pending_msg_events();
9676-
nodes[1].node.get_and_clear_pending_msg_events();
9677-
9678-
{
9688+
// The funding tx should be broadcasted after either a manual broadcast call or if the funding
9689+
// transaction appears on chain.
9690+
if force_broadcast {
96799691
let monitor = get_monitor!(&nodes[0], channel_id);
9680-
// manual override
96819692
monitor.broadcast_latest_holder_commitment_txn(
96829693
&nodes[0].tx_broadcaster,
96839694
&nodes[0].fee_estimator,
96849695
&nodes[0].logger,
96859696
);
9697+
} else {
9698+
mine_transaction(&nodes[0], &funding_tx);
9699+
mine_transaction(&nodes[1], &funding_tx);
96869700
}
9701+
96879702
let funding_txid = funding_tx.compute_txid();
96889703
let broadcasts = nodes[0].tx_broadcaster.txn_broadcast();
9689-
assert!(!broadcasts.is_empty());
9704+
assert_eq!(broadcasts.len(), if close_by_timeout { 2 } else { 1 });
96909705
let commitment_tx = broadcasts
96919706
.iter()
96929707
.find(|tx| {
@@ -9702,10 +9717,30 @@ fn test_manual_broadcast_skips_commitment_until_funding_seen() {
97029717
assert_eq!(commitment_input.previous_output.txid, funding_txid);
97039718
assert_eq!(commitment_input.previous_output.vout, u32::from(funding_outpoint.index));
97049719

9720+
if close_by_timeout {
9721+
let htlc_tx = broadcasts
9722+
.iter()
9723+
.find(|tx| {
9724+
tx.input
9725+
.iter()
9726+
.any(|input| input.previous_output.txid == commitment_tx.compute_txid())
9727+
})
9728+
.expect("HTLC claim transaction not broadcast");
9729+
check_spends!(htlc_tx, commitment_tx);
9730+
}
9731+
97059732
let monitor_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
97069733
assert!(monitor_events.iter().all(|event| !matches!(event, Event::BumpTransaction(_))));
9707-
assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
9708-
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
9734+
}
9735+
9736+
#[test]
9737+
fn test_manual_broadcast_skips_commitment_until_funding() {
9738+
do_test_manual_broadcast_skips_commitment_until_funding(true, true, true);
9739+
do_test_manual_broadcast_skips_commitment_until_funding(true, false, true);
9740+
do_test_manual_broadcast_skips_commitment_until_funding(true, false, false);
9741+
do_test_manual_broadcast_skips_commitment_until_funding(false, true, true);
9742+
do_test_manual_broadcast_skips_commitment_until_funding(false, false, true);
9743+
do_test_manual_broadcast_skips_commitment_until_funding(false, false, false);
97099744
}
97109745

97119746
#[test]
@@ -9718,7 +9753,7 @@ fn test_manual_broadcast_detects_funding_and_broadcasts_on_timeout() {
97189753
let node_b_id = nodes[1].node.get_our_node_id();
97199754

97209755
let (channel_id, funding_tx, funding_outpoint) =
9721-
create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000);
9756+
create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000, false);
97229757

97239758
let funding_msgs =
97249759
create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &funding_tx);
@@ -9793,7 +9828,7 @@ fn test_manual_broadcast_no_bump_events_before_funding_seen() {
97939828

97949829
let node_b_id = nodes[1].node.get_our_node_id();
97959830

9796-
let (channel_id, _, _) = create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000);
9831+
let (channel_id, _, _) = create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000, false);
97979832

97989833
nodes[0]
97999834
.node

0 commit comments

Comments
 (0)