Skip to content

Introduce RecipientInfo struct for send_payment #1445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed

Introduce RecipientInfo struct for send_payment #1445

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3876fbf
Define the `PaymentMetadata` feature to be used in invoices
TheBlueMatt Dec 21, 2021
b690365
Unset the optional bit for a feature when setting the required bit
TheBlueMatt Dec 21, 2021
f90cf53
Support reading the new `payment_metadata` field in invoices
TheBlueMatt Dec 21, 2021
fda77f2
Support setting the new payment metadata field in invoices
TheBlueMatt Dec 21, 2021
3fa3694
Deserialize payment metadata fields in the onion final hop data
TheBlueMatt Dec 22, 2021
c90e20c
Provide users the payment metadata via `Event::PaymentReceived`
TheBlueMatt Dec 21, 2021
a523f10
Pipe payment metadata through the HTLC send pipeline
TheBlueMatt Dec 22, 2021
4d5b91e
Expose payment metadata in the sending path via `send_payment`.
TheBlueMatt Dec 22, 2021
afb468a
Introduce RecipientInfo struct for send_payment
andozw Apr 22, 2022
40e436e
f fix comments
andozw May 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6566,6 +6566,7 @@ mod tests {
first_hop_htlc_msat: 548,
payment_id: PaymentId([42; 32]),
payment_secret: None,
payment_metadata: None,
payment_params: None,
}
});
Expand Down
46 changes: 29 additions & 17 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ pub(crate) enum HTLCSource {
first_hop_htlc_msat: u64,
payment_id: PaymentId,
payment_secret: Option<PaymentSecret>,
payment_metadata: Option<Vec<u8>>,
payment_params: Option<PaymentParameters>,
},
}
Expand All @@ -223,12 +224,13 @@ impl core::hash::Hash for HTLCSource {
0u8.hash(hasher);
prev_hop_data.hash(hasher);
},
HTLCSource::OutboundRoute { path, session_priv, payment_id, payment_secret, first_hop_htlc_msat, payment_params } => {
HTLCSource::OutboundRoute { path, session_priv, payment_id, payment_secret, payment_metadata, first_hop_htlc_msat, payment_params } => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're touching long lines like this, we probably want to start using recommended text width of 100 characters.

1u8.hash(hasher);
path.hash(hasher);
session_priv[..].hash(hasher);
payment_id.hash(hasher);
payment_secret.hash(hasher);
payment_metadata.hash(hasher);
first_hop_htlc_msat.hash(hasher);
payment_params.hash(hasher);
},
Expand All @@ -245,6 +247,7 @@ impl HTLCSource {
first_hop_htlc_msat: 0,
payment_id: PaymentId([2; 32]),
payment_secret: None,
payment_metadata: None,
payment_params: None,
}
}
Expand Down Expand Up @@ -470,6 +473,7 @@ pub(crate) enum PendingOutboundPayment {
session_privs: HashSet<[u8; 32]>,
payment_hash: PaymentHash,
payment_secret: Option<PaymentSecret>,
payment_metadata: Option<Vec<u8>>,
pending_amt_msat: u64,
/// Used to track the fee paid. Only present if the payment was serialized on 0.0.103+.
pending_fee_msat: Option<u64>,
Expand Down Expand Up @@ -2331,15 +2335,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}

// Only public for testing, this should otherwise never be called direcly
pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_params: &Option<PaymentParameters>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>) -> Result<(), APIError> {
pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_params: &Option<PaymentParameters>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, payment_metadata: &Option<Vec<u8>>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>) -> Result<(), APIError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, would be great to consider the 100 char text width.

log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first().unwrap().short_channel_id);
let prng_seed = self.keys_manager.get_secure_random_bytes();
let session_priv_bytes = self.keys_manager.get_secure_random_bytes();
let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");

let onion_keys = onion_utils::construct_onion_keys(&self.secp_ctx, &path, &session_priv)
.map_err(|_| APIError::RouteError{err: "Pubkey along hop was maliciously selected"})?;
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(path, total_value, payment_secret, cur_height, keysend_preimage)?;
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(path, total_value, payment_secret, payment_metadata.clone(), cur_height, keysend_preimage)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, 100 char text width, etc.

if onion_utils::route_size_insane(&onion_payloads) {
return Err(APIError::RouteError{err: "Route size too large considering onion data"});
}
Expand Down Expand Up @@ -2373,6 +2377,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
pending_fee_msat: Some(0),
payment_hash: *payment_hash,
payment_secret: *payment_secret,
payment_metadata: payment_metadata.clone(),
starting_block_height: self.best_block.read().unwrap().height(),
total_msat: total_value,
});
Expand All @@ -2396,6 +2401,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
first_hop_htlc_msat: htlc_msat,
payment_id,
payment_secret: payment_secret.clone(),
payment_metadata: payment_metadata.clone(),
payment_params: payment_params.clone(),
}, onion_packet, &self.logger),
channel_state, chan)
Expand Down Expand Up @@ -2480,10 +2486,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
/// bit set (either as required or as available). If multiple paths are present in the Route,
/// we assume the invoice had the basic_mpp feature set.
pub fn send_payment(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>) -> Result<PaymentId, PaymentSendFailure> {
self.send_payment_internal(route, payment_hash, payment_secret, None, None, None)
self.send_payment_internal(route, payment_hash, payment_secret, None, None, None, None)
}

fn send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, keysend_preimage: Option<PaymentPreimage>, payment_id: Option<PaymentId>, recv_value_msat: Option<u64>) -> Result<PaymentId, PaymentSendFailure> {
fn send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_metadata: Option<Vec<u8>>, keysend_preimage: Option<PaymentPreimage>, payment_id: Option<PaymentId>, recv_value_msat: Option<u64>) -> Result<PaymentId, PaymentSendFailure> {
if route.paths.len() < 1 {
return Err(PaymentSendFailure::ParameterError(APIError::RouteError{err: "There must be at least one path to send over"}));
}
Expand Down Expand Up @@ -2525,7 +2531,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
let cur_height = self.best_block.read().unwrap().height() + 1;
let mut results = Vec::new();
for path in route.paths.iter() {
results.push(self.send_payment_along_path(&path, &route.payment_params, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage));
results.push(self.send_payment_along_path(&path, &route.payment_params, &payment_hash, payment_secret, &payment_metadata, total_value, cur_height, payment_id, &keysend_preimage));
}
let mut has_ok = false;
let mut has_err = false;
Expand Down Expand Up @@ -2588,20 +2594,20 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}
}

let (total_msat, payment_hash, payment_secret) = {
let (total_msat, payment_hash, payment_secret, payment_metadata) = {
let outbounds = self.pending_outbound_payments.lock().unwrap();
if let Some(payment) = outbounds.get(&payment_id) {
match payment {
PendingOutboundPayment::Retryable {
total_msat, payment_hash, payment_secret, pending_amt_msat, ..
total_msat, payment_hash, payment_secret, pending_amt_msat, payment_metadata, ..
} => {
let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
err: format!("retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat).to_string()
}))
}
(*total_msat, *payment_hash, *payment_secret)
(*total_msat, *payment_hash, *payment_secret, payment_metadata.clone())
},
PendingOutboundPayment::Legacy { .. } => {
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
Expand All @@ -2625,7 +2631,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}))
}
};
return self.send_payment_internal(route, payment_hash, &payment_secret, None, Some(payment_id), Some(total_msat)).map(|_| ())
return self.send_payment_internal(route, payment_hash, &payment_secret, payment_metadata, None, Some(payment_id), Some(total_msat)).map(|_| ())
}

/// Signals that no further retries for the given payment will occur.
Expand Down Expand Up @@ -2679,7 +2685,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
None => PaymentPreimage(self.keys_manager.get_secure_random_bytes()),
};
let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
match self.send_payment_internal(route, payment_hash, &None, Some(preimage), None, None) {
match self.send_payment_internal(route, payment_hash, &None, None, Some(preimage), None, None) {
Ok(payment_id) => Ok((payment_hash, payment_id)),
Err(e) => Err(e)
}
Expand Down Expand Up @@ -6152,13 +6158,15 @@ impl Readable for HTLCSource {
let mut payment_id = None;
let mut payment_secret = None;
let mut payment_params = None;
let mut payment_metadata = None;
read_tlv_fields!(reader, {
(0, session_priv, required),
(1, payment_id, option),
(2, first_hop_htlc_msat, required),
(3, payment_secret, option),
(4, path, vec_type),
(5, payment_params, option),
(7, payment_metadata, option),
});
if payment_id.is_none() {
// For backwards compat, if there was no payment_id written, use the session_priv bytes
Expand All @@ -6172,6 +6180,7 @@ impl Readable for HTLCSource {
payment_id: payment_id.unwrap(),
payment_secret,
payment_params,
payment_metadata,
})
}
1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)),
Expand All @@ -6183,7 +6192,7 @@ impl Readable for HTLCSource {
impl Writeable for HTLCSource {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::io::Error> {
match self {
HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret, payment_params } => {
HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret, ref payment_metadata, payment_params } => {
0u8.write(writer)?;
let payment_id_opt = Some(payment_id);
write_tlv_fields!(writer, {
Expand All @@ -6193,6 +6202,7 @@ impl Writeable for HTLCSource {
(3, payment_secret, option),
(4, path, vec_type),
(5, payment_params, option),
(7, payment_metadata, option),
});
}
HTLCSource::PreviousHopData(ref field) => {
Expand Down Expand Up @@ -6247,6 +6257,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
(0, session_privs, required),
(1, pending_fee_msat, option),
(2, payment_hash, required),
(3, payment_metadata, option),
(4, payment_secret, option),
(6, total_msat, required),
(8, pending_amt_msat, required),
Expand Down Expand Up @@ -6709,7 +6720,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
for (_, monitor) in args.channel_monitors {
if by_id.get(&monitor.get_funding_txo().0.to_channel_id()).is_none() {
for (htlc_source, htlc) in monitor.get_pending_outbound_htlcs() {
if let HTLCSource::OutboundRoute { payment_id, session_priv, path, payment_secret, .. } = htlc_source {
if let HTLCSource::OutboundRoute { payment_id, session_priv, path, payment_secret, payment_metadata, .. } = htlc_source {
if path.is_empty() {
log_error!(args.logger, "Got an empty path for a pending payment");
return Err(DecodeError::InvalidValue);
Expand All @@ -6729,6 +6740,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
payment_hash: htlc.payment_hash,
payment_secret,
payment_metadata,
pending_amt_msat: path_amt,
pending_fee_msat: Some(path_fee),
total_msat: path_amt,
Expand Down Expand Up @@ -7004,7 +7016,7 @@ mod tests {
// Use the utility function send_payment_along_path to send the payment with MPP data which
// indicates there are more HTLCs coming.
let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match.
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), &None, 200_000, cur_height, payment_id, &None).unwrap();
check_added_monitors!(nodes[0], 1);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
Expand Down Expand Up @@ -7034,7 +7046,7 @@ mod tests {
expect_payment_failed!(nodes[0], our_payment_hash, true);

// Send the second half of the original MPP payment.
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), &None, 200_000, cur_height, payment_id, &None).unwrap();
check_added_monitors!(nodes[0], 1);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
Expand Down Expand Up @@ -7228,7 +7240,7 @@ mod tests {

let test_preimage = PaymentPreimage([42; 32]);
let mismatch_payment_hash = PaymentHash([43; 32]);
let _ = nodes[0].node.send_payment_internal(&route, mismatch_payment_hash, &None, Some(test_preimage), None, None).unwrap();
let _ = nodes[0].node.send_payment_internal(&route, mismatch_payment_hash, &None, None, Some(test_preimage), None, None).unwrap();
check_added_monitors!(nodes[0], 1);

let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
Expand Down Expand Up @@ -7273,7 +7285,7 @@ mod tests {
let test_preimage = PaymentPreimage([42; 32]);
let test_secret = PaymentSecret([43; 32]);
let payment_hash = PaymentHash(Sha256::hash(&test_preimage.0).into_inner());
let _ = nodes[0].node.send_payment_internal(&route, payment_hash, &Some(test_secret), Some(test_preimage), None, None).unwrap();
let _ = nodes[0].node.send_payment_internal(&route, payment_hash, &Some(test_secret), None, Some(test_preimage), None, None).unwrap();
check_added_monitors!(nodes[0], 1);

let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
Expand Down
12 changes: 6 additions & 6 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1371,7 +1371,7 @@ fn test_fee_spike_violation_fails_htlc() {
let cur_height = nodes[1].node.best_block.read().unwrap().height() + 1;

let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3460001, &Some(payment_secret), cur_height, &None).unwrap();
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3460001, &Some(payment_secret), None, cur_height, &None).unwrap();
let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
let msg = msgs::UpdateAddHTLC {
channel_id: chan.2,
Expand Down Expand Up @@ -1548,7 +1548,7 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
let cur_height = nodes[1].node.best_block.read().unwrap().height() + 1;
let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 700_000, &Some(payment_secret), cur_height, &None).unwrap();
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 700_000, &Some(payment_secret), None, cur_height, &None).unwrap();
let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
let msg = msgs::UpdateAddHTLC {
channel_id: chan.2,
Expand Down Expand Up @@ -1718,7 +1718,7 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() {
let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1;
let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route_2.paths[0], &session_priv).unwrap();
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route_2.paths[0], recv_value_2, &None, cur_height, &None).unwrap();
let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route_2.paths[0], recv_value_2, &None, None, cur_height, &None).unwrap();
let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash_1);
let msg = msgs::UpdateAddHTLC {
channel_id: chan.2,
Expand Down Expand Up @@ -3317,7 +3317,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
let secp_ctx = Secp256k1::new();
let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
let current_height = nodes[1].node.best_block.read().unwrap().height() + 1;
let (onion_payloads, _amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(&route.paths[0], 50_000, &Some(payment_secret), current_height, &None).unwrap();
let (onion_payloads, _amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(&route.paths[0], 50_000, &Some(payment_secret), None, current_height, &None).unwrap();
let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap();
let onion_routing_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);

Expand Down Expand Up @@ -4207,7 +4207,7 @@ fn do_test_htlc_timeout(send_partial_mpp: bool) {
// indicates there are more HTLCs coming.
let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match.
let payment_id = PaymentId([42; 32]);
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200000, cur_height, payment_id, &None).unwrap();
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), &None, 200000, cur_height, payment_id, &None).unwrap();
check_added_monitors!(nodes[0], 1);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
Expand Down Expand Up @@ -6670,7 +6670,7 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() {
let session_priv = SecretKey::from_slice(&[42; 32]).unwrap();
let cur_height = nodes[0].node.best_block.read().unwrap().height() + 1;
let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::signing_only(), &route.paths[0], &session_priv).unwrap();
let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3999999, &Some(our_payment_secret), cur_height, &None).unwrap();
let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3999999, &Some(our_payment_secret), None, cur_height, &None).unwrap();
let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash);

let mut msg = msgs::UpdateAddHTLC {
Expand Down
Loading