Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Fix work-notify. #9104

Merged
merged 1 commit into from
Jul 13, 2018
Merged

Fix work-notify. #9104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ futures-cpupool = "0.1"
fdlimit = "0.1"
ctrlc = { git = "https://github.com/paritytech/rust-ctrlc.git" }
jsonrpc-core = { git = "https://github.com/paritytech/jsonrpc.git", branch = "parity-1.11" }
ethcore = { path = "ethcore", features = ["work-notify", "price-info", "stratum"] }
ethcore = { path = "ethcore", features = ["parity"] }
parity-bytes = { git = "https://github.com/paritytech/parity-common" }
ethcore-io = { path = "util/io" }
ethcore-light = { path = "ethcore/light" }
Expand Down
10 changes: 7 additions & 3 deletions ethcore/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ tempdir = "0.3"
trie-standardmap = { git = "https://github.com/paritytech/parity-common" }

[features]
parity = ["work-notify", "price-info", "stratum"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this?
I feel like we would get a compilation error in Parity anyway if we forget to enable a feature.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is just more future-proof in case we extract more optional features, because it's self-contained within ethcore. You don't have to go to all other crates and add your own feature, which is error-prone.
That would be especially important for things that might be disabled by default if the feature is not enabled.

Obviously can revert if you don't think it's necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not very proper, but it's fine I think.

# Large optional features that are enabled by default for Parity,
# but might be omitted for other dependent crates.
work-notify = ["ethcore-miner/work-notify"]
price-info = ["ethcore-miner/price-info"]
stratum = ["ethcore-stratum"]

# Display EVM debug traces.
evm-debug = ["evm/evm-debug"]
# Display EVM debug traces when running tests.
Expand All @@ -97,6 +104,3 @@ test-heavy = []
benches = []
# Compile test helpers
test-helpers = ["tempdir"]
work-notify = ["ethcore-miner/work-notify"]
price-info = ["ethcore-miner/price-info"]
stratum = ["ethcore-stratum"]
12 changes: 4 additions & 8 deletions parity/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ use ethcore_logger::{Config as LogConfig, RotatingLogger};
use ethcore_service::ClientService;
use ethereum_types::Address;
use sync::{self, SyncConfig};
#[cfg(feature = "work-notify")]
use miner::work_notify::WorkPoster;
use futures::IntoFuture;
use futures_cpupool::CpuPool;
Expand Down Expand Up @@ -510,13 +509,10 @@ fn execute_impl<Cr, Rr>(cmd: RunCmd, logger: Arc<RotatingLogger>, on_client_rq:
miner.set_gas_range_target(cmd.miner_extras.gas_range_target);
miner.set_extra_data(cmd.miner_extras.extra_data);

#[cfg(feature = "work-notify")]
{
if !cmd.miner_extras.work_notify.is_empty() {
miner.add_work_listener(Box::new(
WorkPoster::new(&cmd.miner_extras.work_notify, fetch.clone(), event_loop.remote())
));
}
if !cmd.miner_extras.work_notify.is_empty() {
miner.add_work_listener(Box::new(
WorkPoster::new(&cmd.miner_extras.work_notify, fetch.clone(), event_loop.remote())
));
}

let engine_signer = cmd.miner_extras.engine_signer;
Expand Down