Skip to content

Commit 9de2a91

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #2504 from alecchendev/2023-08-custom-tlvs-followup
Followup: custom HTLC TLVs
2 parents 9b13862 + 83f0dbc commit 9de2a91

File tree

4 files changed

+44
-78
lines changed

4 files changed

+44
-78
lines changed

‎lightning/src/ln/msgs.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -1987,10 +1987,8 @@ impl Writeable for OutboundOnionPayload {
19871987
// We need to update [`ln::outbound_payment::RecipientOnionFields::with_custom_tlvs`]
19881988
// to reject any reserved types in the experimental range if new ones are ever
19891989
// standardized.
1990-
let preimage = if let Some(ref preimage) = keysend_preimage {
1991-
Some((5482373484, preimage.encode()))
1992-
} else { None };
1993-
let mut custom_tlvs: Vec<&(u64, Vec<u8>)> = custom_tlvs.iter().chain(preimage.iter()).collect();
1990+
let keysend_tlv = keysend_preimage.map(|preimage| (5482373484, preimage.encode()));
1991+
let mut custom_tlvs: Vec<&(u64, Vec<u8>)> = custom_tlvs.iter().chain(keysend_tlv.iter()).collect();
19941992
custom_tlvs.sort_unstable_by_key(|(typ, _)| *typ);
19951993
_encode_varint_length_prefixed_tlv!(w, {
19961994
(2, HighZeroBytesDroppedBigSize(*amt_msat), required),

‎lightning/src/ln/outbound_payment.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -515,9 +515,9 @@ impl RecipientOnionFields {
515515
let tlvs = &mut self.custom_tlvs;
516516
let further_tlvs = &mut further_htlc_fields.custom_tlvs;
517517

518-
let even_tlvs: Vec<&(u64, Vec<u8>)> = tlvs.iter().filter(|(typ, _)| *typ % 2 == 0).collect();
519-
let further_even_tlvs: Vec<&(u64, Vec<u8>)> = further_tlvs.iter().filter(|(typ, _)| *typ % 2 == 0).collect();
520-
if even_tlvs != further_even_tlvs { return Err(()) }
518+
let even_tlvs = tlvs.iter().filter(|(typ, _)| *typ % 2 == 0);
519+
let further_even_tlvs = further_tlvs.iter().filter(|(typ, _)| *typ % 2 == 0);
520+
if even_tlvs.ne(further_even_tlvs) { return Err(()) }
521521

522522
tlvs.retain(|tlv| further_tlvs.iter().any(|further_tlv| tlv == further_tlv));
523523
further_tlvs.retain(|further_tlv| tlvs.iter().any(|tlv| tlv == further_tlv));

‎lightning/src/ln/payment_tests.rs

+28-56
Original file line numberDiff line numberDiff line change
@@ -3451,17 +3451,7 @@ fn do_test_custom_tlvs(spontaneous: bool, even_tlvs: bool, known_tlvs: bool) {
34513451
let events = nodes[1].node.get_and_clear_pending_events();
34523452
assert_eq!(events.len(), 1);
34533453
match events[0] {
3454-
Event::PaymentClaimable { ref purpose, amount_msat, ref onion_fields, .. } => {
3455-
match &purpose {
3456-
PaymentPurpose::InvoicePayment { payment_secret, .. } => {
3457-
assert_eq!(our_payment_secret, *payment_secret);
3458-
assert_eq!(Some(*payment_secret), onion_fields.as_ref().unwrap().payment_secret);
3459-
},
3460-
PaymentPurpose::SpontaneousPayment(payment_preimage) => {
3461-
assert_eq!(our_payment_preimage, *payment_preimage);
3462-
},
3463-
}
3464-
assert_eq!(amount_msat, amt_msat);
3454+
Event::PaymentClaimable { ref onion_fields, .. } => {
34653455
assert_eq!(onion_fields.clone().unwrap().custom_tlvs().clone(), custom_tlvs);
34663456
},
34673457
_ => panic!("Unexpected event"),
@@ -3518,26 +3508,12 @@ fn test_retry_custom_tlvs() {
35183508
nodes[0].node.send_payment(payment_hash, onion_fields,
35193509
payment_id, route_params.clone(), Retry::Attempts(1)).unwrap();
35203510
check_added_monitors!(nodes[0], 1); // one monitor per path
3521-
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
3522-
assert_eq!(events.len(), 1);
35233511

35243512
// Add the HTLC along the first hop.
3525-
let fail_path_msgs_1 = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
3526-
let (update_add, commitment_signed) = match fail_path_msgs_1 {
3527-
MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate {
3528-
ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs,
3529-
ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed }
3530-
} => {
3531-
assert_eq!(update_add_htlcs.len(), 1);
3532-
assert!(update_fail_htlcs.is_empty());
3533-
assert!(update_fulfill_htlcs.is_empty());
3534-
assert!(update_fail_malformed_htlcs.is_empty());
3535-
assert!(update_fee.is_none());
3536-
(update_add_htlcs[0].clone(), commitment_signed.clone())
3537-
},
3538-
_ => panic!("Unexpected event"),
3539-
};
3540-
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add);
3513+
let htlc_updates = get_htlc_update_msgs(&nodes[0], &nodes[1].node.get_our_node_id());
3514+
let msgs::CommitmentUpdate { update_add_htlcs, commitment_signed, .. } = htlc_updates;
3515+
assert_eq!(update_add_htlcs.len(), 1);
3516+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add_htlcs[0]);
35413517
commitment_signed_dance!(nodes[1], nodes[0], commitment_signed, false);
35423518

35433519
// Attempt to forward the payment and complete the path's failure.
@@ -3547,15 +3523,14 @@ fn test_retry_custom_tlvs() {
35473523
node_id: Some(nodes[2].node.get_our_node_id()),
35483524
channel_id: chan_2_id
35493525
}]);
3550-
let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
3551-
assert!(htlc_updates.update_add_htlcs.is_empty());
3552-
assert_eq!(htlc_updates.update_fail_htlcs.len(), 1);
3553-
assert!(htlc_updates.update_fulfill_htlcs.is_empty());
3554-
assert!(htlc_updates.update_fail_malformed_htlcs.is_empty());
35553526
check_added_monitors!(nodes[1], 1);
3556-
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(),
3557-
&htlc_updates.update_fail_htlcs[0]);
3558-
commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false);
3527+
3528+
let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
3529+
let msgs::CommitmentUpdate { update_fail_htlcs, commitment_signed, .. } = htlc_updates;
3530+
assert_eq!(update_fail_htlcs.len(), 1);
3531+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &update_fail_htlcs[0]);
3532+
commitment_signed_dance!(nodes[0], nodes[1], commitment_signed, false);
3533+
35593534
let mut events = nodes[0].node.get_and_clear_pending_events();
35603535
match events[1] {
35613536
Event::PendingHTLCsForwardable { .. } => {},
@@ -3577,11 +3552,12 @@ fn test_retry_custom_tlvs() {
35773552
assert_eq!(events.len(), 1);
35783553
let payment_claimable = pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000,
35793554
payment_hash, Some(payment_secret), events.pop().unwrap(), true, None).unwrap();
3580-
let onion_fields = match payment_claimable {
3581-
Event::PaymentClaimable { onion_fields, .. } => onion_fields,
3555+
match payment_claimable {
3556+
Event::PaymentClaimable { onion_fields, .. } => {
3557+
assert_eq!(onion_fields.unwrap().custom_tlvs(), &custom_tlvs);
3558+
},
35823559
_ => panic!("Unexpected event"),
35833560
};
3584-
assert_eq!(onion_fields.unwrap().custom_tlvs(), &custom_tlvs);
35853561
claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage);
35863562
}
35873563

@@ -3665,7 +3641,8 @@ fn do_test_custom_tlvs_consistency(first_tlvs: Vec<(u64, Vec<u8>)>, second_tlvs:
36653641
{
36663642
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
36673643
assert_eq!(events.len(), 1);
3668-
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], amt_msat, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), false, None);
3644+
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], amt_msat, our_payment_hash,
3645+
Some(our_payment_secret), events.pop().unwrap(), false, None);
36693646
}
36703647
assert!(nodes[3].node.get_and_clear_pending_events().is_empty());
36713648

@@ -3704,26 +3681,16 @@ fn do_test_custom_tlvs_consistency(first_tlvs: Vec<(u64, Vec<u8>)>, second_tlvs:
37043681
if let Some(expected_tlvs) = expected_receive_tlvs {
37053682
// Claim and match expected
37063683
let events = nodes[3].node.get_and_clear_pending_events();
3707-
println!("events: {:?}", events);
37083684
assert_eq!(events.len(), 1);
37093685
match events[0] {
3710-
Event::PaymentClaimable { ref purpose, amount_msat, ref onion_fields, .. } => {
3711-
match &purpose {
3712-
PaymentPurpose::InvoicePayment { payment_secret, .. } => {
3713-
assert_eq!(our_payment_secret, *payment_secret);
3714-
assert_eq!(Some(*payment_secret), onion_fields.as_ref().unwrap().payment_secret);
3715-
},
3716-
PaymentPurpose::SpontaneousPayment(payment_preimage) => {
3717-
assert_eq!(our_payment_preimage, *payment_preimage);
3718-
},
3719-
}
3720-
assert_eq!(amount_msat, amt_msat);
3686+
Event::PaymentClaimable { ref onion_fields, .. } => {
37213687
assert_eq!(onion_fields.clone().unwrap().custom_tlvs, expected_tlvs);
37223688
},
37233689
_ => panic!("Unexpected event"),
37243690
}
37253691

3726-
do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
3692+
do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]],
3693+
false, our_payment_preimage);
37273694
expect_payment_sent(&nodes[0], our_payment_preimage, Some(Some(2000)), true);
37283695
} else {
37293696
// Expect fail back
@@ -3735,14 +3702,19 @@ fn do_test_custom_tlvs_consistency(first_tlvs: Vec<(u64, Vec<u8>)>, second_tlvs:
37353702
nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
37363703
commitment_signed_dance!(nodes[2], nodes[3], fail_updates_1.commitment_signed, false);
37373704

3738-
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_2_3.2 }]);
3705+
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![
3706+
HTLCDestination::NextHopChannel {
3707+
node_id: Some(nodes[3].node.get_our_node_id()),
3708+
channel_id: chan_2_3.2
3709+
}]);
37393710
check_added_monitors!(nodes[2], 1);
37403711

37413712
let fail_updates_2 = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id());
37423713
nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &fail_updates_2.update_fail_htlcs[0]);
37433714
commitment_signed_dance!(nodes[0], nodes[2], fail_updates_2.commitment_signed, false);
37443715

3745-
expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());
3716+
expect_payment_failed_conditions(&nodes[0], our_payment_hash, true,
3717+
PaymentFailedConditions::new().mpp_parts_remain());
37463718
}
37473719
}
37483720

‎lightning/src/util/ser_macros.rs

+11-15
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ macro_rules! encode_tlv_stream {
143143
#[macro_export]
144144
macro_rules! _encode_tlv_stream {
145145
($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { {
146+
$crate::_encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* }, &[])
147+
} };
148+
($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}, $extra_tlvs: expr) => { {
146149
#[allow(unused_imports)]
147150
use $crate::{
148151
ln::msgs::DecodeError,
@@ -154,6 +157,10 @@ macro_rules! _encode_tlv_stream {
154157
$(
155158
$crate::_encode_tlv!($stream, $type, $field, $fieldty);
156159
)*
160+
for tlv in $extra_tlvs {
161+
let (typ, value): &(u64, Vec<u8>) = tlv;
162+
$crate::_encode_tlv!($stream, *typ, *value, required_vec);
163+
}
157164

158165
#[allow(unused_mut, unused_variables, unused_assignments)]
159166
#[cfg(debug_assertions)]
@@ -162,18 +169,8 @@ macro_rules! _encode_tlv_stream {
162169
$(
163170
$crate::_check_encoded_tlv_order!(last_seen, $type, $fieldty);
164171
)*
165-
}
166-
} };
167-
($stream: expr, $tlvs: expr) => { {
168-
for tlv in $tlvs {
169-
let (typ, value): &&(u64, Vec<u8>) = tlv;
170-
$crate::_encode_tlv!($stream, *typ, *value, required_vec);
171-
}
172-
173-
#[cfg(debug_assertions)] {
174-
let mut last_seen: Option<u64> = None;
175-
for tlv in $tlvs {
176-
let (typ, _): &&(u64, Vec<u8>) = tlv;
172+
for tlv in $extra_tlvs {
173+
let (typ, _): &(u64, Vec<u8>) = tlv;
177174
$crate::_check_encoded_tlv_order!(last_seen, *typ, required_vec);
178175
}
179176
}
@@ -246,14 +243,13 @@ macro_rules! _encode_varint_length_prefixed_tlv {
246243
$crate::_get_varint_length_prefixed_tlv_length!(len, $type, $field, $fieldty);
247244
)*
248245
for tlv in $extra_tlvs {
249-
let (typ, value): &&(u64, Vec<u8>) = tlv;
246+
let (typ, value): &(u64, Vec<u8>) = tlv;
250247
$crate::_get_varint_length_prefixed_tlv_length!(len, *typ, *value, required_vec);
251248
}
252249
len.0
253250
};
254251
BigSize(len as u64).write($stream)?;
255-
$crate::_encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* });
256-
$crate::_encode_tlv_stream!($stream, $extra_tlvs);
252+
$crate::_encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* }, $extra_tlvs);
257253
} };
258254
}
259255

0 commit comments

Comments
 (0)