Skip to content

Commit ee4b2b3

Browse files
tchardingtcharding
committed
Make fail_htlc_backwards_internal borrow parameters
Currently `fail_htlc_backwards_internal` takes ownership of its source and reason parameters however they are not consumed so we can borrow them. Includes refactoring to use local variables before the function call.
1 parent 7bca3a1 commit ee4b2b3

File tree

1 file changed

+48
-40
lines changed

1 file changed

+48
-40
lines changed

‎lightning/src/ln/channelmanager.rs

Lines changed: 48 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,8 +1853,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
18531853
};
18541854

18551855
for htlc_source in failed_htlcs.drain(..) {
1856+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
18561857
let receiver = HTLCDestination::NextHopChannel { node_id: Some(*counterparty_node_id), channel_id: *channel_id };
1857-
self.fail_htlc_backwards_internal(htlc_source.0, &htlc_source.1, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
1858+
self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
18581859
}
18591860

18601861
let _ = handle_error!(self, result, *counterparty_node_id);
@@ -1911,8 +1912,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
19111912
log_debug!(self.logger, "Finishing force-closure of channel with {} HTLCs to fail", failed_htlcs.len());
19121913
for htlc_source in failed_htlcs.drain(..) {
19131914
let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
1915+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
19141916
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id };
1915-
self.fail_htlc_backwards_internal(source, &payment_hash, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
1917+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
19161918
}
19171919
if let Some((funding_txo, monitor_update)) = monitor_update_option {
19181920
// There isn't anything we can do if we get an update failure - we're already
@@ -3471,7 +3473,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
34713473
}
34723474

34733475
for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
3474-
self.fail_htlc_backwards_internal(htlc_source, &payment_hash, failure_reason, destination);
3476+
self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);
34753477
}
34763478
self.forward_htlcs(&mut phantom_receives);
34773479

@@ -3734,8 +3736,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
37343736
});
37353737

37363738
for htlc_source in timed_out_mpp_htlcs.drain(..) {
3739+
let source = HTLCSource::PreviousHopData(htlc_source.0.clone());
3740+
let reason = HTLCFailReason::from_failure_code(23);
37373741
let receiver = HTLCDestination::FailedPayment { payment_hash: htlc_source.1 };
3738-
self.fail_htlc_backwards_internal(HTLCSource::PreviousHopData(htlc_source.0.clone()), &htlc_source.1, HTLCFailReason::from_failure_code(23), receiver );
3742+
self.fail_htlc_backwards_internal(&source, &htlc_source.1, &reason, receiver);
37393743
}
37403744

37413745
for (err, counterparty_node_id) in handle_errors.drain(..) {
@@ -3770,10 +3774,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
37703774
let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
37713775
htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
37723776
self.best_block.read().unwrap().height()));
3773-
self.fail_htlc_backwards_internal(
3774-
HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
3775-
HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
3776-
HTLCDestination::FailedPayment { payment_hash: *payment_hash });
3777+
let source = HTLCSource::PreviousHopData(htlc.prev_hop);
3778+
let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
3779+
let receiver = HTLCDestination::FailedPayment { payment_hash: *payment_hash };
3780+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
37773781
}
37783782
}
37793783
}
@@ -3841,14 +3845,15 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
38413845
hash_map::Entry::Vacant(_) => (0x4000|10, Vec::new())
38423846
};
38433847

3848+
let reason = HTLCFailReason::reason(failure_code, onion_failure_data);
38443849
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id };
3845-
self.fail_htlc_backwards_internal(htlc_src, &payment_hash, HTLCFailReason::reason(failure_code, onion_failure_data), receiver);
3850+
self.fail_htlc_backwards_internal(&htlc_src, &payment_hash, &reason, receiver);
38463851
}
38473852
}
38483853

38493854
/// Fails an HTLC backwards to the sender of it to us.
38503855
/// Note that we do not assume that channels corresponding to failed HTLCs are still available.
3851-
fn fail_htlc_backwards_internal(&self, source: HTLCSource, payment_hash: &PaymentHash, onion_error: HTLCFailReason,destination: HTLCDestination) {
3856+
fn fail_htlc_backwards_internal(&self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, destination: HTLCDestination) {
38523857
#[cfg(debug_assertions)]
38533858
{
38543859
// Ensure that the `channel_state` lock is not held when calling this function.
@@ -3867,13 +3872,13 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
38673872
// from block_connected which may run during initialization prior to the chain_monitor
38683873
// being fully configured. See the docs for `ChannelManagerReadArgs` for more.
38693874
match source {
3870-
HTLCSource::OutboundRoute { ref path, session_priv, payment_id, ref payment_params, .. } => {
3875+
HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, ref payment_params, .. } => {
38713876
let mut session_priv_bytes = [0; 32];
38723877
session_priv_bytes.copy_from_slice(&session_priv[..]);
38733878
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
38743879
let mut all_paths_failed = false;
38753880
let mut full_failure_ev = None;
3876-
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
3881+
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) {
38773882
if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
38783883
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
38793884
return;
@@ -3886,7 +3891,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
38863891
all_paths_failed = true;
38873892
if payment.get().abandoned() {
38883893
full_failure_ev = Some(events::Event::PaymentFailed {
3889-
payment_id,
3894+
payment_id: *payment_id,
38903895
payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
38913896
});
38923897
payment.remove();
@@ -3916,13 +3921,13 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
39163921
if self.payment_is_probe(payment_hash, &payment_id) {
39173922
if !payment_retryable {
39183923
events::Event::ProbeSuccessful {
3919-
payment_id,
3924+
payment_id: *payment_id,
39203925
payment_hash: payment_hash.clone(),
39213926
path: path.clone(),
39223927
}
39233928
} else {
39243929
events::Event::ProbeFailed {
3925-
payment_id,
3930+
payment_id: *payment_id,
39263931
payment_hash: payment_hash.clone(),
39273932
path: path.clone(),
39283933
short_channel_id,
@@ -3936,7 +3941,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
39363941
retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
39373942
}
39383943
events::Event::PaymentPathFailed {
3939-
payment_id: Some(payment_id),
3944+
payment_id: Some(*payment_id),
39403945
payment_hash: payment_hash.clone(),
39413946
payment_failed_permanently: !payment_retryable,
39423947
network_update,
@@ -3969,14 +3974,14 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
39693974

39703975
if self.payment_is_probe(payment_hash, &payment_id) {
39713976
events::Event::ProbeFailed {
3972-
payment_id,
3977+
payment_id: *payment_id,
39733978
payment_hash: payment_hash.clone(),
39743979
path: path.clone(),
39753980
short_channel_id: Some(scid),
39763981
}
39773982
} else {
39783983
events::Event::PaymentPathFailed {
3979-
payment_id: Some(payment_id),
3984+
payment_id: Some(*payment_id),
39803985
payment_hash: payment_hash.clone(),
39813986
payment_failed_permanently: false,
39823987
network_update: None,
@@ -3996,22 +4001,22 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
39964001
pending_events.push(path_failure);
39974002
if let Some(ev) = full_failure_ev { pending_events.push(ev); }
39984003
},
3999-
HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, phantom_shared_secret, outpoint }) => {
4004+
HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
40004005
let err_packet = match onion_error {
4001-
HTLCFailReason::Reason { failure_code, data } => {
4006+
HTLCFailReason::Reason { ref failure_code, ref data } => {
40024007
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code);
40034008
if let Some(phantom_ss) = phantom_shared_secret {
4004-
let phantom_packet = onion_utils::build_failure_packet(&phantom_ss, failure_code, &data[..]).encode();
4005-
let encrypted_phantom_packet = onion_utils::encrypt_failure_packet(&phantom_ss, &phantom_packet);
4006-
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &encrypted_phantom_packet.data[..])
4009+
let phantom_packet = onion_utils::build_failure_packet(phantom_ss, *failure_code, &data[..]).encode();
4010+
let encrypted_phantom_packet = onion_utils::encrypt_failure_packet(phantom_ss, &phantom_packet);
4011+
onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &encrypted_phantom_packet.data[..])
40074012
} else {
4008-
let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
4009-
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet)
4013+
let packet = onion_utils::build_failure_packet(incoming_packet_shared_secret, *failure_code, &data[..]).encode();
4014+
onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &packet)
40104015
}
40114016
},
40124017
HTLCFailReason::LightningError { err } => {
40134018
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards with pre-built LightningError", log_bytes!(payment_hash.0));
4014-
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &err.data)
4019+
onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &err.data)
40154020
}
40164021
};
40174022

@@ -4020,12 +4025,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
40204025
if forward_htlcs.is_empty() {
40214026
forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS));
40224027
}
4023-
match forward_htlcs.entry(short_channel_id) {
4028+
match forward_htlcs.entry(*short_channel_id) {
40244029
hash_map::Entry::Occupied(mut entry) => {
4025-
entry.get_mut().push(HTLCForwardInfo::FailHTLC { htlc_id, err_packet });
4030+
entry.get_mut().push(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet });
40264031
},
40274032
hash_map::Entry::Vacant(entry) => {
4028-
entry.insert(vec!(HTLCForwardInfo::FailHTLC { htlc_id, err_packet }));
4033+
entry.insert(vec!(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet }));
40294034
}
40304035
}
40314036
mem::drop(forward_htlcs);
@@ -4037,7 +4042,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
40374042
}
40384043
pending_events.push(events::Event::HTLCHandlingFailed {
40394044
prev_channel_id: outpoint.to_channel_id(),
4040-
failed_next_destination: destination
4045+
failed_next_destination: destination,
40414046
});
40424047
},
40434048
}
@@ -4166,10 +4171,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
41664171
let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
41674172
htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
41684173
self.best_block.read().unwrap().height()));
4169-
self.fail_htlc_backwards_internal(
4170-
HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash,
4171-
HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
4172-
HTLCDestination::FailedPayment { payment_hash } );
4174+
let source = HTLCSource::PreviousHopData(htlc.prev_hop);
4175+
let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
4176+
let receiver = HTLCDestination::FailedPayment { payment_hash };
4177+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
41734178
}
41744179
}
41754180

@@ -4498,7 +4503,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
44984503
self.finalize_claims(finalized_claims);
44994504
for failure in pending_failures.drain(..) {
45004505
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id: funding_txo.to_channel_id() };
4501-
self.fail_htlc_backwards_internal(failure.0, &failure.1, failure.2, receiver);
4506+
self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver);
45024507
}
45034508
}
45044509

@@ -4866,7 +4871,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
48664871
};
48674872
for htlc_source in dropped_htlcs.drain(..) {
48684873
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id: msg.channel_id };
4869-
self.fail_htlc_backwards_internal(htlc_source.0, &htlc_source.1, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
4874+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
4875+
self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
48704876
}
48714877

48724878
let _ = handle_error!(self, result, *counterparty_node_id);
@@ -5157,7 +5163,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
51575163
{
51585164
for failure in pending_failures.drain(..) {
51595165
let receiver = HTLCDestination::NextHopChannel { node_id: Some(*counterparty_node_id), channel_id: channel_outpoint.to_channel_id() };
5160-
self.fail_htlc_backwards_internal(failure.0, &failure.1, failure.2, receiver);
5166+
self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver);
51615167
}
51625168
self.forward_htlcs(&mut [(short_channel_id, channel_outpoint, pending_forwards)]);
51635169
self.finalize_claims(finalized_claim_htlcs);
@@ -5317,7 +5323,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
53175323
} else {
53185324
log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
53195325
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
5320-
self.fail_htlc_backwards_internal(htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
5326+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
5327+
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);
53215328
}
53225329
},
53235330
MonitorEvent::CommitmentTxConfirmed(funding_outpoint) |
@@ -6087,7 +6094,7 @@ where
60876094
self.handle_init_event_channel_failures(failed_channels);
60886095

60896096
for (source, payment_hash, reason, destination) in timed_out_htlcs.drain(..) {
6090-
self.fail_htlc_backwards_internal(source, &payment_hash, reason, destination);
6097+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, destination);
60916098
}
60926099
}
60936100

@@ -7560,7 +7567,8 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
75607567
for htlc_source in failed_htlcs.drain(..) {
75617568
let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
75627569
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id };
7563-
channel_manager.fail_htlc_backwards_internal(source, &payment_hash, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
7570+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
7571+
channel_manager.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
75647572
}
75657573

75667574
//TODO: Broadcast channel update for closed channels, but only after we've made a

0 commit comments

Comments
 (0)