Skip to content

Commit c21378f

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 555cb40 commit c21378f

File tree

1 file changed

+50
-42
lines changed

1 file changed

+50
-42
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 50 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1886,8 +1886,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
18861886
};
18871887

18881888
for htlc_source in failed_htlcs.drain(..) {
1889+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
18891890
let receiver = HTLCDestination::NextHopChannel { node_id: Some(*counterparty_node_id), channel_id: *channel_id };
1890-
self.fail_htlc_backwards_internal(htlc_source.0, &htlc_source.1, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
1891+
self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
18911892
}
18921893

18931894
let _ = handle_error!(self, result, *counterparty_node_id);
@@ -1944,8 +1945,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
19441945
log_debug!(self.logger, "Finishing force-closure of channel with {} HTLCs to fail", failed_htlcs.len());
19451946
for htlc_source in failed_htlcs.drain(..) {
19461947
let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
1948+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
19471949
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id };
1948-
self.fail_htlc_backwards_internal(source, &payment_hash, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
1950+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
19491951
}
19501952
if let Some((funding_txo, monitor_update)) = monitor_update_option {
19511953
// There isn't anything we can do if we get an update failure - we're already
@@ -3152,7 +3154,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
31523154

31533155
let failure_reason = HTLCFailReason::from_failure_code(0x4000 | 10);
31543156
let destination = HTLCDestination::UnknownNextHop { requested_forward_scid: short_channel_id };
3155-
self.fail_htlc_backwards_internal(htlc_source, &payment.forward_info.payment_hash, failure_reason, destination);
3157+
self.fail_htlc_backwards_internal(&htlc_source, &payment.forward_info.payment_hash, &failure_reason, destination);
31563158
} else { unreachable!() } // Only `PendingHTLCRouting::Forward`s are intercepted
31573159

31583160
Ok(())
@@ -3609,7 +3611,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
36093611
}
36103612

36113613
for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
3612-
self.fail_htlc_backwards_internal(htlc_source, &payment_hash, failure_reason, destination);
3614+
self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);
36133615
}
36143616
self.forward_htlcs(&mut phantom_receives);
36153617

@@ -3872,8 +3874,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
38723874
});
38733875

38743876
for htlc_source in timed_out_mpp_htlcs.drain(..) {
3877+
let source = HTLCSource::PreviousHopData(htlc_source.0.clone());
3878+
let reason = HTLCFailReason::from_failure_code(23);
38753879
let receiver = HTLCDestination::FailedPayment { payment_hash: htlc_source.1 };
3876-
self.fail_htlc_backwards_internal(HTLCSource::PreviousHopData(htlc_source.0.clone()), &htlc_source.1, HTLCFailReason::from_failure_code(23), receiver);
3880+
self.fail_htlc_backwards_internal(&source, &htlc_source.1, &reason, receiver);
38773881
}
38783882

38793883
for (err, counterparty_node_id) in handle_errors.drain(..) {
@@ -3908,10 +3912,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
39083912
let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
39093913
htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
39103914
self.best_block.read().unwrap().height()));
3911-
self.fail_htlc_backwards_internal(
3912-
HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
3913-
HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
3914-
HTLCDestination::FailedPayment { payment_hash: *payment_hash });
3915+
let source = HTLCSource::PreviousHopData(htlc.prev_hop);
3916+
let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
3917+
let receiver = HTLCDestination::FailedPayment { payment_hash: *payment_hash };
3918+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
39153919
}
39163920
}
39173921
}
@@ -3979,14 +3983,15 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
39793983
hash_map::Entry::Vacant(_) => (0x4000|10, Vec::new())
39803984
};
39813985

3986+
let reason = HTLCFailReason::reason(failure_code, onion_failure_data);
39823987
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id };
3983-
self.fail_htlc_backwards_internal(htlc_src, &payment_hash, HTLCFailReason::reason(failure_code, onion_failure_data), receiver);
3988+
self.fail_htlc_backwards_internal(&htlc_src, &payment_hash, &reason, receiver);
39843989
}
39853990
}
39863991

39873992
/// Fails an HTLC backwards to the sender of it to us.
39883993
/// Note that we do not assume that channels corresponding to failed HTLCs are still available.
3989-
fn fail_htlc_backwards_internal(&self, source: HTLCSource, payment_hash: &PaymentHash, onion_error: HTLCFailReason,destination: HTLCDestination) {
3994+
fn fail_htlc_backwards_internal(&self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, destination: HTLCDestination) {
39903995
#[cfg(debug_assertions)]
39913996
{
39923997
// Ensure that the `channel_state` lock is not held when calling this function.
@@ -4005,13 +4010,13 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
40054010
// from block_connected which may run during initialization prior to the chain_monitor
40064011
// being fully configured. See the docs for `ChannelManagerReadArgs` for more.
40074012
match source {
4008-
HTLCSource::OutboundRoute { ref path, session_priv, payment_id, ref payment_params, .. } => {
4013+
HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, ref payment_params, .. } => {
40094014
let mut session_priv_bytes = [0; 32];
40104015
session_priv_bytes.copy_from_slice(&session_priv[..]);
40114016
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
40124017
let mut all_paths_failed = false;
40134018
let mut full_failure_ev = None;
4014-
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(payment_id) {
4019+
if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) {
40154020
if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
40164021
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
40174022
return;
@@ -4024,7 +4029,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
40244029
all_paths_failed = true;
40254030
if payment.get().abandoned() {
40264031
full_failure_ev = Some(events::Event::PaymentFailed {
4027-
payment_id,
4032+
payment_id: *payment_id,
40284033
payment_hash: payment.get().payment_hash().expect("PendingOutboundPayments::RetriesExceeded always has a payment hash set"),
40294034
});
40304035
payment.remove();
@@ -4054,13 +4059,13 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
40544059
if self.payment_is_probe(payment_hash, &payment_id) {
40554060
if !payment_retryable {
40564061
events::Event::ProbeSuccessful {
4057-
payment_id,
4062+
payment_id: *payment_id,
40584063
payment_hash: payment_hash.clone(),
40594064
path: path.clone(),
40604065
}
40614066
} else {
40624067
events::Event::ProbeFailed {
4063-
payment_id,
4068+
payment_id: *payment_id,
40644069
payment_hash: payment_hash.clone(),
40654070
path: path.clone(),
40664071
short_channel_id,
@@ -4074,7 +4079,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
40744079
retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
40754080
}
40764081
events::Event::PaymentPathFailed {
4077-
payment_id: Some(payment_id),
4082+
payment_id: Some(*payment_id),
40784083
payment_hash: payment_hash.clone(),
40794084
payment_failed_permanently: !payment_retryable,
40804085
network_update,
@@ -4107,14 +4112,14 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
41074112

41084113
if self.payment_is_probe(payment_hash, &payment_id) {
41094114
events::Event::ProbeFailed {
4110-
payment_id,
4115+
payment_id: *payment_id,
41114116
payment_hash: payment_hash.clone(),
41124117
path: path.clone(),
41134118
short_channel_id: Some(scid),
41144119
}
41154120
} else {
41164121
events::Event::PaymentPathFailed {
4117-
payment_id: Some(payment_id),
4122+
payment_id: Some(*payment_id),
41184123
payment_hash: payment_hash.clone(),
41194124
payment_failed_permanently: false,
41204125
network_update: None,
@@ -4134,22 +4139,22 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
41344139
pending_events.push(path_failure);
41354140
if let Some(ev) = full_failure_ev { pending_events.push(ev); }
41364141
},
4137-
HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret, phantom_shared_secret, outpoint }) => {
4142+
HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
41384143
let err_packet = match onion_error {
4139-
HTLCFailReason::Reason { failure_code, data } => {
4144+
HTLCFailReason::Reason { ref failure_code, ref data } => {
41404145
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code);
41414146
if let Some(phantom_ss) = phantom_shared_secret {
4142-
let phantom_packet = onion_utils::build_failure_packet(&phantom_ss, failure_code, &data[..]).encode();
4143-
let encrypted_phantom_packet = onion_utils::encrypt_failure_packet(&phantom_ss, &phantom_packet);
4144-
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &encrypted_phantom_packet.data[..])
4147+
let phantom_packet = onion_utils::build_failure_packet(phantom_ss, *failure_code, &data[..]).encode();
4148+
let encrypted_phantom_packet = onion_utils::encrypt_failure_packet(phantom_ss, &phantom_packet);
4149+
onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &encrypted_phantom_packet.data[..])
41454150
} else {
4146-
let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
4147-
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet)
4151+
let packet = onion_utils::build_failure_packet(incoming_packet_shared_secret, *failure_code, &data[..]).encode();
4152+
onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &packet)
41484153
}
41494154
},
41504155
HTLCFailReason::LightningError { err } => {
41514156
log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards with pre-built LightningError", log_bytes!(payment_hash.0));
4152-
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &err.data)
4157+
onion_utils::encrypt_failure_packet(incoming_packet_shared_secret, &err.data)
41534158
}
41544159
};
41554160

@@ -4158,12 +4163,12 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
41584163
if forward_htlcs.is_empty() {
41594164
forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS));
41604165
}
4161-
match forward_htlcs.entry(short_channel_id) {
4166+
match forward_htlcs.entry(*short_channel_id) {
41624167
hash_map::Entry::Occupied(mut entry) => {
4163-
entry.get_mut().push(HTLCForwardInfo::FailHTLC { htlc_id, err_packet });
4168+
entry.get_mut().push(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet });
41644169
},
41654170
hash_map::Entry::Vacant(entry) => {
4166-
entry.insert(vec!(HTLCForwardInfo::FailHTLC { htlc_id, err_packet }));
4171+
entry.insert(vec!(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet }));
41674172
}
41684173
}
41694174
mem::drop(forward_htlcs);
@@ -4175,7 +4180,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
41754180
}
41764181
pending_events.push(events::Event::HTLCHandlingFailed {
41774182
prev_channel_id: outpoint.to_channel_id(),
4178-
failed_next_destination: destination
4183+
failed_next_destination: destination,
41794184
});
41804185
},
41814186
}
@@ -4304,10 +4309,10 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
43044309
let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
43054310
htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
43064311
self.best_block.read().unwrap().height()));
4307-
self.fail_htlc_backwards_internal(
4308-
HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash,
4309-
HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data),
4310-
HTLCDestination::FailedPayment { payment_hash } );
4312+
let source = HTLCSource::PreviousHopData(htlc.prev_hop);
4313+
let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
4314+
let receiver = HTLCDestination::FailedPayment { payment_hash };
4315+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
43114316
}
43124317
}
43134318

@@ -4631,7 +4636,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
46314636
self.finalize_claims(finalized_claims);
46324637
for failure in pending_failures.drain(..) {
46334638
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id: funding_txo.to_channel_id() };
4634-
self.fail_htlc_backwards_internal(failure.0, &failure.1, failure.2, receiver);
4639+
self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver);
46354640
}
46364641
}
46374642

@@ -4999,7 +5004,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
49995004
};
50005005
for htlc_source in dropped_htlcs.drain(..) {
50015006
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id: msg.channel_id };
5002-
self.fail_htlc_backwards_internal(htlc_source.0, &htlc_source.1, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
5007+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
5008+
self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
50035009
}
50045010

50055011
let _ = handle_error!(self, result, *counterparty_node_id);
@@ -5270,7 +5276,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
52705276
}
52715277

52725278
for (htlc_source, payment_hash, failure_reason, destination) in failed_intercept_forwards.drain(..) {
5273-
self.fail_htlc_backwards_internal(htlc_source, &payment_hash, failure_reason, destination);
5279+
self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);
52745280
}
52755281

52765282
if !new_intercept_events.is_empty() {
@@ -5345,7 +5351,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
53455351
{
53465352
for failure in pending_failures.drain(..) {
53475353
let receiver = HTLCDestination::NextHopChannel { node_id: Some(*counterparty_node_id), channel_id: channel_outpoint.to_channel_id() };
5348-
self.fail_htlc_backwards_internal(failure.0, &failure.1, failure.2, receiver);
5354+
self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver);
53495355
}
53505356
self.forward_htlcs(&mut [(short_channel_id, channel_outpoint, user_channel_id, pending_forwards)]);
53515357
self.finalize_claims(finalized_claim_htlcs);
@@ -5505,7 +5511,8 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
55055511
} else {
55065512
log_trace!(self.logger, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
55075513
let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id: funding_outpoint.to_channel_id() };
5508-
self.fail_htlc_backwards_internal(htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
5514+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
5515+
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);
55095516
}
55105517
},
55115518
MonitorEvent::CommitmentTxConfirmed(funding_outpoint) |
@@ -6315,7 +6322,7 @@ where
63156322
self.handle_init_event_channel_failures(failed_channels);
63166323

63176324
for (source, payment_hash, reason, destination) in timed_out_htlcs.drain(..) {
6318-
self.fail_htlc_backwards_internal(source, &payment_hash, reason, destination);
6325+
self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, destination);
63196326
}
63206327
}
63216328

@@ -7802,7 +7809,8 @@ impl<'a, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
78027809
for htlc_source in failed_htlcs.drain(..) {
78037810
let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
78047811
let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id), channel_id };
7805-
channel_manager.fail_htlc_backwards_internal(source, &payment_hash, HTLCFailReason::from_failure_code(0x4000 | 8), receiver);
7812+
let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
7813+
channel_manager.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
78067814
}
78077815

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

0 commit comments

Comments
 (0)