Skip to content

Commit 9aedf4e

Browse files
ViktorTigerstromViktorTigerstrom
committed
Avoid retaking locks
1 parent 8b7e095 commit 9aedf4e

File tree

1 file changed

+57
-64
lines changed

1 file changed

+57
-64
lines changed

‎lightning/src/ln/channelmanager.rs

Lines changed: 57 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1704,12 +1704,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
17041704
return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
17051705
}
17061706

1707-
let channel = {
1708-
let per_peer_state = self.per_peer_state.read().unwrap();
1709-
match per_peer_state.get(&their_network_key) {
1710-
Some(peer_state) => {
1707+
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
1708+
// We want to make sure the lock is actually acquired by PersistenceNotifierGuard.
1709+
debug_assert!(&self.total_consistency_lock.try_write().is_err());
1710+
1711+
let mut channel_state = self.channel_state.lock().unwrap();
1712+
let per_peer_state = self.per_peer_state.read().unwrap();
1713+
match per_peer_state.get(&their_network_key) {
1714+
None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
1715+
Some(peer_state) => {
1716+
let mut peer_state = peer_state.lock().unwrap();
1717+
let channel = {
17111718
let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
1712-
let peer_state = peer_state.lock().unwrap();
17131719
let their_features = &peer_state.latest_features;
17141720
let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
17151721
match Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key,
@@ -1722,38 +1728,29 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
17221728
return Err(e);
17231729
},
17241730
}
1725-
},
1726-
None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
1727-
}
1728-
};
1729-
let res = channel.get_open_channel(self.genesis_hash.clone());
1731+
};
1732+
let res = channel.get_open_channel(self.genesis_hash.clone());
17301733

1731-
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
1732-
// We want to make sure the lock is actually acquired by PersistenceNotifierGuard.
1733-
debug_assert!(&self.total_consistency_lock.try_write().is_err());
1734+
let temporary_channel_id = channel.channel_id();
17341735

1735-
let temporary_channel_id = channel.channel_id();
1736-
let mut channel_state = self.channel_state.lock().unwrap();
1737-
let per_peer_state = self.per_peer_state.read().unwrap();
1738-
if let Some(peer_state_mutex) = per_peer_state.get(&their_network_key){
1739-
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
1740-
let peer_state = &mut *peer_state_lock;
1741-
match peer_state.channel_by_id.entry(temporary_channel_id) {
1742-
hash_map::Entry::Occupied(_) => {
1743-
if cfg!(fuzzing) {
1744-
return Err(APIError::APIMisuseError { err: "Fuzzy bad RNG".to_owned() });
1745-
} else {
1746-
panic!("RNG is bad???");
1747-
}
1748-
},
1749-
hash_map::Entry::Vacant(entry) => { entry.insert(channel); }
1736+
match peer_state.channel_by_id.entry(temporary_channel_id) {
1737+
hash_map::Entry::Occupied(_) => {
1738+
if cfg!(fuzzing) {
1739+
return Err(APIError::APIMisuseError { err: "Fuzzy bad RNG".to_owned() });
1740+
} else {
1741+
panic!("RNG is bad???");
1742+
}
1743+
},
1744+
hash_map::Entry::Vacant(entry) => { entry.insert(channel); }
1745+
}
1746+
1747+
channel_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannel {
1748+
node_id: their_network_key,
1749+
msg: res,
1750+
});
1751+
Ok(temporary_channel_id)
17501752
}
1751-
} else { unreachable!() }
1752-
channel_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannel {
1753-
node_id: their_network_key,
1754-
msg: res,
1755-
});
1756-
Ok(temporary_channel_id)
1753+
}
17571754
}
17581755

17591756
fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<Signer>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
@@ -2843,12 +2840,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
28432840
fn funding_transaction_generated_intern<FundingOutput: Fn(&Channel<Signer>, &Transaction) -> Result<OutPoint, APIError>>(
28442841
&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
28452842
) -> Result<(), APIError> {
2846-
let (chan, msg) = {
2847-
let (res, chan) = {
2848-
let per_peer_state = self.per_peer_state.read().unwrap();
2849-
if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
2850-
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
2851-
let peer_state = &mut *peer_state_lock;
2843+
let mut channel_state_lock = self.channel_state.lock().unwrap();
2844+
let channel_state = &mut *channel_state_lock;
2845+
let per_peer_state = self.per_peer_state.read().unwrap();
2846+
if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
2847+
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
2848+
let peer_state = &mut *peer_state_lock;
2849+
2850+
let (chan, msg) = {
2851+
let (res, chan) = {
28522852
match peer_state.channel_by_id.remove(temporary_channel_id) {
28532853
Some(mut chan) => {
28542854
let funding_txo = find_funding_output(&chan, &funding_transaction)?;
@@ -2861,31 +2861,22 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
28612861
},
28622862
None => { return Err(APIError::ChannelUnavailable { err: "No such channel".to_owned() }) },
28632863
}
2864-
} else {
2865-
return Err(APIError::APIMisuseError { err: format!("Can't find a peer with a node_id matching the passed counterparty_node_id {}", counterparty_node_id) })
2864+
};
2865+
match handle_error!(self, res, chan.get_counterparty_node_id()) {
2866+
Ok(funding_msg) => {
2867+
(chan, funding_msg)
2868+
},
2869+
Err(_) => { return Err(APIError::ChannelUnavailable {
2870+
err: "Error deriving keys or signing initial commitment transactions - either our RNG or our counterparty's RNG is broken or the Signer refused to sign".to_owned()
2871+
}) },
28662872
}
28672873
};
2868-
match handle_error!(self, res, chan.get_counterparty_node_id()) {
2869-
Ok(funding_msg) => {
2870-
(chan, funding_msg)
2871-
},
2872-
Err(_) => { return Err(APIError::ChannelUnavailable {
2873-
err: "Error deriving keys or signing initial commitment transactions - either our RNG or our counterparty's RNG is broken or the Signer refused to sign".to_owned()
2874-
}) },
2875-
}
2876-
};
28772874

2878-
let mut channel_state_lock = self.channel_state.lock().unwrap();
2879-
let channel_state = &mut *channel_state_lock;
2880-
channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
2881-
node_id: chan.get_counterparty_node_id(),
2882-
msg,
2883-
});
2884-
let per_peer_state = self.per_peer_state.read().unwrap();
2885-
let chan_id = chan.channel_id();
2886-
if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
2887-
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
2888-
let peer_state = &mut *peer_state_lock;
2875+
channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
2876+
node_id: chan.get_counterparty_node_id(),
2877+
msg,
2878+
});
2879+
let chan_id = chan.channel_id();
28892880
match peer_state.channel_by_id.entry(chan_id) {
28902881
hash_map::Entry::Occupied(_) => {
28912882
panic!("Generated duplicate funding txid?");
@@ -2898,8 +2889,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
28982889
e.insert(chan);
28992890
}
29002891
}
2901-
} else { unreachable!(); }
2902-
Ok(())
2892+
Ok(())
2893+
} else {
2894+
return Err(APIError::APIMisuseError { err: format!("Can't find a peer with a node_id matching the passed counterparty_node_id {}", counterparty_node_id) })
2895+
}
29032896
}
29042897

29052898
#[cfg(test)]

0 commit comments

Comments
 (0)