From 9762cf312069358667a658cf93bdef593eef85f0 Mon Sep 17 00:00:00 2001 From: "Arthur Franco (afm)" Date: Mon, 23 Oct 2023 12:32:05 -0300 Subject: [PATCH] chore: improve Error enum names so we can remove clippy::enum_variant_name (#85) --- .../src/pruned_utreexo/chain_state.rs | 20 ++--- .../src/pruned_utreexo/error.rs | 20 ++--- .../src/pruned_utreexo/partial_chain.rs | 13 ++- .../src/electrum_protocol.rs | 10 +-- crates/floresta-electrum/src/error.rs | 6 +- crates/floresta-wire/src/p2p_wire/error.rs | 10 +-- crates/floresta-wire/src/p2p_wire/node.rs | 21 ++--- crates/floresta-wire/src/p2p_wire/peer.rs | 6 +- florestad/src/error.rs | 84 +++++++++---------- florestad/src/main.rs | 6 +- 10 files changed, 95 insertions(+), 101 deletions(-) diff --git a/crates/floresta-chain/src/pruned_utreexo/chain_state.rs b/crates/floresta-chain/src/pruned_utreexo/chain_state.rs index 180c5706..481fb993 100644 --- a/crates/floresta-chain/src/pruned_utreexo/chain_state.rs +++ b/crates/floresta-chain/src/pruned_utreexo/chain_state.rs @@ -145,9 +145,9 @@ impl ChainState { return Err(BlockValidationErrors::NotEnoughPow.into()); } - let block_hash = block_header.validate_pow(&actual_target).map_err(|_| { - BlockchainError::BlockValidationError(BlockValidationErrors::NotEnoughPow) - })?; + let block_hash = block_header + .validate_pow(&actual_target) + .map_err(|_| BlockchainError::BlockValidation(BlockValidationErrors::NotEnoughPow))?; Ok(block_hash) } @@ -655,24 +655,24 @@ impl ChainState { ) -> Result<(), BlockchainError> { let prev_block = self.get_ancestor(&block.header)?; if block.header.prev_blockhash != prev_block.block_hash() { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BlockExtendsAnOrphanChain, )); } if !block.check_merkle_root() { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BadMerkleRoot, )); } if height >= self.chain_params().bip34_activation_height && block.bip34_block_height() != Ok(height as u64) { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BadBip34, )); } if !block.check_witness_commitment() { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BadWitnessCommitment, )); } @@ -686,9 +686,9 @@ impl ChainState { let flags = 0; Consensus::verify_block_transactions(inputs, &block.txdata, subsidy, verify_script, flags) .map_err(|err| { - BlockchainError::BlockValidationError(BlockValidationErrors::InvalidTx( - alloc::format!("{:?}", err), - )) + BlockchainError::BlockValidation(BlockValidationErrors::InvalidTx(alloc::format!( + "{:?}", err + ))) })?; Ok(()) } diff --git a/crates/floresta-chain/src/pruned_utreexo/error.rs b/crates/floresta-chain/src/pruned_utreexo/error.rs index 205c3521..8c14a5fd 100644 --- a/crates/floresta-chain/src/pruned_utreexo/error.rs +++ b/crates/floresta-chain/src/pruned_utreexo/error.rs @@ -12,16 +12,16 @@ pub enum BlockchainError { #[cfg(feature = "cli-blockchain")] #[error("Json-Rpc error")] JsonRpcError(#[from] UtreexodError), - ParsingError(bitcoin::hashes::hex::Error), - BlockValidationError(BlockValidationErrors), + Parsing(bitcoin::hashes::hex::Error), + BlockValidation(BlockValidationErrors), InvalidProof, UtreexoError(String), - DatabaseError(Box), - ConsensusDecodeError(bitcoin::consensus::encode::Error), + Database(Box), + ConsensusDecode(bitcoin::consensus::encode::Error), ChainNotInitialized, InvalidTip(String), ScriptValidationFailed(script::Error), - IoError(ioError), + Io(ioError), } #[derive(Clone, Debug, PartialEq)] @@ -72,18 +72,18 @@ impl Display for BlockValidationErrors { impl From for BlockchainError { fn from(value: T) -> Self { - BlockchainError::DatabaseError(Box::new(value)) + BlockchainError::Database(Box::new(value)) } } -impl_error_from!(BlockchainError, ioError, IoError); +impl_error_from!(BlockchainError, ioError, Io); impl_error_from!( BlockchainError, bitcoin::consensus::encode::Error, - ConsensusDecodeError + ConsensusDecode ); -impl_error_from!(BlockchainError, BlockValidationErrors, BlockValidationError); -impl_error_from!(BlockchainError, bitcoin::hashes::hex::Error, ParsingError); +impl_error_from!(BlockchainError, BlockValidationErrors, BlockValidation); +impl_error_from!(BlockchainError, bitcoin::hashes::hex::Error, Parsing); impl_error_from!(BlockchainError, String, UtreexoError); impl_error_from!(BlockchainError, script::Error, ScriptValidationFailed); diff --git a/crates/floresta-chain/src/pruned_utreexo/partial_chain.rs b/crates/floresta-chain/src/pruned_utreexo/partial_chain.rs index f6ea6359..3802a85d 100644 --- a/crates/floresta-chain/src/pruned_utreexo/partial_chain.rs +++ b/crates/floresta-chain/src/pruned_utreexo/partial_chain.rs @@ -137,8 +137,7 @@ impl PartialChainState { del_hashes: Vec, ) -> bool { let height = self.current_height + 1; - if let Err(BlockchainError::BlockValidationError(e)) = - self.validate_block(block, height, inputs) + if let Err(BlockchainError::BlockValidation(e)) = self.validate_block(block, height, inputs) { self.error = Some(e); return false; @@ -175,25 +174,25 @@ impl PartialChainState { inputs: HashMap, ) -> Result<(), BlockchainError> { if !block.check_merkle_root() { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BadMerkleRoot, )); } if height >= self.chain_params().bip34_activation_height && block.bip34_block_height() != Ok(height as u64) { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BadBip34, )); } if !block.check_witness_commitment() { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BadWitnessCommitment, )); } let prev_block = self.get_ancestor(height)?; if block.header.prev_blockhash != prev_block.block_hash() { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::BlockExtendsAnOrphanChain, )); } @@ -212,7 +211,7 @@ impl PartialChainState { flags, )?; if !valid { - return Err(BlockchainError::BlockValidationError( + return Err(BlockchainError::BlockValidation( BlockValidationErrors::InvalidTx(String::from("invalid block transactions")), )); } diff --git a/crates/floresta-electrum/src/electrum_protocol.rs b/crates/floresta-electrum/src/electrum_protocol.rs index 77035920..dea8e6b6 100644 --- a/crates/floresta-electrum/src/electrum_protocol.rs +++ b/crates/floresta-electrum/src/electrum_protocol.rs @@ -125,7 +125,7 @@ impl ElectrumServer { let header = self .chain .get_block_header(&hash) - .map_err(|e| super::error::Error::ChainError(Box::new(e)))?; + .map_err(|e| super::error::Error::Blockchain(Box::new(e)))?; let header = serialize(&header).to_hex(); json_rpc_res!(request, header) } @@ -143,7 +143,7 @@ impl ElectrumServer { let header = self .chain .get_block_header(&hash) - .map_err(|e| super::error::Error::ChainError(Box::new(e)))?; + .map_err(|e| super::error::Error::Blockchain(Box::new(e)))?; let header = serialize(&header).to_hex(); headers.push_str(&header); } @@ -158,11 +158,11 @@ impl ElectrumServer { let (height, hash) = self .chain .get_best_block() - .map_err(|e| super::error::Error::ChainError(Box::new(e)))?; + .map_err(|e| super::error::Error::Blockchain(Box::new(e)))?; let header = self .chain .get_block_header(&hash) - .map_err(|e| super::error::Error::ChainError(Box::new(e)))?; + .map_err(|e| super::error::Error::Blockchain(Box::new(e)))?; let result = json!({ "height": height, "hex": serialize(&header).to_hex() @@ -269,7 +269,7 @@ impl ElectrumServer { deserialize(&hex).map_err(|_| super::error::Error::InvalidParams)?; self.chain .broadcast(&tx) - .map_err(|e| super::error::Error::ChainError(Box::new(e)))?; + .map_err(|e| super::error::Error::Blockchain(Box::new(e)))?; let id = tx.txid(); let updated = self .address_cache diff --git a/crates/floresta-electrum/src/error.rs b/crates/floresta-electrum/src/error.rs index e487ed16..05e79cb0 100644 --- a/crates/floresta-electrum/src/error.rs +++ b/crates/floresta-electrum/src/error.rs @@ -11,9 +11,9 @@ pub enum Error { #[error("Invalid params passed in")] InvalidParams, #[error("Invalid json string {0}")] - ParsingError(#[from] serde_json::Error), + Parsing(#[from] serde_json::Error), #[error("Blockchain error")] - ChainError(Box), + Blockchain(Box), #[error("IO error")] - IoError(#[from] std::io::Error), + Io(#[from] std::io::Error), } diff --git a/crates/floresta-wire/src/p2p_wire/error.rs b/crates/floresta-wire/src/p2p_wire/error.rs index c79096dc..999272b2 100644 --- a/crates/floresta-wire/src/p2p_wire/error.rs +++ b/crates/floresta-wire/src/p2p_wire/error.rs @@ -9,9 +9,9 @@ use super::peer::PeerError; #[derive(Error, Debug)] pub enum WireError { #[error("Blockchain error")] - BlockchainError(BlockchainError), + Blockchain(BlockchainError), #[error("Error while writing into a channel")] - ChannelSendError(async_std::channel::SendError), + ChannelSend(async_std::channel::SendError), #[error("Peer error")] PeerError(PeerError), #[error("Coinbase didn't mature")] @@ -23,13 +23,13 @@ pub enum WireError { #[error("Our peer is misbehaving")] PeerMisbehaving, #[error("Error while reading from a channel")] - ChannelRecvError(#[from] async_std::channel::RecvError), + ChannelRecv(#[from] async_std::channel::RecvError), #[error("Generic io error")] - IoError(std::io::Error), + Io(std::io::Error), #[error("We don't have any utreexo peers")] NoUtreexoPeersAvailable, #[error("We couldn't find a peer to send the request")] NoPeerToSendRequest, } impl_error_from!(WireError, PeerError, PeerError); -impl_error_from!(WireError, BlockchainError, BlockchainError); +impl_error_from!(WireError, BlockchainError, Blockchain); diff --git a/crates/floresta-wire/src/p2p_wire/node.rs b/crates/floresta-wire/src/p2p_wire/node.rs index e9af1204..f95f8767 100644 --- a/crates/floresta-wire/src/p2p_wire/node.rs +++ b/crates/floresta-wire/src/p2p_wire/node.rs @@ -364,7 +364,7 @@ where peer.channel .send(req) .await - .map_err(WireError::ChannelSendError)?; + .map_err(WireError::ChannelSend)?; } } Ok(()) @@ -456,7 +456,7 @@ where peer.channel .send(req.clone()) .await - .map_err(WireError::ChannelSendError)?; + .map_err(WireError::ChannelSend)?; return Ok(idx); } } @@ -474,7 +474,7 @@ where self.network, &get_chain_dns_seeds(self.network), ) - .map_err(WireError::IoError)?; + .map_err(WireError::Io)?; for address in anchors { self.open_connection(false, address.id, address).await; } @@ -507,14 +507,14 @@ where peer.channel .send(NodeRequest::BroadcastTransaction(txid)) .await - .map_err(WireError::ChannelSendError)?; + .map_err(WireError::ChannelSend)?; } let stale = self.mempool.write().await.get_stale(); for tx in stale { peer.channel .send(NodeRequest::BroadcastTransaction(tx)) .await - .map_err(WireError::ChannelSendError)?; + .map_err(WireError::ChannelSend)?; } } Ok(()) @@ -528,7 +528,7 @@ where fn save_peers(&self) -> Result<(), WireError> { self.address_man .dump_peers(&self.datadir) - .map_err(WireError::IoError) + .map_err(WireError::Io) } fn get_blocks_to_download(&mut self) -> Result, WireError> { let mut blocks = Vec::new(); @@ -745,7 +745,7 @@ where try_and_log!(chain .connect_block(&block.block, proof, inputs, del_hashes) .map_err(|e| { - if let BlockchainError::BlockValidationError(_) = &e { + if let BlockchainError::BlockValidation(_) = &e { try_and_log!(chain.invalidate_block(block.block.block_hash())); } error!( @@ -824,10 +824,7 @@ where if self.state == NodeState::DownloadBlocks { self.process_queued_blocks().await.or_else(|err| { // This usually means we just processed all blocks, and we are done. - if matches!( - err, - WireError::BlockchainError(BlockchainError::BlockNotPresent) - ) { + if matches!(err, WireError::Blockchain(BlockchainError::BlockNotPresent)) { info!("Finished downloading blocks"); self.chain.toggle_ibd(false); Ok(()) @@ -1336,7 +1333,7 @@ where { error!("Invalid block received by peer {} reason: {:?}", peer, e); - if let BlockchainError::BlockValidationError(e) = e { + if let BlockchainError::BlockValidation(e) = e { // Because the proof isn't committed to the block, we can't invalidate // it if the proof is invalid. Any other error should cause the block // to be invalidated. diff --git a/crates/floresta-wire/src/p2p_wire/peer.rs b/crates/floresta-wire/src/p2p_wire/peer.rs index 6417138e..dfac544b 100644 --- a/crates/floresta-wire/src/p2p_wire/peer.rs +++ b/crates/floresta-wire/src/p2p_wire/peer.rs @@ -79,11 +79,11 @@ pub struct Peer { #[derive(Debug, Error)] pub enum PeerError { #[error("Error while sending to peer")] - SendError, + Send, #[error("Error while reading from peer")] - ReadError(#[from] std::io::Error), + Read(#[from] std::io::Error), #[error("Error while parsing message")] - ParseError(#[from] bitcoin::consensus::encode::Error), + Parse(#[from] bitcoin::consensus::encode::Error), #[error("Peer sent us a message that we aren't expecting")] UnexpectedMessage, #[error("Peer sent us a message that is too big")] diff --git a/florestad/src/error.rs b/florestad/src/error.rs index 06672e38..6afc314a 100644 --- a/florestad/src/error.rs +++ b/florestad/src/error.rs @@ -6,41 +6,41 @@ use floresta_chain::{BlockValidationErrors, BlockchainError}; pub enum Error { #[cfg(feature = "cli-blockchain")] UtreexodError(UtreexodError), - ParsingError(bitcoin::hashes::hex::Error), - EncodeError(encode::Error), - DbError(kv::Error), - ParseNumError(std::num::ParseIntError), - RustreexoError(String), - IoError(std::io::Error), - BlockValidationError(BlockValidationErrors), - ScriptValidationError(bitcoin::blockdata::script::Error), - ChainError(BlockchainError), - SerdeJsonError(serde_json::Error), - TomlParsingError(toml::de::Error), - WalletInputError(slip132::Error), - AddressParsingError(bitcoin::util::address::Error), - MiniscriptError(miniscript::Error), + Parsing(bitcoin::hashes::hex::Error), + Encode(encode::Error), + Db(kv::Error), + ParseNum(std::num::ParseIntError), + Rustreexo(String), + Io(std::io::Error), + BlockValidation(BlockValidationErrors), + ScriptValidation(bitcoin::blockdata::script::Error), + Blockchain(BlockchainError), + SerdeJson(serde_json::Error), + TomlParsing(toml::de::Error), + WalletInput(slip132::Error), + AddressParsing(bitcoin::util::address::Error), + Miniscript(miniscript::Error), } impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Error::EncodeError(err) => write!(f, "Encode error: {err}"), - Error::ParsingError(err) => write!(f, "Parsing Error {err}"), + Error::Encode(err) => write!(f, "Encode error: {err}"), + Error::Parsing(err) => write!(f, "Parsing Error {err}"), #[cfg(feature = "cli-blockchain")] Error::UtreexodError(_) => write!(f, "UtreexodError"), - Error::DbError(err) => write!(f, "Database error {err}"), - Error::ParseNumError(err) => write!(f, "int parse error: {err}"), - Error::RustreexoError(err) => write!(f, "Rustreexo error: {err}"), - Error::IoError(err) => write!(f, "Io error {err}"), - Error::ScriptValidationError(err) => write!(f, "Error during script evaluation: {err}"), - Error::ChainError(err) => write!(f, "Error with our blockchain backend: {:?}", err), - Error::SerdeJsonError(err) => write!(f, "Error serializing object {err}"), - Error::WalletInputError(err) => write!(f, "Error while parsing user input {:?}", err), - Error::TomlParsingError(err) => write!(f, "Error deserializing toml file {err}"), - Error::AddressParsingError(err) => write!(f, "Invalid address {err}"), - Error::MiniscriptError(err) => write!(f, "Miniscript error: {err}"), - Error::BlockValidationError(err) => write!(f, "Error while validating block: {err:?}"), + Error::Db(err) => write!(f, "Database error {err}"), + Error::ParseNum(err) => write!(f, "int parse error: {err}"), + Error::Rustreexo(err) => write!(f, "Rustreexo error: {err}"), + Error::Io(err) => write!(f, "Io error {err}"), + Error::ScriptValidation(err) => write!(f, "Error during script evaluation: {err}"), + Error::Blockchain(err) => write!(f, "Error with our blockchain backend: {:?}", err), + Error::SerdeJson(err) => write!(f, "Error serializing object {err}"), + Error::WalletInput(err) => write!(f, "Error while parsing user input {:?}", err), + Error::TomlParsing(err) => write!(f, "Error deserializing toml file {err}"), + Error::AddressParsing(err) => write!(f, "Invalid address {err}"), + Error::Miniscript(err) => write!(f, "Miniscript error: {err}"), + Error::BlockValidation(err) => write!(f, "Error while validating block: {err:?}"), } } } @@ -55,21 +55,21 @@ macro_rules! impl_from_error { } }; } -impl_from_error!(ParsingError, bitcoin::hashes::hex::Error); +impl_from_error!(Parsing, bitcoin::hashes::hex::Error); #[cfg(feature = "cli-blockchain")] impl_from_error!(UtreexodError, UtreexodError); -impl_from_error!(EncodeError, encode::Error); -impl_from_error!(DbError, kv::Error); -impl_from_error!(ParseNumError, std::num::ParseIntError); -impl_from_error!(RustreexoError, String); -impl_from_error!(IoError, std::io::Error); -impl_from_error!(ScriptValidationError, bitcoin::blockdata::script::Error); -impl_from_error!(ChainError, BlockchainError); -impl_from_error!(SerdeJsonError, serde_json::Error); -impl_from_error!(WalletInputError, slip132::Error); -impl_from_error!(TomlParsingError, toml::de::Error); -impl_from_error!(BlockValidationError, BlockValidationErrors); -impl_from_error!(AddressParsingError, bitcoin::util::address::Error); -impl_from_error!(MiniscriptError, miniscript::Error); +impl_from_error!(Encode, encode::Error); +impl_from_error!(Db, kv::Error); +impl_from_error!(ParseNum, std::num::ParseIntError); +impl_from_error!(Rustreexo, String); +impl_from_error!(Io, std::io::Error); +impl_from_error!(ScriptValidation, bitcoin::blockdata::script::Error); +impl_from_error!(Blockchain, BlockchainError); +impl_from_error!(SerdeJson, serde_json::Error); +impl_from_error!(WalletInput, slip132::Error); +impl_from_error!(TomlParsing, toml::de::Error); +impl_from_error!(BlockValidation, BlockValidationErrors); +impl_from_error!(AddressParsing, bitcoin::util::address::Error); +impl_from_error!(Miniscript, miniscript::Error); impl std::error::Error for Error {} diff --git a/florestad/src/main.rs b/florestad/src/main.rs index e8bebcfe..35aebf40 100644 --- a/florestad/src/main.rs +++ b/florestad/src/main.rs @@ -12,8 +12,6 @@ // Coding conventions (lexicographically sorted) #![deny(arithmetic_overflow)] #![deny(clippy::all)] -// FIXME: Rethink enum variant naming -#![allow(clippy::enum_variant_names)] #![deny(missing_docs)] #![deny(non_camel_case_types)] #![deny(non_snake_case)] @@ -284,12 +282,12 @@ fn get_config_file(params: &cli::Cli) -> ConfigFile { data } else { match data.unwrap_err() { - error::Error::TomlParsingError(e) => { + error::Error::TomlParsing(e) => { error!("Error while parsing config file, ignoring it"); debug!("{e}"); ConfigFile::default() } - error::Error::IoError(e) => { + error::Error::Io(e) => { error!("Error reading config file, ignoring it"); debug!("{e}"); ConfigFile::default()