Skip to content

Commit 183fd66

Browse files
wpaulinowpaulino
committed
Enable decoding new incoming HTLC onions when fully committed
This commit ensures all new incoming HTLCs going forward will have their onion decoded when they become fully committed to decide how we should proceed with each one. As a result, we'll obtain `HTLCHandlingFailed` events for _any_ failed HTLC that comes across a channel. We will now start writing channels with the new serialization version (4), and we will still be able to downgrade back to the commit that introduced it since reading version 4 is supported. Note that existing pending inbound HTLCs may already have their resolution if they were received in a previous version of LDK. We must support those until we no longer allow downgrading beyond this commit.
1 parent 19ef855 commit 183fd66

12 files changed

+338
-283
lines changed

‎fuzz/src/full_stack.rs

+12-12
Original file line numberDiff line numberDiff line change
@@ -1344,8 +1344,8 @@ mod tests {
13441344
// end of update_add_htlc from 0 to 1 via client and mac
13451345
ext_from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ab00000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
13461346

1347-
// Two feerate requests to check dust exposure
1348-
ext_from_hex("00fd00fd", &mut test);
1347+
// One feerate request to check dust exposure
1348+
ext_from_hex("00fd", &mut test);
13491349

13501350
// inbound read from peer id 0 of len 18
13511351
ext_from_hex("030012", &mut test);
@@ -1368,8 +1368,8 @@ mod tests {
13681368

13691369
// process the now-pending HTLC forward
13701370
ext_from_hex("07", &mut test);
1371-
// Three feerate requests to check dust exposure
1372-
ext_from_hex("00fd00fd00fd", &mut test);
1371+
// Four feerate requests to check dust exposure while forwarding the HTLC
1372+
ext_from_hex("00fd00fd00fd00fd", &mut test);
13731373
// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7: UpdateHTLCs event for node 03020000 with 1 HTLCs for channel 3f000000)
13741374

13751375
// we respond with commitment_signed then revoke_and_ack (a weird, but valid, order)
@@ -1445,8 +1445,8 @@ mod tests {
14451445
// end of update_add_htlc from 0 to 1 via client and mac
14461446
ext_from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ab00000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
14471447

1448-
// Two feerate requests to check dust exposure
1449-
ext_from_hex("00fd00fd", &mut test);
1448+
// One feerate request to check dust exposure
1449+
ext_from_hex("00fd", &mut test);
14501450

14511451
// now respond to the update_fulfill_htlc+commitment_signed messages the client sent to peer 0
14521452
// inbound read from peer id 0 of len 18
@@ -1480,8 +1480,8 @@ mod tests {
14801480
// process the now-pending HTLC forward
14811481
ext_from_hex("07", &mut test);
14821482

1483-
// Three feerate requests to check dust exposure
1484-
ext_from_hex("00fd00fd00fd", &mut test);
1483+
// Four feerate requests to check dust exposure while forwarding the HTLC
1484+
ext_from_hex("00fd00fd00fd00fd", &mut test);
14851485

14861486
// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7 duplicate)
14871487
// we respond with revoke_and_ack, then commitment_signed, then update_fail_htlc
@@ -1580,8 +1580,8 @@ mod tests {
15801580
// end of update_add_htlc from 0 to 1 via client and mac
15811581
ext_from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff 5300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
15821582

1583-
// Two feerate requests to check dust exposure
1584-
ext_from_hex("00fd00fd", &mut test);
1583+
// One feerate request to check dust exposure
1584+
ext_from_hex("00fd", &mut test);
15851585

15861586
// inbound read from peer id 0 of len 18
15871587
ext_from_hex("030012", &mut test);
@@ -1604,8 +1604,8 @@ mod tests {
16041604

16051605
// process the now-pending HTLC forward
16061606
ext_from_hex("07", &mut test);
1607-
// Three feerate requests to check dust exposure
1608-
ext_from_hex("00fd00fd00fd", &mut test);
1607+
// Four feerate requests to check dust exposure while forwarding the HTLC
1608+
ext_from_hex("00fd00fd00fd00fd", &mut test);
16091609
// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7 duplicate)
16101610

16111611
// connect a block with one transaction of len 125

‎lightning/src/events/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1361,6 +1361,8 @@ pub enum Event {
13611361
/// * When an unknown SCID is requested for forwarding a payment.
13621362
/// * Expected MPP amount has already been reached
13631363
/// * The HTLC has timed out
1364+
/// * The HTLC failed to meet the forwarding requirements (i.e. insufficient fees paid, or a
1365+
/// CLTV that is too soon)
13641366
///
13651367
/// This event, however, does not get generated if an HTLC fails to meet the forwarding
13661368
/// requirements (i.e. insufficient fees paid, or a CLTV that is too soon).

‎lightning/src/ln/blinded_payment_tests.rs

+54-10
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,10 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) {
290290
// We need the session priv to construct a bogus onion packet later.
291291
*nodes[0].keys_manager.override_random_bytes.lock().unwrap() = Some([3; 32]);
292292
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
293-
let chan_upd_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents;
294-
let chan_upd_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).0.contents;
293+
let chan_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0);
294+
let chan_upd_1_2 = chan_1_2.0.contents;
295+
let chan_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0);
296+
let chan_upd_2_3 = chan_2_3.0.contents;
295297

296298
let amt_msat = 5000;
297299
let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[3], Some(amt_msat), None);
@@ -345,18 +347,27 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) {
345347
check_added_monitors!(nodes[1], 0);
346348
do_commitment_signed_dance(&nodes[1], &nodes[0], &updates_0_1.commitment_signed, true, true);
347349

350+
expect_pending_htlcs_forwardable!(nodes[1]);
351+
check_added_monitors!(nodes[1], 1);
352+
348353
if intro_fails {
349354
let mut updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
350355
nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
351356
do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false);
357+
let failed_destination = match check {
358+
ForwardCheckFail::InboundOnionCheck => HTLCDestination::InvalidOnion,
359+
ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::FailedPayment { payment_hash },
360+
ForwardCheckFail::OutboundChannelCheck =>
361+
HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_1_2.2 },
362+
};
363+
expect_htlc_handling_failed_destinations!(
364+
nodes[1].node.get_and_clear_pending_events(), &[failed_destination.clone()]
365+
);
352366
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
353367
PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32]));
354368
return
355369
}
356370

357-
expect_pending_htlcs_forwardable!(nodes[1]);
358-
check_added_monitors!(nodes[1], 1);
359-
360371
let mut updates_1_2 = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
361372
let mut update_add = &mut updates_1_2.update_add_htlcs[0];
362373

@@ -366,6 +377,17 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) {
366377
check_added_monitors!(nodes[2], 0);
367378
do_commitment_signed_dance(&nodes[2], &nodes[1], &updates_1_2.commitment_signed, true, true);
368379

380+
expect_pending_htlcs_forwardable!(nodes[2]);
381+
let failed_destination = match check {
382+
ForwardCheckFail::InboundOnionCheck|ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::InvalidOnion,
383+
ForwardCheckFail::OutboundChannelCheck =>
384+
HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_2_3.2 },
385+
};
386+
expect_htlc_handling_failed_destinations!(
387+
nodes[2].node.get_and_clear_pending_events(), &[failed_destination.clone()]
388+
);
389+
check_added_monitors!(nodes[2], 1);
390+
369391
let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
370392
let update_malformed = &mut updates.update_fail_malformed_htlcs[0];
371393
assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING);
@@ -425,7 +447,10 @@ fn failed_backwards_to_intro_node() {
425447
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), &payment_event.msgs[0]);
426448
check_added_monitors!(nodes[2], 0);
427449
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event.commitment_msg, true, true);
428-
nodes[2].node.process_pending_htlc_forwards();
450+
451+
expect_pending_htlcs_forwardable!(nodes[2]);
452+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::InvalidOnion]);
453+
check_added_monitors(&nodes[2], 1);
429454

430455
let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
431456
let mut update_malformed = &mut updates.update_fail_malformed_htlcs[0];
@@ -502,7 +527,7 @@ fn do_forward_fail_in_process_pending_htlc_fwds(check: ProcessPendingHTLCsCheck,
502527
// intro node will error backwards.
503528
$curr_node.node.peer_disconnected($next_node.node.get_our_node_id());
504529
expect_pending_htlcs_forwardable!($curr_node);
505-
expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!($curr_node,
530+
expect_htlc_handling_failed_destinations!($curr_node.node.get_and_clear_pending_events(),
506531
vec![HTLCDestination::NextHopChannel { node_id: Some($next_node.node.get_our_node_id()), channel_id: $failed_chan_id }]);
507532
},
508533
ProcessPendingHTLCsCheck::FwdChannelClosed => {
@@ -518,12 +543,12 @@ fn do_forward_fail_in_process_pending_htlc_fwds(check: ProcessPendingHTLCsCheck,
518543
crate::events::Event::ChannelClosed { .. } => {},
519544
_ => panic!("Unexpected event {:?}", events),
520545
}
546+
check_closed_broadcast(&$curr_node, 1, true);
547+
check_added_monitors!($curr_node, 1);
521548

522549
$curr_node.node.process_pending_htlc_forwards();
523-
expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!($curr_node,
550+
expect_htlc_handling_failed_destinations!($curr_node.node.get_and_clear_pending_events(),
524551
vec![HTLCDestination::UnknownNextHop { requested_forward_scid: $failed_scid }]);
525-
check_closed_broadcast(&$curr_node, 1, true);
526-
check_added_monitors!($curr_node, 1);
527552
$curr_node.node.process_pending_htlc_forwards();
528553
},
529554
}
@@ -609,6 +634,7 @@ fn do_blinded_intercept_payment(intercept_node_fails: bool) {
609634
};
610635
nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
611636
commitment_signed_dance!(nodes[1], nodes[0], &payment_event.commitment_msg, false, true);
637+
expect_pending_htlcs_forwardable!(nodes[1]);
612638

613639
let events = nodes[1].node.get_and_clear_pending_events();
614640
assert_eq!(events.len(), 1);
@@ -914,13 +940,19 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
914940
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), update_add);
915941
check_added_monitors!(nodes[2], 0);
916942
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true);
943+
expect_pending_htlcs_forwardable!(nodes[2]);
944+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::InvalidOnion]);
945+
check_added_monitors(&nodes[2], 1);
917946
},
918947
ReceiveCheckFail::ReceiveRequirements => {
919948
let update_add = &mut payment_event_1_2.msgs[0];
920949
update_add.amount_msat -= 1;
921950
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), update_add);
922951
check_added_monitors!(nodes[2], 0);
923952
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true);
953+
expect_pending_htlcs_forwardable!(nodes[2]);
954+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]);
955+
check_added_monitors(&nodes[2], 1);
924956
},
925957
ReceiveCheckFail::ChannelCheck => {
926958
nodes[2].node.close_channel(&chan_id_1_2, &nodes[1].node.get_our_node_id()).unwrap();
@@ -934,6 +966,9 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
934966

935967
nodes[2].node.handle_shutdown(nodes[1].node.get_our_node_id(), &node_1_shutdown);
936968
commitment_signed_dance!(nodes[2], nodes[1], (), false, true, false, false);
969+
expect_pending_htlcs_forwardable!(nodes[2]);
970+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]);
971+
check_added_monitors(&nodes[2], 1);
937972
},
938973
ReceiveCheckFail::ProcessPendingHTLCsCheck => {
939974
assert_eq!(payment_event_1_2.msgs[0].cltv_expiry, nodes[0].best_block_info().1 + 1 + excess_final_cltv_delta_opt.unwrap() as u32 + TEST_FINAL_CLTV);
@@ -949,6 +984,9 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
949984
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]);
950985
check_added_monitors!(nodes[2], 0);
951986
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true);
987+
expect_pending_htlcs_forwardable!(nodes[2]);
988+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]);
989+
check_added_monitors(&nodes[2], 1);
952990
}
953991
}
954992

@@ -1149,6 +1187,12 @@ fn min_htlc() {
11491187
nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event_0_1.msgs[0]);
11501188
check_added_monitors!(nodes[1], 0);
11511189
do_commitment_signed_dance(&nodes[1], &nodes[0], &payment_event_0_1.commitment_msg, true, true);
1190+
expect_pending_htlcs_forwardable!(nodes[1]);
1191+
expect_htlc_handling_failed_destinations!(
1192+
nodes[1].node.get_and_clear_pending_events(),
1193+
&[HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_1_2.2 }]
1194+
);
1195+
check_added_monitors(&nodes[1], 1);
11521196
let mut updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
11531197
nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
11541198
do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false);

0 commit comments

Comments
 (0)