From 7eda7cb0ee9fff9ab16f37cd90cba5ed21a3a576 Mon Sep 17 00:00:00 2001 From: danda Date: Sat, 26 Nov 2022 16:42:44 -0800 Subject: [PATCH] refactor(error): improve error handling Addresses issue #247. (more to come later) kindelia_core: * removes all unwrap() from net.rs * defines error enums in net.rs: ProtoCommError and ParseAddressError * return Result from affected fn in net.rs * change IPV6 panic!() to unimplemented!() * accept addr arg for Node::new() so it can avoid returning Result * update test and bench as necessary kindelia: * add dep on anyhow crate * return anyhow::Result from config methods * return anyhow::Result from main methods, including main() * return anyhow::Result from FileInput::read_to_string() --- Cargo.lock | 70 +++++++++++++++ kindelia/Cargo.toml | 2 +- kindelia/src/config.rs | 102 +++++++++++---------- kindelia/src/files.rs | 10 ++- kindelia/src/main.rs | 142 +++++++++++++++++------------- kindelia_core/benches/bench.rs | 4 +- kindelia_core/src/net.rs | 72 +++++++++++---- kindelia_core/src/node.rs | 4 +- kindelia_core/src/test/network.rs | 9 +- 9 files changed, 280 insertions(+), 135 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 71b24b34..0bc7442c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,21 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "addr2line" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9ecd88a8c8378ca913a680cd98f0f13ac67383d35993f86c90a70e3f137816b" +dependencies = [ + "gimli", +] + +[[package]] +name = "adler" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" + [[package]] name = "aho-corasick" version = "0.7.19" @@ -20,6 +35,15 @@ dependencies = [ "libc", ] +[[package]] +name = "anyhow" +version = "1.0.66" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "216261ddc8289130e551ddcd5ce8a064710c0d064a4d2895c67151c92b5443f6" +dependencies = [ + "backtrace", +] + [[package]] name = "arrayvec" version = "0.7.2" @@ -232,6 +256,21 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "backtrace" +version = "0.3.66" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cab84319d616cfb654d03394f38ab7e6f0919e181b1b57e1fd15e7fb4077d9a7" +dependencies = [ + "addr2line", + "cc", + "cfg-if", + "libc", + "miniz_oxide", + "object", + "rustc-demangle", +] + [[package]] name = "base64" version = "0.13.1" @@ -1099,6 +1138,12 @@ dependencies = [ "wasi 0.11.0+wasi-snapshot-preview1", ] +[[package]] +name = "gimli" +version = "0.26.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22030e2c5a68ec659fde1e949a745124b48e6fa8b045b7ed5bd1fe4ccc5c4e5d" + [[package]] name = "gloo-timers" version = "0.2.4" @@ -1440,6 +1485,7 @@ checksum = "f9b7d56ba4a8344d6be9729995e6b06f928af29998cdf79fe390cbf6b1fee838" name = "kindelia" version = "0.1.5" dependencies = [ + "anyhow", "assert_cmd", "clap 4.0.22", "clap_complete", @@ -1691,6 +1737,15 @@ dependencies = [ "unicase", ] +[[package]] +name = "miniz_oxide" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96590ba8f175222643a85693f33d26e9c8a015f599c216509b1a6894af675d34" +dependencies = [ + "adler", +] + [[package]] name = "mio" version = "0.8.5" @@ -1783,6 +1838,15 @@ dependencies = [ "libc", ] +[[package]] +name = "object" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21158b2c33aa6d4561f1c0a6ea283ca92bc54802a93b263e910746d679a7eb53" +dependencies = [ + "memchr", +] + [[package]] name = "once_cell" version = "1.16.0" @@ -2472,6 +2536,12 @@ dependencies = [ "syn", ] +[[package]] +name = "rustc-demangle" +version = "0.1.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ef03e0a2b150c7a90d01faf6254c9c48a41e95fb2a8c2ac1c6f0d2b9aefc342" + [[package]] name = "rustc-hex" version = "2.1.0" diff --git a/kindelia/Cargo.toml b/kindelia/Cargo.toml index 0b6e04c1..29e23979 100644 --- a/kindelia/Cargo.toml +++ b/kindelia/Cargo.toml @@ -47,7 +47,7 @@ serde_json = "1.0" # Events API tokio = { version = "1.19.1", features = ["sync"] } - +anyhow = { version = "1.0.66", features = ["backtrace"] } [dev-dependencies] assert_cmd = "1.0.1" diff --git a/kindelia/src/config.rs b/kindelia/src/config.rs index dab851a3..40c8ae1d 100644 --- a/kindelia/src/config.rs +++ b/kindelia/src/config.rs @@ -1,6 +1,7 @@ use std::{path::PathBuf, str::FromStr}; use crate::files::FileInput; +use anyhow::anyhow; // ConfigSettings // ============== @@ -10,7 +11,7 @@ use crate::files::FileInput; pub struct ConfigSettings where T: Clone + Sized, - F: Fn() -> Result, + F: Fn() -> anyhow::Result, { #[builder(default)] env: Option<&'static str>, @@ -22,7 +23,7 @@ where impl ConfigSettings where T: Clone + Sized, - F: Fn() -> Result, + F: Fn() -> anyhow::Result, { /// Resolve config value. /// @@ -35,7 +36,7 @@ where self, cli_value: Option, config_values: Option<&toml::Value>, - ) -> Result + ) -> anyhow::Result where T: ArgumentFrom + ArgumentFrom, { @@ -59,7 +60,7 @@ where pub fn resolve_from_file_only( self, config_values: Option<&toml::Value>, - ) -> Result + ) -> anyhow::Result where T: ArgumentFrom, { @@ -77,7 +78,7 @@ where pub fn resolve_from_file_opt( self, config_values: Option<&toml::Value>, - ) -> Result, String> + ) -> anyhow::Result> where T: ArgumentFrom, { @@ -86,10 +87,10 @@ where let value = Self::get_prop(config_values, &prop_path); if let Some(value) = value { return T::arg_from(value).map(|v| Some(v)).map_err(|e| { - format!( + anyhow!(format!( "Could not convert value of '{}' into desired type: {}", prop_path, e - ) + )) }); } } @@ -102,17 +103,18 @@ where fn resolve_from_config_aux( config_values: &toml::Value, prop_path: &str, - ) -> Result + ) -> anyhow::Result where T: ArgumentFrom, { - let value = Self::get_prop(config_values, prop_path) - .ok_or(format!("Could not find prop '{}' in config file.", prop_path))?; + let value = Self::get_prop(config_values, prop_path).ok_or_else(|| { + anyhow!(format!("Could not find prop '{}' in config file.", prop_path)) + })?; T::arg_from(value).map_err(|e| { - format!( + anyhow!(format!( "Could not convert value of '{}' into desired type: {}", prop_path, e - ) + )) }) } @@ -134,127 +136,133 @@ where /// it has the From for Vec implementation. /// As like From, the conversion must be perfect. pub trait ArgumentFrom: Sized { - fn arg_from(value: T) -> Result; + fn arg_from(value: T) -> anyhow::Result; } impl ArgumentFrom for String { - fn arg_from(t: String) -> Result { + fn arg_from(t: String) -> anyhow::Result { Ok(t) } } impl ArgumentFrom for u32 { - fn arg_from(t: String) -> Result { - t.parse().map_err(|e| format!("Invalid integer: `{}`", e)) + fn arg_from(t: String) -> anyhow::Result { + t.parse().map_err(|e| anyhow!(format!("Invalid integer: `{}`", e))) } } impl ArgumentFrom for u64 { - fn arg_from(t: String) -> Result { - t.parse().map_err(|e| format!("Invalid integer: `{}`", e)) + fn arg_from(t: String) -> anyhow::Result { + t.parse().map_err(|e| anyhow!(format!("Invalid integer: `{}`", e))) } } impl ArgumentFrom for u32 { - fn arg_from(value: toml::Value) -> Result { + fn arg_from(value: toml::Value) -> anyhow::Result { match value { toml::Value::Integer(i) => Ok(i as Self), toml::Value::String(s) => { let s = s.trim_start_matches("0x"); - let num = u32::from_str_radix(s, 16) - .map_err(|e| format!("Invalid hexadecimal '{}': {}", s, e))?; + let num = u32::from_str_radix(s, 16).map_err(|e| { + anyhow!(format!("Invalid hexadecimal '{}': {}", s, e)) + })?; Ok(num) } - _ => Err(format!("Invalid integer '{}'", value)), + _ => Err(anyhow!(format!("Invalid integer '{}'", value))), } } } impl ArgumentFrom for u64 { - fn arg_from(value: toml::Value) -> Result { + fn arg_from(value: toml::Value) -> anyhow::Result { match value { toml::Value::Integer(i) => Ok(i as u64), toml::Value::String(s) => { let s = s.trim_start_matches("0x"); - let num = u64::from_str_radix(s, 16) - .map_err(|e| format!("Invalid hexadecimal '{}': {}", s, e))?; + let num = u64::from_str_radix(s, 16).map_err(|e| { + anyhow!(format!("Invalid hexadecimal '{}': {}", s, e)) + })?; Ok(num) } - _ => Err(format!("Invalid integer '{}'", value)), + _ => Err(anyhow!(format!("Invalid integer '{}'", value))), } } } impl ArgumentFrom for Vec { - fn arg_from(t: String) -> Result { + fn arg_from(t: String) -> anyhow::Result { Ok(t.split(',').map(|x| x.to_string()).collect()) } } impl ArgumentFrom for bool { - fn arg_from(t: String) -> Result { + fn arg_from(t: String) -> anyhow::Result { if t == "true" { Ok(true) } else if t == "false" { Ok(false) } else { - Err(format!("Invalid boolean value: {}", t)) + Err(anyhow!(format!("Invalid boolean value: {}", t))) } } } impl ArgumentFrom for PathBuf { - fn arg_from(t: String) -> Result { + fn arg_from(t: String) -> anyhow::Result { if let Some(path) = t.strip_prefix("~/") { - let home_dir = - dirs::home_dir().ok_or("Could not find $HOME directory.")?; + let home_dir = dirs::home_dir() + .ok_or_else(|| anyhow!("Could not find $HOME directory."))?; Ok(home_dir.join(path)) } else { - PathBuf::from_str(&t).map_err(|_| format!("Invalid path: {}", t)) + PathBuf::from_str(&t).map_err(|_| anyhow!(format!("Invalid path: {}", t))) } } } impl ArgumentFrom for PathBuf { - fn arg_from(value: toml::Value) -> Result { - let t: String = - value.try_into().map_err(|_| "Could not convert value to PahtBuf")?; + fn arg_from(value: toml::Value) -> anyhow::Result { + let t: String = value + .try_into() + .map_err(|_| anyhow!("Could not convert value to PathBuf"))?; PathBuf::arg_from(t) } } impl ArgumentFrom for String { - fn arg_from(t: toml::Value) -> Result { - t.try_into().map_err(|_| "Could not convert value into String".to_string()) + fn arg_from(t: toml::Value) -> anyhow::Result { + t.try_into() + .map_err(|_| anyhow!("Could not convert value into String".to_string())) } } impl ArgumentFrom for Vec { - fn arg_from(t: toml::Value) -> Result { - t.try_into().map_err(|_| "Could not convert value into array".to_string()) + fn arg_from(t: toml::Value) -> anyhow::Result { + t.try_into() + .map_err(|_| anyhow!("Could not convert value into array".to_string())) } } impl ArgumentFrom for bool { - fn arg_from(t: toml::Value) -> Result { - t.as_bool().ok_or(format!("Invalid boolean value: {}", t)) + fn arg_from(t: toml::Value) -> anyhow::Result { + t.as_bool().ok_or_else(|| anyhow!(format!("Invalid boolean value: {}", t))) } } impl ArgumentFrom for kindelia_core::config::ApiConfig { - fn arg_from(t: toml::Value) -> Result { - t.try_into().map_err(|_| "Could not convert value into array".to_string()) + fn arg_from(t: toml::Value) -> anyhow::Result { + t.try_into() + .map_err(|_| anyhow!("Could not convert value into array".to_string())) } } pub fn arg_from_file_or_stdin>( file: FileInput, -) -> Result { +) -> anyhow::Result { match file { FileInput::Path { path } => { // read from file let content = std::fs::read_to_string(&path).map_err(|err| { - format!("Cannot read from '{:?}' file: {}", path, err) + anyhow!(format!("Cannot read from '{:?}' file: {}", path, err)) })?; T::arg_from(content) } @@ -263,7 +271,7 @@ pub fn arg_from_file_or_stdin>( let mut input = String::new(); match std::io::stdin().read_line(&mut input) { Ok(_) => T::arg_from(input.trim().to_string()), - Err(err) => Err(format!("Could not read from stdin: {}", err)), + Err(err) => Err(anyhow!(format!("Could not read from stdin: {}", err))), } } } diff --git a/kindelia/src/files.rs b/kindelia/src/files.rs index 1e68864e..fa909cc4 100644 --- a/kindelia/src/files.rs +++ b/kindelia/src/files.rs @@ -1,3 +1,4 @@ +use anyhow::anyhow; use std::fmt::{Display, Formatter}; use std::io::Read; use std::path::PathBuf; @@ -40,19 +41,20 @@ impl Display for FileInput { // TODO: alternative that do not read the whole file immediately impl FileInput { - pub fn read_to_string(&self) -> Result { + pub fn read_to_string(&self) -> anyhow::Result { match self { FileInput::Path { path } => { // read from file - std::fs::read_to_string(path) - .map_err(|e| format!("Cannot read from '{:?}' file: {}", path, e)) + std::fs::read_to_string(path).map_err(|e| { + anyhow!(format!("Cannot read from '{:?}' file: {}", path, e)) + }) } FileInput::Stdin => { // read from stdin let mut buff = String::new(); std::io::stdin() .read_to_string(&mut buff) - .map_err(|e| format!("Could not read from stdin: {}", e))?; + .map_err(|e| anyhow!(format!("Could not read from stdin: {}", e)))?; Ok(buff) } } diff --git a/kindelia/src/main.rs b/kindelia/src/main.rs index 517870e3..e7d56004 100644 --- a/kindelia/src/main.rs +++ b/kindelia/src/main.rs @@ -3,6 +3,7 @@ mod config; mod files; mod util; +use anyhow::anyhow; use std::future::Future; use std::net::{SocketAddr, UdpSocket}; use std::path::Path; @@ -25,7 +26,7 @@ use kindelia_core::bits::ProtoSerialize; use kindelia_core::config::{ApiConfig, MineConfig, NodeConfig, UiConfig}; use kindelia_core::net::{Address, ProtoComm}; use kindelia_core::node::{ - spawn_miner, Node, Transaction, TransactionError, MAX_TRANSACTION_SIZE, + spawn_miner, Node, Transaction, MAX_TRANSACTION_SIZE, }; use kindelia_core::persistence::{ get_ordered_blocks_path, SimpleFileStorage, BLOCKS_DIR, @@ -40,7 +41,7 @@ use util::{ use crate::cli::{GetStatsKind, NodeCleanBlocksCommand, NodeCleanCommand}; use crate::util::init_config_file; -fn main() -> Result<(), String> { +fn main() -> anyhow::Result<()> { run_cli() } @@ -58,7 +59,8 @@ macro_rules! resolve_cfg { .default_value($default) .build() .unwrap() - .resolve($cli, None)? + .resolve($cli, None) + .map_err(|e| anyhow!(e))? }; // No default value @@ -69,7 +71,8 @@ macro_rules! resolve_cfg { .default_value($default) .build() .unwrap() - .resolve($cli, None)? + .resolve($cli, None) + .map_err(|e| anyhow!(e))? }; // Resolve config value with file @@ -83,7 +86,8 @@ macro_rules! resolve_cfg { .default_value(|| Ok($default)) .build() .unwrap() - .resolve($cli, $cfg)? + .resolve($cli, $cfg) + .map_err(|e| anyhow!(e))? }; // No default value @@ -94,21 +98,23 @@ macro_rules! resolve_cfg { .default_value(|| Err($default)) .build() .unwrap() - .resolve($cli, $cfg)? + .resolve($cli, $cfg) + .map_err(|e| anyhow!(e))? }; } // TODO: create own error with `thiserror` // TODO: separate file in `mod`'s? /// Parse CLI arguments and run -pub fn run_cli() -> Result<(), String> { +pub fn run_cli() -> anyhow::Result<()> { let parsed = Cli::parse(); let default_base_path = || { - let home_dir = dirs::home_dir().ok_or("Could not find $HOME path")?; - Ok::<_, String>(home_dir.join(".kindelia")) + let home_dir = + dirs::home_dir().ok_or_else(|| anyhow!("Could not find $HOME path"))?; + Ok::<_, anyhow::Error>(home_dir.join(".kindelia")) }; let default_node_data_path = - || Ok::<_, String>(default_base_path()?.join("state")); + || Ok::<_, anyhow::Error>(default_base_path()?.join("state")); let default_config_path = || Ok(default_base_path()?.join("kindelia.toml")); let default_api_url = || Ok("http://localhost:8000".to_string()); @@ -136,14 +142,12 @@ pub fn run_cli() -> Result<(), String> { let stmts = if encoded { statements_from_hex_seq(&code)? } else { - parser::parse_code(&code)? + parser::parse_code(&code).map_err(|e| anyhow!(e))? }; match command { cli::CheckCommand::Transaction => { for ref stmt in stmts { - let transaction: Transaction = stmt - .try_into() - .map_err(|err: TransactionError| err.to_string())?; + let transaction: Transaction = stmt.try_into()?; // size printing { @@ -180,15 +184,17 @@ pub fn run_cli() -> Result<(), String> { let skey: String = arg_from_file_or_stdin(secret_file.into())?; let skey = skey.trim(); let skey = hex::decode(skey).map_err(|err| { - format!("Secret key should be valid hex string: {}", err) + anyhow!(format!("Secret key should be valid hex string: {}", err)) + })?; + let skey: [u8; 32] = skey.try_into().map_err(|_| { + anyhow!("Secret key should have exactly 64 bytes".to_string()) })?; - let skey: [u8; 32] = skey - .try_into() - .map_err(|_| "Secret key should have exactly 64 bytes".to_string())?; let code = load_code(file, encoded)?; let statement = match &code[..] { [stmt] => sign_code(stmt, &skey), - _ => Err("Input file should contain exactly one statement".to_string()), + _ => Err(anyhow!( + "Input file should contain exactly one statement".to_string() + )), }?; if encoded_output { println!("{}", hex::encode(statement.proto_serialized().to_bytes())); @@ -205,7 +211,7 @@ pub fn run_cli() -> Result<(), String> { let stmts = if encoded { statements_from_hex_seq(&code)? } else { - parser::parse_code(&code)? + parser::parse_code(&code).map_err(|e| anyhow!(e))? }; let results = run_on_remote(&api_url, stmts, f)?; for result in results { @@ -218,7 +224,7 @@ pub fn run_cli() -> Result<(), String> { let stmts = if encoded { statements_from_hex_seq(&code)? } else { - parser::parse_code(&code)? + parser::parse_code(&code).map_err(|e| anyhow!(e))? }; publish_code(&api_url, stmts, hosts) } @@ -228,22 +234,22 @@ pub fn run_cli() -> Result<(), String> { } CliCommand::Get { kind, json } => { let prom = get_info(kind, json, &api_url); - run_async_blocking(prom) + run_async_blocking(prom).map_err(|e| anyhow!(e)) } CliCommand::Init => { let path = default_config_path()?; eprintln!("Writing default configuration to '{}'...", path.display()); - init_config_file(&path)?; + init_config_file(&path).map_err(|e| anyhow!(e))?; Ok(()) } CliCommand::Node { command, data_dir, network_id } => { - let config = handle_config_file(&config_path)?; + let config = handle_config_file(&config_path).map_err(|e| anyhow!(e))?; let config = Some(&config); let network_id = resolve_cfg!( env = "KINDELIA_NETWORK_ID", prop = "node.network.network_id".to_string(), - no_default = "Missing `network_id` parameter.".to_string(), + no_default = anyhow!("Missing `network_id` parameter.".to_string()), cli_val = network_id, cfg = config, ); @@ -258,8 +264,11 @@ pub fn run_cli() -> Result<(), String> { .join(format!("{:#02X}", network_id)); match command { - NodeCommand::Clean { command } => clean(&data_path, command) - .map_err(|err| format!("Could not clean kindelia's data: {}", err)), + NodeCommand::Clean { command } => { + clean(&data_path, command).map_err(|err| { + anyhow!(format!("Could not clean kindelia's data: {}", err)) + }) + } NodeCommand::Start { initial_peers, mine, json } => { // TODO: refactor config resolution out of command handling (how?) @@ -299,9 +308,9 @@ pub fn run_cli() -> Result<(), String> { // Start let node_comm = init_socket().expect("Could not open a UDP socket"); let initial_peers = initial_peers - .iter() - .map(|x| net::parse_address(x)) - .collect::>(); + .into_iter() + .map(|x| net::parse_address(&x).map_err(|e| anyhow!(e))) + .collect::, anyhow::Error>>()?; let node_cfg = NodeConfig { network_id, @@ -315,9 +324,7 @@ pub fn run_cli() -> Result<(), String> { }; let api_config = Some(api_config); - start_node(node_cfg, api_config, node_comm, initial_peers); - - Ok(()) + start_node(node_cfg, api_config, node_comm, initial_peers) } } } @@ -330,8 +337,8 @@ pub fn run_cli() -> Result<(), String> { .map(|s| s.strip_prefix("0x").unwrap_or(s)) .map(hex::decode) .collect(); - let data = - data.map_err(|err| format!("Invalid hex string: {}", err))?; + let data = data + .map_err(|err| anyhow!(format!("Invalid hex string: {}", err)))?; let nums = data.iter().map(|v| bytes_to_u128(v)); for num in nums { if let Some(num) = num { @@ -356,14 +363,14 @@ fn run_on_remote( api_url: &str, stmts: Vec, f: F, -) -> Result +) -> anyhow::Result where F: FnOnce(ApiClient, Vec) -> P, P: Future>, { let stmts: Vec = stmts.into_iter().map(|s| s.into()).collect(); - let client = ApiClient::new(api_url, None).map_err(|e| e.to_string())?; - run_async_blocking(f(client, stmts)) + let client = ApiClient::new(api_url, None)?; + run_async_blocking(f(client, stmts)).map_err(|e| anyhow!(e)) } fn print_json_else( @@ -512,7 +519,7 @@ pub fn serialize_code(code: &str) { } } -pub fn deserialize_code(content: &str) -> Result<(), String> { +pub fn deserialize_code(content: &str) -> anyhow::Result<()> { let statements = statements_from_hex_seq(content)?; for statement in statements { println!("{}", statement) @@ -523,7 +530,7 @@ pub fn deserialize_code(content: &str) -> Result<(), String> { pub fn sign_code( statement: &ast::Statement, skey: &[u8; 32], -) -> Result { +) -> anyhow::Result { let user = crypto::Account::from_private_key(skey); let hash = runtime::hash_statement(statement); let sign = user.sign(&hash); @@ -533,7 +540,7 @@ pub fn sign_code( | ast::Statement::Run { sign, .. } | ast::Statement::Reg { sign, .. } => { if sign.is_some() { - return Err("Statement already has a signature.".to_string()); + return Err(anyhow!("Statement already has a signature.".to_string())); } } }; @@ -541,20 +548,26 @@ pub fn sign_code( Ok(stat) } -fn load_code(file: FileInput, encoded: bool) -> Result, String> { +fn load_code( + file: FileInput, + encoded: bool, +) -> anyhow::Result> { let code = file.read_to_string()?; handle_code(&code, encoded) } -fn handle_code(code: &str, encoded: bool) -> Result, String> { +fn handle_code( + code: &str, + encoded: bool, +) -> anyhow::Result> { if encoded { statements_from_hex_seq(code) } else { - parser::parse_code(code) + parser::parse_code(code).map_err(|e| anyhow!(e)) } } -fn statements_from_hex_seq(txt: &str) -> Result, String> { +fn statements_from_hex_seq(txt: &str) -> anyhow::Result> { txt .trim() .split(|c: char| c.is_whitespace()) @@ -562,27 +575,28 @@ fn statements_from_hex_seq(txt: &str) -> Result, String> { .collect() } -fn statement_from_hex(hex: &str) -> Result { - let bytes = hex::decode(hex) - .map_err(|err| format!("Invalid hexadecimal '{}': {}", hex, err))?; +fn statement_from_hex(hex: &str) -> anyhow::Result { + let bytes = hex::decode(hex).map_err(|err| { + anyhow!(format!("Invalid hexadecimal '{}': {}", hex, err)) + })?; ast::Statement::proto_deserialized(&bytes_to_bitvec(&bytes)) - .ok_or(format!("Failed to deserialize '{}'", hex)) + .ok_or_else(|| anyhow!(format!("Failed to deserialize '{}'", hex))) } pub fn publish_code( api_url: &str, stmts: Vec, hosts: Vec, -) -> Result<(), String> { +) -> anyhow::Result<()> { // setup tokio runtime and unordered joinset (tasks). - let runtime = tokio::runtime::Runtime::new().map_err(|e| e.to_string())?; + let runtime = tokio::runtime::Runtime::new()?; let mut tasks = tokio::task::JoinSet::new(); - let client = ApiClient::new(api_url, None).map_err(|e| e.to_string())?; + let client = ApiClient::new(api_url, None)?; let peer_urls: Vec = if hosts.is_empty() { // obtain list of active peers known to "our" node. let prom = async move { client.get_peers::(false).await }; - let peers = runtime.block_on(prom)?; + let peers = runtime.block_on(prom).map_err(|e| anyhow!(e))?; let mut urls: Vec = peers .iter() .map(|p| match p.address { @@ -611,7 +625,7 @@ pub fn publish_code( for peer_url in peer_urls.into_iter() { // these are move'd into the spawned task, so they must be // created for each iteration. - let client = ApiClient::new(peer_url, None).map_err(|e| e.to_string())?; + let client = ApiClient::new(peer_url, None)?; let stmts_hex = stmts_hex.clone(); // spawn a new task for contacting each peer. Because it is @@ -647,7 +661,7 @@ pub fn publish_code( async fn join_all( mut tasks: tokio::task::JoinSet>, -) -> Result<(), String> { +) -> anyhow::Result<()> { while let Some(_res) = tasks.join_next().await {} Ok(()) } @@ -797,18 +811,19 @@ pub fn start_node( api_config: Option, comm: C, initial_peers: Vec, -) { +) -> anyhow::Result<()> { eprintln!("Starting Kindelia node..."); eprintln!("Store path: {:?}", node_config.data_path); eprintln!("Network ID: {:#X}", node_config.network_id); + let addr = comm.get_addr()?; + // Threads let mut threads = vec![]; // Events #[cfg(feature = "events")] let event_tx = { - let addr = comm.get_addr(); let (event_tx, event_thrds) = spawn_event_handlers( node_config.ws.unwrap_or_default(), node_config.ui, @@ -830,6 +845,7 @@ pub fn start_node( let (node_query_sender, node) = Node::new( node_config.data_path, node_config.network_id, + addr, initial_peers, comm, miner_comm, @@ -856,20 +872,24 @@ pub fn start_node( for thread in threads { thread.join().unwrap(); } + + Ok(()) } // Shell completion // ================ // prints completions for a given shell, eg bash. -fn print_shell_completions(shell: Shell) -> Result<(), String> { +fn print_shell_completions(shell: Shell) -> anyhow::Result<()> { // obtain name of present executable let exec_name = std::env::current_exe() - .map_err(|e| format!("Error getting current executable: {}", e))? + .map_err(|e| anyhow!(format!("Error getting current executable: {}", e)))? .file_name() - .ok_or_else(|| "Error getting executable file name".to_string())? + .ok_or_else(|| anyhow!("Error getting executable file name".to_string()))? .to_str() - .ok_or_else(|| "Error decoding executable name as utf8".to_string())? + .ok_or_else(|| { + anyhow!("Error decoding executable name as utf8".to_string()) + })? .to_string(); // Generates completions for and prints to stdout diff --git a/kindelia_core/benches/bench.rs b/kindelia_core/benches/bench.rs index 03421203..0371beb8 100644 --- a/kindelia_core/benches/bench.rs +++ b/kindelia_core/benches/bench.rs @@ -9,6 +9,7 @@ use primitive_types::U256; use kindelia_core::bits::ProtoSerialize; use kindelia_core::{constants, runtime, net, node, util}; +use kindelia_core::net::ProtoComm; // KHVM // ==== @@ -150,10 +151,11 @@ fn block_loading(c: &mut Criterion) { // empty `ProtoComm` to pass the created node let comm = net::EmptySocket; + let addr = comm.get_addr().unwrap(); // create Node let (_, mut node) = - node::Node::new(dir.clone(), 0, vec![], comm, None, storage, None); + node::Node::new(dir.clone(), 0, addr, vec![], comm, None, storage, None); // benchmark block loading c.bench_function("block_loading", |b| b.iter(|| node.load_blocks())); diff --git a/kindelia_core/src/net.rs b/kindelia_core/src/net.rs index ccaf2412..75a09e12 100644 --- a/kindelia_core/src/net.rs +++ b/kindelia_core/src/net.rs @@ -2,9 +2,11 @@ use std::fmt::{Debug, Display}; use std::hash::Hash; pub use std::net::UdpSocket; use std::net::{Ipv4Addr, SocketAddrV4}; +use std::num::ParseIntError; use bit_vec::BitVec; use serde; +use thiserror::Error; use crate::bits::ProtoSerialize; use crate::node::Message; @@ -29,6 +31,18 @@ where { } +/// Errors associated with the ProtoComm trait. +/// +/// Source error types are dynamic to enable trait implementors +/// to provide their own error type if needed. +#[derive(Error, Debug)] +pub enum ProtoCommError { + #[error("Address is unavailable")] + AddrUnavailable{source: Box}, + #[error(transparent)] + Other(#[from] Box), +} + /// Defines how the messages will be sent and received /// by the chain nodes. pub trait ProtoComm @@ -36,13 +50,14 @@ where Self: Sized + Send, { type Address: ProtoAddr; + fn proto_send( &mut self, addresses: Vec, message: &Message, ); fn proto_recv(&mut self) -> Vec<(Self::Address, Message)>; - fn get_addr(&self) -> Self::Address; + fn get_addr(&self) -> Result; } // UDP Implementation @@ -72,23 +87,46 @@ impl std::fmt::Display for Address { } } +#[derive(Error, Debug)] +pub enum ParseAddressError { + #[error("Invalid IP Address '{value:?}'")] + InvalidAddress { + value: String, + source: ParseIntError, + }, + #[error("Invalid Port '{value:?}'")] + InvalidPort { + value: String, + source: ParseIntError, + }, +} + /// Converts a string to an UDP Address. /// TODO: UNSAFE. -pub fn parse_address(code: &str) -> Address { +pub fn parse_address(code: &str) -> Result { let strs = code.split(':').collect::>(); - let vals = - strs[0].split('.').map(|o| o.parse::().unwrap()).collect::>(); - let port = strs.get(1).map(|s| s.parse::().unwrap()).unwrap_or(UDP_PORT); - Address::IPv4 { + let vals = strs[0] + .split('.') + .map(|o| { + o.parse::() + .map_err(|e| ParseAddressError::InvalidAddress{value: strs[0].to_string(), source: e}) }) + .collect::, ParseAddressError>>()?; + let port = match strs.get(1) { + Some(s) => s + .parse::() + .map_err(|e| ParseAddressError::InvalidAddress{value: strs[0].to_string(), source: e})?, + None => UDP_PORT, + }; + Ok(Address::IPv4 { val0: vals[0], val1: vals[1], val2: vals[2], val3: vals[3], port, - } + }) } -/// The UDP implementation based on `std::netUdpSocket` struct +/// The UDP implementation based on `std::net::UdpSocket` struct impl ProtoComm for UdpSocket { type Address = Address; fn proto_send( @@ -119,7 +157,7 @@ impl ProtoComm for UdpSocket { Address::IPv4 { val0, val1, val2, val3, port: sender_addr.port() } } _ => { - panic!("TODO: IPv6") + unimplemented!("TODO: IPv6") } }; messages.push((addr, msge)); @@ -127,14 +165,16 @@ impl ProtoComm for UdpSocket { } messages } - fn get_addr(&self) -> Self::Address { - // TODO: remove unwrap and panic - let addr = self.local_addr().unwrap(); + fn get_addr(&self) -> Result { + // TODO: impl IPV6 + let addr = self + .local_addr() + .map_err(|e| ProtoCommError::AddrUnavailable { source: Box::new(e) })?; if let std::net::IpAddr::V4(v4addr) = addr.ip() { let [val0, val1, val2, val3] = v4addr.octets(); - Address::IPv4 { val0, val1, val2, val3, port: addr.port() } + Ok(Address::IPv4 { val0, val1, val2, val3, port: addr.port() }) } else { - panic!("TODO: IPv6") + unimplemented!("TODO: IPv6") } } } @@ -168,8 +208,8 @@ impl ProtoAddr for EmptyAddress {} impl ProtoComm for EmptySocket { type Address = EmptyAddress; - fn get_addr(&self) -> Self::Address { - EmptyAddress + fn get_addr(&self) -> Result { + Ok(EmptyAddress) } fn proto_recv(&mut self) -> Vec<(Self::Address, Message)> { vec![] diff --git a/kindelia_core/src/node.rs b/kindelia_core/src/node.rs index ff60229c..64a09436 100644 --- a/kindelia_core/src/node.rs +++ b/kindelia_core/src/node.rs @@ -760,9 +760,11 @@ pub fn miner_loop( // ---- impl Node { + #[allow(clippy::too_many_arguments)] pub fn new( data_path: PathBuf, network_id: u32, + addr: C::Address, initial_peers: Vec, comm: C, miner_comm: Option, @@ -784,7 +786,7 @@ impl Node { #[rustfmt::skip] let mut node = Node { network_id, - addr: comm.get_addr(), + addr, comm, runtime, pool : PriorityQueue:: new(), diff --git a/kindelia_core/src/test/network.rs b/kindelia_core/src/test/network.rs index 3fdf1018..a09f3b8f 100644 --- a/kindelia_core/src/test/network.rs +++ b/kindelia_core/src/test/network.rs @@ -8,7 +8,7 @@ use std::thread; use crate::config; #[cfg(feature = "events")] use crate::events; -use crate::net::{self, ProtoComm}; +use crate::net::{self, ProtoComm, ProtoCommError}; use crate::node; use crate::{bits, persistence}; @@ -103,7 +103,7 @@ fn start_simulation( initial_peers: Vec, tags: Vec, ) { - let addr = comm.get_addr(); + let addr = comm.get_addr().unwrap(); // Events #[cfg(feature = "events")] @@ -121,6 +121,7 @@ fn start_simulation( let (node_query_sender, node) = node::Node::new( node_config.data_path, node_config.network_id, + addr, initial_peers, comm, miner_comm, @@ -239,8 +240,8 @@ impl net::ProtoComm for SocketMock { } } - fn get_addr(&self) -> Self::Address { - self.addr + fn get_addr(&self) -> Result { + Ok(self.addr) } }