Less cloning and additional pattern simplifications (#3223)

* API Cleanup * Chain Cleanup * Core Cleanup * Keychain Cleanup * P2P Cleanup * Pool Cleanup * Store Cleanup * Util Cleanup * Cleanup clone_from_slice * Address jasper comments
mimblewimble · Feb 12, 2020 · 04a0123 · 04a0123
1 parent c4e6971
commit 04a0123
Show file tree

Hide file tree

Showing 43 changed files with 267 additions and 326 deletions.
diff --git a/api/src/auth.rs b/api/src/auth.rs
@@ -128,7 +128,7 @@ impl Handler for BasicAuthURIMiddleware {
 				unauthorized_response(&self.basic_realm)
 			}
 		} else {
-			return next_handler.call(req, handlers);
+			next_handler.call(req, handlers)
 		}
 	}
 }

diff --git a/api/src/client.rs b/api/src/client.rs
@@ -34,7 +34,7 @@ pub type ClientResponseFuture<T> = Box<dyn Future<Item = T, Error = Error> + Sen
 /// Helper function to easily issue a HTTP GET request against a given URL that
 /// returns a JSON object. Handles request building, JSON deserialization and
 /// response code checking.
-pub fn get<'a, T>(url: &'a str, api_secret: Option<String>) -> Result<T, Error>
+pub fn get<T>(url: &str, api_secret: Option<String>) -> Result<T, Error>
 where
 	for<'de> T: Deserialize<'de>,
 {
@@ -44,7 +44,7 @@ where
 /// Helper function to easily issue an async HTTP GET request against a given
 /// URL that returns a future. Handles request building, JSON deserialization
 /// and response code checking.
-pub fn get_async<'a, T>(url: &'a str, api_secret: Option<String>) -> ClientResponseFuture<T>
+pub fn get_async<T>(url: &str, api_secret: Option<String>) -> ClientResponseFuture<T>
 where
 	for<'de> T: Deserialize<'de> + Send + 'static,
 {

diff --git a/api/src/handlers.rs b/api/src/handlers.rs
@@ -191,14 +191,10 @@ impl OwnerAPIHandlerV2 {
 
 impl crate::router::Handler for OwnerAPIHandlerV2 {
 	fn post(&self, req: Request<Body>) -> ResponseFuture {
-		Box::new(
-			self.handle_post_request(req)
-				.and_then(|r| ok(r))
-				.or_else(|e| {
-					error!("Request Error: {:?}", e);
-					ok(create_error_response(e))
-				}),
-		)
+		Box::new(self.handle_post_request(req).and_then(ok).or_else(|e| {
+			error!("Request Error: {:?}", e);
+			ok(create_error_response(e))
+		}))
 	}
 
 	fn options(&self, _req: Request<Body>) -> ResponseFuture {
@@ -260,14 +256,10 @@ impl ForeignAPIHandlerV2 {
 
 impl crate::router::Handler for ForeignAPIHandlerV2 {
 	fn post(&self, req: Request<Body>) -> ResponseFuture {
-		Box::new(
-			self.handle_post_request(req)
-				.and_then(|r| ok(r))
-				.or_else(|e| {
-					error!("Request Error: {:?}", e);
-					ok(create_error_response(e))
-				}),
-		)
+		Box::new(self.handle_post_request(req).and_then(ok).or_else(|e| {
+			error!("Request Error: {:?}", e);
+			ok(create_error_response(e))
+		}))
 	}
 
 	fn options(&self, _req: Request<Body>) -> ResponseFuture {

diff --git a/api/src/handlers/blocks_api.rs b/api/src/handlers/blocks_api.rs
@@ -43,7 +43,7 @@ impl HeaderHandler {
 		if let Ok(height) = input.parse() {
 			match w(&self.chain)?.get_header_by_height(height) {
 				Ok(header) => return Ok(BlockHeaderPrintable::from_header(&header)),
-				Err(_) => return Err(ErrorKind::NotFound)?,
+				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}
 		}
 		check_block_param(&input)?;
@@ -60,14 +60,14 @@ impl HeaderHandler {
 		let oid = get_output(&self.chain, &commit_id)?.1;
 		match w(&self.chain)?.get_header_for_output(&oid) {
 			Ok(header) => Ok(BlockHeaderPrintable::from_header(&header)),
-			Err(_) => Err(ErrorKind::NotFound)?,
+			Err(_) => Err(ErrorKind::NotFound.into()),
 		}
 	}
 
 	pub fn get_header_v2(&self, h: &Hash) -> Result<BlockHeaderPrintable, Error> {
 		let chain = w(&self.chain)?;
 		let header = chain.get_block_header(h).context(ErrorKind::NotFound)?;
-		return Ok(BlockHeaderPrintable::from_header(&header));
+		Ok(BlockHeaderPrintable::from_header(&header))
 	}
 
 	// Try to get hash from height, hash or output commit
@@ -80,7 +80,7 @@ impl HeaderHandler {
 		if let Some(height) = height {
 			match w(&self.chain)?.get_header_by_height(height) {
 				Ok(header) => return Ok(header.hash()),
-				Err(_) => return Err(ErrorKind::NotFound)?,
+				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}
 		}
 		if let Some(hash) = hash {
@@ -90,12 +90,10 @@ impl HeaderHandler {
 			let oid = get_output_v2(&self.chain, &commit, false, false)?.1;
 			match w(&self.chain)?.get_header_for_output(&oid) {
 				Ok(header) => return Ok(header.hash()),
-				Err(_) => return Err(ErrorKind::NotFound)?,
+				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}
 		}
-		return Err(ErrorKind::Argument(
-			"not a valid hash, height or output commit".to_owned(),
-		))?;
+		Err(ErrorKind::Argument("not a valid hash, height or output commit".to_owned()).into())
 	}
 }
 
@@ -145,7 +143,7 @@ impl BlockHandler {
 		if let Ok(height) = input.parse() {
 			match w(&self.chain)?.get_header_by_height(height) {
 				Ok(header) => return Ok(header.hash()),
-				Err(_) => return Err(ErrorKind::NotFound)?,
+				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}
 		}
 		check_block_param(&input)?;
@@ -164,7 +162,7 @@ impl BlockHandler {
 		if let Some(height) = height {
 			match w(&self.chain)?.get_header_by_height(height) {
 				Ok(header) => return Ok(header.hash()),
-				Err(_) => return Err(ErrorKind::NotFound)?,
+				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}
 		}
 		if let Some(hash) = hash {
@@ -174,23 +172,19 @@ impl BlockHandler {
 			let oid = get_output_v2(&self.chain, &commit, false, false)?.1;
 			match w(&self.chain)?.get_header_for_output(&oid) {
 				Ok(header) => return Ok(header.hash()),
-				Err(_) => return Err(ErrorKind::NotFound)?,
+				Err(_) => return Err(ErrorKind::NotFound.into()),
 			}
 		}
-		return Err(ErrorKind::Argument(
-			"not a valid hash, height or output commit".to_owned(),
-		))?;
+		Err(ErrorKind::Argument("not a valid hash, height or output commit".to_owned()).into())
 	}
 }
 
-fn check_block_param(input: &String) -> Result<(), Error> {
+fn check_block_param(input: &str) -> Result<(), Error> {
 	lazy_static! {
 		static ref RE: Regex = Regex::new(r"[0-9a-fA-F]{64}").unwrap();
 	}
 	if !RE.is_match(&input) {
-		return Err(ErrorKind::Argument(
-			"Not a valid hash or height.".to_owned(),
-		))?;
+		return Err(ErrorKind::Argument("Not a valid hash or height.".to_owned()).into());
 	}
 	Ok(())
 }

diff --git a/api/src/handlers/chain_api.rs b/api/src/handlers/chain_api.rs
@@ -170,7 +170,7 @@ impl OutputHandler {
 				outputs = [&outputs[..], &block_output_batch[..]].concat();
 			}
 		}
-		return Ok(outputs);
+		Ok(outputs)
 	}
 
 	// allows traversal of utxo set
@@ -327,7 +327,7 @@ impl OutputHandler {
 		let mut return_vec = vec![];
 		for i in (start_height..=end_height).rev() {
 			if let Ok(res) = self.outputs_at_height(i, commitments.clone(), include_rp) {
-				if res.outputs.len() > 0 {
+				if !res.outputs.is_empty() {
 					return_vec.push(res);
 				}
 			}
@@ -359,7 +359,7 @@ impl OutputHandler {
 				include_rproof,
 				include_merkle_proof,
 			) {
-				if res.len() > 0 {
+				if !res.is_empty() {
 					return_vec = [&return_vec[..], &res[..]].concat();
 				}
 			}
@@ -394,7 +394,7 @@ impl KernelHandler {
 			.trim_end_matches('/')
 			.rsplit('/')
 			.next()
-			.ok_or(ErrorKind::RequestError("missing excess".into()))?;
+			.ok_or_else(|| ErrorKind::RequestError("missing excess".into()))?;
 		let excess = util::from_hex(excess.to_owned())
 			.map_err(|_| ErrorKind::RequestError("invalid excess hex".into()))?;
 		if excess.len() != 33 {
@@ -447,7 +447,7 @@ impl KernelHandler {
 		min_height: Option<u64>,
 		max_height: Option<u64>,
 	) -> Result<LocatedTxKernel, Error> {
-		let excess = util::from_hex(excess.to_owned())
+		let excess = util::from_hex(excess)
 			.map_err(|_| ErrorKind::RequestError("invalid excess hex".into()))?;
 		if excess.len() != 33 {
 			return Err(ErrorKind::RequestError("invalid excess length".into()).into());

diff --git a/api/src/handlers/pool_api.rs b/api/src/handlers/pool_api.rs
@@ -84,10 +84,10 @@ impl PoolHandler {
 			.blockchain
 			.chain_head()
 			.context(ErrorKind::Internal("Failed to get chain head".to_owned()))?;
-		let res = tx_pool
+		tx_pool
 			.add_to_pool(source, tx, !fluff.unwrap_or(false), &header)
 			.context(ErrorKind::Internal("Failed to update pool".to_owned()))?;
-		Ok(res)
+		Ok(())
 	}
 }
 /// Dummy wrapper for the hex-encoded serialized transaction.
@@ -141,10 +141,10 @@ impl PoolPushHandler {
 						.blockchain
 						.chain_head()
 						.context(ErrorKind::Internal("Failed to get chain head".to_owned()))?;
-					let res = tx_pool
+					tx_pool
 						.add_to_pool(source, tx, !fluff, &header)
 						.context(ErrorKind::Internal("Failed to update pool".to_owned()))?;
-					Ok(res)
+					Ok(())
 				}),
 		)
 	}

diff --git a/api/src/handlers/server_api.rs b/api/src/handlers/server_api.rs
@@ -54,7 +54,7 @@ impl Handler for KernelDownloadHandler {
 		} else {
 			response(
 				StatusCode::INTERNAL_SERVER_ERROR,
-				format!("requesting kernel data from peer failed (no peers)"),
+				"requesting kernel data from peer failed (no peers)".to_string(),
 			)
 		}
 	}

diff --git a/api/src/handlers/utils.rs b/api/src/handlers/utils.rs
@@ -70,7 +70,7 @@ pub fn get_output(
 			}
 		}
 	}
-	Err(ErrorKind::NotFound)?
+	Err(ErrorKind::NotFound.into())
 }
 
 /// Retrieves an output from the chain given a commit id (a tiny bit iteratively)
@@ -102,10 +102,11 @@ pub fn get_output_v2(
 		match res {
 			Ok(output_pos) => match chain.get_unspent_output_at(output_pos.position) {
 				Ok(output) => {
-					let mut header = None;
-					if include_merkle_proof && output.is_coinbase() {
-						header = chain.get_header_by_height(output_pos.height).ok();
-					}
+					let header = if include_merkle_proof && output.is_coinbase() {
+						chain.get_header_by_height(output_pos.height).ok()
+					} else {
+						None
+					};
 					match OutputPrintable::from_output(
 						&output,
 						chain.clone(),
@@ -124,7 +125,7 @@ pub fn get_output_v2(
 						}
 					}
 				}
-				Err(_) => return Err(ErrorKind::NotFound)?,
+				Err(_) => return Err(ErrorKind::NotFound.into()),
 			},
 			Err(e) => {
 				trace!(
@@ -136,5 +137,5 @@ pub fn get_output_v2(
 			}
 		}
 	}
-	Err(ErrorKind::NotFound)?
+	Err(ErrorKind::NotFound.into())
 }
diff --git a/api/src/handlers/version_api.rs b/api/src/handlers/version_api.rs
@@ -21,7 +21,7 @@ use crate::web::*;
 use hyper::{Body, Request};
 use std::sync::Weak;
 
-const CRATE_VERSION: &'static str = env!("CARGO_PKG_VERSION");
+const CRATE_VERSION: &str = env!("CARGO_PKG_VERSION");
 
 /// Version handler. Get running node API version
 /// GET /v1/version

diff --git a/api/src/rest.rs b/api/src/rest.rs
@@ -137,9 +137,7 @@ impl TLSConfig {
 		let keys = pemfile::pkcs8_private_keys(&mut reader)
 			.map_err(|_| ErrorKind::Internal("failed to load private key".to_string()))?;
 		if keys.len() != 1 {
-			return Err(ErrorKind::Internal(
-				"expected a single private key".to_string(),
-			))?;
+			return Err(ErrorKind::Internal("expected a single private key".to_string()).into());
 		}
 		Ok(keys[0].clone())
 	}
@@ -193,7 +191,8 @@ impl ApiServer {
 		if self.shutdown_sender.is_some() {
 			return Err(ErrorKind::Internal(
 				"Can't start HTTP API server, it's running already".to_string(),
-			))?;
+			)
+			.into());
 		}
 		let (tx, _rx) = oneshot::channel::<()>();
 		self.shutdown_sender = Some(tx);
@@ -222,7 +221,8 @@ impl ApiServer {
 		if self.shutdown_sender.is_some() {
 			return Err(ErrorKind::Internal(
 				"Can't start HTTPS API server, it's running already".to_string(),
-			))?;
+			)
+			.into());
 		}
 
 		let tls_conf = conf.build_server_config()?;

diff --git a/api/src/router.rs b/api/src/router.rs
@@ -70,16 +70,16 @@ pub trait Handler {
 		req: Request<Body>,
 		mut _handlers: Box<dyn Iterator<Item = HandlerObj>>,
 	) -> ResponseFuture {
-		match req.method() {
-			&Method::GET => self.get(req),
-			&Method::POST => self.post(req),
-			&Method::PUT => self.put(req),
-			&Method::DELETE => self.delete(req),
-			&Method::PATCH => self.patch(req),
-			&Method::OPTIONS => self.options(req),
-			&Method::CONNECT => self.connect(req),
-			&Method::TRACE => self.trace(req),
-			&Method::HEAD => self.head(req),
+		match *req.method() {
+			Method::GET => self.get(req),
+			Method::POST => self.post(req),
+			Method::PUT => self.put(req),
+			Method::DELETE => self.delete(req),
+			Method::PATCH => self.patch(req),
+			Method::OPTIONS => self.options(req),
+			Method::CONNECT => self.connect(req),
+			Method::TRACE => self.trace(req),
+			Method::HEAD => self.head(req),
 			_ => not_found(),
 		}
 	}