Replace `NodeRpcClient` with direct `NodeClient` implementation for the client

[dastansam]

closes #27

Notes

SubspaceRpcApiServer

Implementation is mostly borrowed from SubspaceRpcApiServer implementation. I removed deny_unsafe feature, since this implementation is not exposed to outside and modified subscription functions.

MaybeNodeClient

I am still using the old way of instantiating a NodeClient implementation, i.e first default and then inject it once inner value is instantiated, to ensure it is easy to follow the changes. Should we change this, i.e don't wrap it in struct and ArcSwapOption?

Dynamic client type

I spent some time trying to implement your suggestion about dynamic FullClientT trait, however, couldn't manage to do it. The main blocker was the ProvideRuntimeApi which has an associated Api type which should be specified to make the custom trait object safe:

pub trait FullClientT: ProvideRuntimeApi<Block, Api = SomeApi> + HeaderBackend<Block> + ...;

impl<T> FullClientT for T where ProvideRuntimeApi<Block, Api = SomeApi> + HeaderBackend<Block> + ...;

// and this was the main issue
pub struct InnerNodeClient {
    client: Arc<dyn FullClientT>
}

maybe I was missing something, so if you have some suggestion for this, I can tackle it again

[dastansam]

I am mostly done with resolving the comments except this one:

Modify MaybeNodeClient to work with Box or Arc or something similar depending on what works best

do you mean having a dynamic NodeClient like this:

/// Wrapper around `NodeClient`
#[derive(Clone, Default)]
pub(in super::super) struct MaybeNodeClient {
    inner: Arc<dyn NodeClient>,
}

or

/// Inner node client that is injected into wrapper `MaybeNodeClient` once it is fully initialized
pub(in super::super) struct DirectNodeClient {
    client: Box<dyn FullClientT>,

// where `FullClientT` is super trait of all traits needed for a client

The first one fails because NodeClient trait does not have Sized trait constraint, second one I explained in the PR description

[nazar-pc]

I meant the first one, but since it is a MaybeNodeClient, it'll be something like this instead:

#[derive(Debug, Clone, Default)]
pub(in super::super) struct MaybeNodeRpcClient {
    inner: Arc<ArcSwapOption<dyn NodeClient>>,
}

or this, or whatever similar thing works:

#[derive(Debug, Clone, Default)]
pub(in super::super) struct MaybeNodeRpcClient {
    inner: Arc<ArcSwapOption<Box<dyn NodeClient>>>,
}

Basically we need the same logic we had before, just with dyn trait instead of concrete type of the client so you don't have to mess with generics if you don't want to (generics are still an option though).

[dastansam]

ok, finally managed to squash everything into two commits. First commit makes MaybeNodeRpcClient work with dynamic NodeClientExt and updates upstream dependencies. Also adds SubspaceRpc copy which is not compiled but necessary for commit diff. Second commit adds DirectNodeClient and completely removes NodeRpcClient dependency

[nazar-pc]

You shouldn't need NodeClientExt, just NodeClient. NodeClientExt provides a single additional method that can be implemented with SegmentHeadersStore directly where it is used (it is intentionally NodeClientExt for this reason because only CLI farmer will actually need it, not Space Acres).

[dastansam]

some comments in general:

• NodeClientExt is used in create_network() function to fetch the last segment headers. Doesn't this make it necessary?
• I am starting to get a lot more comfortable with squashing and rebasing, but tips for making it easier would help for sure
• sometimes compiler hangs for a while and for some reason if I add a whitespace and remove it, it starts compiling again. so, sometimes those whitespaces accidentally end up in the commit. But I will be careful and run fmt before pushing :)

[nazar-pc]

NodeClientExt is used in create_network() function to fetch the last segment headers. Doesn't this make it necessary?

Yes, the only reason it is there is to provide segment headers, but segment headers will come from SegmentHeadersStore. Not a blocker though, can always be refactored later.

I am starting to get a lot more comfortable with squashing and rebasing, but tips for making it easier would help for sure

JetBrains IDEs have excellent UI for them. Very easy to cherry-pick, squash, rebase, reorder commits, I'll show you.

sometimes compiler hangs for a while and for some reason if I add a whitespace and remove it, it starts compiling again. so, sometimes those whitespaces accidentally end up in the commit. But I will be careful and run fmt before pushing :)

I would recommend configuring IDE to always remove all whitespaces (except trailing new line) on save, so it never ends up being committed. Never had compiler getting stuck though :thinking:

[dastansam]

JetBrains IDEs have excellent UI for them. Very easy to cherry-pick, squash, rebase, reorder commits, I'll show you.

will take a look, but it's quite hard to switch IDEs once you are used to it :)

I would recommend configuring IDE to always remove all whitespaces (except trailing new line) on save, so it never ends up being committed. Never had compiler getting stuck though

I have all of this. Most of the times it does all the fixes and everything is good. But my machine struggles a little bit during the context/branch switches. And compiler getting stuck is probably due to couple of cargo processes running at the same time. but yeah, will be more cautious for sure

[dastansam]

Left a bunch of small comments, but overall makes sense and does what it should, thanks for patience with me 🙂

I force pushed the changes to last comments, didn't want to create new commit for nits. hope that's ok