Hold times for successful payments

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
- path,
+ path: path.clone(),
  session_priv,
  first_hop_htlc_msat: 0,
  payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
  None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+ OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+ lightning::ln::decode_fulfill_attribution_data(
+ &secp_ctx,
+ &logger,
+ &path,
+ &session_priv,
+ attribution_data,
+ );
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
  payment_id: PaymentId([42; 32]),
  payment_hash: None,
  path: path.clone(),
+ hold_times: Vec::new(),
  });
  let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
  match event {

lightning/src/events/mod.rs

@@ -1102,6 +1102,12 @@ pub enum Event {
  ///
  /// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
  path: Path,
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
+ hold_times: Vec<u32>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1153,7 +1159,11 @@ pub enum Event {
  error_code: Option<u16>,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data: Option<Vec<u8>>,
- #[cfg(any(test, feature = "_test_utils"))]
+ /// The hold times as reported by each hop. The unit in which the hold times are expressed are 100's of
+ /// milliseconds. So a hop reporting 2 is a hold time that corresponds to roughly 200 milliseconds. As earlier
+ /// hops hold on to an HTLC for longer, the hold times in the list are expected to decrease. When our peer
+ /// didn't provide attribution data, the list is empty. The same applies to HTLCs that were resolved onchain.
+ /// Because of unavailability of hold times, the list may be shorter than the number of hops in the path.
  hold_times: Vec<u32>,
 	},
 	/// Indicates that a probe payment we sent returned successful, i.e., only failed at the destination.
@@ -1792,16 +1802,13 @@ impl Writeable for Event {
  ref error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  ref error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  ref hold_times,
  } => {
  3u8.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_code.write(writer)?;
  #[cfg(any(test, feature = "_test_utils"))]
  error_data.write(writer)?;
- #[cfg(any(test, feature = "_test_utils"))]
- hold_times.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_hash, required),
  (1, None::<NetworkUpdate>, option), // network_update in LDK versions prior to 0.0.114
@@ -1813,6 +1820,7 @@ impl Writeable for Event {
  (9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
  (11, payment_id, option),
  (13, failure, required),
+ (15, *hold_times, optional_vec),
  });
  },
  &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -1910,10 +1918,16 @@ impl Writeable for Event {
  (4, funding_info, required),
  })
  },
- &Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+ &Event::PaymentPathSuccessful {
+ ref payment_id,
+ ref payment_hash,
+ ref path,
+ ref hold_times,
+ } => {
  13u8.write(writer)?;
  write_tlv_fields!(writer, {
  (0, payment_id, required),
+ (1, *hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path.hops, required_vec),
  (6, path.blinded_tail, option),
@@ -2232,8 +2246,6 @@ impl MaybeReadable for Event {
  let error_code = Readable::read(reader)?;
  #[cfg(any(test, feature = "_test_utils"))]
  let error_data = Readable::read(reader)?;
- #[cfg(any(test, feature = "_test_utils"))]
- let hold_times = Readable::read(reader)?;
  let mut payment_hash = PaymentHash([0; 32]);
  let mut payment_failed_permanently = false;
  let mut network_update = None;
@@ -2242,6 +2254,7 @@ impl MaybeReadable for Event {
  let mut short_channel_id = None;
  let mut payment_id = None;
  let mut failure_opt = None;
+ let mut hold_times = None;
  read_tlv_fields!(reader, {
  (0, payment_hash, required),
  (1, network_update, upgradable_option),
@@ -2253,7 +2266,9 @@ impl MaybeReadable for Event {
  (7, short_channel_id, option),
  (11, payment_id, option),
  (13, failure_opt, upgradable_option),
+ (15, hold_times, optional_vec),
  });
+ let hold_times = hold_times.unwrap_or(Vec::new());
  let failure =
  failure_opt.unwrap_or_else(|| PathFailure::OnPath { network_update });
  Ok(Some(Event::PaymentPathFailed {
@@ -2267,7 +2282,6 @@ impl MaybeReadable for Event {
  error_code,
  #[cfg(any(test, feature = "_test_utils"))]
  error_data,
- #[cfg(any(test, feature = "_test_utils"))]
  hold_times,
  }))
  };
@@ -2413,14 +2427,19 @@ impl MaybeReadable for Event {
  let mut f = || {
  _init_and_read_len_prefixed_tlv_fields!(reader, {
  (0, payment_id, required),
+ (1, hold_times, optional_vec),
  (2, payment_hash, option),
  (4, path, required_vec),
  (6, blinded_tail, option),
  });
+
+ let hold_times = hold_times.unwrap_or(Vec::new());
+
  Ok(Some(Event::PaymentPathSuccessful {
  payment_id: payment_id.0.unwrap(),
  payment_hash,
  path: Path { hops: path, blinded_tail },
+ hold_times,
  }))
  };
  f()

lightning/src/ln/async_payments_tests.rs

@@ -605,7 +605,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev);
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
-	let res =
+	let (res, _) =
  claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
 }
@@ -1723,7 +1723,7 @@ fn refresh_static_invoices() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(updated_invoice)));
 }
 
 #[cfg_attr(feature = "std", ignore)]
@@ -2053,5 +2053,5 @@ fn invoice_server_is_not_channel_peer() {
 	let claimable_ev = do_pass_along_path(args).unwrap();
 	let keysend_preimage = extract_payment_preimage(&claimable_ev);
 	let res = claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
-	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
+	assert_eq!(res.0, Some(PaidBolt12Invoice::StaticInvoice(invoice)));
 }

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
  None,
  ).unwrap();
 
+ // Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+ // this and won't be able to decode the fulfill hold times.
  let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
  let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,8 +2890,12 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg =
- msgs::UpdateFulfillHTLC { channel_id: chan_id_2, htlc_id: 0, payment_preimage };
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
+ channel_id: chan_id_2,
+ htlc_id: 0,
+ payment_preimage,
+ attribution_data: None,
+	};
 	if second_fails {
  nodes[2].node.fail_htlc_backwards(&payment_hash);
  expect_pending_htlcs_forwardable_and_htlc_handling_failed!(
@@ -2900,15 +2904,24 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
  );
  check_added_monitors!(nodes[2], 1);
  get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
  nodes[2].node.claim_funds(payment_preimage);
  check_added_monitors!(nodes[2], 1);
  expect_payment_claimed!(nodes[2], payment_hash, 100_000);
 
  let cs_updates = get_htlc_update_msgs!(nodes[2], node_b_id);
  assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
- // Check that the message we're about to deliver matches the one generated:
- assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+ // Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+ assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+ assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+ assert_eq!(
+ fulfill_msg.payment_preimage,
+ cs_updates.update_fulfill_htlcs[0].payment_preimage
+ );
+ fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);