Skip to content

Commit a706159

Browse files
valentinewallacevalentinewallace
authored
Merge pull request lightningdevkit#2933 from wpaulino/enable-decode-htlc-onion-until-committed
Enable decoding HTLC onions when fully committed
2 parents 8307cc6 + d8d9dc7 commit a706159

File tree

12 files changed

+367
-319
lines changed

12 files changed

+367
-319
lines changed

‎fuzz/src/full_stack.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,8 +1338,8 @@ mod tests {
13381338
// end of update_add_htlc from 0 to 1 via client and mac
13391339
ext_from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ab00000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
13401340

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

13441344
// inbound read from peer id 0 of len 18
13451345
ext_from_hex("030012", &mut test);
@@ -1362,8 +1362,8 @@ mod tests {
13621362

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

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

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

14451445
// now respond to the update_fulfill_htlc+commitment_signed messages the client sent to peer 0
14461446
// inbound read from peer id 0 of len 18
@@ -1474,8 +1474,8 @@ mod tests {
14741474
// process the now-pending HTLC forward
14751475
ext_from_hex("07", &mut test);
14761476

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

14801480
// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7 duplicate)
14811481
// we respond with revoke_and_ack, then commitment_signed, then update_fail_htlc
@@ -1574,8 +1574,8 @@ mod tests {
15741574
// end of update_add_htlc from 0 to 1 via client and mac
15751575
ext_from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff 5300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
15761576

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

15801580
// inbound read from peer id 0 of len 18
15811581
ext_from_hex("030012", &mut test);
@@ -1598,8 +1598,8 @@ mod tests {
15981598

15991599
// process the now-pending HTLC forward
16001600
ext_from_hex("07", &mut test);
1601-
// Three feerate requests to check dust exposure
1602-
ext_from_hex("00fd00fd00fd", &mut test);
1601+
// Four feerate requests to check dust exposure while forwarding the HTLC
1602+
ext_from_hex("00fd00fd00fd00fd", &mut test);
16031603
// client now sends id 1 update_add_htlc and commitment_signed (CHECK 7 duplicate)
16041604

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

‎lightning/src/events/mod.rs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,16 +1431,6 @@ pub enum Event {
14311431
/// Indicates that the HTLC was accepted, but could not be processed when or after attempting to
14321432
/// forward it.
14331433
///
1434-
/// Some scenarios where this event may be sent include:
1435-
/// * Insufficient capacity in the outbound channel
1436-
/// * While waiting to forward the HTLC, the channel it is meant to be forwarded through closes
1437-
/// * When an unknown SCID is requested for forwarding a payment.
1438-
/// * Expected MPP amount has already been reached
1439-
/// * The HTLC has timed out
1440-
///
1441-
/// This event, however, does not get generated if an HTLC fails to meet the forwarding
1442-
/// requirements (i.e. insufficient fees paid, or a CLTV that is too soon).
1443-
///
14441434
/// # Failure Behavior and Persistence
14451435
/// This event will eventually be replayed after failures-to-handle (i.e., the event handler
14461436
/// returning `Err(ReplayEvent ())`) and will be persisted across restarts.

‎lightning/src/ln/blinded_payment_tests.rs‎

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,10 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) {
305305
// We need the session priv to construct a bogus onion packet later.
306306
*nodes[0].keys_manager.override_random_bytes.lock().unwrap() = Some([3; 32]);
307307
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
308-
let chan_upd_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents;
309-
let chan_upd_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).0.contents;
308+
let chan_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0);
309+
let chan_upd_1_2 = chan_1_2.0.contents;
310+
let chan_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0);
311+
let chan_upd_2_3 = chan_2_3.0.contents;
310312

311313
let amt_msat = 5000;
312314
let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[3], Some(amt_msat), None);
@@ -360,18 +362,27 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) {
360362
check_added_monitors!(nodes[1], 0);
361363
do_commitment_signed_dance(&nodes[1], &nodes[0], &updates_0_1.commitment_signed, true, true);
362364

365+
expect_pending_htlcs_forwardable!(nodes[1]);
366+
check_added_monitors!(nodes[1], 1);
367+
363368
if intro_fails {
364369
let mut updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
365370
nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
366371
do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false);
372+
let failed_destination = match check {
373+
ForwardCheckFail::InboundOnionCheck => HTLCDestination::InvalidOnion,
374+
ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::FailedPayment { payment_hash },
375+
ForwardCheckFail::OutboundChannelCheck =>
376+
HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_1_2.2 },
377+
};
378+
expect_htlc_handling_failed_destinations!(
379+
nodes[1].node.get_and_clear_pending_events(), &[failed_destination.clone()]
380+
);
367381
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
368382
PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32]));
369383
return
370384
}
371385

372-
expect_pending_htlcs_forwardable!(nodes[1]);
373-
check_added_monitors!(nodes[1], 1);
374-
375386
let mut updates_1_2 = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
376387
let mut update_add = &mut updates_1_2.update_add_htlcs[0];
377388

@@ -381,6 +392,17 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) {
381392
check_added_monitors!(nodes[2], 0);
382393
do_commitment_signed_dance(&nodes[2], &nodes[1], &updates_1_2.commitment_signed, true, true);
383394

395+
expect_pending_htlcs_forwardable!(nodes[2]);
396+
let failed_destination = match check {
397+
ForwardCheckFail::InboundOnionCheck|ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::InvalidOnion,
398+
ForwardCheckFail::OutboundChannelCheck =>
399+
HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_2_3.2 },
400+
};
401+
expect_htlc_handling_failed_destinations!(
402+
nodes[2].node.get_and_clear_pending_events(), &[failed_destination.clone()]
403+
);
404+
check_added_monitors!(nodes[2], 1);
405+
384406
let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
385407
let update_malformed = &mut updates.update_fail_malformed_htlcs[0];
386408
assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING);
@@ -440,7 +462,10 @@ fn failed_backwards_to_intro_node() {
440462
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), &payment_event.msgs[0]);
441463
check_added_monitors!(nodes[2], 0);
442464
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event.commitment_msg, true, true);
443-
nodes[2].node.process_pending_htlc_forwards();
465+
466+
expect_pending_htlcs_forwardable!(nodes[2]);
467+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::InvalidOnion]);
468+
check_added_monitors(&nodes[2], 1);
444469

445470
let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
446471
let mut update_malformed = &mut updates.update_fail_malformed_htlcs[0];
@@ -517,7 +542,7 @@ fn do_forward_fail_in_process_pending_htlc_fwds(check: ProcessPendingHTLCsCheck,
517542
// intro node will error backwards.
518543
$curr_node.node.peer_disconnected($next_node.node.get_our_node_id());
519544
expect_pending_htlcs_forwardable!($curr_node);
520-
expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!($curr_node,
545+
expect_htlc_handling_failed_destinations!($curr_node.node.get_and_clear_pending_events(),
521546
vec![HTLCDestination::NextHopChannel { node_id: Some($next_node.node.get_our_node_id()), channel_id: $failed_chan_id }]);
522547
},
523548
ProcessPendingHTLCsCheck::FwdChannelClosed => {
@@ -533,12 +558,12 @@ fn do_forward_fail_in_process_pending_htlc_fwds(check: ProcessPendingHTLCsCheck,
533558
crate::events::Event::ChannelClosed { .. } => {},
534559
_ => panic!("Unexpected event {:?}", events),
535560
}
561+
check_closed_broadcast(&$curr_node, 1, true);
562+
check_added_monitors!($curr_node, 1);
536563

537564
$curr_node.node.process_pending_htlc_forwards();
538-
expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!($curr_node,
565+
expect_htlc_handling_failed_destinations!($curr_node.node.get_and_clear_pending_events(),
539566
vec![HTLCDestination::UnknownNextHop { requested_forward_scid: $failed_scid }]);
540-
check_closed_broadcast(&$curr_node, 1, true);
541-
check_added_monitors!($curr_node, 1);
542567
$curr_node.node.process_pending_htlc_forwards();
543568
},
544569
}
@@ -624,6 +649,7 @@ fn do_blinded_intercept_payment(intercept_node_fails: bool) {
624649
};
625650
nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
626651
commitment_signed_dance!(nodes[1], nodes[0], &payment_event.commitment_msg, false, true);
652+
expect_pending_htlcs_forwardable!(nodes[1]);
627653

628654
let events = nodes[1].node.get_and_clear_pending_events();
629655
assert_eq!(events.len(), 1);
@@ -929,13 +955,19 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
929955
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), update_add);
930956
check_added_monitors!(nodes[2], 0);
931957
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true);
958+
expect_pending_htlcs_forwardable!(nodes[2]);
959+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::InvalidOnion]);
960+
check_added_monitors(&nodes[2], 1);
932961
},
933962
ReceiveCheckFail::ReceiveRequirements => {
934963
let update_add = &mut payment_event_1_2.msgs[0];
935964
update_add.amount_msat -= 1;
936965
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), update_add);
937966
check_added_monitors!(nodes[2], 0);
938967
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true);
968+
expect_pending_htlcs_forwardable!(nodes[2]);
969+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]);
970+
check_added_monitors(&nodes[2], 1);
939971
},
940972
ReceiveCheckFail::ChannelCheck => {
941973
nodes[2].node.close_channel(&chan_id_1_2, &nodes[1].node.get_our_node_id()).unwrap();
@@ -949,6 +981,9 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
949981

950982
nodes[2].node.handle_shutdown(nodes[1].node.get_our_node_id(), &node_1_shutdown);
951983
commitment_signed_dance!(nodes[2], nodes[1], (), false, true, false, false);
984+
expect_pending_htlcs_forwardable!(nodes[2]);
985+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]);
986+
check_added_monitors(&nodes[2], 1);
952987
},
953988
ReceiveCheckFail::ProcessPendingHTLCsCheck => {
954989
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);
@@ -964,6 +999,9 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
964999
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]);
9651000
check_added_monitors!(nodes[2], 0);
9661001
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true);
1002+
expect_pending_htlcs_forwardable!(nodes[2]);
1003+
expect_htlc_handling_failed_destinations!(nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]);
1004+
check_added_monitors(&nodes[2], 1);
9671005
}
9681006
}
9691007

@@ -1164,6 +1202,12 @@ fn min_htlc() {
11641202
nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event_0_1.msgs[0]);
11651203
check_added_monitors!(nodes[1], 0);
11661204
do_commitment_signed_dance(&nodes[1], &nodes[0], &payment_event_0_1.commitment_msg, true, true);
1205+
expect_pending_htlcs_forwardable!(nodes[1]);
1206+
expect_htlc_handling_failed_destinations!(
1207+
nodes[1].node.get_and_clear_pending_events(),
1208+
&[HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_1_2.2 }]
1209+
);
1210+
check_added_monitors(&nodes[1], 1);
11671211
let mut updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
11681212
nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
11691213
do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false);
@@ -1350,9 +1394,11 @@ fn fails_receive_tlvs_authentication() {
13501394
let mut payment_event = SendEvent::from_event(ev);
13511395

13521396
nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
1353-
check_added_monitors!(nodes[1], 0);
13541397
do_commitment_signed_dance(&nodes[1], &nodes[0], &payment_event.commitment_msg, true, true);
1398+
expect_pending_htlcs_forwardable!(nodes[1]);
13551399
nodes[1].node.process_pending_htlc_forwards();
1400+
check_added_monitors!(nodes[1], 1);
1401+
expect_htlc_handling_failed_destinations!(nodes[1].node.get_and_clear_pending_events(), &[HTLCDestination::InvalidOnion]);
13561402

13571403
let mut update_fail = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
13581404
assert!(update_fail.update_fail_htlcs.len() == 1);

0 commit comments

Comments
 (0)