Sync stand-alone binary and feature-gated dependencies refactoring #1637

NikVolf · 2016-07-16T14:01:39Z

No description provided.

coveralls · 2016-07-16T14:18:27Z

Coverage increased (+0.04%) to 76.259% when pulling ac8db8f on sync-svc into 6441759 on master.

coveralls · 2016-07-16T15:09:05Z

Coverage increased (+0.09%) to 76.314% when pulling a74b72f on sync-svc into 6441759 on master.

rphmeier · 2016-07-16T15:53:06Z

parity/sync/main.rs

+
+	run_service("ipc:///tmp/parity-sync.ipc", stop.clone(), &(sync.clone() as Arc<SyncProvider>));
+	run_service("ipc:///tmp/parity-manage-net.ipc", stop.clone(), &(sync.clone() as Arc<ManageNetwork>));
+	run_service("ipc:///tmp/parity-sync-notify.ipc", stop.clone(), &(sync.clone() as Arc<ChainNotify>));


why pass by reference here at all? you are cloning just so you can pass by-ref just so run_service can clone again?

rphmeier · 2016-07-16T16:00:38Z

parity/sync/main.rs

+use nanoipc::*;
+use ethsync::{SyncProvider, SyncConfig, EthSync, ManageNetwork, NetworkConfiguration};
+use std::thread;
+use util::numbers::*;


seems like just U256 is used here? prefer without glob import. atomic is only used for AtomicBool and Ordering.
nanoipc is used for Worker, but that's referenced explicitly from nanoipc

NikVolf · 2016-07-16T17:30:50Z

@rphmeier
all issues addressed, really helpful, thanx!

coveralls · 2016-07-16T18:17:34Z

Coverage increased (+0.03%) to 76.251% when pulling 9eed51e on sync-svc into 6441759 on master.

gavofyork · 2016-07-17T07:11:26Z

parity/sync/main.rs

+  --public-address IP      Public address.
+  --boot-nodes LIST        List of boot nodes.
+  --reserved-nodes LIST    List of reserved peers,
+  --secret HEX             Use node key hash


if this is passed on the CLI, won't it be visible to anyone able to run ps? maybe better to pass via stdin/out.

NikVolf · 2016-07-17T08:41:34Z

@gavofyork i moved hashing of secret to the sync side, so ps-runner won't get any new information

coveralls · 2016-07-17T09:01:40Z

Coverage increased (+0.1%) to 76.334% when pulling a44e95b on sync-svc into 6441759 on master.

rphmeier · 2016-07-18T11:28:49Z

parity/sync/main.rs

@@ -38,6 +38,7 @@ use std::thread;
 use util::numbers::{U256, H256};
 use std::str::FromStr;
 use nanoipc::IpcInterface;
+use util::sha3::*;


only used for Hashable trait?

NikVolf · 2016-07-18T13:22:57Z

@rphmeier updated

coveralls · 2016-07-18T13:46:19Z

Coverage increased (+0.002%) to 76.449% when pulling 028d6f6 on sync-svc into 3d00a91 on master.

arkpar · 2016-07-18T14:27:58Z

lgtm, although I don't like the idea of passing network config on the command line. It should be serialized much as other type so we don't have to worry about IPC when adding a new config option.

NikVolf · 2016-07-18T14:29:26Z

@arkpar #1657

NikVolf added 11 commits July 14, 2016 12:29

sorting out the multi-interface dispatch scenario

2310ecb

codegen expansion for traits

f380340

fix rwlock

c60e02d

basic layout of sync executable and minor fixes in the api

507a4ea

flush work

4a9d57d

hypervisor refactoring

b1a67bf

dependancies layout

98ddff1

rpc dependencies relayout

4cb1c90

toml file fix

fea05dc

Merge branch 'master' into sync-svc

be25e93

redundant crate

ac8db8f

NikVolf added A0-pleasereview 🤓 Pull request needs code review. A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Jul 16, 2016

submodules update

a74b72f

NikVolf changed the title ~~Sync stand-alone binary and rpc dependencies refactoring~~ Sync stand-alone binary and feature-gated dependencies refactoring Jul 16, 2016

rphmeier reviewed Jul 16, 2016
View reviewed changes

NikVolf added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 16, 2016

rphmeier reviewed Jul 16, 2016
View reviewed changes

NikVolf added 4 commits July 16, 2016 18:51

review fixes

8e26977

got rid of Arc<T> dispatch

77bbab0

got rid of the generic parameter for ipc interface

c3ef5c1

comments -

9eed51e

NikVolf added the A0-pleasereview 🤓 Pull request needs code review. label Jul 16, 2016

NikVolf removed the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Jul 16, 2016

rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 16, 2016

NikVolf removed the A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. label Jul 16, 2016

gavofyork reviewed Jul 17, 2016
View reviewed changes

gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jul 17, 2016

hashing secret on the sync side

a44e95b

NikVolf added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jul 17, 2016

NikVolf mentioned this pull request Jul 17, 2016

Removing unused client code #1645

Merged

rphmeier reviewed Jul 18, 2016
View reviewed changes

NikVolf mentioned this pull request Jul 18, 2016

IPC (feature-gated) #1654

Merged

NikVolf added 2 commits July 18, 2016 15:20

remove import glob

6a47fd1

Merge branch 'master' into sync-svc

028d6f6

rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 18, 2016

NikVolf merged commit 18f1661 into master Jul 18, 2016

NikVolf deleted the sync-svc branch July 18, 2016 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync stand-alone binary and feature-gated dependencies refactoring #1637

Sync stand-alone binary and feature-gated dependencies refactoring #1637

NikVolf commented Jul 16, 2016

coveralls commented Jul 16, 2016 •

edited

Loading

coveralls commented Jul 16, 2016 •

edited

Loading

rphmeier Jul 16, 2016

rphmeier Jul 16, 2016 •

edited

Loading

NikVolf commented Jul 16, 2016

coveralls commented Jul 16, 2016 •

edited

Loading

gavofyork Jul 17, 2016

NikVolf commented Jul 17, 2016

coveralls commented Jul 17, 2016 •

edited

Loading

rphmeier Jul 18, 2016

NikVolf commented Jul 18, 2016

coveralls commented Jul 18, 2016 •

edited

Loading

arkpar commented Jul 18, 2016

NikVolf commented Jul 18, 2016

Sync stand-alone binary and feature-gated dependencies refactoring #1637

Sync stand-alone binary and feature-gated dependencies refactoring #1637

Conversation

NikVolf commented Jul 16, 2016

coveralls commented Jul 16, 2016 • edited Loading

coveralls commented Jul 16, 2016 • edited Loading

rphmeier Jul 16, 2016

Choose a reason for hiding this comment

rphmeier Jul 16, 2016 • edited Loading

Choose a reason for hiding this comment

NikVolf commented Jul 16, 2016

coveralls commented Jul 16, 2016 • edited Loading

gavofyork Jul 17, 2016

Choose a reason for hiding this comment

NikVolf commented Jul 17, 2016

coveralls commented Jul 17, 2016 • edited Loading

rphmeier Jul 18, 2016

Choose a reason for hiding this comment

NikVolf commented Jul 18, 2016

coveralls commented Jul 18, 2016 • edited Loading

arkpar commented Jul 18, 2016

NikVolf commented Jul 18, 2016

coveralls commented Jul 16, 2016 •

edited

Loading

coveralls commented Jul 16, 2016 •

edited

Loading

rphmeier Jul 16, 2016 •

edited

Loading

coveralls commented Jul 16, 2016 •

edited

Loading

coveralls commented Jul 17, 2016 •

edited

Loading

coveralls commented Jul 18, 2016 •

edited

Loading