Clean up unused dependencies from Entropy node #459
Conversation
This uses the `kitchensink_runtime` which we don't want
This copies some code from the Node Template to do this locally. The `node-executor` pulls in the `kitchensink-runtime` which we don't want.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Just quickly checked, this removes removes ~50 crates from the compilation process. Note that the most significant savings come from not having to build the extra runtime (which happens in a single thread, so it's slow). |
We don't actually use this RPC and it's adding extra dependencies.
We don't make use of this and it's only pulling in more dependencies into the build.
This also pulls in another runtime which we don't use and just slows down the compilaton process.
HCastano
changed the title
| } | ||
|
|
||
| fn copyright_start_year() -> i32 { | ||
| 2017 | ||
| 2022 |
There was a problem hiding this comment.
I made this date up, not sure what we should actually be using here. Can somebody tell me? lol
There was a problem hiding this comment.
It looks like this doesn't matter yet, so we'll leave it as 2022 for now
|
@ameba23 I'd appreciate if your "review" were asking a bunch of clarifying questions. This can help both of us:
|
| use super::*; | ||
| use crate::service::{new_full_base, NewFullBase}; | ||
|
|
||
| fn local_testnet_genesis_instant_single() -> RuntimeGenesisConfig { |
There was a problem hiding this comment.
Why don't we need these tests anymore?
There was a problem hiding this comment.
From what I can tell these were more intended for Substrate core to ensure that everything was working correctly. I don't think it's important for us to check that a genesis config is valid through unit tests
| /// Create a transaction using the given `call`. | ||
| /// | ||
| /// Note: Should only be used for benchmarking. | ||
| pub fn create_benchmark_extrinsic( |
There was a problem hiding this comment.
Why does removing these dependencies mean we need this function?
There was a problem hiding this comment.
If you look at some of the helpers for running benchmarks you'll see that we need to call into the runtime to perform some actions.
Previously the create_extrinsic function was being used, and it was calling into the kitchensink-runtime. I've swapped this out for create_benchmark_extrinsic which a) is better named and b) uses the entropy-runtime under the hood
My initial goal with this PR was to remove the
kitchensink-runtimefrom the node. As I went through this I was also able to remove two unused components:
the statement store, and the
substrate-test-runtime.For a bit of background:
The
kitchensink-runtimeis something provided by Substrate as a sort of demo of usingall the available pallets, runtime APIs, etc. We were pulling it in directly and
through the
node-executorcrate. However We don't need to be pulling in all this codewasting compilation cycles on this. Instead we should be able to running benchmarks and
tests against our own runtime.
The Statement store is a way to
store data off-chain. This is also something that we don't use, but since it was included
in the Substrate kitchensink node which is why we've pulled it in.
The
substrate-test-runtimeis used by the service itself to test some networkingfunctionality. We pulled these tests in, but don't really have a need for them. In fact,
we were even
#[ignore]-ing the only test that actually used the runtime. Despite thatwe were pulling this into our normal builds and our test builds.
Overall this PR reduces the number of crates we pull in by ~70 and removes two runtimes
from the compilation process (and since each runtime is compiled in a single threaded way
we save a bit of time with this).
P.S. There's a lot more unused code we can remove from the node, but I'll open up a separate
issue in the future to track this.