From a70efda4c7a10338b4f4b2164de7f56e0c222437 Mon Sep 17 00:00:00 2001 From: debris Date: Sun, 14 Jul 2019 19:37:19 +0200 Subject: [PATCH 1/2] cleanup stratum errors --- ethcore/light/src/on_demand/request.rs | 1 - ethcore/src/miner/stratum.rs | 80 +++++++------------------- miner/stratum/src/lib.rs | 30 +++++----- miner/stratum/src/traits.rs | 30 +--------- 4 files changed, 37 insertions(+), 104 deletions(-) diff --git a/ethcore/light/src/on_demand/request.rs b/ethcore/light/src/on_demand/request.rs index d57699a798c..15b04ca6355 100644 --- a/ethcore/light/src/on_demand/request.rs +++ b/ethcore/light/src/on_demand/request.rs @@ -26,7 +26,6 @@ use common_types::receipt::Receipt; use common_types::transaction::SignedTransaction; use ethcore::engines::{Engine, StateDependentProof}; use ethcore::executive_state::{ProvedExecution, self}; -use account_state; use ethereum_types::{H256, U256, Address}; use ethtrie::{TrieError, TrieDB}; use hash::{KECCAK_NULL_RLP, KECCAK_EMPTY, KECCAK_EMPTY_LIST_RLP, keccak}; diff --git a/ethcore/src/miner/stratum.rs b/ethcore/src/miner/stratum.rs index aad646753a3..9b8483488c5 100644 --- a/ethcore/src/miner/stratum.rs +++ b/ethcore/src/miner/stratum.rs @@ -17,8 +17,7 @@ //! Client-side stratum job dispatcher and mining notifier handler use std::sync::{Arc, Weak}; -use std::net::{SocketAddr, AddrParseError}; -use std::fmt; +use std::net::SocketAddr; use client::{Client, ImportSealedBlock}; use ethereum_types::{H64, H256, U256}; @@ -27,9 +26,7 @@ use ethash::{self, SeedHashCompute}; use ethcore_miner::work_notify::NotifyWork; #[cfg(feature = "work-notify")] use ethcore_stratum::PushWorkHandler; -use ethcore_stratum::{ - JobDispatcher, Stratum as StratumService, Error as StratumServiceError, -}; +use ethcore_stratum::{JobDispatcher, Stratum as StratumService}; use miner::{Miner, MinerService}; use parking_lot::Mutex; use rlp::encode; @@ -62,16 +59,16 @@ struct SubmitPayload { } impl SubmitPayload { - fn from_args(payload: Vec) -> Result { + fn from_args(payload: Vec) -> Result { if payload.len() != 3 { - return Err(PayloadError::ArgumentsAmountUnexpected(payload.len())); + return Err(format!("Stratum received unexpected amount of arguments {}", payload.len())); } let nonce = match clean_0x(&payload[0]).parse::() { Ok(nonce) => nonce, Err(e) => { warn!(target: "stratum", "submit_work ({}): invalid nonce ({:?})", &payload[0], e); - return Err(PayloadError::InvalidNonce(payload[0].clone())) + return Err(format!("Stratum received invalid nonce: {}", payload[0])); } }; @@ -79,7 +76,7 @@ impl SubmitPayload { Ok(pow_hash) => pow_hash, Err(e) => { warn!(target: "stratum", "submit_work ({}): invalid hash ({:?})", &payload[1], e); - return Err(PayloadError::InvalidPowHash(payload[1].clone())); + return Err(format!("Stratum received invalid pow hash: {}", payload[1])); } }; @@ -87,32 +84,18 @@ impl SubmitPayload { Ok(mix_hash) => mix_hash, Err(e) => { warn!(target: "stratum", "submit_work ({}): invalid mix-hash ({:?})", &payload[2], e); - return Err(PayloadError::InvalidMixHash(payload[2].clone())); + return Err(format!("Stratum received invalid mix hash: {}", payload[2])); } }; Ok(SubmitPayload { - nonce: nonce, - pow_hash: pow_hash, - mix_hash: mix_hash, + nonce, + pow_hash, + mix_hash, }) } } -#[derive(Debug)] -enum PayloadError { - ArgumentsAmountUnexpected(usize), - InvalidNonce(String), - InvalidPowHash(String), - InvalidMixHash(String), -} - -impl fmt::Display for PayloadError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Debug::fmt(&self, f) - } -} - /// Job dispatcher for stratum service pub struct StratumJobDispatcher { seed_compute: Mutex, @@ -132,10 +115,8 @@ impl JobDispatcher for StratumJobDispatcher { })) } - fn submit(&self, payload: Vec) -> Result<(), StratumServiceError> { - let payload = SubmitPayload::from_args(payload).map_err(|e| - StratumServiceError::Dispatch(e.to_string()) - )?; + fn submit(&self, payload: Vec) -> Result<(), String> { + let payload = SubmitPayload::from_args(payload)?; trace!( target: "stratum", @@ -154,7 +135,7 @@ impl JobDispatcher for StratumJobDispatcher { Ok(_) => Ok(()), Err(e) => { warn!(target: "stratum", "submit_seal error: {:?}", e); - Err(StratumServiceError::Dispatch(e.to_string())) + Err(format!("Stratum dispatch failed with error: {}", e)) } } }) @@ -187,7 +168,7 @@ impl StratumJobDispatcher { self.client.upgrade().and_then(|client| self.miner.upgrade().and_then(|miner| (f)(client, miner))) } - fn with_core_result(&self, f: F) -> Result<(), StratumServiceError> where F: Fn(Arc, Arc) -> Result<(), StratumServiceError> { + fn with_core_result(&self, f: F) -> Result<(), String> where F: Fn(Arc, Arc) -> Result<(), String> { match (self.client.upgrade(), self.miner.upgrade()) { (Some(client), Some(miner)) => f(client, miner), _ => Ok(()), @@ -201,46 +182,27 @@ pub struct Stratum { service: Arc, } -#[derive(Debug)] -/// Stratum error -pub enum Error { - /// IPC sockets error - Service(StratumServiceError), - /// Invalid network address - Address(AddrParseError), -} - -impl From for Error { - fn from(service_err: StratumServiceError) -> Error { Error::Service(service_err) } -} - -impl From for Error { - fn from(err: AddrParseError) -> Error { Error::Address(err) } -} - #[cfg(feature = "work-notify")] impl NotifyWork for Stratum { fn notify(&self, pow_hash: H256, difficulty: U256, number: u64) { trace!(target: "stratum", "Notify work"); - - self.service.push_work_all( - self.dispatcher.payload(pow_hash, difficulty, number) - ).unwrap_or_else( - |e| warn!(target: "stratum", "Error while pushing work: {:?}", e) - ); + self.service.push_work_all(self.dispatcher.payload(pow_hash, difficulty, number)) } } impl Stratum { /// New stratum job dispatcher, given the miner, client and dedicated stratum service - pub fn start(options: &Options, miner: Weak, client: Weak) -> Result { + pub fn start(options: &Options, miner: Weak, client: Weak) -> Result { use std::net::IpAddr; let dispatcher = Arc::new(StratumJobDispatcher::new(miner, client)); + let listen_addr = options.listen_addr.parse::() + .map_err(|e| format!("Stratum cannot parse listen address: {:?}", e))?; + let stratum_svc = StratumService::start( - &SocketAddr::new(options.listen_addr.parse::()?, options.port), + &SocketAddr::new(listen_addr, options.port), dispatcher.clone(), options.secret.clone(), )?; @@ -253,7 +215,7 @@ impl Stratum { /// Start STRATUM job dispatcher and register it in the miner #[cfg(feature = "work-notify")] - pub fn register(cfg: &Options, miner: Arc, client: Weak) -> Result<(), Error> { + pub fn register(cfg: &Options, miner: Arc, client: Weak) -> Result<(), String> { let stratum = Stratum::start(cfg, Arc::downgrade(&miner.clone()), client)?; miner.add_work_listener(Box::new(stratum) as Box); Ok(()) diff --git a/miner/stratum/src/lib.rs b/miner/stratum/src/lib.rs index 5ee0296dacf..5a31b1637d4 100644 --- a/miner/stratum/src/lib.rs +++ b/miner/stratum/src/lib.rs @@ -31,7 +31,7 @@ extern crate parking_lot; mod traits; pub use traits::{ - JobDispatcher, PushWorkHandler, Error, ServiceConfiguration, + JobDispatcher, PushWorkHandler, ServiceConfiguration, }; use jsonrpc_tcp_server::{ @@ -72,7 +72,7 @@ impl Stratum { addr: &SocketAddr, dispatcher: Arc, secret: Option, - ) -> Result, Error> { + ) -> Result, String> { let implementation = Arc::new(StratumImpl { subscribers: RwLock::default(), @@ -93,7 +93,7 @@ impl Stratum { let server_builder = JsonRpcServerBuilder::new(handler); let tcp_dispatcher = server_builder.dispatcher(); let server_builder = server_builder.session_meta_extractor(PeerMetaExtractor::new(tcp_dispatcher.clone())); - let server = server_builder.start(addr)?; + let server = server_builder.start(addr).map_err(|e| format!("Stratum server cannot be started: {:?}", e))?; let stratum = Arc::new(Stratum { rpc_server: Some(server), @@ -106,11 +106,11 @@ impl Stratum { } impl PushWorkHandler for Stratum { - fn push_work_all(&self, payload: String) -> Result<(), Error> { + fn push_work_all(&self, payload: String) { self.implementation.push_work_all(payload, &self.tcp_dispatcher) } - fn push_work(&self, payloads: Vec) -> Result<(), Error> { + fn push_work(&self, payloads: Vec) -> Result<(), String> { self.implementation.push_work(payloads, &self.tcp_dispatcher) } } @@ -204,13 +204,11 @@ impl StratumImpl { /// Helper method fn update_peers(&self, tcp_dispatcher: &Dispatcher) { if let Some(job) = self.dispatcher.job() { - if let Err(e) = self.push_work_all(job, tcp_dispatcher) { - warn!("Failed to update some of the peers: {:?}", e); - } + self.push_work_all(job, tcp_dispatcher) } } - fn push_work_all(&self, payload: String, tcp_dispatcher: &Dispatcher) -> Result<(), Error> { + fn push_work_all(&self, payload: String, tcp_dispatcher: &Dispatcher) { let hup_peers = { let workers = self.workers.read(); let next_request_id = { @@ -243,18 +241,16 @@ impl StratumImpl { let mut workers = self.workers.write(); for hup_peer in hup_peers { workers.remove(&hup_peer); } } - - Ok(()) } - fn push_work(&self, payloads: Vec, tcp_dispatcher: &Dispatcher) -> Result<(), Error> { + fn push_work(&self, payloads: Vec, tcp_dispatcher: &Dispatcher) -> Result<(), String> { if !payloads.len() > 0 { - return Err(Error::NoWork); + return Err("Stratum has no work".into()); } let workers = self.workers.read(); let addrs = workers.keys().collect::>(); if !workers.len() > 0 { - return Err(Error::NoWorkers); + return Err("Stratum has no workers".into()); } let mut que = payloads; let mut addr_index = 0; @@ -264,7 +260,7 @@ impl StratumImpl { tcp_dispatcher.push_message( next_worker, next_payload.nth(0).expect("drained successfully of 0..1, so 0-th element should exist") - )?; + ).map_err(|e| format!("Stratum cannot push more work: {:?}", e))?; addr_index = addr_index + 1; } Ok(()) @@ -330,7 +326,7 @@ mod tests { pub struct VoidManager; impl JobDispatcher for VoidManager { - fn submit(&self, _payload: Vec) -> Result<(), Error> { + fn submit(&self, _payload: Vec) -> Result<(), String> { Ok(()) } } @@ -398,7 +394,7 @@ mod tests { Some(self.initial_payload.clone()) } - fn submit(&self, _payload: Vec) -> Result<(), Error> { + fn submit(&self, _payload: Vec) -> Result<(), String> { Ok(()) } } diff --git a/miner/stratum/src/traits.rs b/miner/stratum/src/traits.rs index 36b95a0169e..c1e53783273 100644 --- a/miner/stratum/src/traits.rs +++ b/miner/stratum/src/traits.rs @@ -14,31 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Ethereum. If not, see . -use std; -use std::error::Error as StdError; use ethereum_types::H256; -use jsonrpc_tcp_server::PushMessageError; - -#[derive(Debug, Clone)] -pub enum Error { - NoWork, - NoWorkers, - Io(String), - Tcp(String), - Dispatch(String), -} - -impl From for Error { - fn from(err: std::io::Error) -> Self { - Error::Io(err.description().to_owned()) - } -} - -impl From for Error { - fn from(err: PushMessageError) -> Self { - Error::Tcp(format!("Push message error: {:?}", err)) - } -} /// Interface that can provide pow/blockchain-specific responses for the clients pub trait JobDispatcher: Send + Sync { @@ -49,16 +25,16 @@ pub trait JobDispatcher: Send + Sync { // json for job update given worker_id (payload manager should split job!) fn job(&self) -> Option { None } // miner job result - fn submit(&self, payload: Vec) -> Result<(), Error>; + fn submit(&self, payload: Vec) -> Result<(), String>; } /// Interface that can handle requests to push job for workers pub trait PushWorkHandler: Send + Sync { /// push the same work package for all workers (`payload`: json of pow-specific set of work specification) - fn push_work_all(&self, payload: String) -> Result<(), Error>; + fn push_work_all(&self, payload: String); /// push the work packages worker-wise (`payload`: json of pow-specific set of work specification) - fn push_work(&self, payloads: Vec) -> Result<(), Error>; + fn push_work(&self, payloads: Vec) -> Result<(), String>; } pub struct ServiceConfiguration { From d1f5b49e66f92aa49851748c14a465922f34ca46 Mon Sep 17 00:00:00 2001 From: debris Date: Mon, 15 Jul 2019 11:10:56 +0200 Subject: [PATCH 2/2] fix compilation errors --- miner/stratum/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/miner/stratum/src/lib.rs b/miner/stratum/src/lib.rs index 5a31b1637d4..679b15cdc96 100644 --- a/miner/stratum/src/lib.rs +++ b/miner/stratum/src/lib.rs @@ -471,8 +471,7 @@ mod tests { .map_err(|err: timeout::Error<()>| panic!("Timeout: {:?}", err)) .and_then(move |stream| { trace!(target: "stratum", "Pusing work to peers"); - stratum.push_work_all(r#"{ "00040008", "100500" }"#.to_owned()) - .expect("Pushing work should produce no errors"); + stratum.push_work_all(r#"{ "00040008", "100500" }"#.to_owned()); Timeout::new(future::ok(stream), ::std::time::Duration::from_millis(100)) }) .map_err(|err: timeout::Error<()>| panic!("Timeout: {:?}", err))