update jsonrpsee to 0.2.0-alpha.6 #266

niklasad1 · 2021-04-21T08:02:40Z

The rationale is behind this bump is paritytech/jsonrpsee#274 and that the client API has been changed to avoid less allocations.

Also we have released the jsonrpsee root crate again but it doesn't expose tokio02 feature to won't work for this repo yet :(

niklasad1 · 2021-04-21T16:30:58Z

client/src/lib.rs

-                                            Output::Success(_) => {
-                                                Ok((
-                                                    send_back_sub,
-                                                    SubscriptionId::Num(request_id),


This was wrong and fixed, it would use the actual request ID. Thus dropping the subscription wouldn't unless the server had configured the actual requestID as subscription ID :)

src/lib.rs

niklasad1 · 2021-04-21T16:33:30Z

src/rpc.rs

@@ -294,8 +290,11 @@ impl<T: Runtime> Rpc<T> {
        key: &StorageKey,
        hash: Option<T::Hash>,
    ) -> Result<Option<StorageData>, Error> {
-        let params = Params::Array(vec![to_json_value(key)?, to_json_value(hash)?]);
-        let data = self.client.request("state_getStorage", params).await?;
+        let params: &[_] = &[to_json_value(key)?, to_json_value(hash)?];


needed to not be treated as an reference to an array i.e, a slice.

It's annoying having to add this type hint, perhaps we could extract the whole pattern of: construct params, do request, then log.

Anyway it's fine for now, hope in the future all these rpc calls can be generated with a macro. Is there an updated one in jsonrpsee client yet? When I first did this there was one but it didn't work.

The macros already supported in the clients but the macros doesn't work when you have two different clients I think, then we don't from which crate the fetch types in each client, technically we wanted to avoid users to explicitly jsonrpsee types which is an "internal crate"

The way it's designed is to use the wrapper jsonrpsee crate similar to what it was before but we don't expose the tokio 02 feature there so can't be used here unfortunately. paritytech/jsonrpsee#271

ascjones

Looks good but I haven't looked properly at the client changes,

I'm getting even more inclined to move that out of this repo since it is quite a bit of maintenance and we do not use it for tests here anymore. Which also means we don't actually know whether it works still - have you tested it?

client/src/lib.rs

src/lib.rs

ascjones · 2021-04-28T11:31:10Z

src/rpc.rs

@@ -294,8 +290,11 @@ impl<T: Runtime> Rpc<T> {
        key: &StorageKey,
        hash: Option<T::Hash>,
    ) -> Result<Option<StorageData>, Error> {
-        let params = Params::Array(vec![to_json_value(key)?, to_json_value(hash)?]);
-        let data = self.client.request("state_getStorage", params).await?;
+        let params: &[_] = &[to_json_value(key)?, to_json_value(hash)?];


It's annoying having to add this type hint, perhaps we could extract the whole pattern of: construct params, do request, then log.

Anyway it's fine for now, hope in the future all these rpc calls can be generated with a macro. Is there an updated one in jsonrpsee client yet? When I first did this there was one but it didn't work.

src/rpc.rs

niklasad1 · 2021-04-28T12:04:46Z

I'm getting even more inclined to move that out of this repo since it is quite a bit of maintenance and we do not use it for tests here anymore. Which also means we don't actually know whether it works still - have you tested it?

No, I agree but I think we could extract that boiler plate from jsonrpsee in an async client abstraction or something

niklasad1 · 2021-04-29T12:00:57Z

src/rpc.rs

@@ -176,28 +172,32 @@ pub enum RpcClient {

 impl RpcClient {
    /// Start a JSON-RPC request.
-    pub async fn request<T: DeserializeOwned>(
+    pub async fn request<'a, T: DeserializeOwned + std::fmt::Debug>(


added debug bound here to make is possible to log here but might be more readable logging on the the concrete type

ascjones

LGTM. If @gregdhill can confirm the client is working then I will go ahead and merge.

gregdhill · 2021-04-30T13:28:06Z

I can confirm it was working with v5, but I have not yet tested with v6.

niklasad1 · 2021-04-30T15:31:12Z

@gregdhill please let us know when you have tested it, because the test-node have been removed from this repo and I found no easy-way to test it 🙏

akru · 2021-05-10T10:48:08Z

I have following issue with this branch:

error: failed to select a version for `jsonrpsee-http-client`.
    ... required by package `remote-externalities v0.9.0 (https://github.com/paritytech/substrate?branch=rococo-v1#2be8fcc4)`
    ... which is depended on by `try-runtime-cli v0.9.0 (https://github.com/paritytech/substrate?branch=rococo-v1#2be8fcc4)`
    ... which is depended on by `polkadot-cli v0.8.30 (https://github.com/paritytech/polkadot?branch=rococo-v1#56d0154f)`
    ... which is depended on by `node-cli v0.28.2 (/home/akru/devel/robonomics/bin/node/cli)`
    ... which is depended on by `robonomics v0.28.2 (/home/akru/devel/robonomics/bin/node)`
versions that meet the requirements `=0.2.0-alpha.5` are: 0.2.0-alpha.5

all possible versions conflict with previously selected packages.

  previously selected package `jsonrpsee-http-client v0.2.0-alpha.6`
    ... which is depended on by `substrate-subxt v0.15.0 (https://github.com/paritytech/substrate-subxt?branch=na-bump-jsonrpsee#77df29c6)`
    ... which is depended on by `robonomics-protocol v0.9.0 (/home/akru/devel/robonomics/robonomics/protocol)`
    ... which is depended on by `robonomics-cli v0.7.0 (/home/akru/devel/robonomics/robonomics/cli)`
    ... which is depended on by `node-cli v0.28.2 (/home/akru/devel/robonomics/bin/node/cli)`
    ... which is depended on by `robonomics v0.28.2 (/home/akru/devel/robonomics/bin/node)`

failed to select a version for `jsonrpsee-http-client` which could resolve this conflict

Is it possible to make bonds a bit softer?

niklasad1 · 2021-05-10T10:58:15Z

Is it possible to make bonds a bit softer?

It should go away once the rocco branch updates substrate, paritytech/substrate#8690

It's an exact dependency because we might introduce breaking changes as this is a "pre-release" but if you have any good ideas how to do it better I'm open for suggestions :)

akru · 2021-05-12T08:57:02Z

@niklasad1 I see, thank you. Hope this will fix it when rococo updated. Don’t want to block merge process.

wuminzhe · 2021-05-18T07:38:30Z

@niklasad1 When can this PR be merged?

niklasad1 · 2021-05-19T10:08:05Z

@wuminzhe

When can this PR be merged?

After @gregdhill has tested the client crate ideally, but maybe I could try it out on their repo.

gregdhill · 2021-05-19T15:44:29Z

Apologies, I've been preoccupied lately. Let me upgrade our clients tomorrow and I will verify that the latest upgrade works with the embedded client.

gregdhill · 2021-05-20T16:58:00Z

@niklasad1 I can confirm that 0.2.0-alpha.6 is working as intended.

niklasad1 added 4 commits April 21, 2021 10:01

update jsonrpsee to 0.2.0-alpha.5

a259338

downgrade subxt client

5167397

cleanup

fc3a972

make subxt-client compile again

caa362b

niklasad1 commented Apr 21, 2021

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

niklasad1 commented Apr 21, 2021

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

niklasad1 commented Apr 21, 2021

View reviewed changes

niklasad1 requested a review from ascjones April 21, 2021 16:33

niklasad1 marked this pull request as ready for review April 21, 2021 16:34

niklasad1 changed the title ~~update jsonrpsee to 0.2.0-alpha.5~~ [DO NOT MERGE] update jsonrpsee to 0.2.0-alpha.5 Apr 26, 2021

update jsonrpsee v0.2.0-alpha.6

87a15db

niklasad1 changed the title ~~[DO NOT MERGE] update jsonrpsee to 0.2.0-alpha.5~~ update jsonrpsee to 0.2.0-alpha.6 Apr 27, 2021

niklasad1 mentioned this pull request Apr 27, 2021

Hardcoded jsonrpsee version to alpha3 #273

Closed

ascjones reviewed Apr 28, 2021

View reviewed changes

niklasad1 added 6 commits April 29, 2021 10:13

fix build again

7b64808

remove needless type hints

3d8beb3

cargo fmt

768e1bc

address grumbles

7d8e1df

remove remaining type hints

6255740

cargo fmt

77df29c

niklasad1 commented Apr 29, 2021

View reviewed changes

ascjones approved these changes Apr 30, 2021

View reviewed changes

niklasad1 merged commit 94db874 into master May 20, 2021

niklasad1 deleted the na-bump-jsonrpsee branch May 20, 2021 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update jsonrpsee to 0.2.0-alpha.6 #266

update jsonrpsee to 0.2.0-alpha.6 #266

niklasad1 commented Apr 21, 2021 •

edited

Loading

niklasad1 Apr 21, 2021

niklasad1 Apr 21, 2021

ascjones Apr 28, 2021

niklasad1 Apr 28, 2021 •

edited

Loading

ascjones left a comment

ascjones Apr 28, 2021

niklasad1 commented Apr 28, 2021 •

edited

Loading

niklasad1 Apr 29, 2021

ascjones left a comment •

edited

Loading

gregdhill commented Apr 30, 2021

niklasad1 commented Apr 30, 2021 •

edited

Loading

akru commented May 10, 2021

niklasad1 commented May 10, 2021 •

edited

Loading

akru commented May 12, 2021

wuminzhe commented May 18, 2021

niklasad1 commented May 19, 2021

gregdhill commented May 19, 2021

gregdhill commented May 20, 2021

update jsonrpsee to 0.2.0-alpha.6 #266

update jsonrpsee to 0.2.0-alpha.6 #266

Conversation

niklasad1 commented Apr 21, 2021 • edited Loading

niklasad1 Apr 21, 2021

Choose a reason for hiding this comment

niklasad1 Apr 21, 2021

Choose a reason for hiding this comment

ascjones Apr 28, 2021

Choose a reason for hiding this comment

niklasad1 Apr 28, 2021 • edited Loading

Choose a reason for hiding this comment

ascjones left a comment

Choose a reason for hiding this comment

ascjones Apr 28, 2021

Choose a reason for hiding this comment

niklasad1 commented Apr 28, 2021 • edited Loading

niklasad1 Apr 29, 2021

Choose a reason for hiding this comment

ascjones left a comment • edited Loading

Choose a reason for hiding this comment

gregdhill commented Apr 30, 2021

niklasad1 commented Apr 30, 2021 • edited Loading

akru commented May 10, 2021

niklasad1 commented May 10, 2021 • edited Loading

akru commented May 12, 2021

wuminzhe commented May 18, 2021

niklasad1 commented May 19, 2021

gregdhill commented May 19, 2021

gregdhill commented May 20, 2021

niklasad1 commented Apr 21, 2021 •

edited

Loading

niklasad1 Apr 28, 2021 •

edited

Loading

niklasad1 commented Apr 28, 2021 •

edited

Loading

ascjones left a comment •

edited

Loading

niklasad1 commented Apr 30, 2021 •

edited

Loading

niklasad1 commented May 10, 2021 •

edited

Loading