Implement parity_versionInfo & parity_setChain on LC; fix parity_setChain #10312

axelchalon · 2019-02-07T16:26:21Z

~~Implement eth_subscribe('syncing') RPC for full node & light node (closes Implement eth_subscribe('syncing') #10080); same output format as eth_syncing rpc~~: we'll use the implentation of this other PR RPC: Implements eth_subscribe("syncing") #10311 instead
parity_setChain RPC: return an error if failed (instead of true)
Light client: implement set_exit_handler & parity_setChain RPC (needed for Fether)
Light client: implement parity_versionInfo RPC (needed for Fether)

cc @amaurymartiny

parity-cla-bot · 2019-02-07T16:26:25Z

It looks like @axelchalon signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

parity-cla-bot · 2019-02-07T16:26:25Z

It looks like @axelchalon signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

axelchalon · 2019-02-07T16:33:23Z

There's another implementation of eth_subscribe('syncing') in this PR: #10311

tomusdrw

Please fix the indentation in all files, apart from that it looks ok. We could optimize the notification though to get events from sync like #10311 is attempting to do.

axelchalon · 2019-02-18T15:18:34Z

FWIW if preferred I can revert the Implement eth_subscribe('syncing') commit if we decide to use the implementation in the other PR #10311

Tbaut · 2019-03-04T11:06:17Z

adding the beta label as we need if for Fether :)

niklasad1 · 2019-03-04T11:43:45Z

parity/run.rs

@@ -379,7 +383,7 @@ fn execute_light_impl(cmd: RunCmd, logger: Arc<RotatingLogger>) -> Result<Runnin

 fn execute_impl<Cr, Rr>(cmd: RunCmd, logger: Arc<RotatingLogger>, on_client_rq: Cr,
 						on_updater_rq: Rr) -> Result<RunningClient, String>
-	where Cr: Fn(String) + 'static + Send,
+	where Cr: Fn(String) -> Result<(), ()> + 'static + Send,


Is this change needed?

It breaks parity-clib compatibility, https://github.com/paritytech/parity-ethereum/blob/master/parity-clib/src/lib.rs#L145 is the problem basically we ignore the return value because the restart function is void i.e, doesn't return any value.

It could be fixed by changing the return value of the callback function to an integer and map 0 to Ok and other values as Err but it requires some changes in the C API

OK, it'd be nice to have the RPC response reflect whether or not the restart succeeded (although not necessary) but changing the return value of the restart client callback to an integer doesn't seem very clean so I'll change back the return value of the callback for now! 👍

niklasad1 · 2019-03-04T11:43:56Z

parity/run.rs

@@ -165,7 +165,9 @@ impl ::local_store::NodeInfo for FullNodeInfo {
 type LightClient = ::light::client::Client<::light_helpers::EpochFetch>;

 // helper for light execution.
-fn execute_light_impl(cmd: RunCmd, logger: Arc<RotatingLogger>) -> Result<RunningClient, String> {
+fn execute_light_impl<Cr>(cmd: RunCmd, logger: Arc<RotatingLogger>, on_client_rq: Cr) -> Result<RunningClient, String>
+	where Cr: Fn(String) -> Result<(), ()> + 'static + Send


Is this change needed?

It breaks parity-clib compatibility

niklasad1 · 2019-03-04T11:45:41Z

rpc/src/v1/impls/parity_set.rs

@@ -240,7 +239,7 @@ impl<C, M, U, F> ParitySet for ParitySetClient<C, M, U, F> where
 		let hash = hash.into();

 		Ok(self.miner.remove_transaction(&hash)
-		   .map(|t| Transaction::from_pending(t.pending().clone()))
+			 .map(|t| Transaction::from_pending(t.pending().clone()))


remove space please :P

niklasad1

Overall the code looks good but the change of the trait bound where Cr: Fn(String) -> Result<(), ()> + 'static + Send breaks compatibility with C library which need to be addressed!

Either by changing back back the trait bound to where Cr: Fn(String) + 'static + Send or fix it in the C API (see inline comments)

…hain (#10312) * Light client: implement parity_versionInfo RPC * Light client: implement set_exit_handler & parity_setChain RPC * parity_setChain RPC: return an error if failed (instead of `true`) * Implement eth_subscribe('syncing') RPC for full node & light node * Fix indentation * Revert commit: Implement eth_subscribe('syncing') * Revert change to Cr callback function

* version: bump beta * Implement parity_versionInfo & parity_setChain on LC; fix parity_setChain (#10312) * Light client: implement parity_versionInfo RPC * Light client: implement set_exit_handler & parity_setChain RPC * parity_setChain RPC: return an error if failed (instead of `true`) * Implement eth_subscribe('syncing') RPC for full node & light node * Fix indentation * Revert commit: Implement eth_subscribe('syncing') * Revert change to Cr callback function * CI publish to aws (#10446) * move publish aws from gitlab.yml to gitlab scripts * gitlab.yml cleaning move publish AWS to gitlab scripts remove dependencies from android build * CI aws git checkout (#10451) * Updating the CI system with the publication of releases and binary files on github Signed-off-by: Denis S. Soldatov aka General-Beck <general.beck@gmail.com> * move publish aws from gitlab.yml to gitlab scripts Signed-off-by: Denis S. Soldatov aka General-Beck <general.beck@gmail.com> * gitlab.yml cleaning move publish AWS to gitlab scripts remove dependencies from android build Signed-off-by: Denis S. Soldatov aka General-Beck <general.beck@gmail.com> * Revert "Updating the CI system with the publication of releases and binary files on github" This reverts commit da87e06. * remove no-git for aws * microfix * no need in no_git then * Revert "CI aws git checkout (#10451)" (#10456) * Revert "CI aws git checkout (#10451)" This reverts commit 3e1d731. * Update .gitlab-ci.yml revert aws script with small fixes * Delete publish-aws.sh * Tests parallelized (#10452) * tests splitted, phase 1 * typo * fix wrong launch commands * typos * rearrangements * use `nproc` function for threads * use nproc for threads * let theads be auto, build-andriod no more in regular run * split val chain and cargo check * renamed some files * wrong phase * check rust files before test jobs * lint error * rust files modivied var * test except changes * add rust_changes except * lint error * fixes * .gitlab-ci.yml can't be excluded * pipeline shouldn't start * pipeline must go * pipeline must go 2 * pipeline must go 3 * pipeline must go 4 * pipeline must go 5 * pipeline must go 6 * pipeline must go 7 * pipeline must not go 1 * pipeline must go 8 * avoid skippng tests yet, reintroducing them after the caching * test theory * parallelized cargo check with combusting helicopters * less uploads * alias for cargo checks * nice template * Ensure static validator set changes are recognized (#10467)

axelchalon added 4 commits February 7, 2019 17:01

Light client: implement parity_versionInfo RPC

8cd2c44

Light client: implement set_exit_handler & parity_setChain RPC

87826de

parity_setChain RPC: return an error if failed (instead of true)

0c25ee1

Implement eth_subscribe('syncing') RPC for full node & light node

f259865

axelchalon added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. labels Feb 7, 2019

axelchalon mentioned this pull request Feb 7, 2019

RPC: Implements eth_subscribe("syncing") #10311

Merged

tomusdrw reviewed Feb 8, 2019

View reviewed changes

5chdn added this to the 2.4 milestone Feb 8, 2019

axelchalon force-pushed the ac-rpc branch from 004fc66 to 468468f Compare February 8, 2019 14:22

Fix indentation

468468f

5chdn modified the milestones: 2.4, 2.5, 2.6 Feb 21, 2019

Revert commit: Implement eth_subscribe('syncing')

304e0c1

axelchalon changed the title ~~Implement eth_subscribe('syncing'); parity_versionInfo & parity_setChain on LC; fix parity_setChain~~ Implement parity_versionInfo & parity_setChain on LC; fix parity_setChain Feb 22, 2019

Merge from master

15c5238

axelchalon requested a review from tomusdrw February 24, 2019 19:39

Merge branch 'master' into ac-rpc

9a5a9aa

tomusdrw approved these changes Feb 25, 2019

View reviewed changes

Merge from master

c0574da

soc1c added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 27, 2019

Merge branch 'master' into ac-rpc

d3f5701

Tbaut requested review from ordian and niklasad1 March 4, 2019 11:03

Tbaut added the B1-patch-beta 🕷🕷 label Mar 4, 2019

niklasad1 reviewed Mar 4, 2019

View reviewed changes

niklasad1 suggested changes Mar 4, 2019

View reviewed changes

niklasad1 added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Mar 4, 2019

Revert change to Cr callback function

3af6838

ordian approved these changes Mar 4, 2019

View reviewed changes

niklasad1 approved these changes Mar 4, 2019

View reviewed changes

niklasad1 added A8-looksgood 🦄 Pull request is reviewed well. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Mar 4, 2019

niklasad1 merged commit 7014642 into master Mar 4, 2019

niklasad1 deleted the ac-rpc branch March 4, 2019 19:24

soc1c mentioned this pull request Mar 13, 2019

2.4.1 beta backports #10471

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement parity_versionInfo & parity_setChain on LC; fix parity_setChain #10312

Implement parity_versionInfo & parity_setChain on LC; fix parity_setChain #10312

axelchalon commented Feb 7, 2019 •

edited

Loading

parity-cla-bot commented Feb 7, 2019

parity-cla-bot commented Feb 7, 2019

axelchalon commented Feb 7, 2019

tomusdrw left a comment

axelchalon commented Feb 18, 2019

Tbaut commented Mar 4, 2019

niklasad1 Mar 4, 2019 •

edited

Loading

axelchalon Mar 4, 2019

niklasad1 Mar 4, 2019

niklasad1 Mar 4, 2019

niklasad1 left a comment

Implement parity_versionInfo & parity_setChain on LC; fix parity_setChain #10312

Implement parity_versionInfo & parity_setChain on LC; fix parity_setChain #10312

Conversation

axelchalon commented Feb 7, 2019 • edited Loading

parity-cla-bot commented Feb 7, 2019

parity-cla-bot commented Feb 7, 2019

axelchalon commented Feb 7, 2019

tomusdrw left a comment

Choose a reason for hiding this comment

axelchalon commented Feb 18, 2019

Tbaut commented Mar 4, 2019

niklasad1 Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

axelchalon Mar 4, 2019

Choose a reason for hiding this comment

niklasad1 Mar 4, 2019

Choose a reason for hiding this comment

niklasad1 Mar 4, 2019

Choose a reason for hiding this comment

niklasad1 left a comment

Choose a reason for hiding this comment

axelchalon commented Feb 7, 2019 •

edited

Loading

niklasad1 Mar 4, 2019 •

edited

Loading