From 52d966ccaa5ae5b438b425ef1c25d3b1f446fd9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= <4142+huitseeker@users.noreply.github.com> Date: Thu, 21 Jan 2021 12:27:35 -0500 Subject: [PATCH] Cleans up a number of Option / Result patterns and warts (#226) --- bin/oe/configuration.rs | 6 +-- bin/oe/helpers.rs | 21 ++++----- bin/oe/logger/src/lib.rs | 9 ++-- crates/accounts/ethkey/src/random.rs | 5 +-- crates/accounts/ethstore/src/json/crypto.rs | 10 +---- crates/accounts/ethstore/src/json/key_file.rs | 15 ++----- crates/concensus/ethash/src/compute.rs | 2 +- crates/ethcore/src/client/client.rs | 44 +++++++------------ .../engines/validator_set/safe_contract.rs | 7 ++- crates/ethcore/src/executive.rs | 20 ++------- crates/ethcore/src/state/account.rs | 20 ++++----- crates/ethcore/src/state/mod.rs | 5 +-- crates/ethcore/sync/src/api.rs | 21 ++++----- crates/net/network-devp2p/src/node_table.rs | 5 +-- crates/rpc/src/v1/impls/eth.rs | 20 ++++----- crates/rpc/src/v1/impls/parity_set.rs | 7 ++- 16 files changed, 76 insertions(+), 141 deletions(-) diff --git a/bin/oe/configuration.rs b/bin/oe/configuration.rs index 5982594e5..fe7a71917 100644 --- a/bin/oe/configuration.rs +++ b/bin/oe/configuration.rs @@ -518,10 +518,8 @@ impl Configuration { } fn ip_filter(&self) -> Result { - match IpFilter::parse(self.args.arg_allow_ips.as_str()) { - Ok(allow_ip) => Ok(allow_ip), - Err(_) => Err("Invalid IP filter value".to_owned()), - } + IpFilter::parse(self.args.arg_allow_ips.as_str()) + .map_err(|_| "Invalid IP filter value".to_owned()) } fn min_peers(&self) -> u32 { diff --git a/bin/oe/helpers.rs b/bin/oe/helpers.rs index 76d28defd..01f831073 100644 --- a/bin/oe/helpers.rs +++ b/bin/oe/helpers.rs @@ -105,10 +105,10 @@ pub fn to_block_id(s: &str) -> Result { pub fn to_u256(s: &str) -> Result { if let Ok(decimal) = U256::from_dec_str(s) { Ok(decimal) - } else if let Ok(hex) = clean_0x(s).parse() { - Ok(hex) } else { - Err(format!("Invalid numeric value: {}", s)) + clean_0x(s) + .parse() + .map_err(|_| format!("Invalid numeric value: {}", s)) } } @@ -171,15 +171,12 @@ pub fn to_price(s: &str) -> Result { } pub fn join_set(set: Option<&HashSet>) -> Option { - match set { - Some(s) => Some( - s.iter() - .map(|s| s.as_str()) - .collect::>() - .join(","), - ), - None => None, - } + set.map(|s| { + s.iter() + .map(|s| s.as_str()) + .collect::>() + .join(",") + }) } /// Flush output buffer. diff --git a/bin/oe/logger/src/lib.rs b/bin/oe/logger/src/lib.rs index 44412a05a..11465225a 100644 --- a/bin/oe/logger/src/lib.rs +++ b/bin/oe/logger/src/lib.rs @@ -157,10 +157,11 @@ pub fn setup_log(config: &Config) -> Result, String> { Ok(logs) }) // couldn't create new logger - try to fall back on previous logger. - .or_else(|err| match ROTATING_LOGGER.lock().upgrade() { - Some(l) => Ok(l), - // no previous logger. fatal. - None => Err(format!("{:?}", err)), + .or_else(|err| { + ROTATING_LOGGER + .lock() + .upgrade() + .ok_or_else(|| format!("{:?}", err)) }) } diff --git a/crates/accounts/ethkey/src/random.rs b/crates/accounts/ethkey/src/random.rs index 6bf833d27..3efc4df97 100644 --- a/crates/accounts/ethkey/src/random.rs +++ b/crates/accounts/ethkey/src/random.rs @@ -25,10 +25,7 @@ impl Generator for Random { fn generate(&mut self) -> Result { let mut rng = OsRng::new()?; - match rng.generate() { - Ok(pair) => Ok(pair), - Err(void) => match void {}, // LLVM unreachable - } + rng.generate().or_else(|void| match void {}) } } diff --git a/crates/accounts/ethstore/src/json/crypto.rs b/crates/accounts/ethstore/src/json/crypto.rs index 2afe5bbd1..74d17daa5 100644 --- a/crates/accounts/ethstore/src/json/crypto.rs +++ b/crates/accounts/ethstore/src/json/crypto.rs @@ -159,10 +159,7 @@ impl<'a> Visitor<'a> for CryptoVisitor { (Some(_), None) => return Err(V::Error::missing_field("cipherparams")), }; - let ciphertext = match ciphertext { - Some(ciphertext) => ciphertext, - None => return Err(V::Error::missing_field("ciphertext")), - }; + let ciphertext = ciphertext.ok_or_else(|| V::Error::missing_field("ciphertext"))?; let kdf = match (kdf, kdfparams) { (Some(KdfSer::Pbkdf2), Some(KdfSerParams::Pbkdf2(params))) => Kdf::Pbkdf2(params), @@ -172,10 +169,7 @@ impl<'a> Visitor<'a> for CryptoVisitor { (Some(_), None) => return Err(V::Error::missing_field("kdfparams")), }; - let mac = match mac { - Some(mac) => mac, - None => return Err(V::Error::missing_field("mac")), - }; + let mac = mac.ok_or_else(|| V::Error::missing_field("mac"))?; let result = Crypto { cipher: cipher, diff --git a/crates/accounts/ethstore/src/json/key_file.rs b/crates/accounts/ethstore/src/json/key_file.rs index 480493a9c..9c2aec51e 100644 --- a/crates/accounts/ethstore/src/json/key_file.rs +++ b/crates/accounts/ethstore/src/json/key_file.rs @@ -169,20 +169,11 @@ impl<'a> Visitor<'a> for KeyFileVisitor { } } - let id = match id { - Some(id) => id, - None => return Err(V::Error::missing_field("id")), - }; + let id = id.ok_or_else(|| V::Error::missing_field("id"))?; - let version = match version { - Some(version) => version, - None => return Err(V::Error::missing_field("version")), - }; + let version = version.ok_or_else(|| V::Error::missing_field("version"))?; - let crypto = match crypto { - Some(crypto) => crypto, - None => return Err(V::Error::missing_field("crypto")), - }; + let crypto = crypto.ok_or_else(|| V::Error::missing_field("crypto"))?; let result = KeyFile { id: id, diff --git a/crates/concensus/ethash/src/compute.rs b/crates/concensus/ethash/src/compute.rs index c378c754f..648b157b1 100644 --- a/crates/concensus/ethash/src/compute.rs +++ b/crates/concensus/ethash/src/compute.rs @@ -198,7 +198,7 @@ fn hash_compute(light: &Light, full_size: usize, header_hash: &H256, nonce: u64) struct MixBuf { half_mix: Node, compress_bytes: [u8; MIX_WORDS], - }; + } if full_size % MIX_WORDS != 0 { panic!("Unaligned full size"); diff --git a/crates/ethcore/src/client/client.rs b/crates/ethcore/src/client/client.rs index c5e8dd7a8..80531f721 100644 --- a/crates/ethcore/src/client/client.rs +++ b/crates/ethcore/src/client/client.rs @@ -1240,10 +1240,7 @@ impl Client { return Some(state); } - let block_number = match self.block_number(id) { - Some(num) => num, - None => return None, - }; + let block_number = self.block_number(id)?; self.block_header(id).and_then(|header| { let db = self.state_db.read().boxed_clone(); @@ -1370,15 +1367,12 @@ impl Client { None => best_block_number.saturating_sub(history), }; - match self.block_hash(BlockId::Number(start_num)) { - Some(h) => h, - None => return Err(snapshot::Error::InvalidStartingBlock(at).into()), - } + self.block_hash(BlockId::Number(start_num)) + .ok_or_else(|| snapshot::Error::InvalidStartingBlock(at))? } - _ => match self.block_hash(at) { - Some(hash) => hash, - None => return Err(snapshot::Error::InvalidStartingBlock(at).into()), - }, + _ => self + .block_hash(at) + .ok_or_else(|| snapshot::Error::InvalidStartingBlock(at))?, }; let processing_threads = self.config.snapshot.processing_threads; @@ -2407,18 +2401,13 @@ impl BlockChainClient for Client { .collect::>() } else { // Otherwise, we use a slower version that finds a link between from_block and to_block. - let from_hash = match Self::block_hash(&chain, filter.from_block) { - Some(val) => val, - None => return Err(filter.from_block.clone()), - }; - let from_number = match chain.block_number(&from_hash) { - Some(val) => val, - None => return Err(BlockId::Hash(from_hash)), - }; - let to_hash = match Self::block_hash(&chain, filter.to_block) { - Some(val) => val, - None => return Err(filter.to_block.clone()), - }; + let from_hash = Self::block_hash(&chain, filter.from_block) + .ok_or_else(|| filter.from_block.clone())?; + let from_number = chain + .block_number(&from_hash) + .ok_or_else(|| BlockId::Hash(from_hash))?; + let to_hash = + Self::block_hash(&chain, filter.to_block).ok_or_else(|| filter.to_block.clone())?; let blooms = filter.bloom_possibilities(); let bloom_match = |header: &encoded::Header| { @@ -2432,10 +2421,9 @@ impl BlockChainClient for Client { let mut current_hash = to_hash; loop { - let header = match chain.block_header_data(¤t_hash) { - Some(val) => val, - None => return Err(BlockId::Hash(current_hash)), - }; + let header = chain + .block_header_data(¤t_hash) + .ok_or_else(|| BlockId::Hash(current_hash))?; if bloom_match(&header) { blocks.push(current_hash); } diff --git a/crates/ethcore/src/engines/validator_set/safe_contract.rs b/crates/ethcore/src/engines/validator_set/safe_contract.rs index 39256c360..6e36015aa 100644 --- a/crates/ethcore/src/engines/validator_set/safe_contract.rs +++ b/crates/ethcore/src/engines/validator_set/safe_contract.rs @@ -292,10 +292,9 @@ impl ValidatorSafeContract { }); // only last log is taken into account - match decoded_events.next() { - None => None, - Some(matched_event) => Some(SimpleList::new(matched_event.new_set)), - } + decoded_events + .next() + .map(|matched_event| SimpleList::new(matched_event.new_set)) } } diff --git a/crates/ethcore/src/executive.rs b/crates/ethcore/src/executive.rs index dddc53ec7..485bb4275 100644 --- a/crates/ethcore/src/executive.rs +++ b/crates/ethcore/src/executive.rs @@ -623,10 +623,7 @@ impl<'a> CallCreateExecutive<'a> { tracer, vm_tracer, ); - match exec.exec(&mut ext) { - Ok(val) => Ok(val.finalize(ext)), - Err(err) => Err(err), - } + exec.exec(&mut ext).map(|val| val.finalize(ext)) }; let res = match out { @@ -694,10 +691,7 @@ impl<'a> CallCreateExecutive<'a> { tracer, vm_tracer, ); - match exec.exec(&mut ext) { - Ok(val) => Ok(val.finalize(ext)), - Err(err) => Err(err), - } + exec.exec(&mut ext).map(|val| val.finalize(ext)) }; let res = match out { @@ -763,10 +757,7 @@ impl<'a> CallCreateExecutive<'a> { tracer, vm_tracer, ); - match exec.exec(&mut ext) { - Ok(val) => Ok(val.finalize(ext)), - Err(err) => Err(err), - } + exec.exec(&mut ext).map(|val| val.finalize(ext)) }; let res = match out { @@ -840,10 +831,7 @@ impl<'a> CallCreateExecutive<'a> { tracer, vm_tracer, ); - match exec.exec(&mut ext) { - Ok(val) => Ok(val.finalize(ext)), - Err(err) => Err(err), - } + exec.exec(&mut ext).map(|val| val.finalize(ext)) }; let res = match out { diff --git a/crates/ethcore/src/state/account.rs b/crates/ethcore/src/state/account.rs index b6504c176..799fc24d6 100644 --- a/crates/ethcore/src/state/account.rs +++ b/crates/ethcore/src/state/account.rs @@ -298,13 +298,10 @@ impl Account { /// Get cached original storage value after last state commitment. Returns `None` if the key is not in the cache. pub fn cached_original_storage_at(&self, key: &H256) -> Option { match &self.original_storage_cache { - Some((_, ref original_storage_cache)) => { - if let Some(value) = original_storage_cache.borrow_mut().get_mut(key) { - Some(value.clone()) - } else { - None - } - } + Some((_, ref original_storage_cache)) => original_storage_cache + .borrow_mut() + .get_mut(key) + .map(|value| value.clone()), None => self.cached_moved_original_storage_at(key), } } @@ -317,11 +314,10 @@ impl Account { return Some(H256::new()); } - if let Some(value) = self.storage_cache.borrow_mut().get_mut(key) { - Some(value.clone()) - } else { - None - } + self.storage_cache + .borrow_mut() + .get_mut(key) + .map(|value| value.clone()) } /// return the balance associated with this account. diff --git a/crates/ethcore/src/state/mod.rs b/crates/ethcore/src/state/mod.rs index 5759f5f4f..f15e2b77d 100644 --- a/crates/ethcore/src/state/mod.rs +++ b/crates/ethcore/src/state/mod.rs @@ -246,10 +246,7 @@ pub fn prove_transaction_virtual + Send + Syn factories, ); - let mut state = match res { - Ok(state) => state, - Err(_) => return None, - }; + let mut state = res.ok()?; let options = TransactOptions::with_no_tracing() .dont_check_nonce() diff --git a/crates/ethcore/sync/src/api.rs b/crates/ethcore/sync/src/api.rs index aa82fe776..a512d7a83 100644 --- a/crates/ethcore/sync/src/api.rs +++ b/crates/ethcore/sync/src/api.rs @@ -294,10 +294,7 @@ impl SyncProvider for EthSync { .into_iter() .zip(peer_info) .filter_map(|(peer_id, peer_info)| { - let session_info = match ctx.session_info(peer_id) { - None => return None, - Some(info) => info, - }; + let session_info = ctx.session_info(peer_id)?; Some(PeerInfo { id: session_info.id.map(|id| format!("{:x}", id)), @@ -746,14 +743,14 @@ impl NetworkConfiguration { Ok(BasicNetworkConfiguration { config_path: self.config_path, net_config_path: self.net_config_path, - listen_address: match self.listen_address { - None => None, - Some(addr) => Some(SocketAddr::from_str(&addr)?), - }, - public_address: match self.public_address { - None => None, - Some(addr) => Some(SocketAddr::from_str(&addr)?), - }, + listen_address: self + .listen_address + .map(|addr| SocketAddr::from_str(&addr)) + .transpose()?, + public_address: self + .public_address + .map(|addr| SocketAddr::from_str(&addr)) + .transpose()?, udp_port: self.udp_port, nat_enabled: self.nat_enabled, discovery_enabled: self.discovery_enabled, diff --git a/crates/net/network-devp2p/src/node_table.rs b/crates/net/network-devp2p/src/node_table.rs index 65e946375..03232e2ff 100644 --- a/crates/net/network-devp2p/src/node_table.rs +++ b/crates/net/network-devp2p/src/node_table.rs @@ -503,10 +503,7 @@ impl Drop for NodeTable { /// Check if node url is valid pub fn validate_node_url(url: &str) -> Option { - match Node::from_str(url) { - Ok(_) => None, - Err(e) => Some(e), - } + Node::from_str(url).err() } mod json { diff --git a/crates/rpc/src/v1/impls/eth.rs b/crates/rpc/src/v1/impls/eth.rs index 928990d4e..aa6529380 100644 --- a/crates/rpc/src/v1/impls/eth.rs +++ b/crates/rpc/src/v1/impls/eth.rs @@ -683,10 +683,10 @@ where let num = num.unwrap_or_default(); try_bf!(check_known(&*self.client, num.clone())); - let res = match self.client.balance(&address, self.get_state(num)) { - Some(balance) => Ok(balance), - None => Err(errors::state_pruned()), - }; + let res = self + .client + .balance(&address, self.get_state(num)) + .ok_or_else(|| errors::state_pruned()); Box::new(future::done(res)) } @@ -779,17 +779,13 @@ where self.client.nonce(&address, BlockId::Latest) }); - match nonce { - Some(nonce) => Ok(nonce), - None => Err(errors::database("latest nonce missing")), - } + nonce.ok_or_else(|| errors::database("latest nonce missing")) } number => { try_bf!(check_known(&*self.client, number.clone())); - match self.client.nonce(&address, block_number_to_id(number)) { - Some(nonce) => Ok(nonce), - None => Err(errors::state_pruned()), - } + self.client + .nonce(&address, block_number_to_id(number)) + .ok_or_else(|| errors::state_pruned()) } }; diff --git a/crates/rpc/src/v1/impls/parity_set.rs b/crates/rpc/src/v1/impls/parity_set.rs index aad0fbf8c..21489d034 100644 --- a/crates/rpc/src/v1/impls/parity_set.rs +++ b/crates/rpc/src/v1/impls/parity_set.rs @@ -110,10 +110,9 @@ where F: Fetch + 'static, { fn set_min_gas_price(&self, gas_price: U256) -> Result { - match self.miner.set_minimal_gas_price(gas_price) { - Ok(success) => Ok(success), - Err(e) => Err(errors::unsupported(e, None)), - } + self.miner + .set_minimal_gas_price(gas_price) + .map_err(|e| errors::unsupported(e, None)) } fn set_transactions_limit(&self, _limit: usize) -> Result {