Skip to content

Commit 7e9ac7b

Browse files
ViktorTigerstromViktorTigerstrom
committed
Avoid retaking locks
1 parent 7b594ee commit 7e9ac7b

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
@@ -1703,12 +1703,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
17031703
return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
17041704
}
17051705

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

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

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

17581755
fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<Signer>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
@@ -2789,12 +2786,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
27892786
fn funding_transaction_generated_intern<FundingOutput: Fn(&Channel<Signer>, &Transaction) -> Result<OutPoint, APIError>>(
27902787
&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
27912788
) -> Result<(), APIError> {
2792-
let (chan, msg) = {
2793-
let (res, chan) = {
2794-
let per_peer_state = self.per_peer_state.read().unwrap();
2795-
if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
2796-
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
2797-
let peer_state = &mut *peer_state_lock;
2789+
let mut channel_state_lock = self.channel_state.lock().unwrap();
2790+
let channel_state = &mut *channel_state_lock;
2791+
let per_peer_state = self.per_peer_state.read().unwrap();
2792+
if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
2793+
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
2794+
let peer_state = &mut *peer_state_lock;
2795+
2796+
let (chan, msg) = {
2797+
let (res, chan) = {
27982798
match peer_state.channel_by_id.remove(temporary_channel_id) {
27992799
Some(mut chan) => {
28002800
let funding_txo = find_funding_output(&chan, &funding_transaction)?;
@@ -2807,31 +2807,22 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
28072807
},
28082808
None => { return Err(APIError::ChannelUnavailable { err: "No such channel".to_owned() }) },
28092809
}
2810-
} else {
2811-
return Err(APIError::APIMisuseError { err: format!("Can't find a peer with a node_id matching the passed counterparty_node_id {}", counterparty_node_id) })
2810+
};
2811+
match handle_error!(self, res, chan.get_counterparty_node_id()) {
2812+
Ok(funding_msg) => {
2813+
(chan, funding_msg)
2814+
},
2815+
Err(_) => { return Err(APIError::ChannelUnavailable {
2816+
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()
2817+
}) },
28122818
}
28132819
};
2814-
match handle_error!(self, res, chan.get_counterparty_node_id()) {
2815-
Ok(funding_msg) => {
2816-
(chan, funding_msg)
2817-
},
2818-
Err(_) => { return Err(APIError::ChannelUnavailable {
2819-
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()
2820-
}) },
2821-
}
2822-
};
28232820

2824-
let mut channel_state_lock = self.channel_state.lock().unwrap();
2825-
let channel_state = &mut *channel_state_lock;
2826-
channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
2827-
node_id: chan.get_counterparty_node_id(),
2828-
msg,
2829-
});
2830-
let per_peer_state = self.per_peer_state.read().unwrap();
2831-
let chan_id = chan.channel_id();
2832-
if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
2833-
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
2834-
let peer_state = &mut *peer_state_lock;
2821+
channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
2822+
node_id: chan.get_counterparty_node_id(),
2823+
msg,
2824+
});
2825+
let chan_id = chan.channel_id();
28352826
match peer_state.channel_by_id.entry(chan_id) {
28362827
hash_map::Entry::Occupied(_) => {
28372828
panic!("Generated duplicate funding txid?");
@@ -2844,8 +2835,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
28442835
}
28452836
}
28462837
}
2847-
} else { unreachable!(); }
2848-
Ok(())
2838+
Ok(())
2839+
} else {
2840+
return Err(APIError::APIMisuseError { err: format!("Can't find a peer with a node_id matching the passed counterparty_node_id {}", counterparty_node_id) })
2841+
}
28492842
}
28502843

28512844
#[cfg(test)]

0 commit comments

Comments
 (0)