Unstable Backend: add back the old chainHead RPCs/tests #1137

jsdw · 2023-08-24T16:05:46Z

This PR isn't as big as it looks; new rustfmt understands let ... else statements now, and otherwise seemed to want to reformat quite a lot! See below for actual changes.

I put back the chainHead methods and tests.
I also added back any legacy RPCs that I removed in the last PR; I decided that there's probably enough breaking changes already, and so want to give people using those RPCs a slightly smoother migration.
I also added some other of the new APIs including to submit transactions, and the now stable chainSpec methods.

The only real changes in terms of files are:

subxt/src/backend/unstable/ - add unstable RPC methods back
testing/integration-tests/src/full_client/client/legacy_rpcs.rs - for the tests

I also removed the #[deny(unused_dependencies)] thing from the integration-tests crate, because IMO it's not really important there and we had more and more use foo as _; things depending on different features being enabled or not.

tadeohepperle

Looks very nice, just left a few comments :)

subxt/src/backend/unstable/rpc_methods.rs

tadeohepperle · 2023-08-28T08:11:31Z

subxt/src/backend/unstable/rpc_methods.rs

+    ///
+    /// # Note
+    ///
+    /// This is present only if the `with_runtime` flag is set for


Do you mean with with_runtime the argument runtime_updates: bool that is passed to chainhead_unstable_follow()? I think we should use a uniform name adressing it throughout the code.
Since it has an effect on the data in many struct, we could also think about making the FollowEvent generic over this flag (maybe with const generics). Just an idea. :)

Yeah; we should prolly call it with_runtime elsewhere too because the spec calls it withRuntime!

I guess generics could be used to "guarantee" that we get a RuntimeEvent if we ask for it, and not if we don't, maybe? But the generics might complicate things more than it's worth because they would bleed out to anything else that cared about this stuff :)

Yeah I agree that we should not complicate it more.

Co-authored-by: Tadeo Hepperle <62739623+tadeohepperle@users.noreply.github.com>

subxt/src/backend/unstable/rpc_methods.rs

niklasad1 · 2023-08-29T09:22:45Z

subxt/src/backend/unstable/rpc_methods.rs

+    use super::*;
+
+    #[test]
+    fn can_deserialize_apis_from_tuple_or_object() {


niklasad1 · 2023-08-29T09:23:08Z

subxt/src/backend/unstable/rpc_methods.rs

+
+/// Attempt to deserialize either a string or integer into an integer.
+/// See <https://github.com/paritytech/json-rpc-interface-spec/issues/83>
+pub(crate) mod unsigned_number_as_string {


add a simple test for this as well?

Yup I should def do this!

niklasad1 · 2023-08-29T09:23:49Z

subxt/src/config/mod.rs

@@ -82,7 +82,7 @@ pub trait Hasher {
 }

 /// This represents the block header type used by a node.
-pub trait Header: Sized + Encode {
+pub trait Header: Sized + Encode + Decode {


why is Decode needed now?

In the old API methods, header came back as JSON, but in the new one it comes back as hex encoded SCALE encoded bytes instead :)

niklasad1 · 2023-08-29T09:24:30Z

testing/integration-tests/src/light_client/mod.rs

@@ -35,19 +35,6 @@ use subxt::{
 };
 use subxt_metadata::Metadata;

-// We don't use these dependencies.
-use assert_matches as _;


niklasad1 · 2023-08-29T09:25:04Z

metadata/src/lib.rs

-    ///
-    /// # Panics
-    ///
-    /// Panics if `self.type_id` is not registered in the provided type registry


Is this not true anymore?

Can't find anything in this PR about this :P

I think the pub fn get_type_hash would still panic if the id passed as param is not present in the PortableRegistry 🤔

I belive the CustomValueMetadata could only panic in practice here if the metadata is broken, since that's where our value is coming from

The comment was left over from when this call was taking in a PortableRegistry as an argument (which might then have different types from what he metadata knew about and thus could panic). That was since removed but I noticed that the comment wasn't updated so I tweaked it :)

niklasad1

Looks good

lexnv · 2023-08-29T09:32:30Z

subxt/src/backend/legacy/rpc_methods.rs

+    pub async fn author_rotate_keys(&self) -> Result<Vec<u8>, Error> {
+        let bytes: Bytes = self
+            .client
+            .request("author_rotateKeys", rpc_params![])


Hmm, are we adding those back to the legacy API since they might be useful for validators? 🤔

I just added back anything that I had removed earlier basically, so that if anybody happened to be using them then there is an easy migration path. I'd like to remove some of these again eventually, but ideally as part of a smaller change!

lexnv · 2023-08-29T09:35:09Z

subxt/src/backend/unstable/rpc_methods.rs

+    /// Failure to do so will result in the subscription being stopped by generating the `Stop` event.
+    pub async fn chainhead_unstable_follow(
+        &self,
+        runtime_updates: bool,


nit: I think this is now named with_runtime from spec

Yup; I renamed it :)

jsdw added 2 commits August 24, 2023 16:56

add back unstable APIs and tests

480c894

clippy fixes

46371d8

jsdw requested a review from a team as a code owner August 24, 2023 16:05

fmt with new rust

d5b9919

jsdw marked this pull request as draft August 24, 2023 16:20

jsdw added 6 commits August 25, 2023 12:15

deserialize from object or array into api versions

dc63561

remove inspect

dec32f1

add some of the other unstable RPC methods we might want

a14c0e1

clippy fix

a06ccbe

no unused deps warning in test crate

204459d

fix doc fails

a68a133

jsdw marked this pull request as ready for review August 25, 2023 17:00

tadeohepperle approved these changes Aug 28, 2023

View reviewed changes

niklasad1 and others added 2 commits August 29, 2023 10:11

Merge branch 'master' into jsdw-unstable-backend-rpcs

2330c94

legacy -> unstable

91ac702

Co-authored-by: Tadeo Hepperle <62739623+tadeohepperle@users.noreply.github.com>

niklasad1 reviewed Aug 29, 2023

View reviewed changes

subxt/src/backend/unstable/rpc_methods.rs Outdated Show resolved Hide resolved

niklasad1 reviewed Aug 29, 2023

View reviewed changes

subxt/src/backend/unstable/rpc_methods.rs Outdated Show resolved Hide resolved

niklasad1 reviewed Aug 29, 2023

View reviewed changes

subxt/src/backend/unstable/rpc_methods.rs

use super::*;

#[test]

fn can_deserialize_apis_from_tuple_or_object() {

Copy link

Member

niklasad1 Aug 29, 2023

Choose a reason for hiding this comment

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

👍

niklasad1 reviewed Aug 29, 2023

View reviewed changes

niklasad1 approved these changes Aug 29, 2023

View reviewed changes

runtime_updates -> with_runtiem

6d72dbb

lexnv reviewed Aug 29, 2023

View reviewed changes

lexnv approved these changes Aug 29, 2023

View reviewed changes

add test and be consistent with naming of unstable RPCs

7b9d87a

jsdw merged commit a3d25db into master Aug 29, 2023
9 checks passed

jsdw deleted the jsdw-unstable-backend-rpcs branch August 29, 2023 11:35

jsdw mentioned this pull request Sep 27, 2023

Prep to release 0.32.0 #1178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unstable Backend: add back the old chainHead RPCs/tests #1137

Unstable Backend: add back the old chainHead RPCs/tests #1137

jsdw commented Aug 24, 2023 •

edited

Loading

tadeohepperle left a comment

tadeohepperle Aug 28, 2023

jsdw Aug 29, 2023

tadeohepperle Aug 29, 2023

niklasad1 Aug 29, 2023

niklasad1 Aug 29, 2023

jsdw Aug 29, 2023

niklasad1 Aug 29, 2023

jsdw Aug 29, 2023

niklasad1 Aug 29, 2023

niklasad1 Aug 29, 2023

lexnv Aug 29, 2023

jsdw Aug 29, 2023

niklasad1 left a comment

lexnv Aug 29, 2023

jsdw Aug 29, 2023

lexnv Aug 29, 2023

jsdw Aug 29, 2023

Unstable Backend: add back the old chainHead RPCs/tests #1137

Unstable Backend: add back the old chainHead RPCs/tests #1137

Conversation

jsdw commented Aug 24, 2023 • edited Loading

tadeohepperle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niklasad1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsdw commented Aug 24, 2023 •

edited

Loading