Skip to content

Commit 049eba4

Browse files
committed
Avoid holding a per_peer_state lock while claiming from a monitor
There's no reason to hold a lock on `per_peer_state` while we're claiming from a since-closed channel via a `ChannelMonitorUpdate`, which we stop doing here.
1 parent e34dab6 commit 049eba4

File tree

1 file changed

+36
-35
lines changed

1 file changed

+36
-35
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4244,45 +4244,46 @@ where
42444244
-> Result<(), (PublicKey, MsgHandleErrInternal)> {
42454245
//TODO: Delay the claimed_funds relaying just like we do outbound relay!
42464246

4247-
let per_peer_state = self.per_peer_state.read().unwrap();
4248-
let chan_id = prev_hop.outpoint.to_channel_id();
4249-
let counterparty_node_id_opt = match self.short_to_chan_info.read().unwrap().get(&prev_hop.short_channel_id) {
4250-
Some((cp_id, _dup_chan_id)) => Some(cp_id.clone()),
4251-
None => None
4252-
};
4247+
{
4248+
let per_peer_state = self.per_peer_state.read().unwrap();
4249+
let chan_id = prev_hop.outpoint.to_channel_id();
4250+
let counterparty_node_id_opt = match self.short_to_chan_info.read().unwrap().get(&prev_hop.short_channel_id) {
4251+
Some((cp_id, _dup_chan_id)) => Some(cp_id.clone()),
4252+
None => None
4253+
};
42534254

4254-
let peer_state_opt = counterparty_node_id_opt.as_ref().map(
4255-
|counterparty_node_id| per_peer_state.get(counterparty_node_id).map(
4256-
|peer_mutex| peer_mutex.lock().unwrap()
4257-
)
4258-
).unwrap_or(None);
4255+
let peer_state_opt = counterparty_node_id_opt.as_ref().map(
4256+
|counterparty_node_id| per_peer_state.get(counterparty_node_id)
4257+
.map(|peer_mutex| peer_mutex.lock().unwrap())
4258+
).unwrap_or(None);
42594259

4260-
if peer_state_opt.is_some() {
4261-
let mut peer_state_lock = peer_state_opt.unwrap();
4262-
let peer_state = &mut *peer_state_lock;
4263-
if let hash_map::Entry::Occupied(mut chan) = peer_state.channel_by_id.entry(chan_id) {
4264-
let counterparty_node_id = chan.get().get_counterparty_node_id();
4265-
let fulfill_res = chan.get_mut().get_update_fulfill_htlc_and_commit(prev_hop.htlc_id, payment_preimage, &self.logger);
4266-
4267-
if let UpdateFulfillCommitFetch::NewClaim { htlc_value_msat, monitor_update } = fulfill_res {
4268-
if let Some(action) = completion_action(Some(htlc_value_msat)) {
4269-
log_trace!(self.logger, "Tracking monitor update completion action for channel {}: {:?}",
4270-
log_bytes!(chan_id), action);
4271-
peer_state.monitor_update_blocked_actions.entry(chan_id).or_insert(Vec::new()).push(action);
4272-
}
4273-
let update_id = monitor_update.update_id;
4274-
let update_res = self.chain_monitor.update_channel(prev_hop.outpoint, monitor_update);
4275-
let res = handle_new_monitor_update!(self, update_res, update_id, peer_state_lock,
4276-
peer_state, per_peer_state, chan);
4277-
if let Err(e) = res {
4278-
// TODO: This is a *critical* error - we probably updated the outbound edge
4279-
// of the HTLC's monitor with a preimage. We should retry this monitor
4280-
// update over and over again until morale improves.
4281-
log_error!(self.logger, "Failed to update channel monitor with preimage {:?}", payment_preimage);
4282-
return Err((counterparty_node_id, e));
4260+
if peer_state_opt.is_some() {
4261+
let mut peer_state_lock = peer_state_opt.unwrap();
4262+
let peer_state = &mut *peer_state_lock;
4263+
if let hash_map::Entry::Occupied(mut chan) = peer_state.channel_by_id.entry(chan_id) {
4264+
let counterparty_node_id = chan.get().get_counterparty_node_id();
4265+
let fulfill_res = chan.get_mut().get_update_fulfill_htlc_and_commit(prev_hop.htlc_id, payment_preimage, &self.logger);
4266+
4267+
if let UpdateFulfillCommitFetch::NewClaim { htlc_value_msat, monitor_update } = fulfill_res {
4268+
if let Some(action) = completion_action(Some(htlc_value_msat)) {
4269+
log_trace!(self.logger, "Tracking monitor update completion action for channel {}: {:?}",
4270+
log_bytes!(chan_id), action);
4271+
peer_state.monitor_update_blocked_actions.entry(chan_id).or_insert(Vec::new()).push(action);
4272+
}
4273+
let update_id = monitor_update.update_id;
4274+
let update_res = self.chain_monitor.update_channel(prev_hop.outpoint, monitor_update);
4275+
let res = handle_new_monitor_update!(self, update_res, update_id, peer_state_lock,
4276+
peer_state, per_peer_state, chan);
4277+
if let Err(e) = res {
4278+
// TODO: This is a *critical* error - we probably updated the outbound edge
4279+
// of the HTLC's monitor with a preimage. We should retry this monitor
4280+
// update over and over again until morale improves.
4281+
log_error!(self.logger, "Failed to update channel monitor with preimage {:?}", payment_preimage);
4282+
return Err((counterparty_node_id, e));
4283+
}
42834284
}
4285+
return Ok(());
42844286
}
4285-
return Ok(());
42864287
}
42874288
}
42884289
let preimage_update = ChannelMonitorUpdate {

0 commit comments

Comments
 (0)