From bbc91d267d7392a74a4bd013df6f96f4e731d3ee Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Tue, 29 Aug 2023 12:57:00 -0400 Subject: [PATCH] Cleanup clippy, add comments, and use thiserror Signed-off-by: Jacinta Ferrant --- Cargo.lock | 1 + libsigner/Cargo.toml | 9 +-- libsigner/README.md | 0 libsigner/src/error.rs | 126 +++++++++--------------------------- libsigner/src/events.rs | 5 ++ libsigner/src/libsigner.rs | 7 ++ libsigner/src/runloop.rs | 1 + libsigner/src/session.rs | 6 ++ libsigner/src/tests/http.rs | 10 +-- libsigner/src/tests/mod.rs | 11 ++-- 10 files changed, 66 insertions(+), 110 deletions(-) create mode 100644 libsigner/README.md diff --git a/Cargo.lock b/Cargo.lock index ba150d6b5..45903ffa7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2000,6 +2000,7 @@ dependencies = [ "slog-json", "slog-term", "stacks-common", + "thiserror", "tiny_http", ] diff --git a/libsigner/Cargo.toml b/libsigner/Cargo.toml index 45737c7cf..8500ef55f 100644 --- a/libsigner/Cargo.toml +++ b/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] diff --git a/libsigner/README.md b/libsigner/README.md new file mode 100644 index 000000000..e69de29bb diff --git a/libsigner/src/error.rs b/libsigner/src/error.rs index fb2933695..fec6a1e8f 100644 --- a/libsigner/src/error.rs +++ b/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 . -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 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 for EventError { - fn from(e: io::Error) -> EventError { - EventError::IO(e) - } -} diff --git a/libsigner/src/events.rs b/libsigner/src/events.rs index 069178107..8ca2547d1 100644 --- a/libsigner/src/events.rs +++ b/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, } @@ -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, @@ -155,6 +159,7 @@ pub struct StackerDBStopSignaler { } impl StackerDBStopSignaler { + /// Make a new stop signaler pub fn new(sig: Arc, local_addr: SocketAddr) -> StackerDBStopSignaler { StackerDBStopSignaler { stop_signal: sig, diff --git a/libsigner/src/libsigner.rs b/libsigner/src/libsigner.rs index 11f6ba614..38819b7f5 100644 --- a/libsigner/src/libsigner.rs +++ b/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 . +#![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)] diff --git a/libsigner/src/runloop.rs b/libsigner/src/runloop.rs index e2f5086f3..03c50bae4 100644 --- a/libsigner/src/runloop.rs +++ b/libsigner/src/runloop.rs @@ -204,6 +204,7 @@ impl< EV: EventReceiver + Send + 'static, > Signer { + /// Create a new signer with the given runloop and event receiver. pub fn new( runloop: SL, event_receiver: EV, diff --git a/libsigner/src/session.rs b/libsigner/src/session.rs index 868c81dfa..75b64dc5a 100644 --- a/libsigner/src/session.rs +++ b/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, RPCError>; + /// query the replica for zero or more chunks fn get_chunks( &mut self, slots_and_versions: &[(u32, u32)], ) -> Result>>, RPCError>; + /// query the replica for zero or more latest chunks fn get_latest_chunks(&mut self, slot_ids: &[u32]) -> Result>>, RPCError>; + /// Upload a chunk to the stacker DB instance fn put_chunk(&mut self, chunk: StackerDBChunkData) -> Result; /// Get a single chunk with the given version diff --git a/libsigner/src/tests/http.rs b/libsigner/src/tests/http.rs index 691e1eda4..be34036ff 100644 --- a/libsigner/src/tests/http.rs +++ b/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())); diff --git a/libsigner/src/tests/mod.rs b/libsigner/src/tests/mod.rs index 27597ac82..fc4d45ddf 100644 --- a/libsigner/src/tests/mod.rs +++ b/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, 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, 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);