refactor: Abstract Transport layer and test it

Start moving the code that implements the Bolt #8 transport layer into a
separate testable module.

This patch makes heavy use of Rust's supported mocking pattern to
create test doubles and leverage them to write simple unit tests for the
public API of Transport.

Author: julianknutsen

lightning/src/ln/peers/handler.rs

@@ -5,22 +5,32 @@
 use bitcoin::secp256k1::{PublicKey, SecretKey};
 
 use ln::peers::conduit::Conduit;
-use ln::peers::handshake::states::{HandshakeState, IHandshakeState};
+use ln::peers::handshake::acts::Act;
+use ln::peers::handshake::states::HandshakeState;
+use ln::peers::transport::IPeerHandshake;
 
 mod acts;
 mod states;
 
+/// Interface used by PeerHandshake to interact with NOISE state machine.
+/// State may transition to the same state in the event there are not yet enough bytes to move
+/// forward with the handshake.
+trait IHandshakeState {
+	/// Returns the next HandshakeState after processing the input bytes
+	fn next(self, input: &[u8]) -> Result<(Option<Act>, HandshakeState), String>;
+}
+
 /// Object for managing handshakes.
 /// Currently requires explicit ephemeral private key specification.
 pub struct PeerHandshake {
 	state: Option<HandshakeState>,
 	ready_to_process: bool,
 }
 
-impl PeerHandshake {
+impl IPeerHandshake for PeerHandshake {
 	/// Instantiate a handshake given the peer's static public key. The ephemeral private key MUST
 	/// generate a new session with strong cryptographic randomness.
-	pub fn new_outbound(initiator_static_private_key: &SecretKey, responder_static_public_key: &PublicKey, initiator_ephemeral_private_key: &SecretKey) -> Self {
+	fn new_outbound(initiator_static_private_key: &SecretKey, responder_static_public_key: &PublicKey, initiator_ephemeral_private_key: &SecretKey) -> Self {
 		let state = HandshakeState::new_initiator(initiator_static_private_key, responder_static_public_key, initiator_ephemeral_private_key);
 
 		Self {
@@ -29,16 +39,8 @@ impl PeerHandshake {
 		}
 	}
 
-	/// Instantiate a handshake in anticipation of a peer's first handshake act
-	pub fn new_inbound(responder_static_private_key: &SecretKey, responder_ephemeral_private_key: &SecretKey) -> Self {
-		Self {
-			state: Some(HandshakeState::new_responder(responder_static_private_key, responder_ephemeral_private_key)),
-			ready_to_process: true,
-		}
-	}
-
 	/// Initializes the outbound handshake and provides the initial bytes to send to the responder
-	pub fn set_up_outbound(&mut self) -> Vec<u8> {
+	fn set_up_outbound(&mut self) -> Vec<u8> {
 		assert!(!self.ready_to_process);
 		self.ready_to_process = true;
 
@@ -49,6 +51,14 @@ impl PeerHandshake {
 		response_vec_option.unwrap()
 	}
 
+	/// Instantiate a new handshake in anticipation of a peer's first handshake act
+	fn new_inbound(responder_static_private_key: &SecretKey, responder_ephemeral_private_key: &SecretKey) -> Self {
+		Self {
+			state: Some(HandshakeState::new_responder(responder_static_private_key, responder_ephemeral_private_key)),
+			ready_to_process: true,
+		}
+	}
+
 	/// Process act dynamically
 	/// # Arguments
 	/// `input`: Byte slice received from peer as part of the handshake protocol
@@ -57,7 +67,7 @@ impl PeerHandshake {
 	/// Returns a tuple with the following components:
 	/// `.0`: Byte vector containing the next act to send back to the peer per the handshake protocol
 	/// `.1`: Some(Conduit, PublicKey) if the handshake was just processed to completion and messages can now be encrypted and decrypted
-	pub fn process_act(&mut self, input: &[u8]) -> Result<(Option<Vec<u8>>, Option<(Conduit, PublicKey)>), String> {
+	fn process_act(&mut self, input: &[u8]) -> Result<(Option<Vec<u8>>, Option<(Conduit, PublicKey)>), String> {
 		assert!(self.ready_to_process);
 		let cur_state = self.state.take().unwrap();
 
@@ -120,15 +130,6 @@ mod test {
 		}
 	}
 
-	macro_rules! assert_matches {
-		($e:expr, $state_match:pat) => {
-			match $e {
-				$state_match => (),
-				_ => panic!()
-			}
-		}
-	}
-
 	macro_rules! do_process_act_or_panic {
 		($handshake:expr, $input:expr) => {
 			$handshake.process_act($input).unwrap().0.unwrap()

lightning/src/ln/peers/handshake/mod.rs

@@ -7,6 +7,7 @@ use bitcoin::secp256k1::{SecretKey, PublicKey};
 use ln::peers::{chacha, hkdf5869rfc};
 use ln::peers::conduit::{Conduit, SymmetricKey};
 use ln::peers::handshake::acts::{Act, ActBuilder, ACT_ONE_LENGTH, ACT_TWO_LENGTH, ACT_THREE_LENGTH, EMPTY_ACT_ONE, EMPTY_ACT_TWO, EMPTY_ACT_THREE};
+use ln::peers::handshake::IHandshakeState;
 
 // Alias type to help differentiate between temporary key and chaining key when passing bytes around
 type ChainingKey = [u8; 32];
@@ -30,13 +31,6 @@ pub(super) enum HandshakeState {
 	Complete(Option<(Conduit, PublicKey)>),
 }
 
-// Trait for all individual states to implement that ensure HandshakeState::next() can
-// delegate to a common function signature. May transition to the same state in the event there are
-// not yet enough bytes to move forward with the handshake.
-pub(super) trait IHandshakeState {
-	fn next(self, input: &[u8]) -> Result<(Option<Act>, HandshakeState), String>;
-}
-
 // Enum dispatch for state machine. Single public interface can statically dispatch to all states
 impl HandshakeState {
 	pub(super) fn new_initiator(initiator_static_private_key: &SecretKey, responder_static_public_key: &PublicKey, initiator_ephemeral_private_key: &SecretKey) -> Self {

lightning/src/ln/peers/handshake/states.rs

@@ -3,10 +3,15 @@
 //! When a handshake completes, it returns an instance of Conduit.
 //! Conduit enables message encryption and decryption, and automatically handles key rotation.
 
+#[cfg(test)]
+#[macro_use]
+mod test_util;
+
 mod chacha;
 pub mod handler;
 mod hkdf5869rfc;
 mod outbound_buffer;
+mod transport;
 
 #[cfg(feature = "fuzztarget")]
 pub mod conduit;

lightning/src/ln/peers/mod.rs

@@ -83,104 +83,7 @@ impl OutboundBuffer {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use std::rc::Rc;
-	use std::cell::RefCell;
-	use std::hash::Hash;
-	use std::cmp;
-
-	/// Mock implementation of the SocketDescriptor trait that can be used in tests to finely control
-	/// the send_data() behavior.
-	///
-	///Additionally, records the actual calls to send_data() for later validation.
-	#[derive(Debug, Eq)]
-	struct SocketDescriptorMock {
-		/// If true, all send_data() calls will succeed
-		unbounded: Rc<RefCell<bool>>,
-
-		/// Amount of free space in the descriptor for send_data() bytes
-		free_space: Rc<RefCell<usize>>,
-
-		/// Vector of arguments and return values to send_data() used for validation
-		send_recording: Rc<RefCell<Vec<(Vec<u8>, bool)>>>,
-	}
-
-	impl SocketDescriptorMock {
-		/// Basic unbounded implementation where send_data() will always succeed
-		fn new() -> Self {
-			Self {
-				unbounded: Rc::new(RefCell::new(true)),
-				send_recording: Rc::new(RefCell::new(Vec::new())),
-				free_space: Rc::new(RefCell::new(0))
-			}
-		}
-
-		/// Used for tests that want to return partial sends after a certain amount of data is sent through send_data()
-		fn with_fixed_size(limit: usize) -> Self {
-			let mut descriptor = Self::new();
-			descriptor.unbounded = Rc::new(RefCell::new(false));
-			descriptor.free_space = Rc::new(RefCell::new(limit));
-
-			descriptor
-		}
-
-		/// Standard Mock api to verify actual vs. expected calls
-		fn assert_called_with(&self, expectation: Vec<(Vec<u8>, bool)>) {
-			assert_eq!(expectation.as_slice(), self.send_recording.borrow().as_slice())
-		}
-
-		/// Allow future send_data() calls to succeed for the next added_room bytes. Not valid for
-		/// unbounded mock descriptors
-		fn make_room(&mut self, added_room: usize) {
-			assert!(!*self.unbounded.borrow());
-			let mut free_space = self.free_space.borrow_mut();
-
-			*free_space += added_room;
-		}
-	}
-
-	impl SocketDescriptor for SocketDescriptorMock {
-		fn send_data(&mut self, data: &[u8], resume_read: bool) -> usize {
-			self.send_recording.borrow_mut().push((data.to_vec(), resume_read));
-
-			let mut free_space = self.free_space.borrow_mut();
-
-			// Unbounded just flush everything
-			return if *self.unbounded.borrow() {
-				data.len()
-			}
-			// Bounded flush up to the free_space limit
-			else {
-				let write_len = cmp::min(data.len(), *free_space);
-				*free_space -= write_len;
-				write_len
-			}
-		}
-
-		fn disconnect_socket(&mut self) {
-			unimplemented!()
-		}
-	}
-
-	impl Clone for SocketDescriptorMock {
-		fn clone(&self) -> Self {
-			Self {
-				unbounded: self.unbounded.clone(),
-				send_recording: self.send_recording.clone(),
-				free_space: self.free_space.clone()
-			}
-		}
-	}
-
-	impl PartialEq for SocketDescriptorMock {
-		fn eq(&self, o: &Self) -> bool {
-			self.send_recording == o.send_recording
-		}
-	}
-	impl Hash for SocketDescriptorMock {
-		fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
-			self.send_recording.borrow().hash(state);
-		}
-	}
+	use ln::peers::test_util::*;
 
 	// Test that a try_flush_one() call with no queued data doesn't write anything
 	#[test]