Skip to content

Commit

Permalink
Merge pull request lightningdevkit#3084 from jkczyz/2024-05-onion-fields
Browse files Browse the repository at this point in the history
Include `RecipientOnionFields` in `Event::PaymentClaimed`
  • Loading branch information
TheBlueMatt authored May 30, 2024
2 parents df01208 + 8499214 commit 7d2d047
Show file tree
Hide file tree
Showing 10 changed files with 176 additions and 69 deletions.
4 changes: 3 additions & 1 deletion lightning-invoice/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1397,7 +1397,9 @@ mod test {
let payment_preimage_opt = if user_generated_pmt_hash { None } else { Some(payment_preimage) };
assert_eq!(other_events.borrow().len(), 1);
check_payment_claimable(&other_events.borrow()[0], payment_hash, payment_secret, payment_amt, payment_preimage_opt, invoice.recover_payee_pub_key());
do_claim_payment_along_route(&nodes[0], &[&vec!(&nodes[fwd_idx])[..]], false, payment_preimage);
do_claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[fwd_idx]]], payment_preimage)
);
expect_payment_sent(&nodes[0], payment_preimage, None, true, true);
}

Expand Down
11 changes: 10 additions & 1 deletion lightning/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,11 @@ pub enum Event {
/// The sender-intended sum total of all the MPP parts. This will be `None` for events
/// serialized prior to LDK version 0.0.117.
sender_intended_total_msat: Option<u64>,
/// The fields in the onion which were received with each HTLC. Only fields which were
/// identical in each HTLC involved in the payment will be included here.
///
/// Payments received on LDK versions prior to 0.0.124 will have this field unset.
onion_fields: Option<RecipientOnionFields>,
},
/// Indicates that a peer connection with a node is needed in order to send an [`OnionMessage`].
///
Expand Down Expand Up @@ -1348,7 +1353,7 @@ impl Writeable for Event {
// We never write the OpenChannelRequest events as, upon disconnection, peers
// drop any channels which have not yet exchanged funding_signed.
},
&Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref htlcs, ref sender_intended_total_msat } => {
&Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref htlcs, ref sender_intended_total_msat, ref onion_fields } => {
19u8.write(writer)?;
write_tlv_fields!(writer, {
(0, payment_hash, required),
Expand All @@ -1357,6 +1362,7 @@ impl Writeable for Event {
(4, amount_msat, required),
(5, *htlcs, optional_vec),
(7, sender_intended_total_msat, option),
(9, onion_fields, option),
});
},
&Event::ProbeSuccessful { ref payment_id, ref payment_hash, ref path } => {
Expand Down Expand Up @@ -1719,13 +1725,15 @@ impl MaybeReadable for Event {
let mut receiver_node_id = None;
let mut htlcs: Option<Vec<ClaimedHTLC>> = Some(vec![]);
let mut sender_intended_total_msat: Option<u64> = None;
let mut onion_fields = None;
read_tlv_fields!(reader, {
(0, payment_hash, required),
(1, receiver_node_id, option),
(2, purpose, upgradable_required),
(4, amount_msat, required),
(5, htlcs, optional_vec),
(7, sender_intended_total_msat, option),
(9, onion_fields, option),
});
Ok(Some(Event::PaymentClaimed {
receiver_node_id,
Expand All @@ -1734,6 +1742,7 @@ impl MaybeReadable for Event {
amount_msat,
htlcs: htlcs.unwrap_or(vec![]),
sender_intended_total_msat,
onion_fields,
}))
};
f()
Expand Down
25 changes: 19 additions & 6 deletions lightning/src/ln/blinded_payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@ fn mpp_to_one_hop_blinded_path() {
let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(),
Some(payment_secret), ev.clone(), true, None);
claim_payment_along_route(&nodes[0], expected_route, false, payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], expected_route, payment_preimage)
);
}

#[test]
Expand Down Expand Up @@ -244,7 +246,9 @@ fn mpp_to_three_hop_blinded_paths() {
let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(),
Some(payment_secret), ev.clone(), true, None);
claim_payment_along_route(&nodes[0], expected_route, false, payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], expected_route, payment_preimage)
);
}

enum ForwardCheckFail {
Expand Down Expand Up @@ -643,7 +647,9 @@ fn do_blinded_intercept_payment(intercept_node_fails: bool) {
expect_pending_htlcs_forwardable!(nodes[2]);

expect_payment_claimable!(&nodes[2], payment_hash, payment_secret, amt_msat, None, nodes[2].node.get_our_node_id());
do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[1], &nodes[2])[..]), false, payment_preimage);
do_claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[2]]], payment_preimage)
);
expect_payment_sent(&nodes[0], payment_preimage, Some(Some(1000)), true, true);
}

Expand Down Expand Up @@ -1217,7 +1223,9 @@ fn blinded_keysend() {

let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
pass_along_path(&nodes[0], expected_route[0], amt_msat, payment_hash, Some(payment_secret), ev.clone(), true, Some(keysend_preimage));
claim_payment_along_route(&nodes[0], expected_route, false, keysend_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], expected_route, keysend_preimage)
);
}

#[test]
Expand Down Expand Up @@ -1268,7 +1276,9 @@ fn blinded_mpp_keysend() {
let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(),
Some(payment_secret), ev.clone(), true, Some(keysend_preimage));
claim_payment_along_route(&nodes[0], expected_route, false, keysend_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], expected_route, keysend_preimage)
);
}

#[test]
Expand Down Expand Up @@ -1316,5 +1326,8 @@ fn custom_tlvs_to_blinded_path() {
.with_payment_secret(payment_secret)
.with_custom_tlvs(recipient_onion_fields.custom_tlvs.clone());
do_pass_along_path(args);
claim_payment(&nodes[0], &[&nodes[1]], payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1]]], payment_preimage)
.with_custom_tlvs(recipient_onion_fields.custom_tlvs.clone())
);
}
4 changes: 3 additions & 1 deletion lightning/src/ln/chanmon_update_fail_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2025,7 +2025,9 @@ fn test_path_paused_mpp() {
assert_eq!(events.len(), 1);
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash.clone(), Some(payment_secret), events.pop().unwrap(), true, None);

claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], payment_preimage)
);
}

#[test]
Expand Down
37 changes: 25 additions & 12 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,13 +681,15 @@ struct ClaimingPayment {
receiver_node_id: PublicKey,
htlcs: Vec<events::ClaimedHTLC>,
sender_intended_value: Option<u64>,
onion_fields: Option<RecipientOnionFields>,
}
impl_writeable_tlv_based!(ClaimingPayment, {
(0, amount_msat, required),
(2, payment_purpose, required),
(4, receiver_node_id, required),
(5, htlcs, optional_vec),
(7, sender_intended_value, option),
(9, onion_fields, option),
});

struct ClaimablePayment {
Expand Down Expand Up @@ -6325,19 +6327,27 @@ where
}
}

let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect();
let sender_intended_value = payment.htlcs.first().map(|htlc| htlc.total_msat);
let dup_purpose = claimable_payments.pending_claiming_payments.insert(payment_hash,
ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.value).sum(),
payment_purpose: payment.purpose, receiver_node_id, htlcs, sender_intended_value
});
if dup_purpose.is_some() {
debug_assert!(false, "Shouldn't get a duplicate pending claim event ever");
log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug",
&payment_hash);
}
let claiming_payment = claimable_payments.pending_claiming_payments
.entry(payment_hash)
.and_modify(|_| {
debug_assert!(false, "Shouldn't get a duplicate pending claim event ever");
log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug",
&payment_hash);
})
.or_insert_with(|| {
let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect();
let sender_intended_value = payment.htlcs.first().map(|htlc| htlc.total_msat);
ClaimingPayment {
amount_msat: payment.htlcs.iter().map(|source| source.value).sum(),
payment_purpose: payment.purpose,
receiver_node_id,
htlcs,
sender_intended_value,
onion_fields: payment.onion_fields,
}
});

if let Some(RecipientOnionFields { ref custom_tlvs, .. }) = payment.onion_fields {
if let Some(RecipientOnionFields { ref custom_tlvs, .. }) = claiming_payment.onion_fields {
if !custom_tlvs_known && custom_tlvs.iter().any(|(typ, _)| typ % 2 == 0) {
log_info!(self.logger, "Rejecting payment with payment hash {} as we cannot accept payment with unknown even TLVs: {}",
&payment_hash, log_iter!(custom_tlvs.iter().map(|(typ, _)| typ).filter(|typ| *typ % 2 == 0)));
Expand Down Expand Up @@ -6744,6 +6754,7 @@ where
receiver_node_id,
htlcs,
sender_intended_value: sender_intended_total_msat,
onion_fields,
}) = payment {
self.pending_events.lock().unwrap().push_back((events::Event::PaymentClaimed {
payment_hash,
Expand All @@ -6752,6 +6763,7 @@ where
receiver_node_id: Some(receiver_node_id),
htlcs,
sender_intended_total_msat,
onion_fields,
}, None));
}
},
Expand Down Expand Up @@ -12273,6 +12285,7 @@ where
amount_msat: claimable_amt_msat,
htlcs: payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(),
sender_intended_total_msat: payment.htlcs.first().map(|htlc| htlc.total_msat),
onion_fields: payment.onion_fields,
}, None));
}
}
Expand Down
44 changes: 26 additions & 18 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2716,18 +2716,12 @@ pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route
(our_payment_preimage, our_payment_hash, our_payment_secret, payment_id)
}

pub fn do_claim_payment_along_route<'a, 'b, 'c>(
origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool,
our_payment_preimage: PaymentPreimage
) -> u64 {
for path in expected_paths.iter() {
assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
pub fn do_claim_payment_along_route(args: ClaimAlongRouteArgs) -> u64 {
for path in args.expected_paths.iter() {
assert_eq!(path.last().unwrap().node.get_our_node_id(), args.expected_paths[0].last().unwrap().node.get_our_node_id());
}
expected_paths[0].last().unwrap().node.claim_funds(our_payment_preimage);
pass_claimed_payment_along_route(
ClaimAlongRouteArgs::new(origin_node, expected_paths, our_payment_preimage)
.skip_last(skip_last)
)
args.expected_paths[0].last().unwrap().node.claim_funds(args.payment_preimage);
pass_claimed_payment_along_route(args)
}

pub struct ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
Expand All @@ -2737,6 +2731,7 @@ pub struct ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
pub expected_min_htlc_overpay: Vec<u32>,
pub skip_last: bool,
pub payment_preimage: PaymentPreimage,
pub custom_tlvs: Vec<(u64, Vec<u8>)>,
// Allow forwarding nodes to have taken 1 msat more fee than expected based on the downstream
// fulfill amount.
//
Expand All @@ -2755,7 +2750,7 @@ impl<'a, 'b, 'c, 'd> ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
Self {
origin_node, expected_paths, expected_extra_fees: vec![0; expected_paths.len()],
expected_min_htlc_overpay: vec![0; expected_paths.len()], skip_last: false, payment_preimage,
allow_1_msat_fee_overpay: false,
allow_1_msat_fee_overpay: false, custom_tlvs: vec![],
}
}
pub fn skip_last(mut self, skip_last: bool) -> Self {
Expand All @@ -2774,12 +2769,16 @@ impl<'a, 'b, 'c, 'd> ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
self.allow_1_msat_fee_overpay = true;
self
}
pub fn with_custom_tlvs(mut self, custom_tlvs: Vec<(u64, Vec<u8>)>) -> Self {
self.custom_tlvs = custom_tlvs;
self
}
}

pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArgs) -> u64 {
pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 {
let ClaimAlongRouteArgs {
origin_node, expected_paths, expected_extra_fees, expected_min_htlc_overpay, skip_last,
payment_preimage: our_payment_preimage, allow_1_msat_fee_overpay,
payment_preimage: our_payment_preimage, allow_1_msat_fee_overpay, custom_tlvs,
} = args;
let claim_event = expected_paths[0].last().unwrap().node.get_and_clear_pending_events();
assert_eq!(claim_event.len(), 1);
Expand All @@ -2793,11 +2792,13 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArg
| PaymentPurpose::Bolt12RefundPayment { payment_preimage: Some(preimage), .. },
amount_msat,
ref htlcs,
ref onion_fields,
..
} => {
assert_eq!(preimage, our_payment_preimage);
assert_eq!(htlcs.len(), expected_paths.len()); // One per path.
assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::<u64>(), amount_msat);
assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, custom_tlvs);
expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc));
fwd_amt_msat = amount_msat;
},
Expand All @@ -2808,11 +2809,13 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArg
payment_hash,
amount_msat,
ref htlcs,
ref onion_fields,
..
} => {
assert_eq!(&payment_hash.0, &Sha256::hash(&our_payment_preimage.0)[..]);
assert_eq!(htlcs.len(), expected_paths.len()); // One per path.
assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::<u64>(), amount_msat);
assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, custom_tlvs);
expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc));
fwd_amt_msat = amount_msat;
}
Expand Down Expand Up @@ -2956,15 +2959,20 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArg

expected_total_fee_msat
}
pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) {
let expected_total_fee_msat = do_claim_payment_along_route(origin_node, expected_paths, skip_last, our_payment_preimage);
pub fn claim_payment_along_route(args: ClaimAlongRouteArgs) {
let origin_node = args.origin_node;
let payment_preimage = args.payment_preimage;
let skip_last = args.skip_last;
let expected_total_fee_msat = do_claim_payment_along_route(args);
if !skip_last {
expect_payment_sent!(origin_node, our_payment_preimage, Some(expected_total_fee_msat));
expect_payment_sent!(origin_node, payment_preimage, Some(expected_total_fee_msat));
}
}

pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], our_payment_preimage: PaymentPreimage) {
claim_payment_along_route(origin_node, &[expected_route], false, our_payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(origin_node, &[expected_route], our_payment_preimage)
);
}

pub const TEST_FINAL_CLTV: u32 = 70;
Expand Down
21 changes: 16 additions & 5 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3812,7 +3812,10 @@ fn test_simple_peer_disconnect() {
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());

claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], true, payment_preimage_3);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[2]]], payment_preimage_3)
.skip_last(true)
);
fail_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], true, payment_hash_5);

let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
Expand Down Expand Up @@ -8296,7 +8299,9 @@ fn test_onion_value_mpp_set_calculation() {
let ev = remove_first_msg_event_to_node(&expected_paths[1][0].node.get_our_node_id(), &mut events);
pass_along_path(&nodes[0], expected_paths[1], 101_000, our_payment_hash.clone(), Some(our_payment_secret), ev, true, None);

claim_payment_along_route(&nodes[0], expected_paths, false, our_payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], expected_paths, our_payment_preimage)
);
}

fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64) {
Expand Down Expand Up @@ -8362,7 +8367,9 @@ fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64) {
pass_along_path(&nodes[src_idx], expected_path, amount_received, our_payment_hash.clone(), Some(our_payment_secret), ev, became_claimable_now, None);
}

claim_payment_along_route(&nodes[src_idx], &expected_paths, false, our_payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[src_idx], &expected_paths, our_payment_preimage)
);
}

#[test]
Expand Down Expand Up @@ -8394,7 +8401,9 @@ fn test_simple_mpp() {
route.paths[1].hops[0].short_channel_id = chan_2_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;
send_along_route_with_secret(&nodes[0], route, &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], 200_000, payment_hash, payment_secret);
claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], payment_preimage)
);
}

#[test]
Expand Down Expand Up @@ -9838,7 +9847,9 @@ fn test_inconsistent_mpp_params() {
assert_eq!(events.len(), 1);
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None);

do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
do_claim_payment_along_route(
ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], our_payment_preimage)
);
expect_payment_sent(&nodes[0], our_payment_preimage, Some(None), true, true);
}

Expand Down
Loading

0 comments on commit 7d2d047

Please sign in to comment.