Skip to content

Commit 36ce228

Browse files
f - take channel_state lock only when needed
1 parent 2292538 commit 36ce228

File tree

1 file changed

+36
-31
lines changed

1 file changed

+36
-31
lines changed

lightning/src/ln/channelmanager.rs

+36-31
Original file line numberDiff line numberDiff line change
@@ -2241,13 +2241,12 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
22412241
}
22422242
};
22432243

2244-
let mut channel_state = self.channel_state.lock().unwrap();
22452244
if let &PendingHTLCStatus::Forward(PendingHTLCInfo { ref routing, ref amt_to_forward, ref outgoing_cltv_value, .. }) = &pending_forward_info {
22462245
// If short_channel_id is 0 here, we'll reject the HTLC as there cannot be a channel
22472246
// with a short_channel_id of 0. This is important as various things later assume
22482247
// short_channel_id is non-0 in any ::Forward.
22492248
if let &PendingHTLCRouting::Forward { ref short_channel_id, .. } = routing {
2250-
let id_option = channel_state.short_to_chan_info.get(&short_channel_id).cloned();
2249+
let id_option = self.channel_state.lock().unwrap().short_to_chan_info.get(&short_channel_id).cloned();
22512250
if let Some((err, code, chan_update)) = loop {
22522251
let forwarding_id_opt = match id_option {
22532252
None => { // unknown_next_peer
@@ -2262,36 +2261,42 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
22622261
Some((_cp_id, chan_id)) => Some(chan_id.clone()),
22632262
};
22642263
let chan_update_opt = if let Some(forwarding_id) = forwarding_id_opt {
2265-
let chan = channel_state.by_id.get_mut(&forwarding_id).unwrap();
2266-
if !chan.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels {
2267-
// Note that the behavior here should be identical to the above block - we
2268-
// should NOT reveal the existence or non-existence of a private channel if
2269-
// we don't allow forwards outbound over them.
2270-
break Some(("Refusing to forward to a private channel based on our config.", 0x4000 | 10, None));
2271-
}
2272-
if chan.get_channel_type().supports_scid_privacy() && *short_channel_id != chan.outbound_scid_alias() {
2273-
// `option_scid_alias` (referred to in LDK as `scid_privacy`) means
2274-
// "refuse to forward unless the SCID alias was used", so we pretend
2275-
// we don't have the channel here.
2276-
break Some(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10, None));
2277-
}
2278-
let chan_update_opt = self.get_channel_update_for_onion(*short_channel_id, chan).ok();
2279-
2280-
// Note that we could technically not return an error yet here and just hope
2281-
// that the connection is reestablished or monitor updated by the time we get
2282-
// around to doing the actual forward, but better to fail early if we can and
2283-
// hopefully an attacker trying to path-trace payments cannot make this occur
2284-
// on a small/per-node/per-channel scale.
2285-
if !chan.is_live() { // channel_disabled
2286-
break Some(("Forwarding channel is not in a ready state.", 0x1000 | 20, chan_update_opt));
2287-
}
2288-
if *amt_to_forward < chan.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
2289-
break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt));
2290-
}
2291-
if let Err((err, code)) = chan.htlc_satisfies_config(&msg, *amt_to_forward, *outgoing_cltv_value) {
2292-
break Some((err, code, chan_update_opt));
2264+
let mut channel_state = self.channel_state.lock().unwrap();
2265+
if let Some(chan) = channel_state.by_id.get_mut(&forwarding_id){
2266+
if !chan.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels {
2267+
// Note that the behavior here should be identical to the above block - we
2268+
// should NOT reveal the existence or non-existence of a private channel if
2269+
// we don't allow forwards outbound over them.
2270+
break Some(("Refusing to forward to a private channel based on our config.", 0x4000 | 10, None));
2271+
}
2272+
if chan.get_channel_type().supports_scid_privacy() && *short_channel_id != chan.outbound_scid_alias() {
2273+
// `option_scid_alias` (referred to in LDK as `scid_privacy`) means
2274+
// "refuse to forward unless the SCID alias was used", so we pretend
2275+
// we don't have the channel here.
2276+
break Some(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10, None));
2277+
}
2278+
let chan_update_opt = self.get_channel_update_for_onion(*short_channel_id, chan).ok();
2279+
2280+
// Note that we could technically not return an error yet here and just hope
2281+
// that the connection is reestablished or monitor updated by the time we get
2282+
// around to doing the actual forward, but better to fail early if we can and
2283+
// hopefully an attacker trying to path-trace payments cannot make this occur
2284+
// on a small/per-node/per-channel scale.
2285+
if !chan.is_live() { // channel_disabled
2286+
break Some(("Forwarding channel is not in a ready state.", 0x1000 | 20, chan_update_opt));
2287+
}
2288+
if *amt_to_forward < chan.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
2289+
break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt));
2290+
}
2291+
if let Err((err, code)) = chan.htlc_satisfies_config(&msg, *amt_to_forward, *outgoing_cltv_value) {
2292+
break Some((err, code, chan_update_opt));
2293+
}
2294+
chan_update_opt
2295+
} else {
2296+
// No channel found for the forwarding_id_opt. The channel could have
2297+
// been dropped before the channel_state_lock was retaken.
2298+
break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
22932299
}
2294-
chan_update_opt
22952300
} else {
22962301
if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + MIN_CLTV_EXPIRY_DELTA as u64 { // incorrect_cltv_expiry
22972302
break Some((

0 commit comments

Comments
 (0)