Improve template node #956
Conversation
template/node/Cargo.toml
Outdated
| "sc-consensus-manual-seal", | ||
| "frontier-template-runtime/manual-seal", | ||
| ] | ||
| aura = ["frontier-template-runtime/aura"] |
There was a problem hiding this comment.
Why this change? Shouldn't sc-consensu-aura stays disabled when manual seal is enabled?
There was a problem hiding this comment.
the current implementation only has the sealing cli option to judge whether to enable the manual seal mode, the default is aura + grandpa.
I think this improvement can reduce the duplication of code caused by different features in the service
There was a problem hiding this comment.
We could even get rid of the aura and manual-seal features if possible, but this would require adding a value in genesis to be checked in the OnTimestampSet part in the runtime.
There was a problem hiding this comment.
like:
pub struct ConsensusOnTimestampSet<T>(PhantomData<T>);
impl<T: pallet_aura::Config> OnTimestampSet<T::Moment> for ConsensusOnTimestampSet<T> {
fn on_timestamp_set(moment: T::Moment) {
if storage::EnableManualSeal::get() {
return
}
<pallet_aura::Pallet<T> as OnTimestampSet<T::Moment>>::on_timestamp_set(moment)
}
}
impl pallet_timestamp::Config for Runtime {
type Moment = Moment;
type OnTimestampSet = ConsensusOnTimestampSet<Self>;
type MinimumPeriod = MinimumPeriod;
type WeightInfo = pallet_timestamp::weights::SubstrateWeight<Runtime>;
}There was a problem hiding this comment.
Seems like a good idea to me. We need to build both aura and manual-seal in CI anyway and this was sometimes forgotten. This is also just a template so simplicity is more important (than not building unused features).
|
@sorpaas PTAL again |
| @@ -29,8 +29,6 @@ jobs: | |||
| repo-token: ${{ secrets.GITHUB_TOKEN }} | |||
| - name: Run tests | |||
| run: cargo test --locked --verbose --all | |||
| - name: Ensure runtime-benchmarks and try-runtime features compiles | |||
| run: cargo check --release --features=runtime-benchmarks,try-runtime | |||
There was a problem hiding this comment.
Why this? We still need to check those features that aren't on by default.
There was a problem hiding this comment.
We have this check in the lint job
auraandmanual-sealfeaturesauraandmanual-sealfeaturesrpc_binary_search_estimate=>rpc-binary-search-estimate