-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Switch node template to use AuRa #3790
Switch node template to use AuRa #3790
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small problem with crypto; maybe a look from someone who knows service code better; looks good otherwise 👍
EDIT:
Small problem with crypto; maybe a look from someone who knows service code better(seeing Pierre's commit); looks good otherwise 👍
Co-Authored-By: Kian Paimani <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM. Why remove GRANDPA though? It's unrelated to the BABE issue and I expect some users will start asking why blocks aren't being finalized.
node-template/src/service.rs
Outdated
babe_link.clone(), | ||
babe_block_import, | ||
.with_import_queue_and_fprb(|_config, client, _backend, _fetcher, _select_chain, _tx_pool| { | ||
let finality_proof_request_builder = Box::new(DummyFinalityProofRequestBuilder::default()) as Box<_>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the explicit cast necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing as Box<_>
gives me an error:
Shawns-MacBook-Pro:substrate shawntabrizi$ cargo build -p node-template
Compiling node-template v2.0.0 (/Users/shawntabrizi/Documents/GitHub/substrate/node-template)
error[E0599]: no method named `build` found for type `substrate_service::builder::ServiceBuilder<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>, node_template_runtime::RuntimeApi, C, node_template_runtime::GenesisConfig, std::option::Option<()>, substrate_client::client::Client<substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>, substrate_client::light::call_executor::GenesisCallExecutor<substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>, substrate_client::call_executor::LocalCallExecutor<substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>, substrate_executor::native_executor::NativeExecutor<service::Executor>>>, sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>, node_template_runtime::RuntimeApi>, alloc::sync::Arc<substrate_network::on_demand_layer::OnDemand<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>>, substrate_client::client::LongestChain<substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>, sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_consensus_common::import_queue::basic_queue::BasicQueue<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, std::boxed::Box<substrate_network::config::DummyFinalityProofRequestBuilder>, alloc::sync::Arc<dyn substrate_network::chain::FinalityProofProvider<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>>, service::NodeProtocol, substrate_transaction_graph::pool::Pool<substrate_transaction_pool::api::FullChainApi<substrate_client::client::Client<substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>, substrate_client::light::call_executor::GenesisCallExecutor<substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>, substrate_client::call_executor::LocalCallExecutor<substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>, substrate_executor::native_executor::NativeExecutor<service::Executor>>>, sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>, node_template_runtime::RuntimeApi>, sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>>, (), substrate_service::builder::LightRpcBuilder<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>, node_template_runtime::RuntimeApi, service::Executor>, substrate_client::light::backend::Backend<substrate_client_db::light::LightStorage<sr_primitives::generic::block::Block<sr_primitives::generic::header::Header<u32, sr_primitives::traits::BlakeTwo256>, sr_primitives::OpaqueExtrinsic>>, substrate_primitives::hasher::blake2::Blake2Hasher>>` in the current scope
--> node-template/src/service.rs:135:4
|
135 | .build()
| ^^^^^
error: aborting due to previous error
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
@andresilva TBH I didn't think that hard about including or not including Grandpa. I just thought it too wasn't that stable ATM, so better not to add it... If you think it should be here, then we can add it back, but I will probably need help writing the service file. |
Maybe looking at previous revisions would help? I agree that the we want some finalization in node-template, otherwise people will keep complaining about it. |
@andresilva @kianenigma this is ready for a final review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BlockImport
is how we allow consensus engines to hook into the block import pipeline. The client also implements BlockImport
and it's where actual block import happens (execution, updating db, etc.). Both BABE and GRANDPA need to hook into the import pipeline in order to check for authority change digests, do some bookkeeping on pending changes and properly validate them. The way this works is that we wrap BlockImport
s passing a final composite one to the import queue. When we have both BABE and GRANDPA the import queue sees a BabeBlockImport
, which wraps a GrandpaBlockImport
, which wraps a client::BlockImport
. Aura doesn't need to hook into the import pipeline, so we just need to pass a GrandpaBlockImport
to the import queue and to the aura proposer task.
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
Co-Authored-By: André Silva <[email protected]>
@@ -105,9 +105,9 @@ pub fn new_full<C: Send + Default + 'static>(config: Configuration<C, GenesisCon | |||
|
|||
let aura = aura::start_aura::<_, _, _, _, _, AuraPair, _, _, _>( | |||
aura::SlotDuration::get_or_compute(&*client)?, | |||
client.clone(), | |||
select_chain, | |||
client, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fact that it's so easy to get this wrong means that we should probably not implement BlockImport
on Client
.
Aura is better suited for development. See paritytech/substrate#3790
Fixes #3779
This PR updates the Substrate Node Template to use AuRa consensus rather than Grandpa/BABE.
Right now Substrate chains brick when a chain is offline for more than one epoch. This is not a very friendly developer experience, so we think it will be better to use AuRa for now.
Tested on
--dev
chain and--alice
/--bob
local network.