Skip to content

Commit 1b99c93

Browse files
committed
Store Payee information in HTLCSource::OutboundRoute.
This stores and tracks HTLC payee information with HTLCSource info, allowing us to provide it back to the user if the HTLC fails and ensuring persistence by keeping it with the HTLC itself as it passes between Channel and ChannelMonitor.
1 parent fe237f9 commit 1b99c93

File tree

4 files changed

+49
-18
lines changed

4 files changed

+49
-18
lines changed

lightning/src/ln/channel.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5768,6 +5768,7 @@ mod tests {
57685768
first_hop_htlc_msat: 548,
57695769
payment_id: PaymentId([42; 32]),
57705770
payment_secret: None,
5771+
payee: None,
57715772
}
57725773
});
57735774

lightning/src/ln/channelmanager.rs

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ use chain::transaction::{OutPoint, TransactionData};
4545
use ln::{PaymentHash, PaymentPreimage, PaymentSecret};
4646
use ln::channel::{Channel, ChannelError, ChannelUpdateStatus, UpdateFulfillCommitFetch};
4747
use ln::features::{InitFeatures, NodeFeatures};
48-
use routing::router::{Route, RouteHop};
48+
use routing::router::{Payee, PaymentPathRetry, Route, RouteHop};
4949
use ln::msgs;
5050
use ln::msgs::NetAddress;
5151
use ln::onion_utils;
@@ -201,6 +201,7 @@ pub(crate) enum HTLCSource {
201201
first_hop_htlc_msat: u64,
202202
payment_id: PaymentId,
203203
payment_secret: Option<PaymentSecret>,
204+
payee: Option<Payee>,
204205
},
205206
}
206207
#[allow(clippy::derive_hash_xor_eq)] // Our Hash is faithful to the data, we just don't have SecretKey::hash
@@ -211,13 +212,14 @@ impl core::hash::Hash for HTLCSource {
211212
0u8.hash(hasher);
212213
prev_hop_data.hash(hasher);
213214
},
214-
HTLCSource::OutboundRoute { path, session_priv, payment_id, payment_secret, first_hop_htlc_msat } => {
215+
HTLCSource::OutboundRoute { path, session_priv, payment_id, payment_secret, first_hop_htlc_msat, payee } => {
215216
1u8.hash(hasher);
216217
path.hash(hasher);
217218
session_priv[..].hash(hasher);
218219
payment_id.hash(hasher);
219220
payment_secret.hash(hasher);
220221
first_hop_htlc_msat.hash(hasher);
222+
payee.hash(hasher);
221223
},
222224
}
223225
}
@@ -231,6 +233,7 @@ impl HTLCSource {
231233
first_hop_htlc_msat: 0,
232234
payment_id: PaymentId([2; 32]),
233235
payment_secret: None,
236+
payee: None,
234237
}
235238
}
236239
}
@@ -2021,7 +2024,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
20212024
}
20222025

20232026
// Only public for testing, this should otherwise never be called direcly
2024-
pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>) -> Result<(), APIError> {
2027+
pub(crate) fn send_payment_along_path(&self, path: &Vec<RouteHop>, payee: &Option<Payee>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>) -> Result<(), APIError> {
20252028
log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first().unwrap().short_channel_id);
20262029
let prng_seed = self.keys_manager.get_secure_random_bytes();
20272030
let session_priv_bytes = self.keys_manager.get_secure_random_bytes();
@@ -2071,6 +2074,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
20712074
first_hop_htlc_msat: htlc_msat,
20722075
payment_id,
20732076
payment_secret: payment_secret.clone(),
2077+
payee: payee.clone(),
20742078
}, onion_packet, &self.logger),
20752079
channel_state, chan);
20762080

@@ -2209,7 +2213,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
22092213
let cur_height = self.best_block.read().unwrap().height() + 1;
22102214
let mut results = Vec::new();
22112215
for path in route.paths.iter() {
2212-
results.push(self.send_payment_along_path(&path, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage));
2216+
results.push(self.send_payment_along_path(&path, &route.payee, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage));
22132217
}
22142218
let mut has_ok = false;
22152219
let mut has_err = false;
@@ -3098,14 +3102,22 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
30983102
self.fail_htlc_backwards_internal(channel_state,
30993103
htlc_src, &payment_hash, HTLCFailReason::Reason { failure_code, data: onion_failure_data});
31003104
},
3101-
HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
3105+
HTLCSource::OutboundRoute { session_priv, payment_id, path, payee, .. } => {
31023106
let mut session_priv_bytes = [0; 32];
31033107
session_priv_bytes.copy_from_slice(&session_priv[..]);
31043108
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
31053109
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
3106-
if payment.get_mut().remove(&session_priv_bytes, Some(path.last().unwrap().fee_msat)) &&
3110+
let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
3111+
if payment.get_mut().remove(&session_priv_bytes, Some(path_last_hop.fee_msat)) &&
31073112
!payment.get().is_fulfilled()
31083113
{
3114+
let retry = if let Some(payee_data) = payee {
3115+
Some(PaymentPathRetry {
3116+
payee: payee_data,
3117+
final_value_msat: path_last_hop.fee_msat,
3118+
final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
3119+
})
3120+
} else { None };
31093121
self.pending_events.lock().unwrap().push(
31103122
events::Event::PaymentPathFailed {
31113123
payment_hash,
@@ -3114,7 +3126,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
31143126
all_paths_failed: payment.get().remaining_parts() == 0,
31153127
path: path.clone(),
31163128
short_channel_id: None,
3117-
retry: None,
3129+
retry,
31183130
#[cfg(test)]
31193131
error_code: None,
31203132
#[cfg(test)]
@@ -3146,13 +3158,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
31463158
// from block_connected which may run during initialization prior to the chain_monitor
31473159
// being fully configured. See the docs for `ChannelManagerReadArgs` for more.
31483160
match source {
3149-
HTLCSource::OutboundRoute { ref path, session_priv, payment_id, .. } => {
3161+
HTLCSource::OutboundRoute { ref path, session_priv, payment_id, ref payee, .. } => {
31503162
let mut session_priv_bytes = [0; 32];
31513163
session_priv_bytes.copy_from_slice(&session_priv[..]);
31523164
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
31533165
let mut all_paths_failed = false;
3166+
let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
31543167
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
3155-
if !payment.get_mut().remove(&session_priv_bytes, Some(path.last().unwrap().fee_msat)) {
3168+
if !payment.get_mut().remove(&session_priv_bytes, Some(path_last_hop.fee_msat)) {
31563169
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
31573170
return;
31583171
}
@@ -3167,8 +3180,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
31673180
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
31683181
return;
31693182
}
3170-
log_trace!(self.logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
31713183
mem::drop(channel_state_lock);
3184+
let retry = if let Some(payee_data) = payee {
3185+
Some(PaymentPathRetry {
3186+
payee: payee_data.clone(),
3187+
final_value_msat: path_last_hop.fee_msat,
3188+
final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
3189+
})
3190+
} else { None };
3191+
log_trace!(self.logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
31723192
match &onion_error {
31733193
&HTLCFailReason::LightningError { ref err } => {
31743194
#[cfg(test)]
@@ -3186,7 +3206,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
31863206
all_paths_failed,
31873207
path: path.clone(),
31883208
short_channel_id,
3189-
retry: None,
3209+
retry,
31903210
#[cfg(test)]
31913211
error_code: onion_error_code,
31923212
#[cfg(test)]
@@ -3215,7 +3235,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
32153235
all_paths_failed,
32163236
path: path.clone(),
32173237
short_channel_id: Some(path.first().unwrap().short_channel_id),
3218-
retry: None,
3238+
retry,
32193239
#[cfg(test)]
32203240
error_code: Some(*failure_code),
32213241
#[cfg(test)]
@@ -5378,12 +5398,14 @@ impl Readable for HTLCSource {
53785398
let mut path = Some(Vec::new());
53795399
let mut payment_id = None;
53805400
let mut payment_secret = None;
5401+
let mut payee = None;
53815402
read_tlv_fields!(reader, {
53825403
(0, session_priv, required),
53835404
(1, payment_id, option),
53845405
(2, first_hop_htlc_msat, required),
53855406
(3, payment_secret, option),
53865407
(4, path, vec_type),
5408+
(5, payee, option),
53875409
});
53885410
if payment_id.is_none() {
53895411
// For backwards compat, if there was no payment_id written, use the session_priv bytes
@@ -5396,6 +5418,7 @@ impl Readable for HTLCSource {
53965418
path: path.unwrap(),
53975419
payment_id: payment_id.unwrap(),
53985420
payment_secret,
5421+
payee,
53995422
})
54005423
}
54015424
1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)),
@@ -5407,7 +5430,7 @@ impl Readable for HTLCSource {
54075430
impl Writeable for HTLCSource {
54085431
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::io::Error> {
54095432
match self {
5410-
HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret } => {
5433+
HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret, payee } => {
54115434
0u8.write(writer)?;
54125435
let payment_id_opt = Some(payment_id);
54135436
write_tlv_fields!(writer, {
@@ -5416,6 +5439,7 @@ impl Writeable for HTLCSource {
54165439
(2, first_hop_htlc_msat, required),
54175440
(3, payment_secret, option),
54185441
(4, path, vec_type),
5442+
(5, payee, option),
54195443
});
54205444
}
54215445
HTLCSource::PreviousHopData(ref field) => {
@@ -6160,7 +6184,7 @@ mod tests {
61606184
// Use the utility function send_payment_along_path to send the payment with MPP data which
61616185
// indicates there are more HTLCs coming.
61626186
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.
6163-
nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
6187+
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payee, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
61646188
check_added_monitors!(nodes[0], 1);
61656189
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
61666190
assert_eq!(events.len(), 1);
@@ -6190,7 +6214,7 @@ mod tests {
61906214
expect_payment_failed!(nodes[0], our_payment_hash, true);
61916215

61926216
// Send the second half of the original MPP payment.
6193-
nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
6217+
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payee, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None).unwrap();
61946218
check_added_monitors!(nodes[0], 1);
61956219
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
61966220
assert_eq!(events.len(), 1);

lightning/src/ln/functional_test_utils.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,9 +1114,12 @@ macro_rules! expect_payment_failed_with_update {
11141114
let events = $node.node.get_and_clear_pending_events();
11151115
assert_eq!(events.len(), 1);
11161116
match events[0] {
1117-
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => {
1117+
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, ref path, ref retry, .. } => {
11181118
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
11191119
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
1120+
assert!(retry.is_some(), "expected retry.is_some()");
1121+
assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
1122+
assert_eq!(retry.as_ref().unwrap().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
11201123
assert!(error_code.is_some(), "expected error_code.is_some() = true");
11211124
assert!(error_data.is_some(), "expected error_data.is_some() = true");
11221125
match network_update {
@@ -1143,9 +1146,12 @@ macro_rules! expect_payment_failed {
11431146
let events = $node.node.get_and_clear_pending_events();
11441147
assert_eq!(events.len(), 1);
11451148
match events[0] {
1146-
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => {
1149+
Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, ref path, ref retry, .. } => {
11471150
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
11481151
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
1152+
assert!(retry.is_some(), "expected retry.is_some()");
1153+
assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
1154+
assert_eq!(retry.as_ref().unwrap().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
11491155
assert!(error_code.is_some(), "expected error_code.is_some() = true");
11501156
assert!(error_data.is_some(), "expected error_data.is_some() = true");
11511157
$(

lightning/src/ln/functional_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3912,7 +3912,7 @@ fn do_test_htlc_timeout(send_partial_mpp: bool) {
39123912
// indicates there are more HTLCs coming.
39133913
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.
39143914
let payment_id = PaymentId([42; 32]);
3915-
nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200000, cur_height, payment_id, &None).unwrap();
3915+
nodes[0].node.send_payment_along_path(&route.paths[0], &route.payee, &our_payment_hash, &Some(payment_secret), 200000, cur_height, payment_id, &None).unwrap();
39163916
check_added_monitors!(nodes[0], 1);
39173917
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
39183918
assert_eq!(events.len(), 1);

0 commit comments

Comments
 (0)