Cleanup clippy, add comments, and use thiserror

Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>

Cargo.lock

@@ -2000,6 +2000,7 @@ dependencies = [
  "slog-json",
  "slog-term",
  "stacks-common",
+ "thiserror",
  "tiny_http",
 ]
 

libsigner/Cargo.toml

@@ -16,16 +16,17 @@ name = "libsigner"
 path = "./src/libsigner.rs"
 
 [dependencies]
+clarity = { path = "../clarity" }
+libc = "0.2"
+libstackerdb = { path = "../libstackerdb" }
 serde = "1"
 serde_derive = "1"
 serde_stacker = "0.1"
-stacks-common = { path = "../stacks-common" }
-clarity = { path = "../clarity" }
-libstackerdb = { path = "../libstackerdb" }
 slog = { version = "2.5.2", features = [ "max_level_trace" ] }
 slog-term = "2.6.0"
 slog-json = { version = "2.3.0", optional = true }
-libc = "0.2"
+stacks-common = { path = "../stacks-common" }
+thiserror = "1.0"
 tiny_http = "0.12"
 
 [dependencies.serde_json]

libsigner/src/error.rs

@@ -14,118 +14,56 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-use std::error;
-use std::fmt;
 use std::io;
 
 /// Errors originating from doing an RPC request to the Stacks node
-#[derive(Debug)]
+#[derive(thiserror::Error, Debug)]
 pub enum RPCError {
-    IO(io::Error),
+    /// IO error
+    #[error("{0}")]
+    IO(#[from] io::Error),
+    /// Deserialization error
+    #[error("{0}")]
     Deserialize(String),
+    /// RPC request when not connected
+    #[error("Not connected")]
     NotConnected,
+    /// Malformed request
+    #[error("Malformed request: {0}")]
     MalformedRequest(String),
+    /// Malformed response
+    #[error("Malformed response: {0}")]
     MalformedResponse(String),
+    /// HTTP error
+    #[error("HTTP code {0}")]
     HttpError(u32),
 }
 
-impl fmt::Display for RPCError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            RPCError::IO(ref s) => fmt::Display::fmt(s, f),
-            RPCError::Deserialize(ref s) => fmt::Display::fmt(s, f),
-            RPCError::HttpError(ref s) => {
-                write!(f, "HTTP code {}", s)
-            }
-            RPCError::MalformedRequest(ref s) => {
-                write!(f, "Malformed request: {}", s)
-            }
-            RPCError::MalformedResponse(ref s) => {
-                write!(f, "Malformed response: {}", s)
-            }
-            RPCError::NotConnected => {
-                write!(f, "Not connected")
-            }
-        }
-    }
-}
-
-impl error::Error for RPCError {
-    fn cause(&self) -> Option<&dyn error::Error> {
-        match *self {
-            RPCError::IO(ref s) => Some(s),
-            RPCError::Deserialize(..) => None,
-            RPCError::HttpError(..) => None,
-            RPCError::MalformedRequest(..) => None,
-            RPCError::MalformedResponse(..) => None,
-            RPCError::NotConnected => None,
-        }
-    }
-}
-
-impl From<io::Error> for RPCError {
-    fn from(e: io::Error) -> RPCError {
-        RPCError::IO(e)
-    }
-}
-
 /// Errors originating from receiving event data from the Stacks node
-#[derive(Debug)]
+#[derive(thiserror::Error, Debug)]
 pub enum EventError {
-    IO(io::Error),
+    /// IO Error
+    #[error("{0}")]
+    IO(#[from] io::Error),
+    /// Deserialization error
+    #[error("{0}")]
     Deserialize(String),
+    /// Malformed request
+    #[error("Malformed request: {0}")]
     MalformedRequest(String),
+    /// Not bound to a port error
+    #[error("Not bound to a port yet")]
     NotBound,
+    /// Listener terminated error
+    #[error("Listener is terminated")]
     Terminated,
+    /// Thread already running error
+    #[error("Thread already running")]
     AlreadyRunning,
+    /// Failed to start thread error
+    #[error("Failed to start thread")]
     FailedToStart,
+    /// Unrecognized event error
+    #[error("Unrecognized event: {0}")]
     UnrecognizedEvent(String),
 }
-
-impl fmt::Display for EventError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            EventError::IO(ref s) => fmt::Display::fmt(s, f),
-            EventError::Deserialize(ref s) => fmt::Display::fmt(s, f),
-            EventError::MalformedRequest(ref s) => {
-                write!(f, "Malformed request: {}", s)
-            }
-            EventError::NotBound => {
-                write!(f, "Not bound to a port yet")
-            }
-            EventError::Terminated => {
-                write!(f, "Listener is terminated")
-            }
-            EventError::AlreadyRunning => {
-                write!(f, "Thread already running")
-            }
-            EventError::FailedToStart => {
-                write!(f, "Failed to start thread")
-            }
-            EventError::UnrecognizedEvent(ref ev) => {
-                write!(f, "Unrecognized event '{}'", &ev)
-            }
-        }
-    }
-}
-
-impl error::Error for EventError {
-    fn cause(&self) -> Option<&dyn error::Error> {
-        match *self {
-            EventError::IO(ref s) => Some(s),
-            EventError::Deserialize(..) => None,
-            EventError::MalformedRequest(..) => None,
-            EventError::NotBound => None,
-            EventError::Terminated => None,
-            EventError::AlreadyRunning => None,
-            EventError::FailedToStart => None,
-            EventError::UnrecognizedEvent(..) => None,
-        }
-    }
-}
-
-impl From<io::Error> for EventError {
-    fn from(e: io::Error) -> EventError {
-        EventError::IO(e)
-    }
-}

libsigner/src/events.rs

@@ -40,7 +40,9 @@ use crate::EventError;
 /// Event structure for newly-arrived StackerDB data
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct StackerDBChunksEvent {
+    /// The contract ID for the StackerDB instance
     pub contract_id: QualifiedContractIdentifier,
+    /// The chunk data for newly-modified slots
     pub modified_slots: Vec<StackerDBChunkData>,
 }
 
@@ -48,6 +50,7 @@ pub struct StackerDBChunksEvent {
 /// The caller calls `send()` and the event receiver loop (which lives in a separate thread) will
 /// terminate.
 pub trait EventStopSignaler {
+    /// Send the stop signal
     fn send(&mut self);
 }
 
@@ -104,6 +107,7 @@ pub trait EventReceiver {
     }
 }
 
+/// Event receiver for StackerDB events
 pub struct StackerDBEventReceiver {
     /// contracts we're listening for
     pub stackerdb_contract_ids: Vec<QualifiedContractIdentifier>,
@@ -155,6 +159,7 @@ pub struct StackerDBStopSignaler {
 }
 
 impl StackerDBStopSignaler {
+    /// Make a new stop signaler
     pub fn new(sig: Arc<AtomicBool>, local_addr: SocketAddr) -> StackerDBStopSignaler {
         StackerDBStopSignaler {
             stop_signal: sig,

libsigner/src/libsigner.rs

@@ -14,6 +14,13 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+#![forbid(missing_docs)]
+/*!
+# libsigner: a library for creating and operating a Stacks Nakamato compliant signer.
+
+Usage documentation can be found in the [README](https://github.com/stacks-network/stacks-blockchain/libsigner/README.md).
+*/
+
 #![allow(unused_imports)]
 #![allow(dead_code)]
 #[macro_use(o, slog_log, slog_trace, slog_debug, slog_info, slog_warn, slog_error)]

libsigner/src/runloop.rs

@@ -204,6 +204,7 @@ impl<
         EV: EventReceiver + Send + 'static,
     > Signer<CMD, R, SL, EV>
 {
+    /// Create a new signer with the given runloop and event receiver.
     pub fn new(
         runloop: SL,
         event_receiver: EV,

libsigner/src/session.rs

@@ -28,18 +28,24 @@ use clarity::vm::types::QualifiedContractIdentifier;
 use crate::error::RPCError;
 use crate::http::run_http_request;
 
+/// Trait for connecting to and querying a signer Stacker DB replica
 pub trait SignerSession {
+    /// connect to the replica
     fn connect(
         &mut self,
         host: SocketAddr,
         stackerdb_contract_id: QualifiedContractIdentifier,
     ) -> Result<(), RPCError>;
+    /// query the replica for a list of chunks
     fn list_chunks(&mut self) -> Result<Vec<SlotMetadata>, RPCError>;
+    /// query the replica for zero or more chunks
     fn get_chunks(
         &mut self,
         slots_and_versions: &[(u32, u32)],
     ) -> Result<Vec<Option<Vec<u8>>>, RPCError>;
+    /// query the replica for zero or more latest chunks
     fn get_latest_chunks(&mut self, slot_ids: &[u32]) -> Result<Vec<Option<Vec<u8>>>, RPCError>;
+    /// Upload a chunk to the stacker DB instance
     fn put_chunk(&mut self, chunk: StackerDBChunkData) -> Result<StackerDBChunkAckData, RPCError>;
 
     /// Get a single chunk with the given version

libsigner/src/tests/http.rs

@@ -166,9 +166,9 @@ fn test_decode_http_body() {
 
             let mut state = HttpChunkedTransferWriterState::new(5);
             let mut buf = vec![];
-            let mut body_bytes = expected_body.as_bytes().to_vec();
+            let body_bytes = expected_body.as_bytes().to_vec();
             let mut fd = HttpChunkedTransferWriter::from_writer_state(&mut buf, &mut state);
-            fd.write_all(&mut body_bytes).unwrap();
+            fd.write_all(&body_bytes).unwrap();
             fd.flush().unwrap();
             (hdrs, buf)
         } else {
@@ -177,7 +177,7 @@ fn test_decode_http_body() {
             (hdrs, body)
         };
 
-        let body = decode_http_body(&headers, &mut &encoded_body).unwrap();
+        let body = decode_http_body(&headers, &encoded_body).unwrap();
         assert_eq!(&body[..], expected_body.as_bytes());
     }
 }
@@ -252,9 +252,9 @@ fn test_run_http_request_with_body() {
         // test with chunking
         let mut state = HttpChunkedTransferWriterState::new(5);
         let mut buf = vec![];
-        let mut body_bytes = "this is the song that never ends".as_bytes().to_vec();
+        let body_bytes = "this is the song that never ends".as_bytes().to_vec();
         let mut fd = HttpChunkedTransferWriter::from_writer_state(&mut buf, &mut state);
-        fd.write_all(&mut body_bytes).unwrap();
+        fd.write_all(&body_bytes).unwrap();
         fd.flush().unwrap();
 
         let mut msock_chunked = MockHTTPSocket::new(format!("HTTP/1.1 200 OK\r\nConnection: close\r\nContent-type: text/plain\r\nTransfer-Encoding: chunked\r\n\r\n{}", str::from_utf8(&buf).unwrap()));

libsigner/src/tests/mod.rs

@@ -23,8 +23,6 @@ use std::sync::mpsc::{channel, Receiver, Sender};
 use std::thread;
 use std::time::Duration;
 
-use serde_json;
-
 use crate::{Signer, SignerRunLoop, StackerDBChunksEvent, StackerDBEventReceiver};
 
 use clarity::vm::types::QualifiedContractIdentifier;
@@ -62,7 +60,7 @@ impl SignerRunLoop<Vec<StackerDBChunksEvent>, Command> for SimpleRunLoop {
     }
 
     fn get_event_timeout(&self) -> Duration {
-        self.poll_timeout.clone()
+        self.poll_timeout
     }
 
     fn run_one_pass(
@@ -77,9 +75,9 @@ impl SignerRunLoop<Vec<StackerDBChunksEvent>, Command> for SimpleRunLoop {
         }
 
         if self.events.len() >= self.max_events {
-            return Some(mem::replace(&mut self.events, vec![]));
+            Some(mem::take(&mut self.events))
         } else {
-            return None;
+            None
         }
     }
 }
@@ -98,7 +96,6 @@ fn test_simple_signer() {
     let (res_send, _res_recv) = channel();
     let mut signer = Signer::new(SimpleRunLoop::new(5), ev, cmd_recv, res_send);
     let endpoint: SocketAddr = "127.0.0.1:30000".parse().unwrap();
-    let thread_endpoint = endpoint.clone();
 
     let mut chunks = vec![];
     for i in 0..5 {
@@ -122,7 +119,7 @@ fn test_simple_signer() {
     let mock_stacks_node = thread::spawn(move || {
         let mut num_sent = 0;
         while num_sent < thread_chunks.len() {
-            let mut sock = match TcpStream::connect(&thread_endpoint) {
+            let mut sock = match TcpStream::connect(endpoint) {
                 Ok(sock) => sock,
                 Err(..) => {
                     sleep_ms(100);