mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-24 15:02:20 +01:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #512 from TheBlueMatt/2020-02-peer_handler-docs

Browse source 
Fix incorrect docs/disconnect handling in peer_handler
This commit is contained in:
Matt Corallo 2020-02-21 19:02:21 +00:00 • committed by GitHub
commit 2be0810e78
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
fuzz/src/full_stack.rs
View file

@ -217,7 +217,7 @@ impl<'a> Drop for MoneyLossDetector<'a> {
// Disconnect all peers
for (idx, peer) in self.peers.borrow().iter().enumerate() {
if *peer {
self.handler.disconnect_event(&Peer{id: idx as u8, peers_connected: &self.peers});
self.handler.socket_disconnected(&Peer{id: idx as u8, peers_connected: &self.peers});
}
}
@ -378,7 +378,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
2 => {
let peer_id = get_slice!(1)[0];
if !peers.borrow()[peer_id as usize] { return; }
loss_detector.handler.disconnect_event(&Peer{id: peer_id, peers_connected: &peers});
loss_detector.handler.socket_disconnected(&Peer{id: peer_id, peers_connected: &peers});
peers.borrow_mut()[peer_id as usize] = false;
},
3 => {

4
lightning-net-tokio/src/lib.rs
View file

@ -59,7 +59,7 @@ impl Connection {
return future::Either::A(blocker.then(|_| { Ok(()) }));
}
}
//TODO: There's a race where we don't meet the requirements of disconnect_socket if its
//TODO: There's a race where we don't meet the requirements of socket_disconnected if its
//called right here, after we release the us_ref lock in the scope above, but before we
//call read_event!
match peer_manager.read_event(&mut SocketDescriptor::new(us_ref.clone(), peer_manager.clone()), pending_read) {
@ -84,7 +84,7 @@ impl Connection {
future::Either::B(future::result(Ok(())))
}).then(move |_| {
if us_close_ref.lock().unwrap().need_disconnect {
peer_manager_ref.disconnect_event(&SocketDescriptor::new(us_close_ref, peer_manager_ref.clone()));
peer_manager_ref.socket_disconnected(&SocketDescriptor::new(us_close_ref, peer_manager_ref.clone()));
println!("Peer disconnected!");
} else {
println!("We disconnected peer!");

60
lightning/src/ln/peer_handler.rs
View file

@ -49,12 +49,12 @@ pub struct MessageHandler<CM: Deref> where CM::Target: msgs::ChannelMessageHandl
/// You probably want to just extend an int and put a file descriptor in a struct and implement
/// send_data. Note that if you are using a higher-level net library that may close() itself, be
/// careful to ensure you don't have races whereby you might register a new connection with an fd
/// the same as a yet-to-be-disconnect_event()-ed.
/// the same as a yet-to-be-socket_disconnected()-ed.
pub trait SocketDescriptor : cmp::Eq + hash::Hash + Clone {
/// Attempts to send some data from the given slice to the peer.
///
/// Returns the amount of data which was sent, possibly 0 if the socket has since disconnected.
/// Note that in the disconnected case, a disconnect_event must still fire and further write
/// Note that in the disconnected case, a socket_disconnected must still fire and further write
/// attempts may occur until that time.
///
/// If the returned size is smaller than data.len(), a write_available event must
@ -67,17 +67,18 @@ pub trait SocketDescriptor : cmp::Eq + hash::Hash + Clone {
/// *not* imply that further read events should be paused.
fn send_data(&mut self, data: &[u8], resume_read: bool) -> usize;
/// Disconnect the socket pointed to by this SocketDescriptor. Once this function returns, no
/// more calls to write_event, read_event or disconnect_event may be made with this descriptor.
/// No disconnect_event should be generated as a result of this call, though obviously races
/// may occur whereby disconnect_socket is called after a call to disconnect_event but prior to
/// that event completing.
/// more calls to write_buffer_space_avail, read_event or socket_disconnected may be made with
/// this descriptor. No socket_disconnected call should be generated as a result of this call,
/// though obviously races may occur whereby disconnect_socket is called after a call to
/// socket_disconnected but prior to that event completing.
fn disconnect_socket(&mut self);
}
/// Error for PeerManager errors. If you get one of these, you must disconnect the socket and
/// generate no further read/write_events for the descriptor, only triggering a single
/// disconnect_event (unless it was provided in response to a new_*_connection event, in which case
/// no such disconnect_event must be generated and the socket be silently disconencted).
/// generate no further read_event/write_buffer_space_avail calls for the descriptor, only
/// triggering a single socket_disconnected call (unless it was provided in response to a
/// new_*_connection event, in which case no such socket_disconnected() must be generated and the
/// socket be silently disconencted).
pub struct PeerHandleError {
/// Used to indicate that we probably can't make any future connections to this peer, implying
/// we should go ahead and force-close any channels we have with it.
@ -201,7 +202,7 @@ macro_rules! encode_msg {
}
/// Manages and reacts to connection events. You probably want to use file descriptors as PeerIds.
/// PeerIds may repeat, but only after disconnect_event() has been called.
/// PeerIds may repeat, but only after socket_disconnected() has been called.
impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where CM::Target: msgs::ChannelMessageHandler {
/// Constructs a new PeerManager with the given message handlers and node_id secret key
/// ephemeral_random_data is used to derive per-connection ephemeral keys and must be
@ -254,13 +255,13 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
}
/// Indicates a new outbound connection has been established to a node with the given node_id.
/// Note that if an Err is returned here you MUST NOT call disconnect_event for the new
/// Note that if an Err is returned here you MUST NOT call socket_disconnected for the new
/// descriptor but must disconnect the connection immediately.
///
/// Returns a small number of bytes to send to the remote node (currently always 50).
///
/// Panics if descriptor is duplicative with some other descriptor which has not yet has a
/// disconnect_event.
/// Panics if descriptor is duplicative with some other descriptor which has not yet had a
/// socket_disconnected().
pub fn new_outbound_connection(&self, their_node_id: PublicKey, descriptor: Descriptor) -> Result<Vec<u8>, PeerHandleError> {
let mut peer_encryptor = PeerChannelEncryptor::new_outbound(their_node_id.clone(), self.get_ephemeral_key());
let res = peer_encryptor.get_act_one().to_vec();
@ -294,11 +295,11 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
///
/// May refuse the connection by returning an Err, but will never write bytes to the remote end
/// (outbound connector always speaks first). Note that if an Err is returned here you MUST NOT
/// call disconnect_event for the new descriptor but must disconnect the connection
/// call socket_disconnected for the new descriptor but must disconnect the connection
/// immediately.
///
/// Panics if descriptor is duplicative with some other descriptor which has not yet has a
/// disconnect_event.
/// Panics if descriptor is duplicative with some other descriptor which has not yet had
/// socket_disconnected called.
pub fn new_inbound_connection(&self, descriptor: Descriptor) -> Result<(), PeerHandleError> {
let peer_encryptor = PeerChannelEncryptor::new_inbound(&self.our_node_secret);
let pending_read_buffer = [0; 50].to_vec(); // Noise act one is 50 bytes
@ -406,10 +407,11 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
///
/// Will most likely call send_data on the descriptor passed in (or the descriptor handed into
/// new_*\_connection) before returning. Thus, be very careful with reentrancy issues! The
/// invariants around calling write_event in case a write did not fully complete must still
/// hold - be ready to call write_event again if a write call generated here isn't sufficient!
/// Panics if the descriptor was not previously registered in a new_\*_connection event.
pub fn write_event(&self, descriptor: &mut Descriptor) -> Result<(), PeerHandleError> {
/// invariants around calling write_buffer_space_avail in case a write did not fully complete
/// must still hold - be ready to call write_buffer_space_avail again if a write call generated
/// here isn't sufficient! Panics if the descriptor was not previously registered in a
/// new_\*_connection event.
pub fn write_buffer_space_avail(&self, descriptor: &mut Descriptor) -> Result<(), PeerHandleError> {
let mut peers = self.peers.lock().unwrap();
match peers.peers.get_mut(descriptor) {
None => panic!("Descriptor for write_event is not already known to PeerManager"),
@ -429,8 +431,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
/// Thus, however, you almost certainly want to call process_events() after any read_event to
/// generate send_data calls to handle responses.
///
/// If Ok(true) is returned, further read_events should not be triggered until a write_event on
/// this file descriptor has resume_read set (preventing DoS issues in the send buffer).
/// If Ok(true) is returned, further read_events should not be triggered until a send_data call
/// on this file descriptor has resume_read set (preventing DoS issues in the send buffer).
///
/// Panics if the descriptor was not previously registered in a new_*_connection event.
pub fn read_event(&self, peer_descriptor: &mut Descriptor, data: Vec<u8>) -> Result<bool, PeerHandleError> {
@ -1036,11 +1038,13 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
/// Indicates that the given socket descriptor's connection is now closed.
///
/// This must be called even if a PeerHandleError was given for a read_event or write_event,
/// but must NOT be called if a PeerHandleError was provided out of a new_\*\_connection event!
/// This must only be called if the socket has been disconnected by the peer or your own
/// decision to disconnect it and must NOT be called in any case where other parts of this
/// library (eg PeerHandleError, explicit disconnect_socket calls) instruct you to disconnect
/// the peer.
///
/// Panics if the descriptor was not previously registered in a successful new_*_connection event.
pub fn disconnect_event(&self, descriptor: &Descriptor) {
pub fn socket_disconnected(&self, descriptor: &Descriptor) {
self.disconnect_event_internal(descriptor, false);
}
@ -1073,10 +1077,12 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
let peers_needing_send = &mut peers.peers_needing_send;
let node_id_to_descriptor = &mut peers.node_id_to_descriptor;
let peers = &mut peers.peers;
let mut descriptors_needing_disconnect = Vec::new();
peers.retain(|descriptor, peer| {
if peer.awaiting_pong {
peers_needing_send.remove(descriptor);
descriptors_needing_disconnect.push(descriptor.clone());
match peer.their_node_id {
Some(node_id) => {
node_id_to_descriptor.remove(&node_id);
@ -1104,6 +1110,10 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
peer.awaiting_pong = true;
true
});
for mut descriptor in descriptors_needing_disconnect.drain(..) {
descriptor.disconnect_socket();
}
}
}
}