-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
NikVolf
commented
Jan 20, 2017
- brand new tokio-powered jsonrpc over tcp/ip (TCP migration to tokio, part I paritytech/jsonrpc#29, TCP migration to tokio, part II paritytech/jsonrpc#31)
- no ipc requirements
- no conditional compilation
Conflicts: ethcore/src/miner/work_dispatcher.rs
Conflicts: Cargo.toml ethcore/Cargo.toml ethcore/src/lib.rs ethcore/src/miner/miner.rs parity/cli/usage.txt parity/run.rs
io: io, | ||
proto: self, | ||
}, &unfulfilled) | ||
let unfulfilled = match self.peers.write().remove(&peer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also part of #4285 pr
@@ -114,7 +114,7 @@ extern crate heapsize; | |||
#[macro_use] | |||
extern crate ethcore_ipc as ipc; | |||
extern crate lru_cache; | |||
|
|||
extern crate ethcore_stratum; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this and lru_cache
should be in the section with the other non-#[macro_use]
crates.
JobDispatcher, PushWorkHandler, | ||
Stratum as StratumService, Error as StratumServiceError, | ||
}; | ||
use super::StratumOptions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not declare it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there was a conditional compilation previously, now yes, why not
let dispatcher = Arc::new(StratumJobDispatcher::new(miner, client)); | ||
|
||
let stratum_svc = StratumService::start( | ||
&SocketAddr::from_str(&format!("{}:{}", options.listen_addr, options.port))?, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StratumOptions
should just contain a SocketAddr
rather than do the conversion in the start
function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
port and listen address are configured separately all over the configuration code
https://github.com/ethcore/parity/blob/master/parity/configuration.rs
but the SocketAddr instantiation should not be done via string, yes
couple more style nits but otherwise all LGTM! |