Unstable Backend: add back the old chainHead RPCs/tests #1137
Conversation
| /// | ||
| /// # Note | ||
| /// | ||
| /// This is present only if the `with_runtime` flag is set for |
There was a problem hiding this comment.
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. :)
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
Yeah I agree that we should not complicate it more.
Co-authored-by: Tadeo Hepperle <62739623+tadeohepperle@users.noreply.github.com>
| use super::*; | ||
|
|
||
| #[test] | ||
| fn can_deserialize_apis_from_tuple_or_object() { |
|
|
||
| /// 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 { |
There was a problem hiding this comment.
add a simple test for this as well?
There was a problem hiding this comment.
Yup I should def do this!
| @@ -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 { | |||
There was a problem hiding this comment.
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 :)
| @@ -35,19 +35,6 @@ use subxt::{ | |||
| }; | |||
| use subxt_metadata::Metadata; | |||
|
|
|||
| // We don't use these dependencies. | |||
| use assert_matches as _; | |||
| /// | ||
| /// # Panics | ||
| /// | ||
| /// Panics if `self.type_id` is not registered in the provided type registry |
There was a problem hiding this comment.
Is this not true anymore?
Can't find anything in this PR about this :P
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 :)
| pub async fn author_rotate_keys(&self) -> Result<Vec<u8>, Error> { | ||
| let bytes: Bytes = self | ||
| .client | ||
| .request("author_rotateKeys", rpc_params![]) |
There was a problem hiding this comment.
Hmm, are we adding those back to the legacy API since they might be useful for validators? 🤔
There was a problem hiding this comment.
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!
| /// 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, |
There was a problem hiding this comment.
nit: I think this is now named with_runtime from spec
This PR isn't as big as it looks; new
rustfmtunderstandslet ... elsestatements now, and otherwise seemed to want to reformat quite a lot! See below for actual changes.chainSpecmethods.The only real changes in terms of files are:
subxt/src/backend/unstable/- add unstable RPC methods backtesting/integration-tests/src/full_client/client/legacy_rpcs.rs- for the testsI 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 moreuse foo as _;things depending on different features being enabled or not.