Introduce Backend trait to allow different RPC (or other) backends to be implemented #1126

jsdw · 2023-08-18T13:38:13Z

Previously, Subxt looked something like:

high level APIs <- RPC calls

This PR introduces a Backend trait inbetween those; a trait which provides the minimal set of things that the high level APIs need (while trying to cater for what the new RPCs can provide). It then implements that trait for the "legacy" API methods, and migrates the high level code to using the trait rather than directly calling methods (in fact; making this impossible). So now we have:

high level APIs <- backend implementation <- RPC calls

(Where the RPC calls are just an implementation detail of the backend implementation)

I'd suggest to look at the subxt/backend folder first to see what this trait looks like, then the subxt/backend/legacy to see the LegacyBackend implementation, which is based on the old RPC methods.

The idea is that we can add a v2 backend which is based on the new methods.

The examples are handy to look at next to see what's changed. Much of the rest of the code is fallout from adding this new backend thing!

There are a number of breaking changes here (mostly I'll cover below in the notes), but the high level APIs haven't changed significantly. Manually calling RPC methods is now more difficult, but still possible via creating an RpcClient manually, and using that to instantiate either/both of LegacyRpcMethods to make RPC calls and OnlineClient to do high level Subxt things.

Random Notes:

I've removed all of the chainHead RPC stuff at the moment. The plan is to put this stuff back as part of the new backend.
block.body().await.extrinsics() is now just block.extrinsics().await; the body of a block only contains extrinsics anyway (the new API inparticular only returns extrinsics, and not justifications, which we didn't expose anyway).
Filling in gaps in finalized blocks is now handled by the backend, whose job it is to return a full stream.
Things now take an RpcClient instead of an Arc<impl RpcClientT>. RpcClient::from_url replaces default_rpc_client(), or you can instantiate RpcClient from an underlying trait. I've done renames so that the raw trait stuff is better separated now (eg Raw* names) to make the difference clearer. One reason for this all is that calling RPC methods requires an RPC client, so it's easier if people can create a single RpcClient quite easily and then use that in the places it's needed).
We have a new BlockRef type that, in the new backend, can try to "hold onto" a block, ie keep it from being unpinned, if it's alive. Things that may need to get details at a particular block should now keep the corresponding BlockRef around until they've finished.
Storage changes include removing the explicit "page_count" from the user API; the backend will now determine the page size to use if applicable, renaming fetch_keys to fetch_keys_raw (it returns raw bytes like fetch_raw), and making iter() hand back a proper Stream which should make it more ergonomic (and changes results from being Result<Option<T>> to Option<Result<T>>). Currently it fetches keys in batches but values individually; we can look to improve that again though if needbe.
No more client.rpc(), because this is now an implementation detail of a specific Backend. Instead, you can OnlineClient::from_backend to pass a backend which has been instantiated with some RPC client that you can also use elsewhere.
tx.dry_run() is now tx.validate() and uses a runtime call for validating (in part because the new RPCs won't expose a dry run call). This returns a different result though, leading to breakage here, so I've also kept the RPC call around so that one can manually call that instead if they need the old behaviour.
tx.submit() used to directly call a submit RPC which handed the hash back. Now we delegate to submit_and_watch and return the hash once the first non-error status comes back, since the new APIs don't (currently) have a way to just submit a TX anyway.
The underlying TransactionProgress events are altered to better fit the new API results (omitting a couple of details that the old API can't replicate). The high level API around submitting and watching is otherwise unchanged.
Stream-like things always have a next() method on them instead of eg next_item() now, which is mostly just a convenience to avoid needing to import StreamExt for this common op.

…m instead of RpcClient

… types prefixed with RawRpc so it's less ocnfusing which is which

…ate(), to signal that the calls are different

tadeohepperle

I looked over the entire thing, looks good. From my understanding we will add a newer backend soon as a replacement of the LegacyBackend? If that is the case, what will be the mechanism by which it is determined what backend is used when an OnlineClient is constructed?

tadeohepperle · 2023-08-21T16:04:48Z

subxt/src/backend/mod.rs

+/// events will be emitted.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum TransactionStatus<Hash> {
+    /// Transaction is part of the future queue.


I like the new TransactionStatus!

It's basically much more similar to the one that the new APIs will return, so making this change now means that hopefully we won't need to change anything else as we implement the new backend :)

tadeohepperle · 2023-08-21T16:08:15Z

subxt/src/backend/mod.rs

+/// Prevent the backend trait being implemented externally.
+#[doc(hidden)]
+pub(crate) mod sealed {
+    pub trait Sealed {}


What is the reason to put Sealed into a separate mod? Would pub(crate) trait Sealed {} not suffice?

Good question! This is a bit of a hack, because if you have pub (crate) trait Sealed on it's own then you run into:

crate-private trait `Sealed` in public interface can't leak crate-private trait

But having the trait be fully pub (but in a module that people can't access, so the thing is pub but nobody can name it) circumvents this. It's weird, but it's a generally established pattern that crates use for this purpose :)

(I have only skimmed it but maybe eg https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/ is interesting; it also talks about another approach via methods that can't be implemented :))

Oh that is very interesting!

tadeohepperle · 2023-08-21T16:16:12Z

subxt/src/backend/legacy/rpc_methods.rs

+
+    /// Fetch system health
+    pub async fn system_health(&self) -> Result<SystemHealth, Error> {
+        self.client.request("system_health", rpc_params![]).await


I did not know these RPC methods were available, maybe we should add an example, that showcases all the little things we can fetch from a node?

I'm only really leaving them here because I felt a little bad stripping away all of the RPC methods that we don't use immediately, but in the long run we'll ultimately deprecate and remove these things once the new RPCs are stable and in widespread usage. For this reason, I'm happy to not really "publicise" these legacy RPC methods, because the more people rely on them, the more they will be disappointed when they go away (which, to be clear, may still be over a year away :))

jsdw · 2023-08-21T16:47:50Z

From my understanding we will add a newer backend soon as a replacement of the LegacyBackend? If that is the case, what will be the mechanism by which it is determined what backend is used when an OnlineClient is constructed?

At the moment, the "simple" constructors on OnlineClient will pick the backend to instantiate. When the new RPCs are in widespread use and stable, we can switch the backend internally to be the new one.

I've also added from_backend (and from_backend_with) which would allow users to pick the backend of choice.

The Backend trait is only sealed for now because it's still liable to changing more as we add the new stuff and update things, so I guess I want to discourage people from doing too much with it right now and then being disappointed (though they can still directly call the methods of it etc). I expect we'll "unseal" it when things settle a little so that people can build their own backends if they like, and then use OnlineClient::from_backend to instantiate a client with it.

lexnv · 2023-08-21T17:32:08Z

subxt/examples/blocks_subscribing.rs

@@ -24,8 +23,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
        println!("  Extrinsics:");

        // Log each of the extrinsic with it's associated events:
-        let body = block.body().await?;
-        for ext in body.extrinsics().iter() {
+        let extrinsics = block.extrinsics().await?;


Nice simplification for the end user!

If (ever) justification turns to be a thing in the future we could expose it on the same level as extrinsics (ie block.extrinsics())

Yup definitely; if they are a thing then they won't be in the block body I guess anyway (since the body of a block afaik is just the extrinsics). I'm not actually sure where they live offhand; maybe theres a storage or runtime API call to fetch them (I haven't looked!)

lexnv · 2023-08-21T18:04:44Z

subxt/src/tx/tx_client.rs

+        // We copy some of the inner types and put the three states (valid, invalid, unknown) into one enum,
+        // because from our perspective, the call was successful regardless.
+        if bytes[0] == 0 {
+            // ok: valid (more detail is available here, but ).


nit: ok: valid (more detail is available here, but we only care about the Result enum variant).

Hehe I feel like I do this quite often; get distracted and forget to finish comments!

lexnv · 2023-08-21T18:06:52Z

subxt/src/tx/tx_client.rs

+        // https://github.com/paritytech/substrate/blob/0cdf7029017b70b7c83c21a4dc0aa1020e7914f6/primitives/runtime/src/transaction_validity.rs#L210
+        // We copy some of the inner types and put the three states (valid, invalid, unknown) into one enum,
+        // because from our perspective, the call was successful regardless.
+        if bytes[0] == 0 {


nit: In case we somehow don't receive an Result<ValidTransaction, TransactionValidityError> and end-up with some empty bytes, would it be beneficial to do a bytes.get(0).expect("Invalid wire format for ValidationResult")? Or return the Error::Unkown?

Good point; I should handle these cases better :)

lexnv · 2023-08-21T18:37:54Z

testing/integration-tests/src/full_client/client/mod.rs

-    let api = ctx.client();
-
-    // Check subscription with runtime updates set on false.
-    let mut blocks = api.rpc().chainhead_unstable_follow(false).await.unwrap();


I think this is fine since we plan to add them back under a dedicated backend for the Spec V2.

In the meanwhile, I remember this is used by polakadot-interceptor here. However I think, the same could be achieved with the new Subxt API (for reference @AndreiEres ) which will allow seamless transition between chainHead and the legacy RPC.

IIRC this was used to create a metric about the duration time between two reported blocks (either best / finalized), we could work together here to find an appropriate solution with the new API (we might merge the 2 streams for best and finalized)

Thank you for mentioning me! I should've expected it by unstable mark in chainHead call :-)

So I see that I should switch our subscription in introspector to new API, right? Which one, btw?

After we release the next subxt version, merging the Backend::stream_best_block_headers and Backend::stream_finalized_block_headers should produce the same information as the chainHead

Would this call also work (if all you care about is the time between any new block)?

api.blocks().subscribe_all().await

or

api.blocks().subscribe_best().await

If you care about all new "best" blocks (likewise there is a version for finalized blocks only. Hopefully that way you can lean on the higher level stable APIs more :)

lexnv

Amazing work here! 👍

…ytes

jsdw added 19 commits August 11, 2023 17:30

WIP backend trait

d6ed36e

WIP converting higher level stuff to using Backend impl

35ff06c

more implementing new backend trait, mainly storage focused

b80ab16

Get core code compiling with new backend bits

06afff2

subxt crate checks passing

9a7aef0

fix tests

83b99b1

Merge branch 'master' into jsdw-backend-trait

2f6fcc8

cargo fmt

9687428

clippy/fixes

1c98bcb

merging and other fixes

1eb7bf4

fix test

903c8e5

fix lightclient code

75f5cec

Fix some broken doc links

f3a105c

another book link fix

c84f42a

fix broken test when moving default_rpc_client

1f3c493

fix dry_run test

f9d9063

fix more tests; lightclient and wasm

0a655b3

fix wasm tests

9bdc872

fix some doc examples

0e20235

jsdw marked this pull request as ready for review August 18, 2023 18:16

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

jsdw added 9 commits August 21, 2023 10:47

use next() instead of next_item()

9a70430

missing next_item() -> next()s

d965059

move legacy RPc methods to LegacyRpcMethods type to host generic para…

113b6e1

…m instead of RpcClient

standardise on all RpcClient types prefixed with Rpc, and 'raw' trait…

b2f63ec

… types prefixed with RawRpc so it's less ocnfusing which is which

rename fixes

ba56077

doc fixes

7cdd9ba

Add back system_dryRun RPC method and rename tx.dry_run() to tx.valid…

eb91fb4

…ate(), to signal that the calls are different

Merge branch 'master' into jsdw-backend-trait

3892d44

Add a test that we return the correct extrinsic hash from submit()

47c8bd7

tadeohepperle approved these changes Aug 21, 2023

View reviewed changes

lexnv reviewed Aug 21, 2023

View reviewed changes

lexnv approved these changes Aug 21, 2023

View reviewed changes

AndreiEres mentioned this pull request Aug 22, 2023

Switch to the new subxt subscription API since it's going to be merged paritytech/polkadot-introspector#508

Open

jsdw added 3 commits August 22, 2023 11:23

add TransactionValid details back, and protect against out of range b…

695ad9a

…ytes

add test for decoding transaction validation from empty bytes

e2153fa

fix clippy warning

09a1d41

jsdw merged commit d7124b5 into master Aug 22, 2023
9 checks passed

jsdw deleted the jsdw-backend-trait branch August 22, 2023 11:32

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

Introduce Backend trait to allow different RPC (or other) backends to be implemented #1126

Introduce Backend trait to allow different RPC (or other) backends to be implemented #1126

jsdw commented Aug 18, 2023 •

edited

tadeohepperle left a comment

tadeohepperle Aug 21, 2023

jsdw Aug 21, 2023

tadeohepperle Aug 21, 2023

jsdw Aug 21, 2023

tadeohepperle Aug 22, 2023

tadeohepperle Aug 21, 2023

jsdw Aug 21, 2023

jsdw commented Aug 21, 2023

lexnv Aug 21, 2023

jsdw Aug 22, 2023

lexnv Aug 21, 2023 •

edited

jsdw Aug 22, 2023

lexnv Aug 21, 2023 •

edited

jsdw Aug 22, 2023

lexnv Aug 21, 2023

AndreiEres Aug 22, 2023

lexnv Aug 22, 2023

jsdw Aug 22, 2023 •

edited

AndreiEres Aug 22, 2023

lexnv left a comment

Introduce Backend trait to allow different RPC (or other) backends to be implemented #1126

Introduce Backend trait to allow different RPC (or other) backends to be implemented #1126

Conversation

jsdw commented Aug 18, 2023 • edited

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

jsdw commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lexnv Aug 21, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lexnv Aug 21, 2023 • edited

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 Aug 22, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lexnv left a comment

Choose a reason for hiding this comment

jsdw commented Aug 18, 2023 •

edited

lexnv Aug 21, 2023 •

edited

lexnv Aug 21, 2023 •

edited

jsdw Aug 22, 2023 •

edited