Skip to content

Commit 01ba5aa

Browse files
TheBlueMattTheBlueMatt
committed
Test reconnecting after lost message(s) during the commitment dance
1 parent 17375b0 commit 01ba5aa

File tree

1 file changed

+249
-20
lines changed

1 file changed

+249
-20
lines changed

‎src/ln/channelmanager.rs‎

Lines changed: 249 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4674,15 +4674,17 @@ mod tests {
46744674
assert_eq!(channel_state.short_to_id.len(), 0);
46754675
}
46764676

4677-
fn reconnect_nodes(node_a: &Node, node_b: &Node, pre_all_htlcs: bool, pending_htlc_claims: (usize, usize), pending_htlc_fails: (usize, usize)) {
4677+
/// pending_htlc_adds includes both the holding cell and in-flight update_add_htlcs, whereas
4678+
/// for claims/fails they are separated out.
4679+
fn reconnect_nodes(node_a: &Node, node_b: &Node, pre_all_htlcs: bool, pending_htlc_adds: (i64, i64), pending_htlc_claims: (usize, usize), pending_cell_htlc_claims: (usize, usize), pending_cell_htlc_fails: (usize, usize), pending_raa: (bool, bool)) {
46784680
let reestablish_1 = node_a.node.peer_connected(&node_b.node.get_our_node_id());
46794681
let reestablish_2 = node_b.node.peer_connected(&node_a.node.get_our_node_id());
46804682

46814683
let mut resp_1 = Vec::new();
46824684
for msg in reestablish_1 {
46834685
resp_1.push(node_b.node.handle_channel_reestablish(&node_a.node.get_our_node_id(), &msg).unwrap());
46844686
}
4685-
if pending_htlc_claims.0 != 0 || pending_htlc_fails.0 != 0 {
4687+
if pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 {
46864688
check_added_monitors!(node_b, 1);
46874689
} else {
46884690
check_added_monitors!(node_b, 0);
@@ -4692,37 +4694,59 @@ mod tests {
46924694
for msg in reestablish_2 {
46934695
resp_2.push(node_a.node.handle_channel_reestablish(&node_b.node.get_our_node_id(), &msg).unwrap());
46944696
}
4695-
if pending_htlc_claims.1 != 0 || pending_htlc_fails.1 != 0 {
4697+
if pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 {
46964698
check_added_monitors!(node_a, 1);
46974699
} else {
46984700
check_added_monitors!(node_a, 0);
46994701
}
47004702

47014703
// We dont yet support both needing updates, as that would require a different commitment dance:
4702-
assert!((pending_htlc_claims.0 == 0 && pending_htlc_fails.0 == 0) || (pending_htlc_claims.1 == 0 && pending_htlc_fails.1 == 0));
4704+
assert!((pending_htlc_adds.0 == 0 && pending_htlc_claims.0 == 0 && pending_cell_htlc_claims.0 == 0 && pending_cell_htlc_fails.0 == 0) ||
4705+
(pending_htlc_adds.1 == 0 && pending_htlc_claims.1 == 0 && pending_cell_htlc_claims.1 == 0 && pending_cell_htlc_fails.1 == 0));
47034706

47044707
for chan_msgs in resp_1.drain(..) {
47054708
if pre_all_htlcs {
4706-
let _announcement_sigs_opt = node_a.node.handle_funding_locked(&node_b.node.get_our_node_id(), &chan_msgs.0.unwrap()).unwrap();
4709+
let a = node_a.node.handle_funding_locked(&node_b.node.get_our_node_id(), &chan_msgs.0.unwrap());
4710+
let _announcement_sigs_opt = a.unwrap();
47074711
//TODO: Test announcement_sigs re-sending when we've implemented it
47084712
} else {
47094713
assert!(chan_msgs.0.is_none());
47104714
}
4711-
assert!(chan_msgs.1.is_none());
4712-
if pending_htlc_claims.0 != 0 || pending_htlc_fails.0 != 0 {
4715+
if pending_raa.0 {
4716+
assert!(node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &chan_msgs.1.unwrap()).unwrap().is_none());
4717+
check_added_monitors!(node_a, 1);
4718+
} else {
4719+
assert!(chan_msgs.1.is_none());
4720+
}
4721+
if pending_htlc_adds.0 != 0 || pending_htlc_claims.0 != 0 || pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 {
47134722
let commitment_update = chan_msgs.2.unwrap();
4714-
assert!(commitment_update.update_add_htlcs.is_empty()); // We can't relay while disconnected
4715-
assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.0);
4716-
assert_eq!(commitment_update.update_fail_htlcs.len(), pending_htlc_fails.0);
4723+
if pending_htlc_adds.0 != -1 { // We use -1 to denote a response commitment_signed
4724+
assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.0 as usize);
4725+
} else {
4726+
assert!(commitment_update.update_add_htlcs.is_empty());
4727+
}
4728+
assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.0 + pending_cell_htlc_claims.0);
4729+
assert_eq!(commitment_update.update_fail_htlcs.len(), pending_cell_htlc_fails.0);
47174730
assert!(commitment_update.update_fail_malformed_htlcs.is_empty());
4731+
for update_add in commitment_update.update_add_htlcs {
4732+
node_a.node.handle_update_add_htlc(&node_b.node.get_our_node_id(), &update_add).unwrap();
4733+
}
47184734
for update_fulfill in commitment_update.update_fulfill_htlcs {
47194735
node_a.node.handle_update_fulfill_htlc(&node_b.node.get_our_node_id(), &update_fulfill).unwrap();
47204736
}
47214737
for update_fail in commitment_update.update_fail_htlcs {
47224738
node_a.node.handle_update_fail_htlc(&node_b.node.get_our_node_id(), &update_fail).unwrap();
47234739
}
47244740

4725-
commitment_signed_dance!(node_a, node_b, commitment_update.commitment_signed, false);
4741+
if pending_htlc_adds.0 != -1 { // We use -1 to denote a response commitment_signed
4742+
commitment_signed_dance!(node_a, node_b, commitment_update.commitment_signed, false);
4743+
} else {
4744+
let (as_revoke_and_ack, as_commitment_signed) = node_a.node.handle_commitment_signed(&node_b.node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
4745+
check_added_monitors!(node_a, 1);
4746+
assert!(as_commitment_signed.is_none());
4747+
assert!(node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
4748+
check_added_monitors!(node_b, 1);
4749+
}
47264750
} else {
47274751
assert!(chan_msgs.2.is_none());
47284752
}
@@ -4735,21 +4759,39 @@ mod tests {
47354759
} else {
47364760
assert!(chan_msgs.0.is_none());
47374761
}
4738-
assert!(chan_msgs.1.is_none());
4739-
if pending_htlc_claims.1 != 0 || pending_htlc_fails.1 != 0 {
4762+
if pending_raa.1 {
4763+
assert!(node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &chan_msgs.1.unwrap()).unwrap().is_none());
4764+
check_added_monitors!(node_b, 1);
4765+
} else {
4766+
assert!(chan_msgs.1.is_none());
4767+
}
4768+
if pending_htlc_adds.1 != 0 || pending_htlc_claims.1 != 0 || pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 {
47404769
let commitment_update = chan_msgs.2.unwrap();
4741-
assert!(commitment_update.update_add_htlcs.is_empty()); // We can't relay while disconnected
4742-
assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.0);
4743-
assert_eq!(commitment_update.update_fail_htlcs.len(), pending_htlc_fails.0);
4770+
if pending_htlc_adds.1 != -1 { // We use -1 to denote a response commitment_signed
4771+
assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.1 as usize);
4772+
}
4773+
assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.0 + pending_cell_htlc_claims.0);
4774+
assert_eq!(commitment_update.update_fail_htlcs.len(), pending_cell_htlc_fails.0);
47444775
assert!(commitment_update.update_fail_malformed_htlcs.is_empty());
4776+
for update_add in commitment_update.update_add_htlcs {
4777+
node_b.node.handle_update_add_htlc(&node_a.node.get_our_node_id(), &update_add).unwrap();
4778+
}
47454779
for update_fulfill in commitment_update.update_fulfill_htlcs {
47464780
node_b.node.handle_update_fulfill_htlc(&node_a.node.get_our_node_id(), &update_fulfill).unwrap();
47474781
}
47484782
for update_fail in commitment_update.update_fail_htlcs {
47494783
node_b.node.handle_update_fail_htlc(&node_a.node.get_our_node_id(), &update_fail).unwrap();
47504784
}
47514785

4752-
commitment_signed_dance!(node_b, node_a, commitment_update.commitment_signed, false);
4786+
if pending_htlc_adds.1 != -1 { // We use -1 to denote a response commitment_signed
4787+
commitment_signed_dance!(node_b, node_a, commitment_update.commitment_signed, false);
4788+
} else {
4789+
let (bs_revoke_and_ack, bs_commitment_signed) = node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
4790+
check_added_monitors!(node_b, 1);
4791+
assert!(bs_commitment_signed.is_none());
4792+
assert!(node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
4793+
check_added_monitors!(node_a, 1);
4794+
}
47534795
} else {
47544796
assert!(chan_msgs.2.is_none());
47554797
}
@@ -4765,7 +4807,7 @@ mod tests {
47654807

47664808
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
47674809
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
4768-
reconnect_nodes(&nodes[0], &nodes[1], true, (0, 0), (0, 0));
4810+
reconnect_nodes(&nodes[0], &nodes[1], true, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
47694811

47704812
let payment_preimage_1 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
47714813
let payment_hash_2 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).1;
@@ -4774,7 +4816,7 @@ mod tests {
47744816

47754817
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
47764818
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
4777-
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0));
4819+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
47784820

47794821
let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
47804822
let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 1000000).0;
@@ -4787,7 +4829,7 @@ mod tests {
47874829
claim_payment_along_route(&nodes[0], &vec!(&nodes[1], &nodes[2]), true, payment_preimage_3);
47884830
fail_payment_along_route(&nodes[0], &[&nodes[1], &nodes[2]], true, payment_hash_5);
47894831

4790-
reconnect_nodes(&nodes[0], &nodes[1], false, (1, 0), (1, 0));
4832+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (1, 0), (1, 0), (false, false));
47914833
{
47924834
let events = nodes[0].node.get_and_clear_pending_events();
47934835
assert_eq!(events.len(), 2);
@@ -4809,6 +4851,193 @@ mod tests {
48094851
fail_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), payment_hash_6);
48104852
}
48114853

4854+
fn do_test_drop_messages_peer_disconnect(messages_delivered: u8) {
4855+
// Test that we can reconnect when in-flight HTLC updates get dropped
4856+
let mut nodes = create_network(2);
4857+
if messages_delivered == 0 {
4858+
create_chan_between_nodes_with_value_a(&nodes[0], &nodes[1], 100000, 10001);
4859+
// nodes[1] doesn't receive the funding_locked message (it'll be re-sent on reconnect)
4860+
} else {
4861+
create_announced_chan_between_nodes(&nodes, 0, 1);
4862+
}
4863+
4864+
let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), Some(&nodes[0].node.list_usable_channels()), &Vec::new(), 1000000, TEST_FINAL_CLTV).unwrap();
4865+
let (payment_preimage_1, payment_hash_1) = get_payment_preimage_hash!(nodes[0]);
4866+
4867+
let payment_event = {
4868+
nodes[0].node.send_payment(route.clone(), payment_hash_1).unwrap();
4869+
check_added_monitors!(nodes[0], 1);
4870+
4871+
let mut events = nodes[0].node.get_and_clear_pending_events();
4872+
assert_eq!(events.len(), 1);
4873+
SendEvent::from_event(events.remove(0))
4874+
};
4875+
assert_eq!(nodes[1].node.get_our_node_id(), payment_event.node_id);
4876+
4877+
if messages_delivered < 2 {
4878+
// Drop the payment_event messages, and let them get re-generated in reconnect_nodes!
4879+
} else {
4880+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
4881+
let (bs_revoke_and_ack, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
4882+
check_added_monitors!(nodes[1], 1);
4883+
4884+
if messages_delivered >= 3 {
4885+
assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
4886+
check_added_monitors!(nodes[0], 1);
4887+
4888+
if messages_delivered >= 4 {
4889+
let (as_revoke_and_ack, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_commitment_signed.unwrap()).unwrap();
4890+
assert!(as_commitment_signed.is_none());
4891+
check_added_monitors!(nodes[0], 1);
4892+
4893+
if messages_delivered >= 5 {
4894+
assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
4895+
check_added_monitors!(nodes[1], 1);
4896+
}
4897+
}
4898+
}
4899+
}
4900+
4901+
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
4902+
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
4903+
if messages_delivered < 2 {
4904+
// Even if the funding_locked messages get exchanged, as long as nothing further was
4905+
// received on either side, both sides will need to resend them.
4906+
reconnect_nodes(&nodes[0], &nodes[1], true, (0, 1), (0, 0), (0, 0), (0, 0), (false, false));
4907+
} else if messages_delivered == 2 {
4908+
// nodes[0] still wants its RAA + commitment_signed
4909+
reconnect_nodes(&nodes[0], &nodes[1], false, (-1, 0), (0, 0), (0, 0), (0, 0), (true, false));
4910+
} else if messages_delivered == 3 {
4911+
// nodes[0] still wants its commitment_signed
4912+
reconnect_nodes(&nodes[0], &nodes[1], false, (-1, 0), (0, 0), (0, 0), (0, 0), (false, false));
4913+
} else if messages_delivered == 4 {
4914+
// nodes[1] still wants its final RAA
4915+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, true));
4916+
} else if messages_delivered == 5 {
4917+
// Everything was delivered...
4918+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
4919+
}
4920+
4921+
let events_1 = nodes[1].node.get_and_clear_pending_events();
4922+
assert_eq!(events_1.len(), 1);
4923+
match events_1[0] {
4924+
Event::PendingHTLCsForwardable { .. } => { },
4925+
_ => panic!("Unexpected event"),
4926+
};
4927+
4928+
nodes[1].node.channel_state.lock().unwrap().next_forward = Instant::now();
4929+
nodes[1].node.process_pending_htlc_forwards();
4930+
4931+
let events_2 = nodes[1].node.get_and_clear_pending_events();
4932+
assert_eq!(events_2.len(), 1);
4933+
match events_2[0] {
4934+
Event::PaymentReceived { ref payment_hash, amt } => {
4935+
assert_eq!(payment_hash_1, *payment_hash);
4936+
assert_eq!(amt, 1000000);
4937+
},
4938+
_ => panic!("Unexpected event"),
4939+
}
4940+
4941+
nodes[1].node.claim_funds(payment_preimage_1);
4942+
check_added_monitors!(nodes[1], 1);
4943+
4944+
let events_3 = nodes[1].node.get_and_clear_pending_events();
4945+
assert_eq!(events_3.len(), 1);
4946+
let (update_fulfill_htlc, commitment_signed) = match events_3[0] {
4947+
Event::UpdateHTLCs { ref node_id, ref updates } => {
4948+
assert_eq!(*node_id, nodes[0].node.get_our_node_id());
4949+
assert!(updates.update_add_htlcs.is_empty());
4950+
assert!(updates.update_fail_htlcs.is_empty());
4951+
assert_eq!(updates.update_fulfill_htlcs.len(), 1);
4952+
assert!(updates.update_fail_malformed_htlcs.is_empty());
4953+
assert!(updates.update_fee.is_none());
4954+
(updates.update_fulfill_htlcs[0].clone(), updates.commitment_signed.clone())
4955+
},
4956+
_ => panic!("Unexpected event"),
4957+
};
4958+
4959+
if messages_delivered >= 1 {
4960+
nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &update_fulfill_htlc).unwrap();
4961+
4962+
let events_4 = nodes[0].node.get_and_clear_pending_events();
4963+
assert_eq!(events_4.len(), 1);
4964+
match events_4[0] {
4965+
Event::PaymentSent { ref payment_preimage } => {
4966+
assert_eq!(payment_preimage_1, *payment_preimage);
4967+
},
4968+
_ => panic!("Unexpected event"),
4969+
}
4970+
4971+
if messages_delivered >= 2 {
4972+
let (as_revoke_and_ack, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
4973+
check_added_monitors!(nodes[0], 1);
4974+
4975+
if messages_delivered >= 3 {
4976+
assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
4977+
check_added_monitors!(nodes[1], 1);
4978+
4979+
if messages_delivered >= 4 {
4980+
let (bs_revoke_and_ack, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_commitment_signed.unwrap()).unwrap();
4981+
assert!(bs_commitment_signed.is_none());
4982+
check_added_monitors!(nodes[1], 1);
4983+
4984+
if messages_delivered >= 5 {
4985+
assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
4986+
check_added_monitors!(nodes[0], 1);
4987+
}
4988+
}
4989+
}
4990+
}
4991+
}
4992+
4993+
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
4994+
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
4995+
if messages_delivered < 2 {
4996+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (1, 0), (0, 0), (0, 0), (false, false));
4997+
//TODO: Deduplicate PaymentSent events, then enable this if:
4998+
//if messages_delivered < 1 {
4999+
let events_4 = nodes[0].node.get_and_clear_pending_events();
5000+
assert_eq!(events_4.len(), 1);
5001+
match events_4[0] {
5002+
Event::PaymentSent { ref payment_preimage } => {
5003+
assert_eq!(payment_preimage_1, *payment_preimage);
5004+
},
5005+
_ => panic!("Unexpected event"),
5006+
}
5007+
//}
5008+
} else if messages_delivered == 2 {
5009+
// nodes[0] still wants its RAA + commitment_signed
5010+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, -1), (0, 0), (0, 0), (0, 0), (false, true));
5011+
} else if messages_delivered == 3 {
5012+
// nodes[0] still wants its commitment_signed
5013+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, -1), (0, 0), (0, 0), (0, 0), (false, false));
5014+
} else if messages_delivered == 4 {
5015+
// nodes[1] still wants its final RAA
5016+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (true, false));
5017+
} else if messages_delivered == 5 {
5018+
// Everything was delivered...
5019+
reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
5020+
}
5021+
5022+
// Channel should still work fine...
5023+
let payment_preimage_2 = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000).0;
5024+
claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
5025+
}
5026+
5027+
#[test]
5028+
fn test_drop_messages_peer_disconnect_a() {
5029+
do_test_drop_messages_peer_disconnect(0);
5030+
do_test_drop_messages_peer_disconnect(1);
5031+
do_test_drop_messages_peer_disconnect(2);
5032+
}
5033+
5034+
#[test]
5035+
fn test_drop_messages_peer_disconnect_b() {
5036+
do_test_drop_messages_peer_disconnect(3);
5037+
do_test_drop_messages_peer_disconnect(4);
5038+
do_test_drop_messages_peer_disconnect(5);
5039+
}
5040+
48125041
#[test]
48135042
fn test_invalid_channel_announcement() {
48145043
//Test BOLT 7 channel_announcement msg requirement for final node, gather data to build customed channel_announcement msgs

0 commit comments

Comments
 (0)