From 9ebf20ce10cb94185080d1925c479b1ca115957c Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 20 May 2025 12:45:49 +0200 Subject: [PATCH 01/38] check inherents in import_block --- .vim/coc-settings.json | 3 + Cargo.lock | 2 + .../consensus/aura/src/collators/basic.rs | 22 +- .../consensus/aura/src/collators/lookahead.rs | 39 +- .../consensus/aura/src/collators/mod.rs | 215 +++++++- .../src/collators/slot_based/block_import.rs | 81 ++- .../client/consensus/aura/src/import_queue.rs | 2 - cumulus/polkadot-omni-node/lib/Cargo.toml | 1 + .../polkadot-omni-node/lib/src/nodes/aura.rs | 76 ++- cumulus/test/service/Cargo.toml | 1 + cumulus/test/service/src/lib.rs | 49 +- polkadot/node/service/src/builder/partial.rs | 29 +- substrate/bin/node/cli/src/service.rs | 29 +- substrate/client/consensus/aura/Cargo.toml | 1 + .../client/consensus/aura/src/import_queue.rs | 150 +----- substrate/client/consensus/aura/src/lib.rs | 96 +++- substrate/client/consensus/babe/Cargo.toml | 2 +- .../client/consensus/babe/rpc/src/lib.rs | 21 +- substrate/client/consensus/babe/src/lib.rs | 501 ++++++++++-------- substrate/client/consensus/babe/src/tests.rs | 74 ++- .../client/consensus/babe/src/verification.rs | 16 +- .../consensus/common/src/import_queue.rs | 2 +- substrate/client/consensus/slots/src/lib.rs | 2 +- .../client/network/sync/src/strategy/state.rs | 24 +- .../primitives/consensus/common/src/error.rs | 12 + templates/parachain/node/src/service.rs | 6 +- templates/solochain/node/src/service.rs | 1 - 27 files changed, 929 insertions(+), 528 deletions(-) create mode 100644 .vim/coc-settings.json diff --git a/.vim/coc-settings.json b/.vim/coc-settings.json new file mode 100644 index 0000000000000..ca882484fdaaf --- /dev/null +++ b/.vim/coc-settings.json @@ -0,0 +1,3 @@ +{ + "rust-analyzer.checkOnSave": false +} diff --git a/Cargo.lock b/Cargo.lock index 4a5259c05fe62..55e4c1f82c92d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5063,6 +5063,7 @@ dependencies = [ "sp-consensus-aura", "sp-core 28.0.0", "sp-genesis-builder", + "sp-inherents", "sp-io 30.0.0", "sp-keyring", "sp-runtime 31.0.1", @@ -15478,6 +15479,7 @@ dependencies = [ "sc-client-api", "sc-client-db", "sc-consensus", + "sc-consensus-aura", "sc-consensus-manual-seal", "sc-executor 0.32.0", "sc-network", diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index a66abf979d683..18818548e7efe 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -23,6 +23,7 @@ //! //! For more information about AuRa, the Substrate crate should be checked. +use super::ValidatingBlockImport; use codec::{Codec, Decode}; use cumulus_client_collator::{ relay_chain_driven::CollationRequest, service::ServiceInterface as CollatorServiceInterface, @@ -39,8 +40,10 @@ use polkadot_primitives::{CollatorPair, Id as ParaId, ValidationCode}; use futures::{channel::mpsc::Receiver, prelude::*}; use sc_client_api::{backend::AuxStore, BlockBackend, BlockOf}; use sc_consensus::BlockImport; +use sc_consensus_slots::InherentDataProviderExt; use sp_api::{CallApiAt, ProvideRuntimeApi}; use sp_application_crypto::AppPublic; +use sp_block_builder::BlockBuilder; use sp_blockchain::HeaderBackend; use sp_consensus_aura::AuraApi; use sp_core::crypto::Pair; @@ -101,14 +104,15 @@ where + Send + Sync + 'static, - Client::Api: AuraApi + CollectCollationInfo, + Client::Api: BlockBuilder + AuraApi + CollectCollationInfo, RClient: RelayChainInterface + Send + Clone + 'static, - CIDP: CreateInherentDataProviders + Send + 'static, - CIDP::InherentDataProviders: Send, + CIDP: CreateInherentDataProviders + Clone + Send + 'static, + CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, + BI::Error: Into, Proposer: ProposerInterface + Send + Sync + 'static, CS: CollatorServiceInterface + Send + Sync + 'static, - P: Pair, + P: Pair + Sync + Send + 'static, P::Public: AppPublic + Member + Codec, P::Signature: TryFrom> + Member + Codec, { @@ -126,8 +130,14 @@ where let mut collator = { let params = collator_util::Params { - create_inherent_data_providers: params.create_inherent_data_providers, - block_import: params.block_import, + create_inherent_data_providers: params.create_inherent_data_providers.clone(), + block_import: ValidatingBlockImport::<_, _, _, _, P>::new( + params.block_import, + params.para_client.clone(), + params.create_inherent_data_providers.clone(), + Default::default(), + Default::default(), + ), relay_client: params.relay_client.clone(), keystore: params.keystore.clone(), para_id: params.para_id, diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index db7dbb2f65fa1..d95502e4d10bc 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -32,6 +32,7 @@ //! The main limitation is block propagation time - i.e. the new blocks created by an author //! must be propagated to the next author before their turn. +use super::ValidatingBlockImport; use codec::{Codec, Encode}; use cumulus_client_collator::service::ServiceInterface as CollatorServiceInterface; use cumulus_client_consensus_common::{self as consensus_common, ParachainBlockImportMarker}; @@ -46,6 +47,8 @@ use polkadot_overseer::Handle as OverseerHandle; use polkadot_primitives::{ vstaging::DEFAULT_CLAIM_QUEUE_OFFSET, CollatorPair, Id as ParaId, OccupiedCoreAssumption, }; +use sc_consensus_slots::InherentDataProviderExt; +use sp_block_builder::BlockBuilder; use crate::{collator as collator_util, export_pov_to_path}; use futures::prelude::*; @@ -114,17 +117,20 @@ where + Send + Sync + 'static, - Client::Api: - AuraApi + CollectCollationInfo + AuraUnincludedSegmentApi, + Client::Api: BlockBuilder + + AuraApi + + CollectCollationInfo + + AuraUnincludedSegmentApi, Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, - CIDP: CreateInherentDataProviders + 'static, - CIDP::InherentDataProviders: Send, + CIDP: CreateInherentDataProviders + Clone + 'static, + CIDP::InherentDataProviders: InherentDataProviderExt + Send, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, + BI::Error: Into, Proposer: ProposerInterface + Send + Sync + 'static, CS: CollatorServiceInterface + Send + Sync + 'static, CHP: consensus_common::ValidationCodeHashProvider + Send + 'static, - P: Pair, + P: Pair + Send + Sync + 'static, P::Public: AppPublic + Member + Codec, P::Signature: TryFrom> + Member + Codec, { @@ -166,17 +172,20 @@ where + Send + Sync + 'static, - Client::Api: - AuraApi + CollectCollationInfo + AuraUnincludedSegmentApi, + Client::Api: BlockBuilder + + AuraApi + + CollectCollationInfo + + AuraUnincludedSegmentApi, Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, - CIDP: CreateInherentDataProviders + 'static, - CIDP::InherentDataProviders: Send, + CIDP: CreateInherentDataProviders + Clone + 'static, + CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, + BI::Error: Into, Proposer: ProposerInterface + Send + Sync + 'static, CS: CollatorServiceInterface + Send + Sync + 'static, CHP: consensus_common::ValidationCodeHashProvider + Send + 'static, - P: Pair, + P: Pair + Send + Sync + 'static, P::Public: AppPublic + Member + Codec, P::Signature: TryFrom> + Member + Codec, { @@ -205,8 +214,14 @@ where let mut collator = { let params = collator_util::Params { - create_inherent_data_providers: params.create_inherent_data_providers, - block_import: params.block_import, + create_inherent_data_providers: params.create_inherent_data_providers.clone(), + block_import: ValidatingBlockImport::<_, _, _, _, P>::new( + params.block_import, + params.para_client.clone(), + params.create_inherent_data_providers.clone(), + Default::default(), + Default::default(), + ), relay_client: params.relay_client.clone(), keystore: params.keystore.clone(), para_id: params.para_id, diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs index e4f243351be47..1eb5d579fa962 100644 --- a/cumulus/client/consensus/aura/src/collators/mod.rs +++ b/cumulus/client/consensus/aura/src/collators/mod.rs @@ -21,9 +21,14 @@ //! included parachain block, as well as the [`lookahead`] collator, which prospectively //! builds on parachain blocks which have not yet been included in the relay chain. -use crate::collator::SlotClaim; +use std::sync::Arc; + +use crate::{collator::SlotClaim, LOG_TARGET}; +use async_trait::async_trait; use codec::Codec; -use cumulus_client_consensus_common::{self as consensus_common, ParentSearchParams}; +use cumulus_client_consensus_common::{ + self as consensus_common, ParachainBlockImportMarker, ParentSearchParams, +}; use cumulus_primitives_aura::{AuraUnincludedSegmentApi, Slot}; use cumulus_primitives_core::{relay_chain::Hash as ParaHash, BlockT, ClaimQueueOffset}; use cumulus_relay_chain_interface::RelayChainInterface; @@ -33,10 +38,16 @@ use polkadot_primitives::{ CoreIndex, Hash as RelayHash, Id as ParaId, OccupiedCoreAssumption, ValidationCodeHash, DEFAULT_SCHEDULING_LOOKAHEAD, }; -use sc_consensus_aura::{standalone as aura_internal, AuraApi}; -use sp_api::{ApiExt, ProvideRuntimeApi, RuntimeApiInfo}; +use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResult}; +use sc_consensus_aura::{find_pre_digest, standalone as aura_internal, AuraApi, CompatibilityMode}; +use sc_consensus_slots::InherentDataProviderExt; +use sp_api::{ApiExt, Core, ProvideRuntimeApi, RuntimeApiInfo}; +use sp_block_builder::BlockBuilder; +use sp_consensus_aura::inherents::AuraInherentData; use sp_core::Pair; +use sp_inherents::InherentDataProvider; use sp_keystore::KeystorePtr; +use sp_runtime::traits::{Header, NumberFor}; use sp_timestamp::Timestamp; pub mod basic; @@ -270,6 +281,202 @@ where .map(|parent| (included_block, parent)) } +struct ValidatingBlockImport { + inner: BI, + client: Arc, + create_inherent_data_providers: CIDP, + check_for_equivocation: CheckForEquivocation, + compatibility_mode: CompatibilityMode, + _phantom: std::marker::PhantomData

, +} + +impl ValidatingBlockImport { + pub fn new( + inner: BI, + client: Arc, + create_inherent_data_providers: CIDP, + check_for_equivocation: CheckForEquivocation, + compatibility_mode: CompatibilityMode, + ) -> Self { + Self { + inner, + client, + create_inherent_data_providers, + check_for_equivocation, + compatibility_mode, + _phantom: Default::default(), + } + } +} + +#[async_trait] +impl BlockImport + for ValidatingBlockImport, P> +where + Block: BlockT, + BI: BlockImport + Sync, + BI::Error: Into, + Client: ProvideRuntimeApi + sc_client_api::backend::AuxStore + Send + Sync, + Client::Api: Core + BlockBuilder + AuraApi::Public>, + P: Pair + Sync, + P::Public: Codec + std::fmt::Debug, + P::Signature: Codec, + CIDP: sp_inherents::CreateInherentDataProviders + Send, + CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, +{ + type Error = sp_consensus::Error; + + async fn check_block( + &self, + block: BlockCheckParams, + ) -> Result { + self.inner.check_block(block).await.map_err(Into::into) + } + + async fn import_block( + &self, + mut block: BlockImportParams, + ) -> Result { + validate_block_import::<_, _, P, _>( + &mut block, + self.client.as_ref(), + &self.create_inherent_data_providers, + self.check_for_equivocation, + &self.compatibility_mode, + ) + .await?; + self.inner.import_block(block).await.map_err(Into::into) + } +} + +impl ParachainBlockImportMarker + for ValidatingBlockImport +{ +} + +async fn validate_block_import( + params: &mut sc_consensus::BlockImportParams, + client: &Client, + create_inherent_data_providers: &CIDP, + check_for_equivocation: CheckForEquivocation, + compatibility_mode: &CompatibilityMode>, +) -> Result<(), sp_consensus::Error> +where + Block: BlockT, + Client: ProvideRuntimeApi + sc_client_api::backend::AuxStore + Send + Sync, + Client::Api: Core + BlockBuilder + AuraApi::Public>, + P: Pair + Sync, + P::Public: Codec + std::fmt::Debug, + P::Signature: Codec, + CIDP: sp_inherents::CreateInherentDataProviders + Send, + CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, +{ + // Check inherents. + let slot = find_pre_digest::(¶ms.header) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + let parent_hash = *params.header.parent_hash(); + // if the body is passed through, we need to use the runtime + // to check that the internally-set timestamp in the inherents + // actually matches the slot set in the seal. + if let Some(inner_body) = params.body.take() { + let new_block = Block::new(params.header.clone(), inner_body); + if client + .runtime_api() + .has_api_with::, _>(parent_hash, |v| v >= 2) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? + { + let create_inherent_data_providers = create_inherent_data_providers + .create_inherent_data_providers(parent_hash, ()) + .await + .map_err(sp_consensus::Error::Other)?; + let mut inherent_data = create_inherent_data_providers + .create_inherent_data() + .await + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + inherent_data.aura_replace_inherent_data(slot); + let inherent_res = client + .runtime_api() + .check_inherents(parent_hash, new_block.clone(), inherent_data) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + + if !inherent_res.ok() { + for (i, e) in inherent_res.into_errors() { + match create_inherent_data_providers.try_handle_error(&i, &e).await { + Some(res) => res.map_err(|e| sp_consensus::Error::InvalidInherents(e))?, + None => return Err(sp_consensus::Error::InvalidInherentsUnhandled(i)), + } + } + } + } + let (_, inner_body) = new_block.deconstruct(); + params.body = Some(inner_body); + } + + // Check for equivocation. + let authorities = sc_consensus_aura::authorities::<

::Public, _, _>( + client, + parent_hash, + *params.header.number(), + compatibility_mode, + ) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + let create_inherent_data_providers = create_inherent_data_providers + .create_inherent_data_providers(parent_hash, ()) + .await + .map_err(sp_consensus::Error::Other)?; + let slot_now = create_inherent_data_providers.slot(); + let expected_author = sc_consensus_aura::standalone::slot_author::

(slot, &authorities); + let should_equiv_check = check_for_equivocation.check_for_equivocation(); + if let (true, Some(expected)) = (should_equiv_check, expected_author) { + if let Some(equivocation_proof) = sc_consensus_slots::check_equivocation( + client, + // we add one to allow for some small drift. + // FIXME #1019 in the future, alter this queue to allow deferring of + // headers + slot_now + 1, + slot, + ¶ms.header, + expected, + ) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? + { + tracing::info!( + target: LOG_TARGET, + "Slot author is equivocating at slot {} with headers {:?} and {:?}", + slot, + equivocation_proof.first_header.hash(), + equivocation_proof.second_header.hash(), + ); + } + } + + Ok(()) +} + +/// Should we check for equivocation of a block author? +#[derive(Debug, Clone, Copy)] +pub enum CheckForEquivocation { + /// Yes, check for equivocation. + /// + /// This is the default setting for this. + Yes, + /// No, don't check for equivocation. + No, +} + +impl CheckForEquivocation { + /// Should we check for equivocation? + fn check_for_equivocation(self) -> bool { + matches!(self, Self::Yes) + } +} + +impl Default for CheckForEquivocation { + fn default() -> Self { + Self::Yes + } +} + #[cfg(test)] mod tests { use crate::collators::can_build_upon; diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs index 27588c661d533..141ec6e8ec07e 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs @@ -15,14 +15,22 @@ // You should have received a copy of the GNU General Public License // along with Cumulus. If not, see . +use codec::Codec; use futures::{stream::FusedStream, StreamExt}; use sc_consensus::{BlockImport, StateAction}; +use sc_consensus_aura::CompatibilityMode; +use sc_consensus_slots::InherentDataProviderExt; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; use sp_api::{ApiExt, CallApiAt, CallContext, Core, ProvideRuntimeApi, StorageProof}; -use sp_runtime::traits::{Block as BlockT, Header as _}; +use sp_block_builder::BlockBuilder as BlockBuilderApi; +use sp_consensus_aura::AuraApi; +use sp_core::Pair; +use sp_runtime::traits::{Block as BlockT, Header as _, NumberFor}; use sp_trie::proof_size_extension::ProofSizeExt; use std::sync::Arc; +use crate::collators::{validate_block_import, CheckForEquivocation}; + /// Handle for receiving the block and the storage proof from the [`SlotBasedBlockImport`]. /// /// This handle should be passed to [`Params`](super::Params) or can also be dropped if the node is @@ -47,40 +55,84 @@ impl SlotBasedBlockImportHandle { } /// Special block import for the slot based collator. -pub struct SlotBasedBlockImport { +pub struct SlotBasedBlockImport { inner: BI, client: Arc, sender: TracingUnboundedSender<(Block, StorageProof)>, + create_inherent_data_providers: CIDP, + check_for_equivocation: CheckForEquivocation, + compatibility_mode: CompatibilityMode, + _phantom: std::marker::PhantomData

, } -impl SlotBasedBlockImport { +impl SlotBasedBlockImport { /// Create a new instance. /// /// The returned [`SlotBasedBlockImportHandle`] needs to be passed to the /// [`Params`](super::Params), so that this block import instance can communicate with the /// collation task. If the node is not running as a collator, just dropping the handle is fine. - pub fn new(inner: BI, client: Arc) -> (Self, SlotBasedBlockImportHandle) { + pub fn new( + inner: BI, + client: Arc, + create_inherent_data_providers: CIDP, + check_for_equivocation: CheckForEquivocation, + compatibility_mode: CompatibilityMode, + ) -> (Self, SlotBasedBlockImportHandle) { let (sender, receiver) = tracing_unbounded("SlotBasedBlockImportChannel", 1000); - (Self { sender, client, inner }, SlotBasedBlockImportHandle { receiver }) + ( + Self { + sender, + client, + inner, + create_inherent_data_providers, + check_for_equivocation, + compatibility_mode, + _phantom: Default::default(), + }, + SlotBasedBlockImportHandle { receiver }, + ) } } -impl Clone for SlotBasedBlockImport { +impl Clone + for SlotBasedBlockImport +where + CIDP: Clone, + N: Clone, +{ fn clone(&self) -> Self { - Self { inner: self.inner.clone(), client: self.client.clone(), sender: self.sender.clone() } + Self { + inner: self.inner.clone(), + client: self.client.clone(), + sender: self.sender.clone(), + create_inherent_data_providers: self.create_inherent_data_providers.clone(), + check_for_equivocation: self.check_for_equivocation, + compatibility_mode: self.compatibility_mode.clone(), + _phantom: Default::default(), + } } } #[async_trait::async_trait] -impl BlockImport for SlotBasedBlockImport +impl BlockImport + for SlotBasedBlockImport> where Block: BlockT, BI: BlockImport + Send + Sync, BI::Error: Into, - Client: ProvideRuntimeApi + CallApiAt + Send + Sync, + Client: ProvideRuntimeApi + + CallApiAt + + sc_client_api::backend::AuxStore + + Send + + Sync, Client::StateBackend: Send, - Client::Api: Core, + Client::Api: Core + BlockBuilderApi + AuraApi::Public>, + P: Pair + Sync, + P::Public: Codec + std::fmt::Debug, + P::Signature: Codec, + CIDP: sp_inherents::CreateInherentDataProviders + Send, + CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, { type Error = sp_consensus::Error; @@ -101,6 +153,15 @@ where // means that the node produced the block itself or the block was imported via state sync. if !self.sender.is_closed() && !matches!(params.state_action, StateAction::ApplyChanges(_)) { + validate_block_import::<_, _, P, _>( + &mut params, + self.client.as_ref(), + &self.create_inherent_data_providers, + self.check_for_equivocation, + &self.compatibility_mode, + ) + .await?; + let mut runtime_api = self.client.runtime_api(); runtime_api.set_call_context(CallContext::Onchain); diff --git a/cumulus/client/consensus/aura/src/import_queue.rs b/cumulus/client/consensus/aura/src/import_queue.rs index bf2bf32e2de61..c2576b7731c26 100644 --- a/cumulus/client/consensus/aura/src/import_queue.rs +++ b/cumulus/client/consensus/aura/src/import_queue.rs @@ -92,7 +92,6 @@ where create_inherent_data_providers, spawner, registry, - check_for_equivocation: sc_consensus_aura::CheckForEquivocation::No, telemetry, compatibility_mode: CompatibilityMode::None, }) @@ -119,7 +118,6 @@ pub fn build_verifier( client, create_inherent_data_providers, telemetry, - check_for_equivocation: sc_consensus_aura::CheckForEquivocation::No, compatibility_mode: CompatibilityMode::None, }) } diff --git a/cumulus/polkadot-omni-node/lib/Cargo.toml b/cumulus/polkadot-omni-node/lib/Cargo.toml index fb5d006f0d187..bfef3e383d361 100644 --- a/cumulus/polkadot-omni-node/lib/Cargo.toml +++ b/cumulus/polkadot-omni-node/lib/Cargo.toml @@ -48,6 +48,7 @@ sc-cli = { workspace = true, default-features = false } sc-client-api = { workspace = true, default-features = true } sc-client-db = { workspace = true, default-features = true } sc-consensus = { workspace = true, default-features = true } +sc-consensus-aura = { workspace = true, default-features = true } sc-consensus-manual-seal = { workspace = true, default-features = true } sc-executor = { workspace = true, default-features = true } sc-network = { workspace = true, default-features = true } diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 6ad2a880230b3..30b8eb0419fbd 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -60,17 +60,16 @@ use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, BlockImportParams, DefaultImportQueue, }; +use sc_consensus_aura::CreateInherentDataProvidersForAuraViaRuntime; use sc_service::{Configuration, Error, TaskManager}; use sc_telemetry::TelemetryHandle; use sc_transaction_pool::TransactionPoolHandle; use sp_api::ProvideRuntimeApi; -use sp_core::traits::SpawnNamed; +use sp_consensus_aura::AuraApi; +use sp_core::{traits::SpawnNamed, Pair}; use sp_inherents::CreateInherentDataProviders; use sp_keystore::KeystorePtr; -use sp_runtime::{ - app_crypto::AppCrypto, - traits::{Block as BlockT, Header as HeaderT}, -}; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor}; use std::{marker::PhantomData, sync::Arc, time::Duration}; struct Verifier { @@ -122,20 +121,17 @@ where telemetry_handle: Option, task_manager: &TaskManager, ) -> sc_service::error::Result> { - let inherent_data_providers = - move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }; let registry = config.prometheus_registry(); let spawner = task_manager.spawn_essential_handle(); let relay_chain_verifier = Box::new(RelayChainVerifier::new(client.clone(), |_, _| async { Ok(()) })); - let equivocation_aura_verifier = - EquivocationVerifier::<::Pair, _, _, _>::new( - client.clone(), - inherent_data_providers, - telemetry_handle, - ); + let equivocation_aura_verifier = EquivocationVerifier::::new( + client.clone(), + move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }, + telemetry_handle, + ); let verifier = Verifier { client, @@ -217,6 +213,7 @@ where + pallet_transaction_payment_rpc::TransactionPaymentRuntimeApi + substrate_frame_rpc_system::AccountNonceApi, AuraId: AuraIdT + Sync, + AuraId::BoundedPair: Send + Sync, { if extra_args.authoring_policy == AuthoringPolicy::SlotBased { Box::new(AuraNode::< @@ -248,6 +245,7 @@ where RuntimeApi: ConstructNodeRuntimeApi>, RuntimeApi::RuntimeApi: AuraRuntimeApi, AuraId: AuraIdT + Sync, + AuraId::BoundedPair: Send + Sync, { #[docify::export_content] fn launch_slot_based_collator( @@ -259,6 +257,12 @@ where Block, Arc>, ParachainClient, + CreateInherentDataProvidersForAuraViaRuntime< + ParachainClient, + ::Public, + >, + AuraId::BoundedPair, + NumberFor, >, >, CIDP, @@ -278,7 +282,7 @@ where CS: CollatorServiceInterface + Send + Sync + Clone + 'static, Spawner: SpawnNamed, { - slot_based::run::::Pair, _, _, _, _, _, _, _, _, _>( + slot_based::run::( params_with_export, ); } @@ -292,6 +296,12 @@ impl, RuntimeApi, AuraId> Block, Arc>, ParachainClient, + CreateInherentDataProvidersForAuraViaRuntime< + ParachainClient, + ::Public, + >, + AuraId::BoundedPair, + NumberFor, >, SlotBasedBlockImportHandle, > for StartSlotBasedAuraConsensus @@ -299,6 +309,7 @@ where RuntimeApi: ConstructNodeRuntimeApi>, RuntimeApi::RuntimeApi: AuraRuntimeApi, AuraId: AuraIdT + Sync, + AuraId::BoundedPair: Send + Sync, { fn start_consensus( client: Arc>, @@ -308,6 +319,12 @@ where Block, Arc>, ParachainClient, + CreateInherentDataProvidersForAuraViaRuntime< + ParachainClient, + ::Public, + >, + AuraId::BoundedPair, + NumberFor, >, >, prometheus_registry: Option<&Registry>, @@ -343,7 +360,9 @@ where let client_for_aura = client.clone(); let params = SlotBasedParams { - create_inherent_data_providers: move |_, ()| async move { Ok(()) }, + create_inherent_data_providers: CreateInherentDataProvidersForAuraViaRuntime::new( + client.clone(), + ), block_import, para_client: client.clone(), para_backend: backend.clone(), @@ -380,19 +399,33 @@ impl, RuntimeApi, AuraId> InitBlockImport>, RuntimeApi::RuntimeApi: AuraRuntimeApi, + RuntimeApi::BoundedRuntimeApi: AuraApi::Public>, AuraId: AuraIdT + Sync, + AuraId::BoundedPair: Send + Sync, { type BlockImport = SlotBasedBlockImport< Block, Arc>, ParachainClient, + sc_consensus_aura::CreateInherentDataProvidersForAuraViaRuntime< + ParachainClient, + ::Public, + >, + AuraId::BoundedPair, + NumberFor, >; type BlockImportAuxiliaryData = SlotBasedBlockImportHandle; fn init_block_import( client: Arc>, ) -> sc_service::error::Result<(Self::BlockImport, Self::BlockImportAuxiliaryData)> { - Ok(SlotBasedBlockImport::new(client.clone(), client)) + Ok(SlotBasedBlockImport::new( + client.clone(), + client.clone(), + CreateInherentDataProvidersForAuraViaRuntime::new(client), + Default::default(), + Default::default(), + )) } } @@ -431,6 +464,7 @@ where RuntimeApi: ConstructNodeRuntimeApi>, RuntimeApi::RuntimeApi: AuraRuntimeApi, AuraId: AuraIdT + Sync, + AuraId::BoundedPair: Send + Sync + 'static, { fn start_consensus( client: Arc>, @@ -468,7 +502,9 @@ where let params = aura::ParamsWithExport { export_pov: node_extra_args.export_pov, params: AuraParams { - create_inherent_data_providers: move |_, ()| async move { Ok(()) }, + create_inherent_data_providers: CreateInherentDataProvidersForAuraViaRuntime::new( + client.clone(), + ), block_import, para_client: client.clone(), para_backend: backend, @@ -494,10 +530,8 @@ where let fut = async move { wait_for_aura(client).await; - aura::run_with_export::::Pair, _, _, _, _, _, _, _, _>( - params, - ) - .await; + aura::run_with_export::(params) + .await; }; task_manager.spawn_essential_handle().spawn("aura", None, fut); diff --git a/cumulus/test/service/Cargo.toml b/cumulus/test/service/Cargo.toml index 5873f4f7762b0..b4fca72211145 100644 --- a/cumulus/test/service/Cargo.toml +++ b/cumulus/test/service/Cargo.toml @@ -59,6 +59,7 @@ sp-runtime = { workspace = true } sp-state-machine = { workspace = true, default-features = true } sp-timestamp = { workspace = true, default-features = true } sp-tracing = { workspace = true, default-features = true } +sp-inherents = { workspace = true } substrate-test-client = { workspace = true } # Polkadot diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 7d69f3fc8dabe..b77da4aa50ca6 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -35,8 +35,12 @@ use cumulus_client_consensus_aura::{ ImportQueueParams, }; use cumulus_client_consensus_proposer::Proposer; +use cumulus_test_runtime::{Hash, Header, NodeBlock as Block, RuntimeApi}; use prometheus::Registry; use runtime::AccountId; +use sc_consensus_aura::{ + CreateInherentDataProvidersForAura, CreateInherentDataProvidersForAuraViaRuntime, +}; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; use sp_consensus_aura::sr25519::AuthorityPair; use std::{ @@ -66,13 +70,11 @@ use cumulus_relay_chain_minimal_node::{ build_minimal_relay_chain_node_light_client, build_minimal_relay_chain_node_with_rpc, }; -use cumulus_test_runtime::{Hash, Header, NodeBlock as Block, RuntimeApi}; - use frame_system_rpc_runtime_api::AccountNonceApi; use polkadot_node_subsystem::{errors::RecoveryError, messages::AvailabilityRecoveryMessage}; use polkadot_overseer::Handle as OverseerHandle; use polkadot_primitives::{CandidateHash, CollatorPair, Hash as PHash, PersistedValidationData}; -use polkadot_service::{IdentifyNetworkBackend, ProvideRuntimeApi}; +use polkadot_service::{IdentifyNetworkBackend, NumberFor, ProvideRuntimeApi}; use sc_consensus::ImportQueue; use sc_network::{ config::{FullNetworkConfiguration, TransportConfig}, @@ -134,8 +136,18 @@ pub type Client = TFullClient; /// The block-import type being used by the test service. -pub type ParachainBlockImport = - TParachainBlockImport, Client>, Backend>; +pub type ParachainBlockImport = TParachainBlockImport< + Block, + SlotBasedBlockImport< + Block, + Arc, + Client, + CreateInherentDataProvidersForAura, + AuthorityPair, + NumberFor, + >, + Backend, +>; /// Transaction pool type used by the test service pub type TransactionPool = Arc>; @@ -221,8 +233,14 @@ pub fn new_partial( )?; let client = Arc::new(client); - let (block_import, slot_based_handle) = - SlotBasedBlockImport::new(client.clone(), client.clone()); + let slot_duration = sc_consensus_aura::slot_duration(&*client)?; + let (block_import, slot_based_handle) = SlotBasedBlockImport::new( + client.clone(), + client.clone(), + CreateInherentDataProvidersForAura::new(slot_duration), + Default::default(), + Default::default(), + ); let block_import = ParachainBlockImport::new(block_import, backend.clone()); let transaction_pool = Arc::from( @@ -236,22 +254,11 @@ pub fn new_partial( .build(), ); - let slot_duration = sc_consensus_aura::slot_duration(&*client)?; let import_queue = cumulus_client_consensus_aura::import_queue::( ImportQueueParams { block_import: block_import.clone(), client: client.clone(), - create_inherent_data_providers: move |_, ()| async move { - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - - let slot = - sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - slot_duration, - ); - - Ok((slot, timestamp)) - }, + create_inherent_data_providers: CreateInherentDataProvidersForAura::new(slot_duration), spawner: &task_manager.spawn_essential_handle(), registry: None, telemetry: None, @@ -516,7 +523,8 @@ where } else { tracing::info!(target: LOG_TARGET, "Starting block authoring with lookahead collator."); let params = AuraParams { - create_inherent_data_providers: move |_, ()| async move { Ok(()) }, + create_inherent_data_providers: + CreateInherentDataProvidersForAuraViaRuntime::new(client.clone()), block_import, para_client: client.clone(), para_backend: backend.clone(), @@ -568,6 +576,7 @@ pub struct TestNode { } #[allow(missing_docs)] +#[derive(Debug)] pub enum Consensus { /// Use Aura consensus. Aura, diff --git a/polkadot/node/service/src/builder/partial.rs b/polkadot/node/service/src/builder/partial.rs index 0926230bff1db..4f8267835f50c 100644 --- a/polkadot/node/service/src/builder/partial.rs +++ b/polkadot/node/service/src/builder/partial.rs @@ -64,6 +64,8 @@ pub(crate) type PolkadotPartialComponents = sc_service::PartialC FullGrandpaBlockImport, ecdsa_crypto::AuthorityId, >, + sc_consensus_babe::CreateInherentDataProvidersForBabe, + ChainSelection, >, sc_consensus_grandpa::LinkHalf, sc_consensus_babe::BabeLink, @@ -184,32 +186,27 @@ where ); let babe_config = sc_consensus_babe::configuration(&*client)?; - let (block_import, babe_link) = - sc_consensus_babe::block_import(babe_config.clone(), beefy_block_import, client.clone())?; + let slot_duration = babe_config.slot_duration(); + let (block_import, babe_link) = sc_consensus_babe::block_import( + babe_config.clone(), + beefy_block_import, + client.clone(), + sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), + select_chain.clone(), + OffchainTransactionPoolFactory::new(transaction_pool.clone()), + )?; - let slot_duration = babe_link.config().slot_duration(); let (import_queue, babe_worker_handle) = sc_consensus_babe::import_queue(sc_consensus_babe::ImportQueueParams { link: babe_link.clone(), block_import: block_import.clone(), justification_import: Some(Box::new(justification_import)), client: client.clone(), - select_chain: select_chain.clone(), - create_inherent_data_providers: move |_, ()| async move { - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - - let slot = - sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - slot_duration, - ); - - Ok((slot, timestamp)) - }, + create_inherent_data_providers: + sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), telemetry: telemetry.as_ref().map(|x| x.handle()), - offchain_tx_pool_factory: OffchainTransactionPoolFactory::new(transaction_pool.clone()), })?; let justification_stream = grandpa_link.justification_stream(); diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 2207c1047b2ec..7a8f77cd0d85d 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -190,6 +190,8 @@ pub fn new_partial( Block, FullClient, FullBeefyBlockImport, + sc_consensus_babe::CreateInherentDataProvidersForBabe, + FullSelectChain, >, grandpa::LinkHalf, sc_consensus_babe::BabeLink, @@ -259,35 +261,28 @@ pub fn new_partial( config.prometheus_registry().cloned(), ); + let babe_config = sc_consensus_babe::configuration(&*client)?; + let slot_duration = babe_config.slot_duration(); let (block_import, babe_link) = sc_consensus_babe::block_import( - sc_consensus_babe::configuration(&*client)?, + babe_config, beefy_block_import, client.clone(), + sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), + select_chain.clone(), + OffchainTransactionPoolFactory::new(transaction_pool.clone()), )?; - let slot_duration = babe_link.config().slot_duration(); let (import_queue, babe_worker_handle) = sc_consensus_babe::import_queue(sc_consensus_babe::ImportQueueParams { link: babe_link.clone(), block_import: block_import.clone(), justification_import: Some(Box::new(justification_import)), client: client.clone(), - select_chain: select_chain.clone(), - create_inherent_data_providers: move |_, ()| async move { - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - - let slot = - sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - slot_duration, - ); - - Ok((slot, timestamp)) - }, + create_inherent_data_providers: + sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), telemetry: telemetry.as_ref().map(|x| x.handle()), - offchain_tx_pool_factory: OffchainTransactionPoolFactory::new(transaction_pool.clone()), })?; let import_setup = (block_import, grandpa_link, babe_link, beefy_voter_links); @@ -408,6 +403,8 @@ pub fn new_full_base::Hash>>( Block, FullClient, FullBeefyBlockImport, + sc_consensus_babe::CreateInherentDataProvidersForBabe, + FullSelectChain, >, &sc_consensus_babe::BabeLink, ), @@ -923,7 +920,7 @@ mod tests { config, None, false, - |block_import: &sc_consensus_babe::BabeBlockImport, + |block_import: &sc_consensus_babe::BabeBlockImport, babe_link: &sc_consensus_babe::BabeLink| { setup_handles = Some((block_import.clone(), babe_link.clone())); }, diff --git a/substrate/client/consensus/aura/Cargo.toml b/substrate/client/consensus/aura/Cargo.toml index 6af6736171182..2abc1a6d81526 100644 --- a/substrate/client/consensus/aura/Cargo.toml +++ b/substrate/client/consensus/aura/Cargo.toml @@ -37,6 +37,7 @@ sp-core = { workspace = true, default-features = true } sp-inherents = { workspace = true, default-features = true } sp-keystore = { workspace = true, default-features = true } sp-runtime = { workspace = true, default-features = true } +sp-timestamp = { workspace = true, default-features = true } thiserror = { workspace = true } [dev-dependencies] diff --git a/substrate/client/consensus/aura/src/import_queue.rs b/substrate/client/consensus/aura/src/import_queue.rs index 79f4faa5ebf97..3dc157f7eb835 100644 --- a/substrate/client/consensus/aura/src/import_queue.rs +++ b/substrate/client/consensus/aura/src/import_queue.rs @@ -23,23 +23,23 @@ use crate::{ LOG_TARGET, }; use codec::Codec; -use log::{debug, info, trace}; +use log::{debug, trace}; use prometheus_endpoint::Registry; use sc_client_api::{backend::AuxStore, BlockOf, UsageProvider}; use sc_consensus::{ block_import::{BlockImport, BlockImportParams, ForkChoiceStrategy}, import_queue::{BasicQueue, BoxJustificationImport, DefaultImportQueue, Verifier}, }; -use sc_consensus_slots::{check_equivocation, CheckedHeader, InherentDataProviderExt}; +use sc_consensus_slots::{CheckedHeader, InherentDataProviderExt}; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE}; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_block_builder::BlockBuilder as BlockBuilderApi; use sp_blockchain::HeaderBackend; use sp_consensus::Error as ConsensusError; -use sp_consensus_aura::{inherents::AuraInherentData, AuraApi}; +use sp_consensus_aura::AuraApi; use sp_consensus_slots::Slot; use sp_core::crypto::Pair; -use sp_inherents::{CreateInherentDataProviders, InherentDataProvider as _}; +use sp_inherents::CreateInherentDataProviders; use sp_runtime::{ traits::{Block as BlockT, Header, NumberFor}, DigestItem, @@ -52,12 +52,10 @@ use std::{fmt::Debug, marker::PhantomData, sync::Arc}; /// /// This digest item will always return `Some` when used with `as_aura_seal`. fn check_header( - client: &C, slot_now: Slot, header: B::Header, hash: B::Hash, authorities: &[AuthorityId

], - check_for_equivocation: CheckForEquivocation, ) -> Result, Error> where P::Public: Codec, @@ -68,26 +66,7 @@ where crate::standalone::check_header_slot_and_seal::(slot_now, header, authorities); match check_result { - Ok((header, slot, seal)) => { - let expected_author = crate::standalone::slot_author::

(slot, &authorities); - let should_equiv_check = check_for_equivocation.check_for_equivocation(); - if let (true, Some(expected)) = (should_equiv_check, expected_author) { - if let Some(equivocation_proof) = - check_equivocation(client, slot_now, slot, &header, expected) - .map_err(Error::Client)? - { - info!( - target: LOG_TARGET, - "Slot author is equivocating at slot {} with headers {:?} and {:?}", - slot, - equivocation_proof.first_header.hash(), - equivocation_proof.second_header.hash(), - ); - } - } - - Ok(CheckedHeader::Checked(header, (slot, seal))) - }, + Ok((header, slot, seal)) => Ok(CheckedHeader::Checked(header, (slot, seal))), Err(SealVerificationError::Deferred(header, slot)) => Ok(CheckedHeader::Deferred(header, slot)), Err(SealVerificationError::Unsealed) => Err(Error::HeaderUnsealed(hash)), @@ -102,7 +81,6 @@ where pub struct AuraVerifier { client: Arc, create_inherent_data_providers: CIDP, - check_for_equivocation: CheckForEquivocation, telemetry: Option, compatibility_mode: CompatibilityMode, _phantom: PhantomData P>, @@ -112,14 +90,12 @@ impl AuraVerifier { pub(crate) fn new( client: Arc, create_inherent_data_providers: CIDP, - check_for_equivocation: CheckForEquivocation, telemetry: Option, compatibility_mode: CompatibilityMode, ) -> Self { Self { client, create_inherent_data_providers, - check_for_equivocation, telemetry, compatibility_mode, _phantom: PhantomData, @@ -127,41 +103,6 @@ impl AuraVerifier { } } -impl AuraVerifier -where - CIDP: Send, -{ - async fn check_inherents( - &self, - block: B, - at_hash: B::Hash, - inherent_data: sp_inherents::InherentData, - create_inherent_data_providers: CIDP::InherentDataProviders, - ) -> Result<(), Error> - where - C: ProvideRuntimeApi, - C::Api: BlockBuilderApi, - CIDP: CreateInherentDataProviders, - { - let inherent_res = self - .client - .runtime_api() - .check_inherents(at_hash, block, inherent_data) - .map_err(|e| Error::Client(e.into()))?; - - if !inherent_res.ok() { - for (i, e) in inherent_res.into_errors() { - match create_inherent_data_providers.try_handle_error(&i, &e).await { - Some(res) => res.map_err(Error::Inherent)?, - None => return Err(Error::UnknownInherentError(i)), - } - } - } - - Ok(()) - } -} - #[async_trait::async_trait] impl Verifier for AuraVerifier> where @@ -205,57 +146,16 @@ where .await .map_err(|e| Error::::Client(sp_blockchain::Error::Application(e)))?; - let mut inherent_data = create_inherent_data_providers - .create_inherent_data() - .await - .map_err(Error::::Inherent)?; - let slot_now = create_inherent_data_providers.slot(); // we add one to allow for some small drift. // FIXME #1019 in the future, alter this queue to allow deferring of // headers - let checked_header = check_header::( - &self.client, - slot_now + 1, - block.header, - hash, - &authorities[..], - self.check_for_equivocation, - ) - .map_err(|e| e.to_string())?; + let checked_header = + check_header::(slot_now + 1, block.header, hash, &authorities[..]) + .map_err(|e| e.to_string())?; match checked_header { - CheckedHeader::Checked(pre_header, (slot, seal)) => { - // if the body is passed through, we need to use the runtime - // to check that the internally-set timestamp in the inherents - // actually matches the slot set in the seal. - if let Some(inner_body) = block.body.take() { - let new_block = B::new(pre_header.clone(), inner_body); - - inherent_data.aura_replace_inherent_data(slot); - - // skip the inherents verification if the runtime API is old or not expected to - // exist. - if self - .client - .runtime_api() - .has_api_with::, _>(parent_hash, |v| v >= 2) - .map_err(|e| e.to_string())? - { - self.check_inherents( - new_block.clone(), - parent_hash, - inherent_data, - create_inherent_data_providers, - ) - .await - .map_err(|e| e.to_string())?; - } - - let (_, inner_body) = new_block.deconstruct(); - block.body = Some(inner_body); - } - + CheckedHeader::Checked(pre_header, (_, seal)) => { trace!(target: LOG_TARGET, "Checked {:?}; importing.", pre_header); telemetry!( self.telemetry; @@ -287,30 +187,6 @@ where } } -/// Should we check for equivocation of a block author? -#[derive(Debug, Clone, Copy)] -pub enum CheckForEquivocation { - /// Yes, check for equivocation. - /// - /// This is the default setting for this. - Yes, - /// No, don't check for equivocation. - No, -} - -impl CheckForEquivocation { - /// Should we check for equivocation? - fn check_for_equivocation(self) -> bool { - matches!(self, Self::Yes) - } -} - -impl Default for CheckForEquivocation { - fn default() -> Self { - Self::Yes - } -} - /// Parameters of [`import_queue`]. pub struct ImportQueueParams<'a, Block: BlockT, I, C, S, CIDP> { /// The block import to use. @@ -325,8 +201,6 @@ pub struct ImportQueueParams<'a, Block: BlockT, I, C, S, CIDP> { pub spawner: &'a S, /// The prometheus registry. pub registry: Option<&'a Registry>, - /// Should we check for equivocation? - pub check_for_equivocation: CheckForEquivocation, /// Telemetry instance used to report telemetry metrics. pub telemetry: Option, /// Compatibility mode that should be used. @@ -344,7 +218,6 @@ pub fn import_queue( create_inherent_data_providers, spawner, registry, - check_for_equivocation, telemetry, compatibility_mode, }: ImportQueueParams, @@ -371,7 +244,6 @@ where let verifier = build_verifier::(BuildVerifierParams { client, create_inherent_data_providers, - check_for_equivocation, telemetry, compatibility_mode, }); @@ -385,8 +257,6 @@ pub struct BuildVerifierParams { pub client: Arc, /// Something that can create the inherent data providers. pub create_inherent_data_providers: CIDP, - /// Should we check for equivocation? - pub check_for_equivocation: CheckForEquivocation, /// Telemetry instance used to report telemetry metrics. pub telemetry: Option, /// Compatibility mode that should be used. @@ -400,7 +270,6 @@ pub fn build_verifier( BuildVerifierParams { client, create_inherent_data_providers, - check_for_equivocation, telemetry, compatibility_mode, }: BuildVerifierParams, @@ -408,7 +277,6 @@ pub fn build_verifier( AuraVerifier::<_, P, _, _>::new( client, create_inherent_data_providers, - check_for_equivocation, telemetry, compatibility_mode, ) diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 2d6264a489295..f9079b4b5c521 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -57,8 +57,7 @@ pub mod standalone; pub use crate::standalone::{find_pre_digest, slot_duration}; pub use import_queue::{ - build_verifier, import_queue, AuraVerifier, BuildVerifierParams, CheckForEquivocation, - ImportQueueParams, + build_verifier, import_queue, AuraVerifier, BuildVerifierParams, ImportQueueParams, }; pub use sc_consensus_slots::SlotProportion; pub use sp_consensus::SyncOracle; @@ -504,7 +503,8 @@ impl From for Error { } } -fn authorities( +#[doc(hidden)] +pub fn authorities( client: &C, parent_hash: B::Hash, context_block_number: NumberFor, @@ -544,6 +544,96 @@ where .ok_or(ConsensusError::InvalidAuthoritiesSet) } +/// Create inherent data providers for AURA. +#[derive(Debug, Clone)] +pub struct CreateInherentDataProvidersForAura { + slot_duration: sp_consensus_aura::SlotDuration, +} + +impl CreateInherentDataProvidersForAura { + /// Create a new instance of `CreateInherentDataProvidersForAura`. + pub fn new(slot_duration: sp_consensus_aura::SlotDuration) -> Self { + Self { slot_duration } + } +} + +#[async_trait::async_trait] +impl + sp_inherents::CreateInherentDataProviders + for CreateInherentDataProvidersForAura +{ + type InherentDataProviders = + (sp_consensus_aura::inherents::InherentDataProvider, sp_timestamp::InherentDataProvider); + + /// Create the inherent data providers at the given `parent` block using the given `extra_args`. + async fn create_inherent_data_providers( + &self, + _parent: Block::Hash, + _extra_args: ExtraArgs, + ) -> Result> { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + self.slot_duration, + ); + Ok((slot, timestamp)) + } +} + +// TODO Question: is this needed? Or can I just use +// let slot_duration = sc_consensus_aura::slot_duration(&*client)?; +// let cidp = CreateInherentDataProvidersForAura::new(slot_duration); +// Like I do in the cumulus test service? +/// Create inherent data providers for AURA, using the runtime API to fetch slot durations. +#[derive(Debug)] +pub struct CreateInherentDataProvidersForAuraViaRuntime { + client: Arc, + _phantom: PhantomData, +} + +impl CreateInherentDataProvidersForAuraViaRuntime { + /// Create a new instance of `CreateInherentDataProvidersForAura`. + pub fn new(client: Arc) -> Self { + Self { client, _phantom: Default::default() } + } +} + +impl Clone for CreateInherentDataProvidersForAuraViaRuntime { + fn clone(&self) -> Self { + Self { client: self.client.clone(), _phantom: Default::default() } + } +} + +#[async_trait::async_trait] +impl + sp_inherents::CreateInherentDataProviders + for CreateInherentDataProvidersForAuraViaRuntime +where + A: Codec + Send + Sync, + C: ProvideRuntimeApi + Send + Sync, + C::Api: AuraApi, +{ + type InherentDataProviders = + (sp_consensus_aura::inherents::InherentDataProvider, sp_timestamp::InherentDataProvider); + + /// Create the inherent data providers at the given `parent` block using the given `extra_args`. + async fn create_inherent_data_providers( + &self, + parent: Block::Hash, + _extra_args: ExtraArgs, + ) -> Result> { + let slot_duration = crate::standalone::slot_duration_at(self.client.as_ref(), parent)?; + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + Ok((slot, timestamp)) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/substrate/client/consensus/babe/Cargo.toml b/substrate/client/consensus/babe/Cargo.toml index 305409b80c787..3b5b1ad2377aa 100644 --- a/substrate/client/consensus/babe/Cargo.toml +++ b/substrate/client/consensus/babe/Cargo.toml @@ -32,6 +32,7 @@ sc-consensus = { workspace = true, default-features = true } sc-consensus-epochs = { workspace = true, default-features = true } sc-consensus-slots = { workspace = true, default-features = true } sc-telemetry = { workspace = true, default-features = true } +sp-timestamp = { workspace = true, default-features = true } sc-transaction-pool-api = { workspace = true, default-features = true } sp-api = { workspace = true, default-features = true } sp-application-crypto = { workspace = true, default-features = true } @@ -51,7 +52,6 @@ thiserror = { workspace = true } sc-block-builder = { workspace = true, default-features = true } sc-network-test = { workspace = true } sp-keyring = { workspace = true, default-features = true } -sp-timestamp = { workspace = true, default-features = true } sp-tracing = { workspace = true, default-features = true } substrate-test-runtime-client = { workspace = true } tokio = { workspace = true, default-features = true } diff --git a/substrate/client/consensus/babe/rpc/src/lib.rs b/substrate/client/consensus/babe/rpc/src/lib.rs index 338d71a432565..4839566609a53 100644 --- a/substrate/client/consensus/babe/rpc/src/lib.rs +++ b/substrate/client/consensus/babe/rpc/src/lib.rs @@ -224,16 +224,26 @@ mod tests { let config = sc_consensus_babe::configuration(&*client).expect("config available"); let slot_duration = config.slot_duration(); - let (block_import, link) = - sc_consensus_babe::block_import(config.clone(), client.clone(), client.clone()) - .expect("can initialize block-import"); + let (block_import, link) = sc_consensus_babe::block_import( + config.clone(), + client.clone(), + client.clone(), + move |_, _| async move { + Ok((InherentDataProvider::from_timestamp_and_slot_duration( + 0.into(), + slot_duration, + ),)) + }, + longest_chain.clone(), + OffchainTransactionPoolFactory::new(RejectAllTxPool::default()), + ) + .expect("can initialize block-import"); let (_, babe_worker_handle) = sc_consensus_babe::import_queue(ImportQueueParams { link: link.clone(), block_import: block_import.clone(), justification_import: None, client: client.clone(), - select_chain: longest_chain.clone(), create_inherent_data_providers: move |_, _| async move { Ok((InherentDataProvider::from_timestamp_and_slot_duration( 0.into(), @@ -243,9 +253,6 @@ mod tests { spawner: &task_executor, registry: None, telemetry: None, - offchain_tx_pool_factory: OffchainTransactionPoolFactory::new( - RejectAllTxPool::default(), - ), }) .unwrap(); diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 4cf66302ec858..bdf409f2004ef 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -332,15 +332,6 @@ pub enum Error { /// Parent block has no associated weight #[error("Parent block of {0} has no associated weight")] ParentBlockNoAssociatedWeight(B::Hash), - /// Check inherents error - #[error("Checking inherents failed: {0}")] - CheckInherents(sp_inherents::Error), - /// Unhandled check inherents error - #[error("Checking inherents unhandled error: {}", String::from_utf8_lossy(.0))] - CheckInherentsUnhandled(sp_inherents::InherentIdentifier), - /// Create inherents error. - #[error("Creating inherents failed: {0}")] - CreateInherents(sp_inherents::Error), /// Background worker is not running and therefore requests cannot be answered. #[error("Background worker is not running")] BackgroundWorkerTerminated, @@ -978,142 +969,16 @@ impl BabeLink { } /// A verifier for Babe blocks. -pub struct BabeVerifier { +pub struct BabeVerifier { client: Arc, - select_chain: SelectChain, create_inherent_data_providers: CIDP, config: BabeConfiguration, epoch_changes: SharedEpochChanges, telemetry: Option, - offchain_tx_pool_factory: OffchainTransactionPoolFactory, -} - -impl BabeVerifier -where - Block: BlockT, - Client: AuxStore + HeaderBackend + HeaderMetadata + ProvideRuntimeApi, - Client::Api: BlockBuilderApi + BabeApi, - SelectChain: sp_consensus::SelectChain, - CIDP: CreateInherentDataProviders, -{ - async fn check_inherents( - &self, - block: Block, - at_hash: Block::Hash, - inherent_data: InherentData, - create_inherent_data_providers: CIDP::InherentDataProviders, - ) -> Result<(), Error> { - let inherent_res = self - .client - .runtime_api() - .check_inherents(at_hash, block, inherent_data) - .map_err(Error::RuntimeApi)?; - - if !inherent_res.ok() { - for (i, e) in inherent_res.into_errors() { - match create_inherent_data_providers.try_handle_error(&i, &e).await { - Some(res) => res.map_err(|e| Error::CheckInherents(e))?, - None => return Err(Error::CheckInherentsUnhandled(i)), - } - } - } - - Ok(()) - } - - async fn check_and_report_equivocation( - &self, - slot_now: Slot, - slot: Slot, - header: &Block::Header, - author: &AuthorityId, - origin: &BlockOrigin, - ) -> Result<(), Error> { - // don't report any equivocations during initial sync - // as they are most likely stale. - if *origin == BlockOrigin::NetworkInitialSync { - return Ok(()) - } - - // check if authorship of this header is an equivocation and return a proof if so. - let equivocation_proof = - match check_equivocation(&*self.client, slot_now, slot, header, author) - .map_err(Error::Client)? - { - Some(proof) => proof, - None => return Ok(()), - }; - - info!( - "Slot author {:?} is equivocating at slot {} with headers {:?} and {:?}", - author, - slot, - equivocation_proof.first_header.hash(), - equivocation_proof.second_header.hash(), - ); - - // get the best block on which we will build and send the equivocation report. - let best_hash = self - .select_chain - .best_chain() - .await - .map(|h| h.hash()) - .map_err(|e| Error::Client(e.into()))?; - - // generate a key ownership proof. we start by trying to generate the - // key ownership proof at the parent of the equivocating header, this - // will make sure that proof generation is successful since it happens - // during the on-going session (i.e. session keys are available in the - // state to be able to generate the proof). this might fail if the - // equivocation happens on the first block of the session, in which case - // its parent would be on the previous session. if generation on the - // parent header fails we try with best block as well. - let generate_key_owner_proof = |at_hash: Block::Hash| { - self.client - .runtime_api() - .generate_key_ownership_proof(at_hash, slot, equivocation_proof.offender.clone()) - .map_err(Error::RuntimeApi) - }; - - let parent_hash = *header.parent_hash(); - let key_owner_proof = match generate_key_owner_proof(parent_hash)? { - Some(proof) => proof, - None => match generate_key_owner_proof(best_hash)? { - Some(proof) => proof, - None => { - debug!( - target: LOG_TARGET, - "Equivocation offender is not part of the authority set." - ); - return Ok(()) - }, - }, - }; - - // submit equivocation report at best block. - let mut runtime_api = self.client.runtime_api(); - - // Register the offchain tx pool to be able to use it from the runtime. - runtime_api - .register_extension(self.offchain_tx_pool_factory.offchain_transaction_pool(best_hash)); - - runtime_api - .submit_report_equivocation_unsigned_extrinsic( - best_hash, - equivocation_proof, - key_owner_proof, - ) - .map_err(Error::RuntimeApi)?; - - info!(target: LOG_TARGET, "Submitted equivocation report for author {:?}", author); - - Ok(()) - } } #[async_trait::async_trait] -impl Verifier - for BabeVerifier +impl Verifier for BabeVerifier where Block: BlockT, Client: HeaderMetadata @@ -1123,7 +988,6 @@ where + Sync + AuxStore, Client::Api: BlockBuilderApi + BabeApi, - SelectChain: sp_consensus::SelectChain, CIDP: CreateInherentDataProviders + Send + Sync, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, { @@ -1208,56 +1072,6 @@ where match check_header { CheckedHeader::Checked(pre_header, verified_info) => { - let babe_pre_digest = verified_info - .pre_digest - .as_babe_pre_digest() - .expect("check_header always returns a pre-digest digest item; qed"); - let slot = babe_pre_digest.slot(); - - // the header is valid but let's check if there was something else already - // proposed at the same slot by the given author. if there was, we will - // report the equivocation to the runtime. - if let Err(err) = self - .check_and_report_equivocation( - slot_now, - slot, - &block.header, - &verified_info.author, - &block.origin, - ) - .await - { - warn!( - target: LOG_TARGET, - "Error checking/reporting BABE equivocation: {}", err - ); - } - - if let Some(inner_body) = block.body { - let new_block = Block::new(pre_header.clone(), inner_body); - if !block.state_action.skip_execution_checks() { - // if the body is passed through and the block was executed, - // we need to use the runtime to check that the internally-set - // timestamp in the inherents actually matches the slot set in the seal. - let mut inherent_data = create_inherent_data_providers - .create_inherent_data() - .await - .map_err(Error::::CreateInherents)?; - inherent_data.babe_replace_inherent_data(slot); - - self.check_inherents( - new_block.clone(), - parent_hash, - inherent_data, - create_inherent_data_providers, - ) - .await?; - } - - let (_, inner_body) = new_block.deconstruct(); - block.body = Some(inner_body); - } - trace!(target: LOG_TARGET, "Checked {:?}; importing.", pre_header); telemetry!( self.telemetry; @@ -1298,36 +1112,62 @@ where /// it is missing. /// /// The epoch change tree should be pruned as blocks are finalized. -pub struct BabeBlockImport { +pub struct BabeBlockImport { inner: I, client: Arc, epoch_changes: SharedEpochChanges, + create_inherent_data_providers: CIDP, config: BabeConfiguration, + // A [`SelectChain`] implementation. + // + // Used to determine the best block that should be used as basis when sending an equivocation + // report. + select_chain: SC, + // The offchain transaction pool factory. + // + // Will be used when sending equivocation reports. + offchain_tx_pool_factory: OffchainTransactionPoolFactory, } -impl Clone for BabeBlockImport { +impl Clone + for BabeBlockImport +{ fn clone(&self) -> Self { BabeBlockImport { inner: self.inner.clone(), client: self.client.clone(), epoch_changes: self.epoch_changes.clone(), config: self.config.clone(), + create_inherent_data_providers: self.create_inherent_data_providers.clone(), + select_chain: self.select_chain.clone(), + offchain_tx_pool_factory: self.offchain_tx_pool_factory.clone(), } } } -impl BabeBlockImport { +impl BabeBlockImport { fn new( client: Arc, epoch_changes: SharedEpochChanges, block_import: I, config: BabeConfiguration, + create_inherent_data_providers: CIDP, + select_chain: SC, + offchain_tx_pool_factory: OffchainTransactionPoolFactory, ) -> Self { - BabeBlockImport { client, inner: block_import, epoch_changes, config } + BabeBlockImport { + client, + inner: block_import, + epoch_changes, + config, + create_inherent_data_providers, + select_chain, + offchain_tx_pool_factory, + } } } -impl BabeBlockImport +impl BabeBlockImport where Block: BlockT, Inner: BlockImport + Send + Sync, @@ -1338,7 +1178,9 @@ where + ProvideRuntimeApi + Send + Sync, - Client::Api: BabeApi + ApiExt, + Client::Api: BlockBuilderApi + BabeApi + ApiExt, + CIDP: CreateInherentDataProviders, + SC: sp_consensus::SelectChain + 'static, { /// Import whole state after warp sync. // This function makes multiple transactions to the DB. If one of them fails we may @@ -1388,10 +1230,125 @@ where Ok(ImportResult::Imported(aux)) } + + async fn check_inherents( + &self, + block: Block, + at_hash: Block::Hash, + inherent_data: InherentData, + create_inherent_data_providers: CIDP::InherentDataProviders, + ) -> Result<(), ConsensusError> { + let inherent_res = self + .client + .runtime_api() + .check_inherents(at_hash, block, inherent_data) + .map_err(|e| ConsensusError::Other(Box::new(e)))?; + + if !inherent_res.ok() { + for (i, e) in inherent_res.into_errors() { + match create_inherent_data_providers.try_handle_error(&i, &e).await { + Some(res) => res.map_err(|e| ConsensusError::InvalidInherents(e))?, + None => return Err(ConsensusError::InvalidInherentsUnhandled(i)), + } + } + } + + Ok(()) + } + + async fn check_and_report_equivocation( + &self, + slot_now: Slot, + slot: Slot, + header: &Block::Header, + author: &AuthorityId, + origin: &BlockOrigin, + ) -> Result<(), Error> { + // don't report any equivocations during initial sync + // as they are most likely stale. + if *origin == BlockOrigin::NetworkInitialSync { + return Ok(()) + } + + // check if authorship of this header is an equivocation and return a proof if so. + let equivocation_proof = + match check_equivocation(&*self.client, slot_now, slot, header, author) + .map_err(Error::Client)? + { + Some(proof) => proof, + None => return Ok(()), + }; + + info!( + "Slot author {:?} is equivocating at slot {} with headers {:?} and {:?}", + author, + slot, + equivocation_proof.first_header.hash(), + equivocation_proof.second_header.hash(), + ); + + // get the best block on which we will build and send the equivocation report. + let best_hash = self + .select_chain + .best_chain() + .await + .map(|h| h.hash()) + .map_err(|e| Error::Client(e.into()))?; + + // generate a key ownership proof. we start by trying to generate the + // key ownership proof at the parent of the equivocating header, this + // will make sure that proof generation is successful since it happens + // during the on-going session (i.e. session keys are available in the + // state to be able to generate the proof). this might fail if the + // equivocation happens on the first block of the session, in which case + // its parent would be on the previous session. if generation on the + // parent header fails we try with best block as well. + let generate_key_owner_proof = |at_hash: Block::Hash| { + self.client + .runtime_api() + .generate_key_ownership_proof(at_hash, slot, equivocation_proof.offender.clone()) + .map_err(Error::RuntimeApi) + }; + + let parent_hash = *header.parent_hash(); + let key_owner_proof = match generate_key_owner_proof(parent_hash)? { + Some(proof) => proof, + None => match generate_key_owner_proof(best_hash)? { + Some(proof) => proof, + None => { + debug!( + target: LOG_TARGET, + "Equivocation offender is not part of the authority set." + ); + return Ok(()) + }, + }, + }; + + // submit equivocation report at best block. + let mut runtime_api = self.client.runtime_api(); + + // Register the offchain tx pool to be able to use it from the runtime. + runtime_api + .register_extension(self.offchain_tx_pool_factory.offchain_transaction_pool(best_hash)); + + runtime_api + .submit_report_equivocation_unsigned_extrinsic( + best_hash, + equivocation_proof, + key_owner_proof, + ) + .map_err(Error::RuntimeApi)?; + + info!(target: LOG_TARGET, "Submitted equivocation report for author {:?}", author); + + Ok(()) + } } #[async_trait::async_trait] -impl BlockImport for BabeBlockImport +impl BlockImport + for BabeBlockImport where Block: BlockT, Inner: BlockImport + Send + Sync, @@ -1402,7 +1359,10 @@ where + ProvideRuntimeApi + Send + Sync, - Client::Api: BabeApi + ApiExt, + Client::Api: BlockBuilderApi + BabeApi + ApiExt, + CIDP: CreateInherentDataProviders, + CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, + SC: SelectChain + 'static, { type Error = ConsensusError; @@ -1413,6 +1373,82 @@ where let hash = block.post_hash(); let number = *block.header.number(); let info = self.client.info(); + let parent_hash = *block.header.parent_hash(); + + let create_inherent_data_providers = self + .create_inherent_data_providers + .create_inherent_data_providers(parent_hash, ()) + .await?; + + let slot_now = create_inherent_data_providers.slot(); + + let babe_pre_digest = find_pre_digest::(&block.header) + .map_err(|e| ConsensusError::Other(Box::new(e)))?; + let slot = babe_pre_digest.slot(); + + // Check inherents. + if let Some(inner_body) = block.body { + let new_block = Block::new(block.header.clone(), inner_body); + if !block.state_action.skip_execution_checks() { + // if the body is passed through and the block was executed, + // we need to use the runtime to check that the internally-set + // timestamp in the inherents actually matches the slot set in the seal. + let create_inherent_data_providers = self + .create_inherent_data_providers + .create_inherent_data_providers(parent_hash, ()) + .await + .map_err(Self::Error::Other)?; + let mut inherent_data = create_inherent_data_providers + .create_inherent_data() + .await + .map_err(|e| ConsensusError::Other(Box::new(e)))?; + inherent_data.babe_replace_inherent_data(slot); + self.check_inherents( + new_block.clone(), + parent_hash, + inherent_data, + create_inherent_data_providers, + ) + .await?; + } + let (_, inner_body) = new_block.deconstruct(); + block.body = Some(inner_body); + } + + // Check for equivocation and report it to the runtime if needed. + let author = { + let parent_header_metadata = self + .client + .header_metadata(parent_hash) + .map_err(|e| ConsensusError::Other(Box::new(e)))?; + let epoch_changes = self.epoch_changes.shared_data(); + let epoch_descriptor = epoch_changes + .epoch_descriptor_for_child_of( + descendent_query(&*self.client), + &parent_hash, + parent_header_metadata.number, + babe_pre_digest.slot(), + ) + .map_err(|e| ConsensusError::Other(Box::new(e)))? + .ok_or_else(|| ConsensusError::EpochUnavailable(parent_hash.to_string()))?; + let viable_epoch = epoch_changes + .viable_epoch(&epoch_descriptor, |slot| Epoch::genesis(&self.config, slot)) + .ok_or_else(|| ConsensusError::EpochUnavailable(parent_hash.to_string()))?; + let epoch = viable_epoch.as_ref(); + match epoch.authorities.get(babe_pre_digest.authority_index() as usize) { + Some(author) => author.0.clone(), + None => return Err(ConsensusError::SlotAuthorNotFound), + } + }; + if let Err(err) = self + .check_and_report_equivocation(slot_now, slot, &block.header, &author, &block.origin) + .await + { + warn!( + target: LOG_TARGET, + "Error checking/reporting BABE equivocation: {}", err + ); + } let block_status = self .client @@ -1731,11 +1767,14 @@ where /// /// Also returns a link object used to correctly instantiate the import queue /// and background worker. -pub fn block_import( +pub fn block_import( config: BabeConfiguration, wrapped_block_import: I, client: Arc, -) -> ClientResult<(BabeBlockImport, BabeLink)> + create_inherent_data_providers: CIDP, + select_chain: SC, + offchain_tx_pool_factory: OffchainTransactionPoolFactory, +) -> ClientResult<(BabeBlockImport, BabeLink)> where Client: AuxStore + HeaderBackend @@ -1761,13 +1800,21 @@ where }; client.register_finality_action(Box::new(on_finality)); - let import = BabeBlockImport::new(client, epoch_changes, wrapped_block_import, config); + let import = BabeBlockImport::new( + client, + epoch_changes, + wrapped_block_import, + config, + create_inherent_data_providers, + select_chain, + offchain_tx_pool_factory, + ); Ok((import, link)) } /// Parameters passed to [`import_queue`]. -pub struct ImportQueueParams<'a, Block: BlockT, BI, Client, CIDP, SelectChain, Spawn> { +pub struct ImportQueueParams<'a, Block: BlockT, BI, Client, CIDP, Spawn> { /// The BABE link that is created by [`block_import`]. pub link: BabeLink, /// The block import that should be wrapped. @@ -1776,11 +1823,6 @@ pub struct ImportQueueParams<'a, Block: BlockT, BI, Client, CIDP, SelectChain, S pub justification_import: Option>, /// The client to interact with the internals of the node. pub client: Arc, - /// A [`SelectChain`] implementation. - /// - /// Used to determine the best block that should be used as basis when sending an equivocation - /// report. - pub select_chain: SelectChain, /// Used to crate the inherent data providers. /// /// These inherent data providers are then used to create the inherent data that is @@ -1792,10 +1834,6 @@ pub struct ImportQueueParams<'a, Block: BlockT, BI, Client, CIDP, SelectChain, S pub registry: Option<&'a Registry>, /// Optional telemetry handle to report telemetry events. pub telemetry: Option, - /// The offchain transaction pool factory. - /// - /// Will be used when sending equivocation reports. - pub offchain_tx_pool_factory: OffchainTransactionPoolFactory, } /// Start an import queue for the BABE consensus algorithm. @@ -1807,19 +1845,17 @@ pub struct ImportQueueParams<'a, Block: BlockT, BI, Client, CIDP, SelectChain, S /// /// The block import object provided must be the `BabeBlockImport` or a wrapper /// of it, otherwise crucial import logic will be omitted. -pub fn import_queue( +pub fn import_queue( ImportQueueParams { link: babe_link, block_import, justification_import, client, - select_chain, create_inherent_data_providers, spawner, registry, telemetry, - offchain_tx_pool_factory, - }: ImportQueueParams<'_, Block, BI, Client, CIDP, SelectChain, Spawn>, + }: ImportQueueParams<'_, Block, BI, Client, CIDP, Spawn>, ) -> ClientResult<(DefaultImportQueue, BabeWorkerHandle)> where BI: BlockImport + Send + Sync + 'static, @@ -1831,7 +1867,6 @@ where + Sync + 'static, Client::Api: BlockBuilderApi + BabeApi + ApiExt, - SelectChain: sp_consensus::SelectChain + 'static, CIDP: CreateInherentDataProviders + Send + Sync + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, Spawn: SpawnEssentialNamed, @@ -1839,13 +1874,11 @@ where const HANDLE_BUFFER_SIZE: usize = 1024; let verifier = BabeVerifier { - select_chain, create_inherent_data_providers, config: babe_link.config.clone(), epoch_changes: babe_link.epoch_changes.clone(), telemetry, client: client.clone(), - offchain_tx_pool_factory, }; let (worker_tx, worker_rx) = channel(HANDLE_BUFFER_SIZE); @@ -1937,3 +1970,39 @@ where client.insert_aux(values, weight_keys.iter()) }) } + +/// Create inherent data providers for BABE. +#[derive(Debug, Clone)] +pub struct CreateInherentDataProvidersForBabe { + slot_duration: sp_consensus_slots::SlotDuration, +} + +impl CreateInherentDataProvidersForBabe { + /// Create a new instance of `InherentDataProvidersFromTimestampAndSlotDuration`. + pub fn new(slot_duration: sp_consensus_slots::SlotDuration) -> Self { + CreateInherentDataProvidersForBabe { slot_duration } + } +} + +#[async_trait::async_trait] +impl CreateInherentDataProviders + for CreateInherentDataProvidersForBabe +{ + type InherentDataProviders = + (sp_consensus_babe::inherents::InherentDataProvider, sp_timestamp::InherentDataProvider); + + /// Create the inherent data providers at the given `parent` block using the given `extra_args`. + async fn create_inherent_data_providers( + &self, + _parent: Block::Hash, + _extra_args: ExtraArgs, + ) -> Result> { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = + sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + self.slot_duration, + ); + Ok((slot, timestamp)) + } +} diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 5c2e0eae959c1..52e69582b31a6 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -19,6 +19,7 @@ //! BABE testsuite use super::*; +use async_trait::async_trait; use authorship::claim_slot; use sc_block_builder::{BlockBuilder, BlockBuilderBuilder}; use sc_client_api::{BlockchainEvents, Finalizer}; @@ -43,6 +44,7 @@ use sp_runtime::{ }; use sp_timestamp::Timestamp; use std::{cell::RefCell, task::Poll, time::Duration}; +use substrate_test_runtime_client::DefaultTestClientBuilderExt; type Item = DigestItem; @@ -63,11 +65,40 @@ enum Stage { type Mutator = Arc; -type BabeBlockImport = - PanickingBlockImport>>; +type BabeBlockImport = PanickingBlockImport< + crate::BabeBlockImport< + TestBlock, + TestClient, + Arc, + CreateInherentDataProviders, + sc_consensus::LongestChain, + >, +>; const SLOT_DURATION_MS: u64 = 1000; +#[derive(Debug, Clone)] +pub struct CreateInherentDataProviders; + +#[async_trait] +impl + sp_inherents::CreateInherentDataProviders for CreateInherentDataProviders +{ + type InherentDataProviders = (InherentDataProvider,); + + async fn create_inherent_data_providers( + &self, + parent: Block::Hash, + extra_args: ExtraArgs, + ) -> Result> { + let slot = InherentDataProvider::from_timestamp_and_slot_duration( + Timestamp::current(), + SlotDuration::from_millis(SLOT_DURATION_MS), + ); + Ok((slot,)) + } +} + #[derive(Clone)] struct DummyFactory { client: Arc, @@ -177,18 +208,7 @@ type TestSelectChain = substrate_test_runtime_client::LongestChain; pub struct TestVerifier { - inner: BabeVerifier< - TestBlock, - PeersFullClient, - TestSelectChain, - Box< - dyn CreateInherentDataProviders< - TestBlock, - (), - InherentDataProviders = (InherentDataProvider,), - >, - >, - >, + inner: BabeVerifier, mutator: Mutator, } @@ -228,8 +248,16 @@ impl TestNetFactory for BabeTestNet { let client = client.as_client(); let config = crate::configuration(&*client).expect("config available"); - let (block_import, link) = crate::block_import(config, client.clone(), client.clone()) - .expect("can initialize block-import"); + let (_, longest_chain) = TestClientBuilder::new().build_with_longest_chain(); + let (block_import, link) = crate::block_import( + config, + client.clone(), + client.clone(), + CreateInherentDataProviders, + longest_chain, + OffchainTransactionPoolFactory::new(RejectAllTxPool::default()), + ) + .expect("can initialize block-import"); let block_import = PanickingBlockImport(block_import); @@ -253,25 +281,13 @@ impl TestNetFactory for BabeTestNet { .as_ref() .expect("babe link always provided to verifier instantiation"); - let (_, longest_chain) = TestClientBuilder::new().build_with_longest_chain(); - TestVerifier { inner: BabeVerifier { client: client.clone(), - select_chain: longest_chain, - create_inherent_data_providers: Box::new(|_, _| async { - let slot = InherentDataProvider::from_timestamp_and_slot_duration( - Timestamp::current(), - SlotDuration::from_millis(SLOT_DURATION_MS), - ); - Ok((slot,)) - }), + create_inherent_data_providers: CreateInherentDataProviders, config: data.link.config.clone(), epoch_changes: data.link.epoch_changes.clone(), telemetry: None, - offchain_tx_pool_factory: OffchainTransactionPoolFactory::new( - RejectAllTxPool::default(), - ), }, mutator: MUTATOR.with(|m| m.borrow().clone()), } diff --git a/substrate/client/consensus/babe/src/verification.rs b/substrate/client/consensus/babe/src/verification.rs index c6e4ec0c10c13..9527c04c80523 100644 --- a/substrate/client/consensus/babe/src/verification.rs +++ b/substrate/client/consensus/babe/src/verification.rs @@ -30,7 +30,7 @@ use sp_consensus_babe::{ CompatibleDigestItem, PreDigest, PrimaryPreDigest, SecondaryPlainPreDigest, SecondaryVRFPreDigest, }, - make_vrf_sign_data, AuthorityId, AuthorityPair, AuthoritySignature, + make_vrf_sign_data, AuthorityPair, AuthoritySignature, }; use sp_consensus_slots::Slot; use sp_core::{ @@ -69,7 +69,6 @@ pub(super) fn check_header( ) -> Result, Error> { let VerificationParams { mut header, pre_digest, slot_now, epoch } = params; - let authorities = &epoch.authorities; let pre_digest = pre_digest.map(Ok).unwrap_or_else(|| find_pre_digest::(&header))?; trace!(target: LOG_TARGET, "Checking header"); @@ -91,11 +90,6 @@ pub(super) fn check_header( return Ok(CheckedHeader::Deferred(header, pre_digest.slot())) } - let author = match authorities.get(pre_digest.authority_index() as usize) { - Some(author) => author.0.clone(), - None => return Err(babe_err(Error::SlotAuthorNotFound)), - }; - match &pre_digest { PreDigest::Primary(primary) => { debug!( @@ -134,18 +128,12 @@ pub(super) fn check_header( _ => return Err(babe_err(Error::SecondarySlotAssignmentsDisabled)), } - let info = VerifiedHeaderInfo { - pre_digest: CompatibleDigestItem::babe_pre_digest(pre_digest), - seal, - author, - }; + let info = VerifiedHeaderInfo { seal }; Ok(CheckedHeader::Checked(header, info)) } pub(super) struct VerifiedHeaderInfo { - pub(super) pre_digest: DigestItem, pub(super) seal: DigestItem, - pub(super) author: AuthorityId, } /// Check a primary slot proposal header. We validate that the given header is diff --git a/substrate/client/consensus/common/src/import_queue.rs b/substrate/client/consensus/common/src/import_queue.rs index 602683907d482..66cdb5b92068d 100644 --- a/substrate/client/consensus/common/src/import_queue.rs +++ b/substrate/client/consensus/common/src/import_queue.rs @@ -97,7 +97,7 @@ pub struct IncomingBlock { /// Verify a justification of a block #[async_trait::async_trait] -pub trait Verifier: Send + Sync { +pub trait Verifier: Send + Sync { /// Verify the given block data and return the `BlockImportParams` to /// continue the block import process. async fn verify(&self, block: BlockImportParams) -> Result, String>; diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index 4f7e85541777a..7da510ace79a6 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -532,7 +532,7 @@ pub async fn start_slot_worker( /// A header which has been checked pub enum CheckedHeader { - /// A header which has slot in the future. this is the full header (not stripped) + /// A header which has slot in the future. This is the full header (not stripped) /// and the slot in which it should be processed. Deferred(H, Slot), /// A header which is fully checked, including signature. This is the pre-header diff --git a/substrate/client/network/sync/src/strategy/state.rs b/substrate/client/network/sync/src/strategy/state.rs index 1abbb96ccd907..a63e817e60892 100644 --- a/substrate/client/network/sync/src/strategy/state.rs +++ b/substrate/client/network/sync/src/strategy/state.rs @@ -37,7 +37,7 @@ use sc_consensus::{BlockImportError, BlockImportStatus, IncomingBlock}; use sc_network::{IfDisconnected, ProtocolName}; use sc_network_common::sync::message::BlockAnnounce; use sc_network_types::PeerId; -use sp_consensus::BlockOrigin; +use sp_consensus::{BlockOrigin, Error as ConsensusError}; use sp_runtime::{ traits::{Block as BlockT, Header, NumberFor}, Justifications, SaturatedConversion, @@ -272,10 +272,24 @@ impl StateStrategy { if !results.is_empty() { // We processed the target block results.iter().filter_map(|result| result.as_ref().err()).for_each(|e| { - error!( - target: LOG_TARGET, - "Failed to import target block with state: {e:?}." - ); + match e { + BlockImportError::Other( + ConsensusError::InvalidInherents(_) | + ConsensusError::InvalidInherentsUnhandled(_), + ) => { + // Ignore inherents errors. + trace!( + target: LOG_TARGET, + "Failed to import target block due to invalid inherent transactions: {e:?}." + ); + }, + _ => { + error!( + target: LOG_TARGET, + "Failed to import target block with state: {e:?}." + ); + }, + } }); self.succeeded |= results.into_iter().any(|result| result.is_ok()); self.actions.push(SyncingAction::Finished); diff --git a/substrate/primitives/consensus/common/src/error.rs b/substrate/primitives/consensus/common/src/error.rs index fb8d0447fe3d6..b56511c8a76f4 100644 --- a/substrate/primitives/consensus/common/src/error.rs +++ b/substrate/primitives/consensus/common/src/error.rs @@ -50,6 +50,18 @@ pub enum Error { /// Signing failed. #[error("Failed to sign: {0}")] CannotSign(String), + /// Invalid inherent transactions. + #[error("Invalid inherent transactions: {0}")] + InvalidInherents(sp_inherents::Error), + /// Invalid inherent transactions. + #[error("Invalid inherent transactions (unhandled): {0:?}")] + InvalidInherentsUnhandled(sp_inherents::InherentIdentifier), + /// Epoch unavailable. + #[error("Epoch unavailable for parent hash: {0}")] + EpochUnavailable(String), + /// Slot author not found. + #[error("Slot author not found")] + SlotAuthorNotFound, /// Some other error. #[error(transparent)] Other(#[from] Box), diff --git a/templates/parachain/node/src/service.rs b/templates/parachain/node/src/service.rs index ef8ba1a2dde34..dbdb6da49070c 100644 --- a/templates/parachain/node/src/service.rs +++ b/templates/parachain/node/src/service.rs @@ -9,7 +9,7 @@ use parachain_template_runtime::{ opaque::{Block, Hash}, }; -use polkadot_sdk::*; +use polkadot_sdk::{sc_consensus_aura::CreateInherentDataProvidersForAuraViaRuntime, *}; // Cumulus Imports use cumulus_client_bootnodes::{start_bootnode_tasks, StartBootnodeTasksParams}; @@ -203,7 +203,9 @@ fn start_consensus( ); let params = AuraParams { - create_inherent_data_providers: move |_, ()| async move { Ok(()) }, + create_inherent_data_providers: CreateInherentDataProvidersForAuraViaRuntime::new( + client.clone(), + ), block_import, para_client: client.clone(), para_backend: backend, diff --git a/templates/solochain/node/src/service.rs b/templates/solochain/node/src/service.rs index 79d97fbab8dfa..e6411db42aa78 100644 --- a/templates/solochain/node/src/service.rs +++ b/templates/solochain/node/src/service.rs @@ -109,7 +109,6 @@ pub fn new_partial(config: &Configuration) -> Result { }, spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), - check_for_equivocation: Default::default(), telemetry: telemetry.as_ref().map(|x| x.handle()), compatibility_mode: Default::default(), })?; From 4431bb1e46c6497638cb8768da66d263ce894219 Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 20 May 2025 12:49:03 +0200 Subject: [PATCH 02/38] empty prdoc --- prdoc/pr_8446.prdoc | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 prdoc/pr_8446.prdoc diff --git a/prdoc/pr_8446.prdoc b/prdoc/pr_8446.prdoc new file mode 100644 index 0000000000000..e69de29bb2d1d From 0f9533be8c4029a0fd422d762f2aa5c01b7f8b97 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 20 May 2025 10:57:53 +0000 Subject: [PATCH 03/38] Update from github-actions[bot] running command 'prdoc --audience node_operator --bump minor --force' --- prdoc/pr_8446.prdoc | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/prdoc/pr_8446.prdoc b/prdoc/pr_8446.prdoc index e69de29bb2d1d..c9c74c8b657e7 100644 --- a/prdoc/pr_8446.prdoc +++ b/prdoc/pr_8446.prdoc @@ -0,0 +1,28 @@ +title: check inherents and equivocations in `import_block` +doc: +- audience: Node Operator + description: |- + When importing blocks, there are essentially two steps: `verify` and `import_block`. With https://github.com/paritytech/polkadot-sdk/issues/65, we would like to re-broadcast blocks between these two steps (i.e. basically right after `verify`). + + This PR is the first step to enable that change, by moving `check_inherents` (and equivocation checks) from the `verify` to the `import_block` step (because `verify` does runtime checks that can potentially be expensive, and we don't really need to do those before re-broadcasting). +crates: +- name: cumulus-client-consensus-aura + bump: minor +- name: polkadot-omni-node-lib + bump: minor +- name: polkadot-service + bump: minor +- name: sc-consensus-aura + bump: minor +- name: sc-consensus-babe + bump: minor +- name: sc-consensus-babe-rpc + bump: minor +- name: sc-consensus + bump: minor +- name: sc-consensus-slots + bump: minor +- name: sc-network-sync + bump: minor +- name: sp-consensus + bump: minor From e3e584625222afc257a7d6338b152acd45d979b2 Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 20 May 2025 14:15:26 +0200 Subject: [PATCH 04/38] fix clippies --- substrate/client/consensus/aura/src/lib.rs | 1 - substrate/client/consensus/babe/src/tests.rs | 9 ++------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index f9079b4b5c521..bdbf452019edf 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -749,7 +749,6 @@ mod tests { ); Ok((slot,)) }), - CheckForEquivocation::Yes, None, CompatibilityMode::None, ) diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 52e69582b31a6..f911808c98489 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -88,8 +88,8 @@ impl async fn create_inherent_data_providers( &self, - parent: Block::Hash, - extra_args: ExtraArgs, + _parent: Block::Hash, + _extra_args: ExtraArgs, ) -> Result> { let slot = InherentDataProvider::from_timestamp_and_slot_duration( Timestamp::current(), @@ -204,9 +204,6 @@ pub struct BabeTestNet { type TestHeader = ::Header; -type TestSelectChain = - substrate_test_runtime_client::LongestChain; - pub struct TestVerifier { inner: BabeVerifier, mutator: Mutator, @@ -271,8 +268,6 @@ impl TestNetFactory for BabeTestNet { } fn make_verifier(&self, client: PeersClient, maybe_link: &Option) -> Self::Verifier { - use substrate_test_runtime_client::DefaultTestClientBuilderExt; - let client = client.as_client(); trace!(target: LOG_TARGET, "Creating a verifier"); From dc09e649a03c911574429d3f4545c959a2647c55 Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 20 May 2025 14:39:06 +0200 Subject: [PATCH 05/38] toml formatting --- cumulus/test/service/Cargo.toml | 2 +- substrate/client/consensus/babe/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/test/service/Cargo.toml b/cumulus/test/service/Cargo.toml index b4fca72211145..f6413cfba7d9b 100644 --- a/cumulus/test/service/Cargo.toml +++ b/cumulus/test/service/Cargo.toml @@ -53,13 +53,13 @@ sp-consensus = { workspace = true, default-features = true } sp-consensus-aura = { workspace = true, default-features = true } sp-core = { workspace = true, default-features = true } sp-genesis-builder = { workspace = true, default-features = true } +sp-inherents = { workspace = true } sp-io = { workspace = true, default-features = true } sp-keyring = { workspace = true, default-features = true } sp-runtime = { workspace = true } sp-state-machine = { workspace = true, default-features = true } sp-timestamp = { workspace = true, default-features = true } sp-tracing = { workspace = true, default-features = true } -sp-inherents = { workspace = true } substrate-test-client = { workspace = true } # Polkadot diff --git a/substrate/client/consensus/babe/Cargo.toml b/substrate/client/consensus/babe/Cargo.toml index 3b5b1ad2377aa..6390ec840d555 100644 --- a/substrate/client/consensus/babe/Cargo.toml +++ b/substrate/client/consensus/babe/Cargo.toml @@ -32,7 +32,6 @@ sc-consensus = { workspace = true, default-features = true } sc-consensus-epochs = { workspace = true, default-features = true } sc-consensus-slots = { workspace = true, default-features = true } sc-telemetry = { workspace = true, default-features = true } -sp-timestamp = { workspace = true, default-features = true } sc-transaction-pool-api = { workspace = true, default-features = true } sp-api = { workspace = true, default-features = true } sp-application-crypto = { workspace = true, default-features = true } @@ -46,6 +45,7 @@ sp-crypto-hashing = { workspace = true, default-features = true } sp-inherents = { workspace = true, default-features = true } sp-keystore = { workspace = true, default-features = true } sp-runtime = { workspace = true, default-features = true } +sp-timestamp = { workspace = true, default-features = true } thiserror = { workspace = true } [dev-dependencies] From 610fd40957039bbb3da0b3e27c4b6f2df1bf397d Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 20 May 2025 16:46:52 +0200 Subject: [PATCH 06/38] update test --- substrate/client/consensus/babe/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index f911808c98489..a4b9a483df9e0 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -440,7 +440,7 @@ async fn authoring_blocks() { } #[tokio::test] -#[should_panic(expected = "valid babe headers must contain a predigest")] +#[should_panic(expected = "importing block failed: Other(NoPreRuntimeDigest)")] async fn rejects_missing_inherent_digest() { run_one_test(|header: &mut TestHeader, stage| { let v = std::mem::take(&mut header.digest_mut().logs); From 09a0896159625eed29ed0627d8238d04ce3b3574 Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 20 May 2025 21:49:53 +0200 Subject: [PATCH 07/38] fix semver --- prdoc/pr_8446.prdoc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/prdoc/pr_8446.prdoc b/prdoc/pr_8446.prdoc index c9c74c8b657e7..d2684141f635b 100644 --- a/prdoc/pr_8446.prdoc +++ b/prdoc/pr_8446.prdoc @@ -7,22 +7,22 @@ doc: This PR is the first step to enable that change, by moving `check_inherents` (and equivocation checks) from the `verify` to the `import_block` step (because `verify` does runtime checks that can potentially be expensive, and we don't really need to do those before re-broadcasting). crates: - name: cumulus-client-consensus-aura - bump: minor + bump: major - name: polkadot-omni-node-lib bump: minor - name: polkadot-service - bump: minor + bump: major - name: sc-consensus-aura - bump: minor + bump: major - name: sc-consensus-babe - bump: minor + bump: major - name: sc-consensus-babe-rpc - bump: minor + bump: patch - name: sc-consensus - bump: minor + bump: patch - name: sc-consensus-slots - bump: minor + bump: patch - name: sc-network-sync - bump: minor + bump: patch - name: sp-consensus - bump: minor + bump: major From 73a9b0a490fef2eea93571ad8f39b090bfd77a6c Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 21 May 2025 20:50:01 +0200 Subject: [PATCH 08/38] remove .vim --- .vim/coc-settings.json | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .vim/coc-settings.json diff --git a/.vim/coc-settings.json b/.vim/coc-settings.json deleted file mode 100644 index ca882484fdaaf..0000000000000 --- a/.vim/coc-settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "rust-analyzer.checkOnSave": false -} From 4eb719712e23ddbc3eb24cc10f30d1233f083f89 Mon Sep 17 00:00:00 2001 From: sistemd Date: Thu, 22 May 2025 12:31:12 +0200 Subject: [PATCH 09/38] use Arc instead of custom CreateInherentDataProvider impls --- .../polkadot-omni-node/lib/src/nodes/aura.rs | 118 +++++++++++++++--- cumulus/test/service/src/lib.rs | 80 ++++++++++-- polkadot/node/service/src/builder/partial.rs | 51 +++++++- substrate/bin/node/cli/src/service.rs | 64 +++++++++- substrate/client/consensus/aura/src/lib.rs | 90 ------------- substrate/client/consensus/babe/src/lib.rs | 36 ------ substrate/client/consensus/babe/src/tests.rs | 66 ++++++---- .../primitives/inherents/src/client_side.rs | 18 +++ templates/parachain/node/src/service.rs | 30 ++++- 9 files changed, 358 insertions(+), 195 deletions(-) diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 30b8eb0419fbd..8b2078e2ef1f0 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -60,7 +60,6 @@ use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, BlockImportParams, DefaultImportQueue, }; -use sc_consensus_aura::CreateInherentDataProvidersForAuraViaRuntime; use sc_service::{Configuration, Error, TaskManager}; use sc_telemetry::TelemetryHandle; use sc_transaction_pool::TransactionPoolHandle; @@ -257,9 +256,15 @@ where Block, Arc>, ParachainClient, - CreateInherentDataProvidersForAuraViaRuntime< - ParachainClient, - ::Public, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, >, AuraId::BoundedPair, NumberFor, @@ -296,9 +301,15 @@ impl, RuntimeApi, AuraId> Block, Arc>, ParachainClient, - CreateInherentDataProvidersForAuraViaRuntime< - ParachainClient, - ::Public, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, >, AuraId::BoundedPair, NumberFor, @@ -319,9 +330,15 @@ where Block, Arc>, ParachainClient, - CreateInherentDataProvidersForAuraViaRuntime< - ParachainClient, - ::Public, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, >, AuraId::BoundedPair, NumberFor, @@ -359,10 +376,32 @@ where ); let client_for_aura = client.clone(); + let client_for_closure = client.clone(); let params = SlotBasedParams { - create_inherent_data_providers: CreateInherentDataProvidersForAuraViaRuntime::new( - client.clone(), - ), + create_inherent_data_providers: Arc::new(move |parent, _| { + let slot_duration = sc_consensus_aura::standalone::slot_duration_at( + client_for_closure.as_ref(), + parent, + ); + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = slot_duration.map(|slot_duration| { + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ) + }); + async move { Ok((slot?, timestamp)) } + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, block_import, para_client: client.clone(), para_backend: backend.clone(), @@ -407,9 +446,15 @@ where Block, Arc>, ParachainClient, - sc_consensus_aura::CreateInherentDataProvidersForAuraViaRuntime< - ParachainClient, - ::Public, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, >, AuraId::BoundedPair, NumberFor, @@ -422,7 +467,18 @@ where Ok(SlotBasedBlockImport::new( client.clone(), client.clone(), - CreateInherentDataProvidersForAuraViaRuntime::new(client), + Arc::new(move |parent, _| { + let slot_duration = + sc_consensus_aura::standalone::slot_duration_at(client.as_ref(), parent); + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = slot_duration.map(|slot_duration| { + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ) + }); + async move { Ok((slot?, timestamp)) } + }), Default::default(), Default::default(), )) @@ -499,12 +555,34 @@ where client.clone(), ); + let client_for_closure = client.clone(); let params = aura::ParamsWithExport { export_pov: node_extra_args.export_pov, params: AuraParams { - create_inherent_data_providers: CreateInherentDataProvidersForAuraViaRuntime::new( - client.clone(), - ), + create_inherent_data_providers: Arc::new(move |parent, _| { + let slot_duration = sc_consensus_aura::standalone::slot_duration_at( + client_for_closure.as_ref(), + parent, + ); + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = slot_duration.map(|slot_duration| { + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ) + }); + async move { Ok((slot?, timestamp)) } + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, block_import, para_client: client.clone(), para_backend: backend, diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index b77da4aa50ca6..d985175113457 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -38,11 +38,9 @@ use cumulus_client_consensus_proposer::Proposer; use cumulus_test_runtime::{Hash, Header, NodeBlock as Block, RuntimeApi}; use prometheus::Registry; use runtime::AccountId; -use sc_consensus_aura::{ - CreateInherentDataProvidersForAura, CreateInherentDataProvidersForAuraViaRuntime, -}; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; use sp_consensus_aura::sr25519::AuthorityPair; +use sp_inherents::CreateInherentDataProviders; use std::{ collections::HashSet, future::Future, @@ -142,7 +140,16 @@ pub type ParachainBlockImport = TParachainBlockImport< Block, Arc, Client, - CreateInherentDataProvidersForAura, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, AuthorityPair, NumberFor, >, @@ -237,7 +244,24 @@ pub fn new_partial( let (block_import, slot_based_handle) = SlotBasedBlockImport::new( client.clone(), client.clone(), - CreateInherentDataProvidersForAura::new(slot_duration), + Arc::new(move |_, _| async move { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + Ok((slot, timestamp)) + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, Default::default(), Default::default(), ); @@ -258,7 +282,24 @@ pub fn new_partial( ImportQueueParams { block_import: block_import.clone(), client: client.clone(), - create_inherent_data_providers: CreateInherentDataProvidersForAura::new(slot_duration), + create_inherent_data_providers: Arc::new(move |_, _| async move { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + Ok((slot, timestamp)) + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, spawner: &task_manager.spawn_essential_handle(), registry: None, telemetry: None, @@ -522,9 +563,32 @@ where slot_based::run::(params); } else { tracing::info!(target: LOG_TARGET, "Starting block authoring with lookahead collator."); + let client_for_closure = client.clone(); let params = AuraParams { - create_inherent_data_providers: - CreateInherentDataProvidersForAuraViaRuntime::new(client.clone()), + create_inherent_data_providers: Arc::new(move |parent, _| { + let slot_duration = sc_consensus_aura::standalone::slot_duration_at( + client_for_closure.as_ref(), + parent, + ); + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = slot_duration.map(|slot_duration| { + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ) + }); + async move { Ok((slot?, timestamp)) } + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, block_import, para_client: client.clone(), para_backend: backend.clone(), diff --git a/polkadot/node/service/src/builder/partial.rs b/polkadot/node/service/src/builder/partial.rs index 4f8267835f50c..407c6e1659bdc 100644 --- a/polkadot/node/service/src/builder/partial.rs +++ b/polkadot/node/service/src/builder/partial.rs @@ -30,6 +30,7 @@ use sc_telemetry::{Telemetry, TelemetryWorker, TelemetryWorkerHandle}; use sc_transaction_pool_api::OffchainTransactionPoolFactory; use sp_consensus::SelectChain; use sp_consensus_beefy::ecdsa_crypto; +use sp_inherents::CreateInherentDataProviders; use std::sync::Arc; type FullSelectChain = relay_chain_selection::SelectRelayChain; @@ -64,7 +65,16 @@ pub(crate) type PolkadotPartialComponents = sc_service::PartialC FullGrandpaBlockImport, ecdsa_crypto::AuthorityId, >, - sc_consensus_babe::CreateInherentDataProvidersForBabe, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_babe::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, ChainSelection, >, sc_consensus_grandpa::LinkHalf, @@ -191,7 +201,24 @@ where babe_config.clone(), beefy_block_import, client.clone(), - sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), + Arc::new(move |_, _| async move { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + Ok((slot, timestamp)) + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_babe::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, select_chain.clone(), OffchainTransactionPoolFactory::new(transaction_pool.clone()), )?; @@ -202,8 +229,24 @@ where block_import: block_import.clone(), justification_import: Some(Box::new(justification_import)), client: client.clone(), - create_inherent_data_providers: - sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), + create_inherent_data_providers: Arc::new(move |_, _| async move { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + Ok((slot, timestamp)) + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_babe::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), telemetry: telemetry.as_ref().map(|x| x.handle()), diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 7a8f77cd0d85d..7c2892e493bb8 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -22,7 +22,7 @@ use polkadot_sdk::{ sc_consensus_beefy as beefy, sc_consensus_grandpa as grandpa, - sp_consensus_beefy as beefy_primitives, *, + sp_consensus_beefy as beefy_primitives, sp_inherents::CreateInherentDataProviders, *, }; use crate::Cli; @@ -190,7 +190,16 @@ pub fn new_partial( Block, FullClient, FullBeefyBlockImport, - sc_consensus_babe::CreateInherentDataProvidersForBabe, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_babe::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, FullSelectChain, >, grandpa::LinkHalf, @@ -267,7 +276,25 @@ pub fn new_partial( babe_config, beefy_block_import, client.clone(), - sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), + Arc::new(move |_, _| async move { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = + sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + Ok((slot, timestamp)) + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_babe::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, select_chain.clone(), OffchainTransactionPoolFactory::new(transaction_pool.clone()), )?; @@ -278,8 +305,24 @@ pub fn new_partial( block_import: block_import.clone(), justification_import: Some(Box::new(justification_import)), client: client.clone(), - create_inherent_data_providers: - sc_consensus_babe::CreateInherentDataProvidersForBabe::new(slot_duration), + create_inherent_data_providers: Arc::new(move |_, _| async move { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + Ok((slot, timestamp)) + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_babe::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), telemetry: telemetry.as_ref().map(|x| x.handle()), @@ -403,7 +446,16 @@ pub fn new_full_base::Hash>>( Block, FullClient, FullBeefyBlockImport, - sc_consensus_babe::CreateInherentDataProvidersForBabe, + Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_babe::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, FullSelectChain, >, &sc_consensus_babe::BabeLink, diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index bdbf452019edf..7acb42cd4b815 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -544,96 +544,6 @@ where .ok_or(ConsensusError::InvalidAuthoritiesSet) } -/// Create inherent data providers for AURA. -#[derive(Debug, Clone)] -pub struct CreateInherentDataProvidersForAura { - slot_duration: sp_consensus_aura::SlotDuration, -} - -impl CreateInherentDataProvidersForAura { - /// Create a new instance of `CreateInherentDataProvidersForAura`. - pub fn new(slot_duration: sp_consensus_aura::SlotDuration) -> Self { - Self { slot_duration } - } -} - -#[async_trait::async_trait] -impl - sp_inherents::CreateInherentDataProviders - for CreateInherentDataProvidersForAura -{ - type InherentDataProviders = - (sp_consensus_aura::inherents::InherentDataProvider, sp_timestamp::InherentDataProvider); - - /// Create the inherent data providers at the given `parent` block using the given `extra_args`. - async fn create_inherent_data_providers( - &self, - _parent: Block::Hash, - _extra_args: ExtraArgs, - ) -> Result> { - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - let slot = - sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - self.slot_duration, - ); - Ok((slot, timestamp)) - } -} - -// TODO Question: is this needed? Or can I just use -// let slot_duration = sc_consensus_aura::slot_duration(&*client)?; -// let cidp = CreateInherentDataProvidersForAura::new(slot_duration); -// Like I do in the cumulus test service? -/// Create inherent data providers for AURA, using the runtime API to fetch slot durations. -#[derive(Debug)] -pub struct CreateInherentDataProvidersForAuraViaRuntime { - client: Arc, - _phantom: PhantomData, -} - -impl CreateInherentDataProvidersForAuraViaRuntime { - /// Create a new instance of `CreateInherentDataProvidersForAura`. - pub fn new(client: Arc) -> Self { - Self { client, _phantom: Default::default() } - } -} - -impl Clone for CreateInherentDataProvidersForAuraViaRuntime { - fn clone(&self) -> Self { - Self { client: self.client.clone(), _phantom: Default::default() } - } -} - -#[async_trait::async_trait] -impl - sp_inherents::CreateInherentDataProviders - for CreateInherentDataProvidersForAuraViaRuntime -where - A: Codec + Send + Sync, - C: ProvideRuntimeApi + Send + Sync, - C::Api: AuraApi, -{ - type InherentDataProviders = - (sp_consensus_aura::inherents::InherentDataProvider, sp_timestamp::InherentDataProvider); - - /// Create the inherent data providers at the given `parent` block using the given `extra_args`. - async fn create_inherent_data_providers( - &self, - parent: Block::Hash, - _extra_args: ExtraArgs, - ) -> Result> { - let slot_duration = crate::standalone::slot_duration_at(self.client.as_ref(), parent)?; - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - let slot = - sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - slot_duration, - ); - Ok((slot, timestamp)) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index bdf409f2004ef..a11df83882463 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -1970,39 +1970,3 @@ where client.insert_aux(values, weight_keys.iter()) }) } - -/// Create inherent data providers for BABE. -#[derive(Debug, Clone)] -pub struct CreateInherentDataProvidersForBabe { - slot_duration: sp_consensus_slots::SlotDuration, -} - -impl CreateInherentDataProvidersForBabe { - /// Create a new instance of `InherentDataProvidersFromTimestampAndSlotDuration`. - pub fn new(slot_duration: sp_consensus_slots::SlotDuration) -> Self { - CreateInherentDataProvidersForBabe { slot_duration } - } -} - -#[async_trait::async_trait] -impl CreateInherentDataProviders - for CreateInherentDataProvidersForBabe -{ - type InherentDataProviders = - (sp_consensus_babe::inherents::InherentDataProvider, sp_timestamp::InherentDataProvider); - - /// Create the inherent data providers at the given `parent` block using the given `extra_args`. - async fn create_inherent_data_providers( - &self, - _parent: Block::Hash, - _extra_args: ExtraArgs, - ) -> Result> { - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - let slot = - sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - self.slot_duration, - ); - Ok((slot, timestamp)) - } -} diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index a4b9a483df9e0..964a9efd33622 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -19,7 +19,6 @@ //! BABE testsuite use super::*; -use async_trait::async_trait; use authorship::claim_slot; use sc_block_builder::{BlockBuilder, BlockBuilderBuilder}; use sc_client_api::{BlockchainEvents, Finalizer}; @@ -70,35 +69,19 @@ type BabeBlockImport = PanickingBlockImport< TestBlock, TestClient, Arc, - CreateInherentDataProviders, + Arc< + dyn CreateInherentDataProviders< + TestBlock, + (), + InherentDataProviders = (InherentDataProvider,), + >, + >, sc_consensus::LongestChain, >, >; const SLOT_DURATION_MS: u64 = 1000; -#[derive(Debug, Clone)] -pub struct CreateInherentDataProviders; - -#[async_trait] -impl - sp_inherents::CreateInherentDataProviders for CreateInherentDataProviders -{ - type InherentDataProviders = (InherentDataProvider,); - - async fn create_inherent_data_providers( - &self, - _parent: Block::Hash, - _extra_args: ExtraArgs, - ) -> Result> { - let slot = InherentDataProvider::from_timestamp_and_slot_duration( - Timestamp::current(), - SlotDuration::from_millis(SLOT_DURATION_MS), - ); - Ok((slot,)) - } -} - #[derive(Clone)] struct DummyFactory { client: Arc, @@ -205,7 +188,17 @@ pub struct BabeTestNet { type TestHeader = ::Header; pub struct TestVerifier { - inner: BabeVerifier, + inner: BabeVerifier< + TestBlock, + PeersFullClient, + Box< + dyn CreateInherentDataProviders< + TestBlock, + (), + InherentDataProviders = (InherentDataProvider,), + >, + >, + >, mutator: Mutator, } @@ -250,7 +243,20 @@ impl TestNetFactory for BabeTestNet { config, client.clone(), client.clone(), - CreateInherentDataProviders, + Arc::new(move |_, _| async { + let slot = InherentDataProvider::from_timestamp_and_slot_duration( + Timestamp::current(), + SlotDuration::from_millis(SLOT_DURATION_MS), + ); + Ok((slot,)) + }) + as Arc< + dyn CreateInherentDataProviders< + TestBlock, + (), + InherentDataProviders = (InherentDataProvider,), + >, + >, longest_chain, OffchainTransactionPoolFactory::new(RejectAllTxPool::default()), ) @@ -279,7 +285,13 @@ impl TestNetFactory for BabeTestNet { TestVerifier { inner: BabeVerifier { client: client.clone(), - create_inherent_data_providers: CreateInherentDataProviders, + create_inherent_data_providers: Box::new(move |_, _| async { + let slot = InherentDataProvider::from_timestamp_and_slot_duration( + Timestamp::current(), + SlotDuration::from_millis(SLOT_DURATION_MS), + ); + Ok((slot,)) + }), config: data.link.config.clone(), epoch_changes: data.link.epoch_changes.clone(), telemetry: None, diff --git a/substrate/primitives/inherents/src/client_side.rs b/substrate/primitives/inherents/src/client_side.rs index 3c299dfa4eea0..c4b4db335dab1 100644 --- a/substrate/primitives/inherents/src/client_side.rs +++ b/substrate/primitives/inherents/src/client_side.rs @@ -15,6 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::sync::Arc; + use crate::{Error, InherentData, InherentIdentifier}; use sp_runtime::traits::Block as BlockT; @@ -77,6 +79,22 @@ impl } } +#[async_trait::async_trait] +impl + CreateInherentDataProviders + for Arc> +{ + type InherentDataProviders = IDPS; + + async fn create_inherent_data_providers( + &self, + parent: Block::Hash, + extra_args: ExtraArgs, + ) -> Result> { + (**self).create_inherent_data_providers(parent, extra_args).await + } +} + /// Something that provides inherent data. #[async_trait::async_trait] pub trait InherentDataProvider: Send + Sync { diff --git a/templates/parachain/node/src/service.rs b/templates/parachain/node/src/service.rs index dbdb6da49070c..e867bf38eba45 100644 --- a/templates/parachain/node/src/service.rs +++ b/templates/parachain/node/src/service.rs @@ -9,7 +9,7 @@ use parachain_template_runtime::{ opaque::{Block, Hash}, }; -use polkadot_sdk::{sc_consensus_aura::CreateInherentDataProvidersForAuraViaRuntime, *}; +use polkadot_sdk::{sp_inherents::CreateInherentDataProviders, *}; // Cumulus Imports use cumulus_client_bootnodes::{start_bootnode_tasks, StartBootnodeTasksParams}; @@ -202,10 +202,32 @@ fn start_consensus( client.clone(), ); + let client_for_closure = client.clone(); let params = AuraParams { - create_inherent_data_providers: CreateInherentDataProvidersForAuraViaRuntime::new( - client.clone(), - ), + create_inherent_data_providers: Arc::new(move |parent, _| { + let slot_duration = sc_consensus_aura::standalone::slot_duration_at( + client_for_closure.as_ref(), + parent, + ); + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot = slot_duration.map(|slot_duration| { + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ) + }); + async move { Ok((slot?, timestamp)) } + }) + as Arc< + dyn CreateInherentDataProviders< + Block, + (), + InherentDataProviders = ( + sp_consensus_aura::inherents::InherentDataProvider, + sp_timestamp::InherentDataProvider, + ), + >, + >, block_import, para_client: client.clone(), para_backend: backend, From cc36b4ac3f723060826fe71719bbdc2eafb5d602 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 23 May 2025 15:03:11 +0200 Subject: [PATCH 10/38] remove CheckForEquivocation --- .../consensus/aura/src/collators/basic.rs | 2 +- .../consensus/aura/src/collators/lookahead.rs | 2 +- .../consensus/aura/src/collators/mod.rs | 33 +++---------------- .../src/collators/slot_based/block_import.rs | 6 ++-- .../polkadot-omni-node/lib/src/nodes/aura.rs | 2 +- cumulus/test/service/src/lib.rs | 2 +- 6 files changed, 11 insertions(+), 36 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index 18818548e7efe..ee092cb60bca4 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -135,7 +135,7 @@ where params.block_import, params.para_client.clone(), params.create_inherent_data_providers.clone(), - Default::default(), + true, Default::default(), ), relay_client: params.relay_client.clone(), diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index d95502e4d10bc..781215860d4be 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -219,7 +219,7 @@ where params.block_import, params.para_client.clone(), params.create_inherent_data_providers.clone(), - Default::default(), + true, Default::default(), ), relay_client: params.relay_client.clone(), diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs index 1eb5d579fa962..4290ff3631357 100644 --- a/cumulus/client/consensus/aura/src/collators/mod.rs +++ b/cumulus/client/consensus/aura/src/collators/mod.rs @@ -285,7 +285,7 @@ struct ValidatingBlockImport { inner: BI, client: Arc, create_inherent_data_providers: CIDP, - check_for_equivocation: CheckForEquivocation, + check_for_equivocation: bool, compatibility_mode: CompatibilityMode, _phantom: std::marker::PhantomData

, } @@ -295,7 +295,7 @@ impl ValidatingBlockImport { inner: BI, client: Arc, create_inherent_data_providers: CIDP, - check_for_equivocation: CheckForEquivocation, + check_for_equivocation: bool, compatibility_mode: CompatibilityMode, ) -> Self { Self { @@ -358,7 +358,7 @@ async fn validate_block_import( params: &mut sc_consensus::BlockImportParams, client: &Client, create_inherent_data_providers: &CIDP, - check_for_equivocation: CheckForEquivocation, + check_for_equivocation: bool, compatibility_mode: &CompatibilityMode>, ) -> Result<(), sp_consensus::Error> where @@ -426,8 +426,7 @@ where .map_err(sp_consensus::Error::Other)?; let slot_now = create_inherent_data_providers.slot(); let expected_author = sc_consensus_aura::standalone::slot_author::

(slot, &authorities); - let should_equiv_check = check_for_equivocation.check_for_equivocation(); - if let (true, Some(expected)) = (should_equiv_check, expected_author) { + if let (true, Some(expected)) = (check_for_equivocation, expected_author) { if let Some(equivocation_proof) = sc_consensus_slots::check_equivocation( client, // we add one to allow for some small drift. @@ -453,30 +452,6 @@ where Ok(()) } -/// Should we check for equivocation of a block author? -#[derive(Debug, Clone, Copy)] -pub enum CheckForEquivocation { - /// Yes, check for equivocation. - /// - /// This is the default setting for this. - Yes, - /// No, don't check for equivocation. - No, -} - -impl CheckForEquivocation { - /// Should we check for equivocation? - fn check_for_equivocation(self) -> bool { - matches!(self, Self::Yes) - } -} - -impl Default for CheckForEquivocation { - fn default() -> Self { - Self::Yes - } -} - #[cfg(test)] mod tests { use crate::collators::can_build_upon; diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs index 141ec6e8ec07e..9df170fc964be 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs @@ -29,7 +29,7 @@ use sp_runtime::traits::{Block as BlockT, Header as _, NumberFor}; use sp_trie::proof_size_extension::ProofSizeExt; use std::sync::Arc; -use crate::collators::{validate_block_import, CheckForEquivocation}; +use crate::collators::validate_block_import; /// Handle for receiving the block and the storage proof from the [`SlotBasedBlockImport`]. /// @@ -60,7 +60,7 @@ pub struct SlotBasedBlockImport { client: Arc, sender: TracingUnboundedSender<(Block, StorageProof)>, create_inherent_data_providers: CIDP, - check_for_equivocation: CheckForEquivocation, + check_for_equivocation: bool, compatibility_mode: CompatibilityMode, _phantom: std::marker::PhantomData

, } @@ -75,7 +75,7 @@ impl SlotBasedBlockImport, create_inherent_data_providers: CIDP, - check_for_equivocation: CheckForEquivocation, + check_for_equivocation: bool, compatibility_mode: CompatibilityMode, ) -> (Self, SlotBasedBlockImportHandle) { let (sender, receiver) = tracing_unbounded("SlotBasedBlockImportChannel", 1000); diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 8b2078e2ef1f0..369b6d6db959e 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -479,7 +479,7 @@ where }); async move { Ok((slot?, timestamp)) } }), - Default::default(), + true, Default::default(), )) } diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index d985175113457..10317ec53e6be 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -262,7 +262,7 @@ pub fn new_partial( ), >, >, - Default::default(), + true, Default::default(), ); let block_import = ParachainBlockImport::new(block_import, backend.clone()); From eb14482646962a046fa552f396729970c753ffd5 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 23 May 2025 15:03:11 +0200 Subject: [PATCH 11/38] typedefs --- .../polkadot-omni-node/lib/src/nodes/aura.rs | 70 ++----------------- cumulus/test/service/src/lib.rs | 49 ++----------- polkadot/node/service/src/builder/partial.rs | 36 ++-------- substrate/bin/node/cli/src/service.rs | 48 ++----------- substrate/client/consensus/babe/src/tests.rs | 21 ++---- .../consensus/aura/src/inherents.rs | 10 +++ .../consensus/babe/src/inherents.rs | 21 ++++++ .../primitives/inherents/src/client_side.rs | 4 +- templates/parachain/node/src/service.rs | 14 +--- 9 files changed, 61 insertions(+), 212 deletions(-) diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 369b6d6db959e..9df8e080d4914 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -64,7 +64,7 @@ use sc_service::{Configuration, Error, TaskManager}; use sc_telemetry::TelemetryHandle; use sc_transaction_pool::TransactionPoolHandle; use sp_api::ProvideRuntimeApi; -use sp_consensus_aura::AuraApi; +use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, AuraApi}; use sp_core::{traits::SpawnNamed, Pair}; use sp_inherents::CreateInherentDataProviders; use sp_keystore::KeystorePtr; @@ -256,16 +256,7 @@ where Block, Arc>, ParachainClient, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + AuraCreateInherentDataProviders, AuraId::BoundedPair, NumberFor, >, @@ -301,16 +292,7 @@ impl, RuntimeApi, AuraId> Block, Arc>, ParachainClient, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + AuraCreateInherentDataProviders, AuraId::BoundedPair, NumberFor, >, @@ -330,16 +312,7 @@ where Block, Arc>, ParachainClient, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + AuraCreateInherentDataProviders, AuraId::BoundedPair, NumberFor, >, @@ -391,17 +364,7 @@ where ) }); async move { Ok((slot?, timestamp)) } - }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + }) as AuraCreateInherentDataProviders, block_import, para_client: client.clone(), para_backend: backend.clone(), @@ -446,16 +409,7 @@ where Block, Arc>, ParachainClient, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + AuraCreateInherentDataProviders, AuraId::BoundedPair, NumberFor, >; @@ -572,17 +526,7 @@ where ) }); async move { Ok((slot?, timestamp)) } - }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + }) as AuraCreateInherentDataProviders, block_import, para_client: client.clone(), para_backend: backend, diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 10317ec53e6be..6f728fbc00634 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -39,8 +39,7 @@ use cumulus_test_runtime::{Hash, Header, NodeBlock as Block, RuntimeApi}; use prometheus::Registry; use runtime::AccountId; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; -use sp_consensus_aura::sr25519::AuthorityPair; -use sp_inherents::CreateInherentDataProviders; +use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, sr25519::AuthorityPair}; use std::{ collections::HashSet, future::Future, @@ -140,16 +139,7 @@ pub type ParachainBlockImport = TParachainBlockImport< Block, Arc, Client, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + AuraCreateInherentDataProviders, AuthorityPair, NumberFor, >, @@ -251,17 +241,7 @@ pub fn new_partial( slot_duration, ); Ok((slot, timestamp)) - }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + }) as AuraCreateInherentDataProviders, true, Default::default(), ); @@ -289,17 +269,7 @@ pub fn new_partial( slot_duration, ); Ok((slot, timestamp)) - }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + }) as AuraCreateInherentDataProviders, spawner: &task_manager.spawn_essential_handle(), registry: None, telemetry: None, @@ -579,16 +549,7 @@ where }); async move { Ok((slot?, timestamp)) } }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + as AuraCreateInherentDataProviders, block_import, para_client: client.clone(), para_backend: backend.clone(), diff --git a/polkadot/node/service/src/builder/partial.rs b/polkadot/node/service/src/builder/partial.rs index 407c6e1659bdc..8d6cc77c963c1 100644 --- a/polkadot/node/service/src/builder/partial.rs +++ b/polkadot/node/service/src/builder/partial.rs @@ -29,8 +29,8 @@ use sc_service::{Configuration, Error as SubstrateServiceError, KeystoreContaine use sc_telemetry::{Telemetry, TelemetryWorker, TelemetryWorkerHandle}; use sc_transaction_pool_api::OffchainTransactionPoolFactory; use sp_consensus::SelectChain; +use sp_consensus_babe::inherents::BabeCreateInherentDataProvidersWithTimestamp; use sp_consensus_beefy::ecdsa_crypto; -use sp_inherents::CreateInherentDataProviders; use std::sync::Arc; type FullSelectChain = relay_chain_selection::SelectRelayChain; @@ -65,16 +65,7 @@ pub(crate) type PolkadotPartialComponents = sc_service::PartialC FullGrandpaBlockImport, ecdsa_crypto::AuthorityId, >, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_babe::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + BabeCreateInherentDataProvidersWithTimestamp, ChainSelection, >, sc_consensus_grandpa::LinkHalf, @@ -208,17 +199,7 @@ where slot_duration, ); Ok((slot, timestamp)) - }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_babe::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + }) as BabeCreateInherentDataProvidersWithTimestamp, select_chain.clone(), OffchainTransactionPoolFactory::new(transaction_pool.clone()), )?; @@ -237,16 +218,7 @@ where ); Ok((slot, timestamp)) }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_babe::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + as BabeCreateInherentDataProvidersWithTimestamp, spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), telemetry: telemetry.as_ref().map(|x| x.handle()), diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 7c2892e493bb8..3ca8756d85af9 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -22,7 +22,8 @@ use polkadot_sdk::{ sc_consensus_beefy as beefy, sc_consensus_grandpa as grandpa, - sp_consensus_beefy as beefy_primitives, sp_inherents::CreateInherentDataProviders, *, + sp_consensus_babe::inherents::BabeCreateInherentDataProvidersWithTimestamp, + sp_consensus_beefy as beefy_primitives, *, }; use crate::Cli; @@ -190,16 +191,7 @@ pub fn new_partial( Block, FullClient, FullBeefyBlockImport, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_babe::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + BabeCreateInherentDataProvidersWithTimestamp, FullSelectChain, >, grandpa::LinkHalf, @@ -284,17 +276,7 @@ pub fn new_partial( slot_duration, ); Ok((slot, timestamp)) - }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_babe::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + }) as BabeCreateInherentDataProvidersWithTimestamp, select_chain.clone(), OffchainTransactionPoolFactory::new(transaction_pool.clone()), )?; @@ -313,16 +295,7 @@ pub fn new_partial( ); Ok((slot, timestamp)) }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_babe::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + as BabeCreateInherentDataProvidersWithTimestamp, spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), telemetry: telemetry.as_ref().map(|x| x.handle()), @@ -446,16 +419,7 @@ pub fn new_full_base::Hash>>( Block, FullClient, FullBeefyBlockImport, - Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_babe::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + BabeCreateInherentDataProvidersWithTimestamp, FullSelectChain, >, &sc_consensus_babe::BabeLink, diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 964a9efd33622..2d8d0c7eda4c2 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -30,8 +30,8 @@ use sc_transaction_pool_api::RejectAllTxPool; use sp_application_crypto::key_types::BABE; use sp_consensus::{DisableProofRecording, NoNetwork as DummyOracle, Proposal}; use sp_consensus_babe::{ - inherents::InherentDataProvider, make_vrf_sign_data, AllowedSlots, AuthorityId, AuthorityPair, - Slot, + inherents::{BabeCreateInherentDataProviders, InherentDataProvider}, + make_vrf_sign_data, AllowedSlots, AuthorityId, AuthorityPair, Slot, }; use sp_consensus_slots::SlotDuration; use sp_core::crypto::Pair; @@ -69,13 +69,7 @@ type BabeBlockImport = PanickingBlockImport< TestBlock, TestClient, Arc, - Arc< - dyn CreateInherentDataProviders< - TestBlock, - (), - InherentDataProviders = (InherentDataProvider,), - >, - >, + BabeCreateInherentDataProviders, sc_consensus::LongestChain, >, >; @@ -249,14 +243,7 @@ impl TestNetFactory for BabeTestNet { SlotDuration::from_millis(SLOT_DURATION_MS), ); Ok((slot,)) - }) - as Arc< - dyn CreateInherentDataProviders< - TestBlock, - (), - InherentDataProviders = (InherentDataProvider,), - >, - >, + }) as BabeCreateInherentDataProviders, longest_chain, OffchainTransactionPoolFactory::new(RejectAllTxPool::default()), ) diff --git a/substrate/primitives/consensus/aura/src/inherents.rs b/substrate/primitives/consensus/aura/src/inherents.rs index 733081dd790a1..b85d9e0a616d9 100644 --- a/substrate/primitives/consensus/aura/src/inherents.rs +++ b/substrate/primitives/consensus/aura/src/inherents.rs @@ -24,6 +24,16 @@ pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"auraslot"; /// The type of the Aura inherent. pub type InherentType = sp_consensus_slots::Slot; +/// Create inherent data providers for Aura. +#[cfg(feature = "std")] +pub type AuraCreateInherentDataProviders = std::sync::Arc< + dyn sp_inherents::CreateInherentDataProviders< + Block, + (), + InherentDataProviders = (InherentDataProvider, sp_timestamp::InherentDataProvider), + >, +>; + /// Auxiliary trait to extract Aura inherent data. pub trait AuraInherentData { /// Get aura inherent data. diff --git a/substrate/primitives/consensus/babe/src/inherents.rs b/substrate/primitives/consensus/babe/src/inherents.rs index 54b7b64401673..9ff789038f473 100644 --- a/substrate/primitives/consensus/babe/src/inherents.rs +++ b/substrate/primitives/consensus/babe/src/inherents.rs @@ -24,6 +24,27 @@ pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"babeslot"; /// The type of the BABE inherent. pub type InherentType = sp_consensus_slots::Slot; + +/// Create inherent data providers for BABE with timestamp. +#[cfg(feature = "std")] +pub type BabeCreateInherentDataProvidersWithTimestamp = std::sync::Arc< + dyn sp_inherents::CreateInherentDataProviders< + Block, + (), + InherentDataProviders = (InherentDataProvider, sp_timestamp::InherentDataProvider), + >, +>; + +/// Create inherent data providers for BABE. +#[cfg(feature = "std")] +pub type BabeCreateInherentDataProviders = std::sync::Arc< + dyn sp_inherents::CreateInherentDataProviders< + Block, + (), + InherentDataProviders = (InherentDataProvider,), + >, +>; + /// Auxiliary trait to extract BABE inherent data. pub trait BabeInherentData { /// Get BABE inherent data. diff --git a/substrate/primitives/inherents/src/client_side.rs b/substrate/primitives/inherents/src/client_side.rs index c4b4db335dab1..32f4821bc34d3 100644 --- a/substrate/primitives/inherents/src/client_side.rs +++ b/substrate/primitives/inherents/src/client_side.rs @@ -26,8 +26,8 @@ use sp_runtime::traits::Block as BlockT; /// `ExtraArgs` generic parameter. /// /// The crate already provides some convince implementations of this trait for -/// `Box` and closures. So, it should not be required to implement -/// this trait manually. +/// `Box`, `Arc` and closures. So, +/// it should not be required to implement this trait manually. #[async_trait::async_trait] pub trait CreateInherentDataProviders: Send + Sync { /// The inherent data providers that will be created. diff --git a/templates/parachain/node/src/service.rs b/templates/parachain/node/src/service.rs index e867bf38eba45..65cf09638bd85 100644 --- a/templates/parachain/node/src/service.rs +++ b/templates/parachain/node/src/service.rs @@ -9,7 +9,7 @@ use parachain_template_runtime::{ opaque::{Block, Hash}, }; -use polkadot_sdk::{sp_inherents::CreateInherentDataProviders, *}; +use polkadot_sdk::{sp_consensus_aura::inherents::AuraCreateInherentDataProviders, *}; // Cumulus Imports use cumulus_client_bootnodes::{start_bootnode_tasks, StartBootnodeTasksParams}; @@ -217,17 +217,7 @@ fn start_consensus( ) }); async move { Ok((slot?, timestamp)) } - }) - as Arc< - dyn CreateInherentDataProviders< - Block, - (), - InherentDataProviders = ( - sp_consensus_aura::inherents::InherentDataProvider, - sp_timestamp::InherentDataProvider, - ), - >, - >, + }) as AuraCreateInherentDataProviders, block_import, para_client: client.clone(), para_backend: backend, From 8966cbf11a9cb4b62952fd2593dcda78146f83eb Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 23 May 2025 15:03:11 +0200 Subject: [PATCH 12/38] remove redundant type parameter --- substrate/client/consensus/common/src/import_queue.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/consensus/common/src/import_queue.rs b/substrate/client/consensus/common/src/import_queue.rs index 66cdb5b92068d..602683907d482 100644 --- a/substrate/client/consensus/common/src/import_queue.rs +++ b/substrate/client/consensus/common/src/import_queue.rs @@ -97,7 +97,7 @@ pub struct IncomingBlock { /// Verify a justification of a block #[async_trait::async_trait] -pub trait Verifier: Send + Sync { +pub trait Verifier: Send + Sync { /// Verify the given block data and return the `BlockImportParams` to /// continue the block import process. async fn verify(&self, block: BlockImportParams) -> Result, String>; From 484579ab6bbb0f5a3b2293b21a1cd095055552a9 Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 27 May 2025 13:26:22 +0200 Subject: [PATCH 13/38] improvements and oversights --- .../client/consensus/aura/src/collators/mod.rs | 15 ++++++--------- .../src/collators/slot_based/block_import.rs | 18 +++++++++--------- .../polkadot-omni-node/lib/src/nodes/aura.rs | 4 +++- substrate/client/consensus/aura/src/lib.rs | 1 + .../primitives/inherents/src/client_side.rs | 2 +- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs index 4290ff3631357..9e89c41f17630 100644 --- a/cumulus/client/consensus/aura/src/collators/mod.rs +++ b/cumulus/client/consensus/aura/src/collators/mod.rs @@ -371,10 +371,15 @@ where CIDP: sp_inherents::CreateInherentDataProviders + Send, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, { - // Check inherents. let slot = find_pre_digest::(¶ms.header) .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; let parent_hash = *params.header.parent_hash(); + let create_inherent_data_providers = create_inherent_data_providers + .create_inherent_data_providers(parent_hash, ()) + .await + .map_err(sp_consensus::Error::Other)?; + + // Check inherents. // if the body is passed through, we need to use the runtime // to check that the internally-set timestamp in the inherents // actually matches the slot set in the seal. @@ -385,10 +390,6 @@ where .has_api_with::, _>(parent_hash, |v| v >= 2) .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? { - let create_inherent_data_providers = create_inherent_data_providers - .create_inherent_data_providers(parent_hash, ()) - .await - .map_err(sp_consensus::Error::Other)?; let mut inherent_data = create_inherent_data_providers .create_inherent_data() .await @@ -420,10 +421,6 @@ where compatibility_mode, ) .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; - let create_inherent_data_providers = create_inherent_data_providers - .create_inherent_data_providers(parent_hash, ()) - .await - .map_err(sp_consensus::Error::Other)?; let slot_now = create_inherent_data_providers.slot(); let expected_author = sc_consensus_aura::standalone::slot_author::

(slot, &authorities); if let (true, Some(expected)) = (check_for_equivocation, expected_author) { diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs index 9df170fc964be..08538cbffee10 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs @@ -147,21 +147,21 @@ where &self, mut params: sc_consensus::BlockImportParams, ) -> Result { + validate_block_import::<_, _, P, _>( + &mut params, + self.client.as_ref(), + &self.create_inherent_data_providers, + self.check_for_equivocation, + &self.compatibility_mode, + ) + .await?; + // If the channel exists and it is required to execute the block, we will execute the block // here. This is done to collect the storage proof and to prevent re-execution, we push // downwards the state changes. `StateAction::ApplyChanges` is ignored, because it either // means that the node produced the block itself or the block was imported via state sync. if !self.sender.is_closed() && !matches!(params.state_action, StateAction::ApplyChanges(_)) { - validate_block_import::<_, _, P, _>( - &mut params, - self.client.as_ref(), - &self.create_inherent_data_providers, - self.check_for_equivocation, - &self.compatibility_mode, - ) - .await?; - let mut runtime_api = self.client.runtime_api(); runtime_api.set_call_context(CallContext::Onchain); diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 9df8e080d4914..411a6f667a44c 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -120,6 +120,8 @@ where telemetry_handle: Option, task_manager: &TaskManager, ) -> sc_service::error::Result> { + let inherent_data_providers = + move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }; let registry = config.prometheus_registry(); let spawner = task_manager.spawn_essential_handle(); @@ -128,7 +130,7 @@ where let equivocation_aura_verifier = EquivocationVerifier::::new( client.clone(), - move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }, + inherent_data_providers, telemetry_handle, ); diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 7acb42cd4b815..62fe15e088d69 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -503,6 +503,7 @@ impl From for Error { } } +/// Return AURA authorities at the given `parent_hash`. #[doc(hidden)] pub fn authorities( client: &C, diff --git a/substrate/primitives/inherents/src/client_side.rs b/substrate/primitives/inherents/src/client_side.rs index 32f4821bc34d3..bbbb72c0eda01 100644 --- a/substrate/primitives/inherents/src/client_side.rs +++ b/substrate/primitives/inherents/src/client_side.rs @@ -25,7 +25,7 @@ use sp_runtime::traits::Block as BlockT; /// It is possible for the caller to provide custom arguments to the callee by setting the /// `ExtraArgs` generic parameter. /// -/// The crate already provides some convince implementations of this trait for +/// The crate already provides some convenience implementations of this trait for /// `Box`, `Arc` and closures. So, /// it should not be required to implement this trait manually. #[async_trait::async_trait] From 9bc4f69c8dd12475329a8ab5614f8da9453bf00b Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 30 May 2025 15:39:02 +0200 Subject: [PATCH 14/38] improve logs --- substrate/client/network/sync/src/strategy/state.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/substrate/client/network/sync/src/strategy/state.rs b/substrate/client/network/sync/src/strategy/state.rs index a63e817e60892..7095113649ea8 100644 --- a/substrate/client/network/sync/src/strategy/state.rs +++ b/substrate/client/network/sync/src/strategy/state.rs @@ -271,8 +271,8 @@ impl StateStrategy { if !results.is_empty() { // We processed the target block - results.iter().filter_map(|result| result.as_ref().err()).for_each(|e| { - match e { + results.iter().filter_map(|result| result.as_ref().err()).for_each(|error| { + match error { BlockImportError::Other( ConsensusError::InvalidInherents(_) | ConsensusError::InvalidInherentsUnhandled(_), @@ -280,13 +280,15 @@ impl StateStrategy { // Ignore inherents errors. trace!( target: LOG_TARGET, - "Failed to import target block due to invalid inherent transactions: {e:?}." + "Failed to import target block due to invalid inherent transactions." + ?error, ); }, _ => { error!( target: LOG_TARGET, - "Failed to import target block with state: {e:?}." + "Failed to import target block with state." + ?error, ); }, } From f9c046ad558bde934a9076a1e75a9ce1caa17860 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 30 May 2025 16:09:28 +0200 Subject: [PATCH 15/38] fix --- substrate/client/network/sync/src/strategy/state.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/substrate/client/network/sync/src/strategy/state.rs b/substrate/client/network/sync/src/strategy/state.rs index 7095113649ea8..d19e99b6e541a 100644 --- a/substrate/client/network/sync/src/strategy/state.rs +++ b/substrate/client/network/sync/src/strategy/state.rs @@ -280,15 +280,13 @@ impl StateStrategy { // Ignore inherents errors. trace!( target: LOG_TARGET, - "Failed to import target block due to invalid inherent transactions." - ?error, + "Failed to import target block due to invalid inherent transactions: {error:?}." ); }, _ => { error!( target: LOG_TARGET, - "Failed to import target block with state." - ?error, + "Failed to import target block with state: {error:?}." ); }, } From fc83d6b7f90305477184f847b0ed28f3af8f84cd Mon Sep 17 00:00:00 2001 From: sistemd Date: Mon, 2 Jun 2025 23:43:22 +0200 Subject: [PATCH 16/38] remove ValidatingBlockImport --- .../consensus/aura/src/collators/basic.rs | 9 +- .../consensus/aura/src/collators/lookahead.rs | 9 +- .../consensus/aura/src/collators/mod.rs | 187 +----------------- .../src/collators/slot_based/block_import.rs | 89 ++++++++- 4 files changed, 86 insertions(+), 208 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index ee092cb60bca4..4eeb7d4be2bfa 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -23,7 +23,6 @@ //! //! For more information about AuRa, the Substrate crate should be checked. -use super::ValidatingBlockImport; use codec::{Codec, Decode}; use cumulus_client_collator::{ relay_chain_driven::CollationRequest, service::ServiceInterface as CollatorServiceInterface, @@ -131,13 +130,7 @@ where let mut collator = { let params = collator_util::Params { create_inherent_data_providers: params.create_inherent_data_providers.clone(), - block_import: ValidatingBlockImport::<_, _, _, _, P>::new( - params.block_import, - params.para_client.clone(), - params.create_inherent_data_providers.clone(), - true, - Default::default(), - ), + block_import: params.block_import, relay_client: params.relay_client.clone(), keystore: params.keystore.clone(), para_id: params.para_id, diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 7c4cb99bee876..0b20583048f84 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -32,7 +32,6 @@ //! The main limitation is block propagation time - i.e. the new blocks created by an author //! must be propagated to the next author before their turn. -use super::ValidatingBlockImport; use codec::{Codec, Encode}; use cumulus_client_collator::service::ServiceInterface as CollatorServiceInterface; use cumulus_client_consensus_common::{self as consensus_common, ParachainBlockImportMarker}; @@ -215,13 +214,7 @@ where let mut collator = { let params = collator_util::Params { create_inherent_data_providers: params.create_inherent_data_providers.clone(), - block_import: ValidatingBlockImport::<_, _, _, _, P>::new( - params.block_import, - params.para_client.clone(), - params.create_inherent_data_providers.clone(), - true, - Default::default(), - ), + block_import: params.block_import, relay_client: params.relay_client.clone(), keystore: params.keystore.clone(), para_id: params.para_id, diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs index 117deb98026ff..72fa7ad3dcdb8 100644 --- a/cumulus/client/consensus/aura/src/collators/mod.rs +++ b/cumulus/client/consensus/aura/src/collators/mod.rs @@ -21,14 +21,9 @@ //! included parachain block, as well as the [`lookahead`] collator, which prospectively //! builds on parachain blocks which have not yet been included in the relay chain. -use std::sync::Arc; - -use crate::{collator::SlotClaim, LOG_TARGET}; -use async_trait::async_trait; +use crate::collator::SlotClaim; use codec::Codec; -use cumulus_client_consensus_common::{ - self as consensus_common, ParachainBlockImportMarker, ParentSearchParams, -}; +use cumulus_client_consensus_common::{self as consensus_common, ParentSearchParams}; use cumulus_primitives_aura::{AuraUnincludedSegmentApi, Slot}; use cumulus_primitives_core::{relay_chain::Header as RelayHeader, BlockT, ClaimQueueOffset}; use cumulus_relay_chain_interface::RelayChainInterface; @@ -38,16 +33,10 @@ use polkadot_primitives::{ CoreIndex, Hash as RelayHash, Id as ParaId, OccupiedCoreAssumption, ValidationCodeHash, DEFAULT_SCHEDULING_LOOKAHEAD, }; -use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResult}; -use sc_consensus_aura::{find_pre_digest, standalone as aura_internal, AuraApi, CompatibilityMode}; -use sc_consensus_slots::InherentDataProviderExt; -use sp_api::{ApiExt, Core, ProvideRuntimeApi, RuntimeApiInfo}; -use sp_block_builder::BlockBuilder; -use sp_consensus_aura::inherents::AuraInherentData; +use sc_consensus_aura::{standalone as aura_internal, AuraApi}; +use sp_api::{ApiExt, ProvideRuntimeApi, RuntimeApiInfo}; use sp_core::Pair; -use sp_inherents::InherentDataProvider; use sp_keystore::KeystorePtr; -use sp_runtime::traits::{Header, NumberFor}; use sp_timestamp::Timestamp; pub mod basic; @@ -281,174 +270,6 @@ where .map(|parent| (included_block, parent)) } -struct ValidatingBlockImport { - inner: BI, - client: Arc, - create_inherent_data_providers: CIDP, - check_for_equivocation: bool, - compatibility_mode: CompatibilityMode, - _phantom: std::marker::PhantomData

, -} - -impl ValidatingBlockImport { - pub fn new( - inner: BI, - client: Arc, - create_inherent_data_providers: CIDP, - check_for_equivocation: bool, - compatibility_mode: CompatibilityMode, - ) -> Self { - Self { - inner, - client, - create_inherent_data_providers, - check_for_equivocation, - compatibility_mode, - _phantom: Default::default(), - } - } -} - -#[async_trait] -impl BlockImport - for ValidatingBlockImport, P> -where - Block: BlockT, - BI: BlockImport + Sync, - BI::Error: Into, - Client: ProvideRuntimeApi + sc_client_api::backend::AuxStore + Send + Sync, - Client::Api: Core + BlockBuilder + AuraApi::Public>, - P: Pair + Sync, - P::Public: Codec + std::fmt::Debug, - P::Signature: Codec, - CIDP: sp_inherents::CreateInherentDataProviders + Send, - CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, -{ - type Error = sp_consensus::Error; - - async fn check_block( - &self, - block: BlockCheckParams, - ) -> Result { - self.inner.check_block(block).await.map_err(Into::into) - } - - async fn import_block( - &self, - mut block: BlockImportParams, - ) -> Result { - validate_block_import::<_, _, P, _>( - &mut block, - self.client.as_ref(), - &self.create_inherent_data_providers, - self.check_for_equivocation, - &self.compatibility_mode, - ) - .await?; - self.inner.import_block(block).await.map_err(Into::into) - } -} - -impl ParachainBlockImportMarker - for ValidatingBlockImport -{ -} - -async fn validate_block_import( - params: &mut sc_consensus::BlockImportParams, - client: &Client, - create_inherent_data_providers: &CIDP, - check_for_equivocation: bool, - compatibility_mode: &CompatibilityMode>, -) -> Result<(), sp_consensus::Error> -where - Block: BlockT, - Client: ProvideRuntimeApi + sc_client_api::backend::AuxStore + Send + Sync, - Client::Api: Core + BlockBuilder + AuraApi::Public>, - P: Pair + Sync, - P::Public: Codec + std::fmt::Debug, - P::Signature: Codec, - CIDP: sp_inherents::CreateInherentDataProviders + Send, - CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, -{ - let slot = find_pre_digest::(¶ms.header) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; - let parent_hash = *params.header.parent_hash(); - let create_inherent_data_providers = create_inherent_data_providers - .create_inherent_data_providers(parent_hash, ()) - .await - .map_err(sp_consensus::Error::Other)?; - - // Check inherents. - // if the body is passed through, we need to use the runtime - // to check that the internally-set timestamp in the inherents - // actually matches the slot set in the seal. - if let Some(inner_body) = params.body.take() { - let new_block = Block::new(params.header.clone(), inner_body); - if client - .runtime_api() - .has_api_with::, _>(parent_hash, |v| v >= 2) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? - { - let mut inherent_data = create_inherent_data_providers - .create_inherent_data() - .await - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; - inherent_data.aura_replace_inherent_data(slot); - let inherent_res = client - .runtime_api() - .check_inherents(parent_hash, new_block.clone(), inherent_data) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; - - if !inherent_res.ok() { - for (i, e) in inherent_res.into_errors() { - match create_inherent_data_providers.try_handle_error(&i, &e).await { - Some(res) => res.map_err(|e| sp_consensus::Error::InvalidInherents(e))?, - None => return Err(sp_consensus::Error::InvalidInherentsUnhandled(i)), - } - } - } - } - let (_, inner_body) = new_block.deconstruct(); - params.body = Some(inner_body); - } - - // Check for equivocation. - let authorities = sc_consensus_aura::authorities::<

::Public, _, _>( - client, - parent_hash, - *params.header.number(), - compatibility_mode, - ) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; - let slot_now = create_inherent_data_providers.slot(); - let expected_author = sc_consensus_aura::standalone::slot_author::

(slot, &authorities); - if let (true, Some(expected)) = (check_for_equivocation, expected_author) { - if let Some(equivocation_proof) = sc_consensus_slots::check_equivocation( - client, - // we add one to allow for some small drift. - // FIXME #1019 in the future, alter this queue to allow deferring of - // headers - slot_now + 1, - slot, - ¶ms.header, - expected, - ) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? - { - tracing::info!( - target: LOG_TARGET, - "Slot author is equivocating at slot {} with headers {:?} and {:?}", - slot, - equivocation_proof.first_header.hash(), - equivocation_proof.second_header.hash(), - ); - } - } - - Ok(()) -} - #[cfg(test)] mod tests { use crate::collators::can_build_upon; diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs index 08538cbffee10..a98c6be9cd58e 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs @@ -18,18 +18,19 @@ use codec::Codec; use futures::{stream::FusedStream, StreamExt}; use sc_consensus::{BlockImport, StateAction}; -use sc_consensus_aura::CompatibilityMode; +use sc_consensus_aura::{find_pre_digest, CompatibilityMode}; use sc_consensus_slots::InherentDataProviderExt; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; use sp_api::{ApiExt, CallApiAt, CallContext, Core, ProvideRuntimeApi, StorageProof}; -use sp_block_builder::BlockBuilder as BlockBuilderApi; -use sp_consensus_aura::AuraApi; +use sp_block_builder::{BlockBuilder as BlockBuilderApi, BlockBuilder}; +use sp_consensus_aura::{inherents::AuraInherentData, AuraApi}; use sp_core::Pair; +use sp_inherents::InherentDataProvider; use sp_runtime::traits::{Block as BlockT, Header as _, NumberFor}; use sp_trie::proof_size_extension::ProofSizeExt; use std::sync::Arc; -use crate::collators::validate_block_import; +use crate::LOG_TARGET; /// Handle for receiving the block and the storage proof from the [`SlotBasedBlockImport`]. /// @@ -147,14 +148,84 @@ where &self, mut params: sc_consensus::BlockImportParams, ) -> Result { - validate_block_import::<_, _, P, _>( - &mut params, + let slot = find_pre_digest::(¶ms.header) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + let parent_hash = *params.header.parent_hash(); + let create_inherent_data_providers = self + .create_inherent_data_providers + .create_inherent_data_providers(parent_hash, ()) + .await + .map_err(sp_consensus::Error::Other)?; + + // Check inherents. + // if the body is passed through, we need to use the runtime + // to check that the internally-set timestamp in the inherents + // actually matches the slot set in the seal. + if let Some(inner_body) = params.body.take() { + let new_block = Block::new(params.header.clone(), inner_body); + if self + .client + .runtime_api() + .has_api_with::, _>(parent_hash, |v| v >= 2) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? + { + let mut inherent_data = create_inherent_data_providers + .create_inherent_data() + .await + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + inherent_data.aura_replace_inherent_data(slot); + let inherent_res = self + .client + .runtime_api() + .check_inherents(parent_hash, new_block.clone(), inherent_data) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + + if !inherent_res.ok() { + for (i, e) in inherent_res.into_errors() { + match create_inherent_data_providers.try_handle_error(&i, &e).await { + Some(res) => + res.map_err(|e| sp_consensus::Error::InvalidInherents(e))?, + None => return Err(sp_consensus::Error::InvalidInherentsUnhandled(i)), + } + } + } + } + let (_, inner_body) = new_block.deconstruct(); + params.body = Some(inner_body); + } + + // Check for equivocation. + let authorities = sc_consensus_aura::authorities::<

::Public, _, _>( self.client.as_ref(), - &self.create_inherent_data_providers, - self.check_for_equivocation, + parent_hash, + *params.header.number(), &self.compatibility_mode, ) - .await?; + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + let slot_now = create_inherent_data_providers.slot(); + let expected_author = sc_consensus_aura::standalone::slot_author::

(slot, &authorities); + if let (true, Some(expected)) = (self.check_for_equivocation, expected_author) { + if let Some(equivocation_proof) = sc_consensus_slots::check_equivocation( + self.client.as_ref(), + // we add one to allow for some small drift. + // FIXME #1019 in the future, alter this queue to allow deferring of + // headers + slot_now + 1, + slot, + ¶ms.header, + expected, + ) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? + { + tracing::info!( + target: LOG_TARGET, + "Slot author is equivocating at slot {} with headers {:?} and {:?}", + slot, + equivocation_proof.first_header.hash(), + equivocation_proof.second_header.hash(), + ); + } + } // If the channel exists and it is required to execute the block, we will execute the block // here. This is done to collect the storage proof and to prevent re-execution, we push From d3f0bc7f233badc5d18fca790b9e6e2efcc828b0 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 6 Jun 2025 16:47:13 +0200 Subject: [PATCH 17/38] remove outdated comment --- substrate/primitives/consensus/aura/src/inherents.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/primitives/consensus/aura/src/inherents.rs b/substrate/primitives/consensus/aura/src/inherents.rs index b85d9e0a616d9..6ec3927517087 100644 --- a/substrate/primitives/consensus/aura/src/inherents.rs +++ b/substrate/primitives/consensus/aura/src/inherents.rs @@ -53,7 +53,6 @@ impl AuraInherentData for InherentData { } /// Provides the slot duration inherent data for `Aura`. -// TODO: Remove in the future. https://github.com/paritytech/substrate/issues/8029 #[cfg(feature = "std")] pub struct InherentDataProvider { slot: InherentType, From a171009ad2d4d3448461051cb0ca856b6a65a70b Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 6 Jun 2025 16:47:13 +0200 Subject: [PATCH 18/38] remove unneeded BlockBuilder trait --- .../consensus/aura/src/collators/lookahead.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 0b20583048f84..e6b8ebbfebdb8 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -47,7 +47,6 @@ use polkadot_primitives::{ vstaging::DEFAULT_CLAIM_QUEUE_OFFSET, CollatorPair, Id as ParaId, OccupiedCoreAssumption, }; use sc_consensus_slots::InherentDataProviderExt; -use sp_block_builder::BlockBuilder; use crate::{collator as collator_util, export_pov_to_path}; use futures::prelude::*; @@ -116,10 +115,8 @@ where + Send + Sync + 'static, - Client::Api: BlockBuilder - + AuraApi - + CollectCollationInfo - + AuraUnincludedSegmentApi, + Client::Api: + AuraApi + CollectCollationInfo + AuraUnincludedSegmentApi, Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, CIDP: CreateInherentDataProviders + Clone + 'static, @@ -171,10 +168,8 @@ where + Send + Sync + 'static, - Client::Api: BlockBuilder - + AuraApi - + CollectCollationInfo - + AuraUnincludedSegmentApi, + Client::Api: + AuraApi + CollectCollationInfo + AuraUnincludedSegmentApi, Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, CIDP: CreateInherentDataProviders + Clone + 'static, From 2130887b84cacf73db63a156bf53982d05e57a21 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 6 Jun 2025 16:47:13 +0200 Subject: [PATCH 19/38] remove unnecessary InherentDataProviderExt constraint --- .../consensus/aura/src/collators/lookahead.rs | 5 ++--- .../primitives/consensus/aura/src/inherents.rs | 7 +++++-- templates/parachain/node/src/service.rs | 17 ++--------------- 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index e6b8ebbfebdb8..5ea75fa006d13 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -46,7 +46,6 @@ use polkadot_overseer::Handle as OverseerHandle; use polkadot_primitives::{ vstaging::DEFAULT_CLAIM_QUEUE_OFFSET, CollatorPair, Id as ParaId, OccupiedCoreAssumption, }; -use sc_consensus_slots::InherentDataProviderExt; use crate::{collator as collator_util, export_pov_to_path}; use futures::prelude::*; @@ -120,7 +119,7 @@ where Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, CIDP: CreateInherentDataProviders + Clone + 'static, - CIDP::InherentDataProviders: InherentDataProviderExt + Send, + CIDP::InherentDataProviders: Send, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, BI::Error: Into, Proposer: ProposerInterface + Send + Sync + 'static, @@ -173,7 +172,7 @@ where Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, CIDP: CreateInherentDataProviders + Clone + 'static, - CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, + CIDP::InherentDataProviders: Send + Sync, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, BI::Error: Into, Proposer: ProposerInterface + Send + Sync + 'static, diff --git a/substrate/primitives/consensus/aura/src/inherents.rs b/substrate/primitives/consensus/aura/src/inherents.rs index 6ec3927517087..adea363936fa0 100644 --- a/substrate/primitives/consensus/aura/src/inherents.rs +++ b/substrate/primitives/consensus/aura/src/inherents.rs @@ -26,11 +26,14 @@ pub type InherentType = sp_consensus_slots::Slot; /// Create inherent data providers for Aura. #[cfg(feature = "std")] -pub type AuraCreateInherentDataProviders = std::sync::Arc< +pub type AuraCreateInherentDataProviders< + Block, + InherentDataProviders = (InherentDataProvider, sp_timestamp::InherentDataProvider), +> = std::sync::Arc< dyn sp_inherents::CreateInherentDataProviders< Block, (), - InherentDataProviders = (InherentDataProvider, sp_timestamp::InherentDataProvider), + InherentDataProviders = InherentDataProviders, >, >; diff --git a/templates/parachain/node/src/service.rs b/templates/parachain/node/src/service.rs index 65cf09638bd85..18696bb426255 100644 --- a/templates/parachain/node/src/service.rs +++ b/templates/parachain/node/src/service.rs @@ -202,22 +202,9 @@ fn start_consensus( client.clone(), ); - let client_for_closure = client.clone(); let params = AuraParams { - create_inherent_data_providers: Arc::new(move |parent, _| { - let slot_duration = sc_consensus_aura::standalone::slot_duration_at( - client_for_closure.as_ref(), - parent, - ); - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - let slot = slot_duration.map(|slot_duration| { - sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - slot_duration, - ) - }); - async move { Ok((slot?, timestamp)) } - }) as AuraCreateInherentDataProviders, + create_inherent_data_providers: Arc::new(move |_, ()| async move { Ok(()) }) + as AuraCreateInherentDataProviders, block_import, para_client: client.clone(), para_backend: backend, From 4e9a465123155ee7c7f27c5b906c84adcd7a1574 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 6 Jun 2025 16:47:14 +0200 Subject: [PATCH 20/38] use simple callback for AuraVerifier instead of CIDP --- .../client/consensus/aura/src/import_queue.rs | 38 ++++----- cumulus/test/service/src/lib.rs | 15 ++-- .../client/consensus/aura/src/import_queue.rs | 78 +++++++------------ templates/solochain/node/src/service.rs | 29 +++---- 4 files changed, 61 insertions(+), 99 deletions(-) diff --git a/cumulus/client/consensus/aura/src/import_queue.rs b/cumulus/client/consensus/aura/src/import_queue.rs index c2576b7731c26..c7107bc0f30c9 100644 --- a/cumulus/client/consensus/aura/src/import_queue.rs +++ b/cumulus/client/consensus/aura/src/import_queue.rs @@ -23,26 +23,24 @@ use prometheus_endpoint::Registry; use sc_client_api::{backend::AuxStore, BlockOf, UsageProvider}; use sc_consensus::{import_queue::DefaultImportQueue, BlockImport}; use sc_consensus_aura::{AuraVerifier, CompatibilityMode}; -use sc_consensus_slots::InherentDataProviderExt; use sc_telemetry::TelemetryHandle; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_block_builder::BlockBuilder as BlockBuilderApi; use sp_blockchain::HeaderBackend; use sp_consensus::Error as ConsensusError; -use sp_consensus_aura::AuraApi; +use sp_consensus_aura::{AuraApi, Slot}; use sp_core::crypto::Pair; -use sp_inherents::CreateInherentDataProviders; use sp_runtime::traits::Block as BlockT; use std::{fmt::Debug, sync::Arc}; /// Parameters for [`import_queue`]. -pub struct ImportQueueParams<'a, I, C, CIDP, S> { +pub struct ImportQueueParams<'a, I, C, GetSlotFn, S> { /// The block import to use. pub block_import: I, /// The client to interact with the chain. pub client: Arc, - /// The inherent data providers, to create the inherent data. - pub create_inherent_data_providers: CIDP, + /// Callback to get the current slot. + pub get_slot: GetSlotFn, /// The spawner to spawn background tasks. pub spawner: &'a S, /// The prometheus registry. @@ -52,15 +50,15 @@ pub struct ImportQueueParams<'a, I, C, CIDP, S> { } /// Start an import queue for the Aura consensus algorithm. -pub fn import_queue( +pub fn import_queue( ImportQueueParams { block_import, client, - create_inherent_data_providers, + get_slot, spawner, registry, telemetry, - }: ImportQueueParams<'_, I, C, CIDP, S>, + }: ImportQueueParams<'_, I, C, GetSlotFn, S>, ) -> Result, sp_consensus::Error> where Block: BlockT, @@ -82,14 +80,13 @@ where P::Public: Debug + Codec, P::Signature: Codec, S: sp_core::traits::SpawnEssentialNamed, - CIDP: CreateInherentDataProviders + Sync + Send + 'static, - CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, + GetSlotFn: Fn(Block::Hash) -> sp_blockchain::Result + Send + Sync + 'static, { sc_consensus_aura::import_queue::(sc_consensus_aura::ImportQueueParams { block_import, justification_import: None, client, - create_inherent_data_providers, + get_slot, spawner, registry, telemetry, @@ -98,25 +95,22 @@ where } /// Parameters of [`build_verifier`]. -pub struct BuildVerifierParams { +pub struct BuildVerifierParams { /// The client to interact with the chain. pub client: Arc, - /// The inherent data providers, to create the inherent data. - pub create_inherent_data_providers: CIDP, + /// Something that can get the current slot. + pub get_slot: GetSlotFn, /// The telemetry handle. pub telemetry: Option, } /// Build the [`AuraVerifier`]. -pub fn build_verifier( - BuildVerifierParams { client, create_inherent_data_providers, telemetry }: BuildVerifierParams< - C, - CIDP, - >, -) -> AuraVerifier { +pub fn build_verifier( + BuildVerifierParams { client, get_slot, telemetry }: BuildVerifierParams, +) -> AuraVerifier { sc_consensus_aura::build_verifier(sc_consensus_aura::BuildVerifierParams { client, - create_inherent_data_providers, + get_slot, telemetry, compatibility_mode: CompatibilityMode::None, }) diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index fbbe47a6adda3..7ef7c45e736c7 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -39,7 +39,7 @@ use cumulus_test_runtime::{Hash, Header, NodeBlock as Block, RuntimeApi}; use prometheus::Registry; use runtime::AccountId; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; -use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, sr25519::AuthorityPair}; +use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, sr25519::AuthorityPair, Slot}; use std::{ collections::HashSet, future::Future, @@ -262,14 +262,11 @@ pub fn new_partial( ImportQueueParams { block_import: block_import.clone(), client: client.clone(), - create_inherent_data_providers: Arc::new(move |_, _| async move { - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - let slot = sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - slot_duration, - ); - Ok((slot, timestamp)) - }) as AuraCreateInherentDataProviders, + get_slot: move |_| { + let timestamp = sp_timestamp::Timestamp::current(); + let slot = Slot::from_timestamp(timestamp, slot_duration); + Ok(slot) + }, spawner: &task_manager.spawn_essential_handle(), registry: None, telemetry: None, diff --git a/substrate/client/consensus/aura/src/import_queue.rs b/substrate/client/consensus/aura/src/import_queue.rs index 3dc157f7eb835..e72efc80d636f 100644 --- a/substrate/client/consensus/aura/src/import_queue.rs +++ b/substrate/client/consensus/aura/src/import_queue.rs @@ -30,7 +30,7 @@ use sc_consensus::{ block_import::{BlockImport, BlockImportParams, ForkChoiceStrategy}, import_queue::{BasicQueue, BoxJustificationImport, DefaultImportQueue, Verifier}, }; -use sc_consensus_slots::{CheckedHeader, InherentDataProviderExt}; +use sc_consensus_slots::CheckedHeader; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE}; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_block_builder::BlockBuilder as BlockBuilderApi; @@ -39,7 +39,6 @@ use sp_consensus::Error as ConsensusError; use sp_consensus_aura::AuraApi; use sp_consensus_slots::Slot; use sp_core::crypto::Pair; -use sp_inherents::CreateInherentDataProviders; use sp_runtime::{ traits::{Block as BlockT, Header, NumberFor}, DigestItem, @@ -78,41 +77,34 @@ where } /// A verifier for Aura blocks. -pub struct AuraVerifier { +pub struct AuraVerifier { client: Arc, - create_inherent_data_providers: CIDP, + get_slot: GetSlotFn, telemetry: Option, compatibility_mode: CompatibilityMode, _phantom: PhantomData P>, } -impl AuraVerifier { +impl AuraVerifier { pub(crate) fn new( client: Arc, - create_inherent_data_providers: CIDP, + get_slot: GetSlotFn, telemetry: Option, compatibility_mode: CompatibilityMode, ) -> Self { - Self { - client, - create_inherent_data_providers, - telemetry, - compatibility_mode, - _phantom: PhantomData, - } + Self { client, get_slot, telemetry, compatibility_mode, _phantom: PhantomData } } } #[async_trait::async_trait] -impl Verifier for AuraVerifier> +impl Verifier for AuraVerifier> where C: ProvideRuntimeApi + Send + Sync + sc_client_api::backend::AuxStore, C::Api: BlockBuilderApi + AuraApi> + ApiExt, P: Pair, P::Public: Codec + Debug, P::Signature: Codec, - CIDP: CreateInherentDataProviders + Send + Sync, - CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, + GetSlotFn: Fn(B::Hash) -> sp_blockchain::Result + Send + Sync, { async fn verify( &self, @@ -140,13 +132,8 @@ where ) .map_err(|e| format!("Could not fetch authorities at {:?}: {}", parent_hash, e))?; - let create_inherent_data_providers = self - .create_inherent_data_providers - .create_inherent_data_providers(parent_hash, ()) - .await - .map_err(|e| Error::::Client(sp_blockchain::Error::Application(e)))?; - - let slot_now = create_inherent_data_providers.slot(); + let slot_now = (self.get_slot)(parent_hash) + .map_err(|e| format!("Could not get slot for parent hash {:?}: {}", parent_hash, e))?; // we add one to allow for some small drift. // FIXME #1019 in the future, alter this queue to allow deferring of @@ -188,15 +175,15 @@ where } /// Parameters of [`import_queue`]. -pub struct ImportQueueParams<'a, Block: BlockT, I, C, S, CIDP> { +pub struct ImportQueueParams<'a, Block: BlockT, I, C, S, GetSlotFn> { /// The block import to use. pub block_import: I, /// The justification import. pub justification_import: Option>, /// The client to interact with the chain. pub client: Arc, - /// Something that can create the inherent data providers. - pub create_inherent_data_providers: CIDP, + /// Something that can get the current slot. + pub get_slot: GetSlotFn, /// The spawner to spawn background tasks. pub spawner: &'a S, /// The prometheus registry. @@ -210,17 +197,17 @@ pub struct ImportQueueParams<'a, Block: BlockT, I, C, S, CIDP> { } /// Start an import queue for the Aura consensus algorithm. -pub fn import_queue( +pub fn import_queue( ImportQueueParams { block_import, justification_import, client, - create_inherent_data_providers, + get_slot, spawner, registry, telemetry, compatibility_mode, - }: ImportQueueParams, + }: ImportQueueParams, ) -> Result, sp_consensus::Error> where Block: BlockT, @@ -238,12 +225,11 @@ where P::Public: Codec + Debug, P::Signature: Codec, S: sp_core::traits::SpawnEssentialNamed, - CIDP: CreateInherentDataProviders + Sync + Send + 'static, - CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, + GetSlotFn: Fn(Block::Hash) -> sp_blockchain::Result + Send + Sync + 'static, { let verifier = build_verifier::(BuildVerifierParams { client, - create_inherent_data_providers, + get_slot, telemetry, compatibility_mode, }); @@ -252,11 +238,11 @@ where } /// Parameters of [`build_verifier`]. -pub struct BuildVerifierParams { +pub struct BuildVerifierParams { /// The client to interact with the chain. pub client: Arc, - /// Something that can create the inherent data providers. - pub create_inherent_data_providers: CIDP, + /// Something that can get the current slot. + pub get_slot: GetSlotFn, /// Telemetry instance used to report telemetry metrics. pub telemetry: Option, /// Compatibility mode that should be used. @@ -266,18 +252,12 @@ pub struct BuildVerifierParams { } /// Build the [`AuraVerifier`] -pub fn build_verifier( - BuildVerifierParams { - client, - create_inherent_data_providers, - telemetry, - compatibility_mode, - }: BuildVerifierParams, -) -> AuraVerifier { - AuraVerifier::<_, P, _, _>::new( - client, - create_inherent_data_providers, - telemetry, - compatibility_mode, - ) +pub fn build_verifier( + BuildVerifierParams { client, get_slot, telemetry, compatibility_mode }: BuildVerifierParams< + C, + GetSlotFn, + N, + >, +) -> AuraVerifier { + AuraVerifier::<_, P, _, _>::new(client, get_slot, telemetry, compatibility_mode) } diff --git a/templates/solochain/node/src/service.rs b/templates/solochain/node/src/service.rs index e6411db42aa78..314d1298df04a 100644 --- a/templates/solochain/node/src/service.rs +++ b/templates/solochain/node/src/service.rs @@ -8,7 +8,7 @@ use sc_service::{error::Error as ServiceError, Configuration, TaskManager, WarpS use sc_telemetry::{Telemetry, TelemetryWorker}; use sc_transaction_pool_api::OffchainTransactionPoolFactory; use solochain_template_runtime::{self, apis::RuntimeApi, opaque::Block}; -use sp_consensus_aura::sr25519::AuthorityPair as AuraPair; +use sp_consensus_aura::{sr25519::AuthorityPair as AuraPair, Slot}; use std::{sync::Arc, time::Duration}; pub(crate) type FullClient = sc_service::TFullClient< @@ -83,29 +83,20 @@ pub fn new_partial(config: &Configuration) -> Result { telemetry.as_ref().map(|x| x.handle()), )?; - let cidp_client = client.clone(); + let get_slot_client = client.clone(); let import_queue = sc_consensus_aura::import_queue::(ImportQueueParams { block_import: grandpa_block_import.clone(), justification_import: Some(Box::new(grandpa_block_import.clone())), client: client.clone(), - create_inherent_data_providers: move |parent_hash, _| { - let cidp_client = cidp_client.clone(); - async move { - let slot_duration = sc_consensus_aura::standalone::slot_duration_at( - &*cidp_client, - parent_hash, - )?; - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - - let slot = - sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - slot_duration, - ); - - Ok((slot, timestamp)) - } + get_slot: move |parent_hash| { + let slot_duration = sc_consensus_aura::standalone::slot_duration_at( + &*get_slot_client, + parent_hash, + )?; + let timestamp = sp_timestamp::Timestamp::current(); + let slot = Slot::from_timestamp(timestamp, slot_duration); + Ok(slot) }, spawner: &task_manager.spawn_essential_handle(), registry: config.prometheus_registry(), From c1c9e97616ad83baf1c0acbe74b870955e88a091 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 6 Jun 2025 18:06:12 +0200 Subject: [PATCH 21/38] fix compiler errors --- cumulus/test/service/src/lib.rs | 2 +- substrate/client/consensus/aura/src/lib.rs | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 27d226fe634c8..58b834cb82ff4 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -71,7 +71,7 @@ use frame_system_rpc_runtime_api::AccountNonceApi; use polkadot_node_subsystem::{errors::RecoveryError, messages::AvailabilityRecoveryMessage}; use polkadot_overseer::Handle as OverseerHandle; use polkadot_primitives::{CandidateHash, CollatorPair, Hash as PHash, PersistedValidationData}; -use polkadot_service::{IdentifyNetworkBackend, NumberFor, ProvideRuntimeApi}; +use polkadot_service::{NumberFor, ProvideRuntimeApi}; use sc_consensus::ImportQueue; use sc_network::{ config::{FullNetworkConfiguration, TransportConfig}, diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 62fe15e088d69..1bc9f5e6f72fc 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -626,11 +626,9 @@ mod tests { PeersFullClient, AuthorityPair, Box< - dyn CreateInherentDataProviders< - TestBlock, - (), - InherentDataProviders = (InherentDataProvider,), - >, + dyn Fn(::Hash) -> sp_blockchain::Result + + Send + + Sync, >, u64, >; @@ -653,12 +651,11 @@ mod tests { assert_eq!(slot_duration.as_millis() as u64, SLOT_DURATION_MS); import_queue::AuraVerifier::new( client, - Box::new(|_, _| async { - let slot = InherentDataProvider::from_timestamp_and_slot_duration( + Box::new(|_| { + Ok(Slot::from_timestamp( Timestamp::current(), SlotDuration::from_millis(SLOT_DURATION_MS), - ); - Ok((slot,)) + )) }), None, CompatibilityMode::None, From 40fd334ac77f6cf3a1315db2ada1213ac2e6c5b2 Mon Sep 17 00:00:00 2001 From: sistemd Date: Sat, 7 Jun 2025 00:32:40 +0200 Subject: [PATCH 22/38] update prdoc --- prdoc/pr_8446.prdoc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/prdoc/pr_8446.prdoc b/prdoc/pr_8446.prdoc index d2684141f635b..3046b0f52790f 100644 --- a/prdoc/pr_8446.prdoc +++ b/prdoc/pr_8446.prdoc @@ -11,18 +11,22 @@ crates: - name: polkadot-omni-node-lib bump: minor - name: polkadot-service - bump: major + bump: patch - name: sc-consensus-aura bump: major - name: sc-consensus-babe bump: major - name: sc-consensus-babe-rpc bump: patch -- name: sc-consensus - bump: patch - name: sc-consensus-slots bump: patch - name: sc-network-sync bump: patch - name: sp-consensus bump: major +- name: sp-inherents + bump: minor +- name: sp-consensus-aura + bump: minor +- name: sp-consensus-babe + bump: minor From ac54928d0a9fccd3f5984854af10d6fcd7498a0b Mon Sep 17 00:00:00 2001 From: sistemd Date: Mon, 9 Jun 2025 22:15:36 +0200 Subject: [PATCH 23/38] remove BlockBuilder constraint --- cumulus/client/consensus/aura/src/collators/basic.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index 4eeb7d4be2bfa..90669501d7f61 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -42,7 +42,6 @@ use sc_consensus::BlockImport; use sc_consensus_slots::InherentDataProviderExt; use sp_api::{CallApiAt, ProvideRuntimeApi}; use sp_application_crypto::AppPublic; -use sp_block_builder::BlockBuilder; use sp_blockchain::HeaderBackend; use sp_consensus_aura::AuraApi; use sp_core::crypto::Pair; @@ -103,7 +102,7 @@ where + Send + Sync + 'static, - Client::Api: BlockBuilder + AuraApi + CollectCollationInfo, + Client::Api: AuraApi + CollectCollationInfo, RClient: RelayChainInterface + Send + Clone + 'static, CIDP: CreateInherentDataProviders + Clone + Send + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, From b8b9b72200a77cc18faddff64923ea05a6b5eeda Mon Sep 17 00:00:00 2001 From: sistemd Date: Mon, 9 Jun 2025 22:18:55 +0200 Subject: [PATCH 24/38] remove doc hidden --- substrate/client/consensus/aura/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 1bc9f5e6f72fc..5e8617044f54f 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -504,7 +504,6 @@ impl From for Error { } /// Return AURA authorities at the given `parent_hash`. -#[doc(hidden)] pub fn authorities( client: &C, parent_hash: B::Hash, From 2d800c0c4a15cce1c66f8f0baaa1826e54a76a64 Mon Sep 17 00:00:00 2001 From: sistemd Date: Mon, 9 Jun 2025 22:19:59 +0200 Subject: [PATCH 25/38] update prdoc --- prdoc/pr_8446.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_8446.prdoc b/prdoc/pr_8446.prdoc index 3046b0f52790f..f3a555d16c914 100644 --- a/prdoc/pr_8446.prdoc +++ b/prdoc/pr_8446.prdoc @@ -1,6 +1,6 @@ title: check inherents and equivocations in `import_block` doc: -- audience: Node Operator +- audience: ["Node Operator", "Node Dev"] description: |- When importing blocks, there are essentially two steps: `verify` and `import_block`. With https://github.com/paritytech/polkadot-sdk/issues/65, we would like to re-broadcast blocks between these two steps (i.e. basically right after `verify`). From 50c8e9c15c69dd4162c7fd63f754a5e9c25a591e Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 11 Jun 2025 15:42:58 +0200 Subject: [PATCH 26/38] remove unneeded constraints --- cumulus/client/consensus/aura/src/collators/basic.rs | 5 ++--- cumulus/client/consensus/aura/src/collators/lookahead.rs | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index 90669501d7f61..4bf1cc982bcb6 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -105,12 +105,11 @@ where Client::Api: AuraApi + CollectCollationInfo, RClient: RelayChainInterface + Send + Clone + 'static, CIDP: CreateInherentDataProviders + Clone + Send + 'static, - CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync, + CIDP::InherentDataProviders: Send, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, - BI::Error: Into, Proposer: ProposerInterface + Send + Sync + 'static, CS: CollatorServiceInterface + Send + Sync + 'static, - P: Pair + Sync + Send + 'static, + P: Pair, P::Public: AppPublic + Member + Codec, P::Signature: TryFrom> + Member + Codec, { diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 5ea75fa006d13..24da945afba86 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -172,13 +172,12 @@ where Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, CIDP: CreateInherentDataProviders + Clone + 'static, - CIDP::InherentDataProviders: Send + Sync, + CIDP::InherentDataProviders: Send, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, - BI::Error: Into, Proposer: ProposerInterface + Send + Sync + 'static, CS: CollatorServiceInterface + Send + Sync + 'static, CHP: consensus_common::ValidationCodeHashProvider + Send + 'static, - P: Pair + Send + Sync + 'static, + P: Pair, P::Public: AppPublic + Member + Codec, P::Signature: TryFrom> + Member + Codec, { From 85663469e9451a0feedb447126af3f28dac39d91 Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 11 Jun 2025 16:59:11 +0200 Subject: [PATCH 27/38] fix warning --- cumulus/client/consensus/aura/src/collators/basic.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index 4bf1cc982bcb6..56d836b3c325e 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -39,7 +39,6 @@ use polkadot_primitives::{CollatorPair, Id as ParaId, ValidationCode}; use futures::{channel::mpsc::Receiver, prelude::*}; use sc_client_api::{backend::AuxStore, BlockBackend, BlockOf}; use sc_consensus::BlockImport; -use sc_consensus_slots::InherentDataProviderExt; use sp_api::{CallApiAt, ProvideRuntimeApi}; use sp_application_crypto::AppPublic; use sp_blockchain::HeaderBackend; From 7a28d3301ee6f054939899f31861f640f23cd533 Mon Sep 17 00:00:00 2001 From: sistemd Date: Thu, 12 Jun 2025 23:08:16 +0200 Subject: [PATCH 28/38] add AuraBlockImport for polkadot-omni-node --- Cargo.lock | 3 + .../aura/src/equivocation_import_queue.rs | 238 +-------------- cumulus/polkadot-omni-node/lib/Cargo.toml | 3 + .../polkadot-omni-node/lib/src/nodes/aura.rs | 285 +++++++++++++++++- templates/parachain/node/src/service.rs | 5 - 5 files changed, 296 insertions(+), 238 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 38b376133084e..0d786b555cc94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15714,6 +15714,8 @@ dependencies = [ "cumulus-primitives-aura", "cumulus-primitives-core", "cumulus-relay-chain-interface", + "cumulus-test-client", + "cumulus-test-relay-sproof-builder", "cumulus-test-runtime", "docify", "frame-benchmarking", @@ -15753,6 +15755,7 @@ dependencies = [ "sc-transaction-pool", "sc-transaction-pool-api", "scale-info", + "schnellru", "serde", "serde_json", "sp-api 26.0.0", diff --git a/cumulus/client/consensus/aura/src/equivocation_import_queue.rs b/cumulus/client/consensus/aura/src/equivocation_import_queue.rs index 748e11bc45d03..429ca9a8a2aa7 100644 --- a/cumulus/client/consensus/aura/src/equivocation_import_queue.rs +++ b/cumulus/client/consensus/aura/src/equivocation_import_queue.rs @@ -22,65 +22,28 @@ /// should be thrown out and which ones should be kept. use codec::Codec; use cumulus_client_consensus_common::ParachainBlockImportMarker; -use parking_lot::Mutex; -use polkadot_primitives::Hash as RHash; use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, BlockImport, BlockImportParams, ForkChoiceStrategy, }; use sc_consensus_aura::standalone as aura_internal; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE}; -use schnellru::{ByLength, LruMap}; use sp_api::ProvideRuntimeApi; use sp_block_builder::BlockBuilder as BlockBuilderApi; -use sp_consensus::{error::Error as ConsensusError, BlockOrigin}; +use sp_consensus::error::Error as ConsensusError; use sp_consensus_aura::{AuraApi, Slot, SlotDuration}; use sp_core::crypto::Pair; -use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor}; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use std::{fmt::Debug, sync::Arc}; -const LRU_WINDOW: u32 = 512; -const EQUIVOCATION_LIMIT: usize = 16; - -struct NaiveEquivocationDefender { - /// We distinguish blocks by `(Slot, BlockNumber, RelayParent)`. - cache: LruMap<(u64, N, RHash), usize>, -} - -impl Default for NaiveEquivocationDefender { - fn default() -> Self { - NaiveEquivocationDefender { cache: LruMap::new(ByLength::new(LRU_WINDOW)) } - } -} - -impl NaiveEquivocationDefender { - // Returns `true` if equivocation is beyond the limit. - fn insert_and_check(&mut self, slot: Slot, block_number: N, relay_chain_parent: RHash) -> bool { - let val = self - .cache - .get_or_insert((*slot, block_number, relay_chain_parent), || 0) - .expect("insertion with ByLength limiter always succeeds; qed"); - - if *val == EQUIVOCATION_LIMIT { - true - } else { - *val += 1; - false - } - } -} - -/// A parachain block import verifier that checks for equivocation limits within each slot. -pub struct Verifier { +/// A parachain block import verifier that checks the header slot and seal. +pub struct Verifier { client: Arc, - create_inherent_data_providers: CIDP, - defender: Mutex>>, telemetry: Option, _phantom: std::marker::PhantomData (Block, P)>, } -impl Verifier +impl Verifier where P: Pair, P::Signature: Codec, @@ -88,28 +51,16 @@ where Block: BlockT, Client: ProvideRuntimeApi + Send + Sync, >::Api: BlockBuilderApi + AuraApi, - - CIDP: CreateInherentDataProviders, { /// Creates a new Verifier instance for handling parachain block import verification in Aura /// consensus. - pub fn new( - client: Arc, - inherent_data_provider: CIDP, - telemetry: Option, - ) -> Self { - Self { - client, - create_inherent_data_providers: inherent_data_provider, - defender: Mutex::new(NaiveEquivocationDefender::default()), - telemetry, - _phantom: std::marker::PhantomData, - } + pub fn new(client: Arc, telemetry: Option) -> Self { + Self { client, telemetry, _phantom: std::marker::PhantomData } } } #[async_trait::async_trait] -impl VerifierT for Verifier +impl VerifierT for Verifier where P: Pair, P::Signature: Codec, @@ -117,8 +68,6 @@ where Block: BlockT, Client: ProvideRuntimeApi + Send + Sync, >::Api: BlockBuilderApi + AuraApi, - - CIDP: CreateInherentDataProviders, { async fn verify( &self, @@ -157,7 +106,7 @@ where ); match res { - Ok((pre_header, slot, seal_digest)) => { + Ok((pre_header, _, seal_digest)) => { telemetry!( self.telemetry; CONSENSUS_TRACE; @@ -165,38 +114,10 @@ where "pre_header" => ?pre_header, ); - // We need some kind of identifier for the relay parent, in the worst case we - // take the all `0` hash. - let relay_parent = - cumulus_primitives_core::rpsr_digest::extract_relay_parent_storage_root( - pre_header.digest(), - ) - .map(|r| r.0) - .unwrap_or_else(|| { - cumulus_primitives_core::extract_relay_parent(pre_header.digest()) - .unwrap_or_default() - }); - block_params.header = pre_header; block_params.post_digests.push(seal_digest); block_params.fork_choice = Some(ForkChoiceStrategy::LongestChain); block_params.post_hash = Some(post_hash); - - // Check for and reject egregious amounts of equivocations. - // - // If the `origin` is `ConsensusBroadcast`, we ignore the result of the - // equivocation check. This `origin` is for example used by pov-recovery. - if self.defender.lock().insert_and_check( - slot, - *block_params.header.number(), - relay_parent, - ) && !matches!(block_params.origin, BlockOrigin::ConsensusBroadcast) - { - return Err(format!( - "Rejecting block {:?} due to excessive equivocations at slot", - post_hash, - )) - } }, Err(aura_internal::SealVerificationError::Deferred(hdr, slot)) => { telemetry!( @@ -221,40 +142,6 @@ where } } - // Check inherents. - if let Some(body) = block_params.body.clone() { - let block = Block::new(block_params.header.clone(), body); - let create_inherent_data_providers = self - .create_inherent_data_providers - .create_inherent_data_providers(parent_hash, ()) - .await - .map_err(|e| format!("Could not create inherent data {:?}", e))?; - - let inherent_data = create_inherent_data_providers - .create_inherent_data() - .await - .map_err(|e| format!("Could not create inherent data {:?}", e))?; - - let inherent_res = self - .client - .runtime_api() - .check_inherents(parent_hash, block, inherent_data) - .map_err(|e| format!("Unable to check block inherents {:?}", e))?; - - if !inherent_res.ok() { - for (i, e) in inherent_res.into_errors() { - match create_inherent_data_providers.try_handle_error(&i, &e).await { - Some(res) => res.map_err(|e| format!("Inherent Error {:?}", e))?, - None => - return Err(format!( - "Unknown inherent error, source {:?}", - String::from_utf8_lossy(&i[..]) - )), - } - } - } - } - Ok(block_params) } } @@ -264,19 +151,18 @@ fn slot_now(slot_duration: SlotDuration) -> Slot { Slot::from_timestamp(timestamp, slot_duration) } -/// Start an import queue for a Cumulus node which checks blocks' seals and inherent data. +/// Start an import queue for a Cumulus node which checks blocks' seals. /// /// Pass in only inherent data providers which don't include aura or parachain consensus inherents, /// e.g. things like timestamp and custom inherents for the runtime. /// /// The others are generated explicitly internally. /// -/// This should only be used for runtimes where the runtime does not check all inherents and +/// This should only be used for runtimes where the runtime does not check /// seals in `execute_block` (see ) -pub fn fully_verifying_import_queue( +pub fn fully_verifying_import_queue( client: Arc, block_import: I, - create_inherent_data_providers: CIDP, spawner: &impl sp_core::traits::SpawnEssentialNamed, registry: Option<&prometheus_endpoint::Registry>, telemetry: Option, @@ -292,105 +178,7 @@ where + 'static, Client: ProvideRuntimeApi + Send + Sync + 'static, >::Api: BlockBuilderApi + AuraApi, - CIDP: CreateInherentDataProviders + 'static, { - let verifier = Verifier:: { - client, - create_inherent_data_providers, - defender: Mutex::new(NaiveEquivocationDefender::default()), - telemetry, - _phantom: std::marker::PhantomData, - }; - + let verifier = Verifier:: { client, telemetry, _phantom: std::marker::PhantomData }; BasicQueue::new(verifier, Box::new(block_import), None, spawner, registry) } - -#[cfg(test)] -mod test { - use super::*; - use codec::Encode; - use cumulus_test_client::{ - runtime::Block, seal_block, Client, InitBlockBuilder, TestClientBuilder, - TestClientBuilderExt, - }; - use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; - use futures::FutureExt; - use polkadot_primitives::{HeadData, PersistedValidationData}; - use sc_client_api::HeaderBackend; - use sp_consensus_aura::sr25519; - use sp_tracing::try_init_simple; - use std::{collections::HashSet, sync::Arc}; - - #[test] - fn import_equivocated_blocks_from_recovery() { - try_init_simple(); - - let client = Arc::new(TestClientBuilder::default().build()); - - let verifier = Verifier:: { - client: client.clone(), - create_inherent_data_providers: |_, _| async move { - Ok(sp_timestamp::InherentDataProvider::from_system_time()) - }, - defender: Mutex::new(NaiveEquivocationDefender::default()), - telemetry: None, - _phantom: std::marker::PhantomData, - }; - - let genesis = client.info().best_hash; - let mut sproof = RelayStateSproofBuilder::default(); - sproof.included_para_head = Some(HeadData(client.header(genesis).unwrap().encode())); - sproof.para_id = cumulus_test_client::runtime::PARACHAIN_ID.into(); - - let validation_data = PersistedValidationData { - relay_parent_number: 1, - parent_head: client.header(genesis).unwrap().encode().into(), - ..Default::default() - }; - - let block_builder = client.init_block_builder(Some(validation_data), sproof); - let block = block_builder.block_builder.build().unwrap(); - - let mut blocks = Vec::new(); - for _ in 0..EQUIVOCATION_LIMIT + 1 { - blocks.push(seal_block(block.block.clone(), &client)) - } - - // sr25519 should generate a different signature every time you sign something and thus, all - // blocks get a different hash (even if they are the same block). - assert_eq!(blocks.iter().map(|b| b.hash()).collect::>().len(), blocks.len()); - - blocks.iter().take(EQUIVOCATION_LIMIT).for_each(|block| { - let mut params = - BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); - params.body = Some(block.extrinsics().to_vec()); - verifier.verify(params).now_or_never().unwrap().unwrap(); - }); - - // Now let's try some previously verified block and a block we have not verified yet. - // - // Verify should fail, because we are above the limit. However, when we change the origin to - // `ConsensusBroadcast`, it should work. - let extra_blocks = - vec![blocks[EQUIVOCATION_LIMIT / 2].clone(), blocks.last().unwrap().clone()]; - - extra_blocks.into_iter().for_each(|block| { - let mut params = - BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); - params.body = Some(block.extrinsics().to_vec()); - assert!(verifier - .verify(params) - .now_or_never() - .unwrap() - .map(drop) - .unwrap_err() - .contains("excessive equivocations at slot")); - - // When it comes from `pov-recovery`, we will accept it - let mut params = - BlockImportParams::new(BlockOrigin::ConsensusBroadcast, block.header().clone()); - params.body = Some(block.extrinsics().to_vec()); - assert!(verifier.verify(params).now_or_never().unwrap().is_ok()); - }); - } -} diff --git a/cumulus/polkadot-omni-node/lib/Cargo.toml b/cumulus/polkadot-omni-node/lib/Cargo.toml index 383d15b352186..0afbea5292f6d 100644 --- a/cumulus/polkadot-omni-node/lib/Cargo.toml +++ b/cumulus/polkadot-omni-node/lib/Cargo.toml @@ -100,9 +100,12 @@ cumulus-primitives-aura = { workspace = true, default-features = true } cumulus-primitives-core = { workspace = true, default-features = true } cumulus-relay-chain-interface = { workspace = true, default-features = true } futures-timer = { workspace = true } +schnellru = { workspace = true } [dev-dependencies] assert_cmd = { workspace = true } +cumulus-test-client = { workspace = true } +cumulus-test-relay-sproof-builder = { workspace = true, default-features = true } cumulus-test-runtime = { workspace = true } nix = { features = ["signal"], workspace = true } tokio = { version = "1.43.1", features = ["macros", "parking_lot", "time"] } diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 411a6f667a44c..31b36c6de562e 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -58,18 +58,26 @@ use sc_client_api::BlockchainEvents; use sc_client_db::DbHash; use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, - BlockImportParams, DefaultImportQueue, + BlockCheckParams, BlockImport, BlockImportParams, DefaultImportQueue, ImportResult, }; +use sc_consensus_aura::find_pre_digest; use sc_service::{Configuration, Error, TaskManager}; use sc_telemetry::TelemetryHandle; use sc_transaction_pool::TransactionPoolHandle; +use schnellru::{ByLength, LruMap}; use sp_api::ProvideRuntimeApi; -use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, AuraApi}; +use sp_block_builder::BlockBuilder as BlockBuilderApi; +use sp_consensus::BlockOrigin; +use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, AuraApi, Slot}; use sp_core::{traits::SpawnNamed, Pair}; -use sp_inherents::CreateInherentDataProviders; +use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; use sp_keystore::KeystorePtr; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor}; -use std::{marker::PhantomData, sync::Arc, time::Duration}; +use std::{ + marker::PhantomData, + sync::{Arc, Mutex}, + time::Duration, +}; struct Verifier { client: Arc, @@ -120,7 +128,7 @@ where telemetry_handle: Option, task_manager: &TaskManager, ) -> sc_service::error::Result> { - let inherent_data_providers = + let create_inherent_data_providers = move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }; let registry = config.prometheus_registry(); let spawner = task_manager.spawn_essential_handle(); @@ -128,23 +136,181 @@ where let relay_chain_verifier = Box::new(RelayChainVerifier::new(client.clone(), |_, _| async { Ok(()) })); - let equivocation_aura_verifier = EquivocationVerifier::::new( + let equivocation_aura_verifier = EquivocationVerifier::::new( client.clone(), - inherent_data_providers, telemetry_handle, ); let verifier = Verifier { - client, + client: client.clone(), aura_verifier: Box::new(equivocation_aura_verifier), relay_chain_verifier, _phantom: Default::default(), }; + let block_import = + AuraBlockImport::new(block_import, client, create_inherent_data_providers); + Ok(BasicQueue::new(verifier, Box::new(block_import), None, &spawner, registry)) } } +struct AuraBlockImport { + inner: BI, + client: Arc, + create_inherent_data_providers: CIDP, + defender: Mutex>>, + _phantom: PhantomData, +} + +impl AuraBlockImport { + fn new(inner: BI, client: Arc, create_inherent_data_providers: CIDP) -> Self { + Self { + inner, + client, + create_inherent_data_providers, + defender: Default::default(), + _phantom: Default::default(), + } + } +} + +#[async_trait::async_trait] +impl BlockImport + for AuraBlockImport +where + Client: ProvideRuntimeApi + Send + Sync, + Client::Api: AuraRuntimeApi, + >::Api: BlockBuilderApi, + AuraId: AuraIdT + Sync, + BI: sc_consensus::BlockImport + Send + Sync, + CIDP: CreateInherentDataProviders + Sync, + AuraId::BoundedPair: Pair, + ::Signature: codec::Codec, +{ + type Error = sp_consensus::Error; + + async fn check_block( + &self, + block: BlockCheckParams, + ) -> Result { + self.inner.check_block(block).await + } + + async fn import_block( + &self, + mut block_params: BlockImportParams, + ) -> Result { + // Check inherents. + if let Some(inner_body) = block_params.body.take() { + let inherent_data_providers = self + .create_inherent_data_providers + .create_inherent_data_providers(*block_params.header.parent_hash(), ()) + .await + .map_err(sp_consensus::Error::Other)?; + + let inherent_data = inherent_data_providers + .create_inherent_data() + .await + .map_err(|e| sp_consensus::Error::Other(e.into()))?; + + let block = Block::new(block_params.header.clone(), inner_body); + + let inherent_res = self + .client + .runtime_api() + .check_inherents(*block.header().parent_hash(), block.clone(), inherent_data) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + + if !inherent_res.ok() { + for (i, e) in inherent_res.into_errors() { + match inherent_data_providers.try_handle_error(&i, &e).await { + Some(res) => res.map_err(|e| sp_consensus::Error::InvalidInherents(e))?, + None => return Err(sp_consensus::Error::InvalidInherentsUnhandled(i)), + } + } + } + + let (_, inner_body) = block.deconstruct(); + block_params.body = Some(inner_body); + } + + if self.client.runtime_api().has_aura_api(*block_params.header.parent_hash()) { + let slot = find_pre_digest::::Signature>( + &block_params.header, + ) + .map_err(|e| sp_consensus::Error::Other(e.into()))?; + + // We need some kind of identifier for the relay parent, in the worst case we + // take the all `0` hash. + let relay_parent = + cumulus_primitives_core::rpsr_digest::extract_relay_parent_storage_root( + block_params.header.digest(), + ) + .map(|r| r.0) + .unwrap_or_else(|| { + cumulus_primitives_core::extract_relay_parent(block_params.header.digest()) + .unwrap_or_default() + }); + + // Check for and reject egregious amounts of equivocations. + // + // If the `origin` is `ConsensusBroadcast`, we ignore the result of the + // equivocation check. This `origin` is for example used by pov-recovery. + if self.defender.lock().unwrap().insert_and_check( + slot, + *block_params.header.number(), + relay_parent, + ) && !matches!(block_params.origin, BlockOrigin::ConsensusBroadcast) + { + return Err(sp_consensus::Error::Other( + format!("Rejecting block {slot:?} due to excessive equivocations at slot",) + .as_str() + .into(), + )) + } + } + + self.inner.import_block(block_params).await + } +} + +const LRU_WINDOW: u32 = 512; +const EQUIVOCATION_LIMIT: usize = 16; + +struct NaiveEquivocationDefender { + /// We distinguish blocks by `(Slot, BlockNumber, RelayParent)`. + cache: LruMap<(u64, N, polkadot_primitives::Hash), usize>, +} + +impl Default for NaiveEquivocationDefender { + fn default() -> Self { + NaiveEquivocationDefender { cache: LruMap::new(ByLength::new(LRU_WINDOW)) } + } +} + +impl NaiveEquivocationDefender { + // Returns `true` if equivocation is beyond the limit. + fn insert_and_check( + &mut self, + slot: Slot, + block_number: N, + relay_chain_parent: polkadot_primitives::Hash, + ) -> bool { + let val = self + .cache + .get_or_insert((*slot, block_number, relay_chain_parent), || 0) + .expect("insertion with ByLength limiter always succeeds; qed"); + + if *val == EQUIVOCATION_LIMIT { + true + } else { + *val += 1; + false + } + } +} + /// Uses the lookahead collator to support async backing. /// /// Start an aura powered parachain node. Some system chains use this. @@ -562,3 +728,106 @@ where Ok(()) } } + +#[cfg(test)] +mod test { + use super::*; + use codec::Encode; + use cumulus_test_client::{ + seal_block, InitBlockBuilder, TestClientBuilder, TestClientBuilderExt, + }; + use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; + use cumulus_test_runtime::AuraId; + use futures::FutureExt; + use polkadot_primitives::{HeadData, PersistedValidationData}; + use sc_client_api::HeaderBackend; + use std::{collections::HashSet, sync::Arc}; + + struct TestBlockImport; + + #[async_trait::async_trait] + impl BlockImport for TestBlockImport { + type Error = sp_consensus::Error; + + async fn check_block( + &self, + _: BlockCheckParams, + ) -> Result { + Ok(ImportResult::Imported(Default::default())) + } + + async fn import_block( + &self, + _: BlockImportParams, + ) -> Result { + Ok(ImportResult::Imported(Default::default())) + } + } + + #[test] + fn import_equivocated_blocks_from_recovery() { + let client = Arc::new(TestClientBuilder::default().build()); + + let block_import: AuraBlockImport<_, _, _, _, AuraId> = + AuraBlockImport::new(TestBlockImport, client.clone(), |_, _| async move { + Ok(sp_timestamp::InherentDataProvider::from_system_time()) + }); + + let genesis = client.info().best_hash; + let mut sproof = RelayStateSproofBuilder::default(); + sproof.included_para_head = Some(HeadData(client.header(genesis).unwrap().encode())); + sproof.para_id = cumulus_test_client::runtime::PARACHAIN_ID.into(); + + let validation_data = PersistedValidationData { + relay_parent_number: 1, + parent_head: client.header(genesis).unwrap().encode().into(), + ..Default::default() + }; + + let block_builder = client.init_block_builder(Some(validation_data), sproof); + let block = block_builder.block_builder.build().unwrap(); + + let mut blocks = Vec::new(); + for _ in 0..EQUIVOCATION_LIMIT + 1 { + blocks.push(seal_block(block.block.clone(), &client)) + } + + // sr25519 should generate a different signature every time you sign something and thus, all + // blocks get a different hash (even if they are the same block). + assert_eq!(blocks.iter().map(|b| b.hash()).collect::>().len(), blocks.len()); + + blocks.iter().take(EQUIVOCATION_LIMIT).for_each(|block| { + let mut params = + BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); + params.body = Some(block.extrinsics().to_vec()); + block_import.import_block(params).now_or_never().unwrap().unwrap(); + }); + + // Now let's try some previously verified block and a block we have not verified yet. + // + // Verify should fail, because we are above the limit. However, when we change the origin to + // `ConsensusBroadcast`, it should work. + let extra_blocks = + vec![blocks[EQUIVOCATION_LIMIT / 2].clone(), blocks.last().unwrap().clone()]; + + extra_blocks.into_iter().for_each(|block| { + let mut params = + BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); + params.body = Some(block.extrinsics().to_vec()); + assert!(block_import + .import_block(params) + .now_or_never() + .unwrap() + .map(drop) + .unwrap_err() + .to_string() + .contains("excessive equivocations at slot")); + + // When it comes from `pov-recovery`, we will accept it + let mut params = + BlockImportParams::new(BlockOrigin::ConsensusBroadcast, block.header().clone()); + params.body = Some(block.extrinsics().to_vec()); + assert!(block_import.import_block(params).now_or_never().unwrap().is_ok()); + }); + } +} diff --git a/templates/parachain/node/src/service.rs b/templates/parachain/node/src/service.rs index 18696bb426255..7fc08af7e7eb9 100644 --- a/templates/parachain/node/src/service.rs +++ b/templates/parachain/node/src/service.rs @@ -154,14 +154,9 @@ fn build_import_queue( _, _, _, - _, >( client, block_import, - move |_, _| async move { - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - Ok(timestamp) - }, &task_manager.spawn_essential_handle(), config.prometheus_registry(), telemetry, From 61478150fcb671416c5718ea5d815ab5aad624f6 Mon Sep 17 00:00:00 2001 From: sistemd Date: Fri, 13 Jun 2025 15:54:37 +0200 Subject: [PATCH 29/38] fixing ci --- cumulus/polkadot-omni-node/lib/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/cumulus/polkadot-omni-node/lib/Cargo.toml b/cumulus/polkadot-omni-node/lib/Cargo.toml index 0afbea5292f6d..9c906eb65e7d7 100644 --- a/cumulus/polkadot-omni-node/lib/Cargo.toml +++ b/cumulus/polkadot-omni-node/lib/Cargo.toml @@ -117,6 +117,7 @@ rococo-native = ["polkadot-cli/rococo-native"] westend-native = ["polkadot-cli/westend-native"] runtime-benchmarks = [ "cumulus-primitives-core/runtime-benchmarks", + "cumulus-test-client/runtime-benchmarks", "frame-benchmarking-cli/runtime-benchmarks", "frame-benchmarking/runtime-benchmarks", "frame-support/runtime-benchmarks", From 4a77f2358162eed6909f7a3bfc06fe4b398aa5c3 Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 25 Jun 2025 15:58:31 +0200 Subject: [PATCH 30/38] remove some unneeded constraints --- cumulus/client/consensus/aura/src/collators/basic.rs | 4 ++-- cumulus/client/consensus/aura/src/collators/lookahead.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/basic.rs b/cumulus/client/consensus/aura/src/collators/basic.rs index 56d836b3c325e..a66abf979d683 100644 --- a/cumulus/client/consensus/aura/src/collators/basic.rs +++ b/cumulus/client/consensus/aura/src/collators/basic.rs @@ -103,7 +103,7 @@ where + 'static, Client::Api: AuraApi + CollectCollationInfo, RClient: RelayChainInterface + Send + Clone + 'static, - CIDP: CreateInherentDataProviders + Clone + Send + 'static, + CIDP: CreateInherentDataProviders + Send + 'static, CIDP::InherentDataProviders: Send, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, Proposer: ProposerInterface + Send + Sync + 'static, @@ -126,7 +126,7 @@ where let mut collator = { let params = collator_util::Params { - create_inherent_data_providers: params.create_inherent_data_providers.clone(), + create_inherent_data_providers: params.create_inherent_data_providers, block_import: params.block_import, relay_client: params.relay_client.clone(), keystore: params.keystore.clone(), diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 24da945afba86..e31cd1566161e 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -171,7 +171,7 @@ where AuraApi + CollectCollationInfo + AuraUnincludedSegmentApi, Backend: sc_client_api::Backend + 'static, RClient: RelayChainInterface + Clone + 'static, - CIDP: CreateInherentDataProviders + Clone + 'static, + CIDP: CreateInherentDataProviders + 'static, CIDP::InherentDataProviders: Send, BI: BlockImport + ParachainBlockImportMarker + Send + Sync + 'static, Proposer: ProposerInterface + Send + Sync + 'static, @@ -206,7 +206,7 @@ where let mut collator = { let params = collator_util::Params { - create_inherent_data_providers: params.create_inherent_data_providers.clone(), + create_inherent_data_providers: params.create_inherent_data_providers, block_import: params.block_import, relay_client: params.relay_client.clone(), keystore: params.keystore.clone(), From 8aae9f1ba5e36ff290a8a2130749f77352f4ccb5 Mon Sep 17 00:00:00 2001 From: sistemd Date: Mon, 30 Jun 2025 16:47:11 +0200 Subject: [PATCH 31/38] move NaiveEquivocationDefender back into import queue --- .../aura/src/equivocation_import_queue.rs | 238 +++++++++++++++++- .../polkadot-omni-node/lib/src/nodes/aura.rs | 200 +-------------- 2 files changed, 231 insertions(+), 207 deletions(-) diff --git a/cumulus/client/consensus/aura/src/equivocation_import_queue.rs b/cumulus/client/consensus/aura/src/equivocation_import_queue.rs index 429ca9a8a2aa7..748e11bc45d03 100644 --- a/cumulus/client/consensus/aura/src/equivocation_import_queue.rs +++ b/cumulus/client/consensus/aura/src/equivocation_import_queue.rs @@ -22,28 +22,65 @@ /// should be thrown out and which ones should be kept. use codec::Codec; use cumulus_client_consensus_common::ParachainBlockImportMarker; +use parking_lot::Mutex; +use polkadot_primitives::Hash as RHash; use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, BlockImport, BlockImportParams, ForkChoiceStrategy, }; use sc_consensus_aura::standalone as aura_internal; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE}; +use schnellru::{ByLength, LruMap}; use sp_api::ProvideRuntimeApi; use sp_block_builder::BlockBuilder as BlockBuilderApi; -use sp_consensus::error::Error as ConsensusError; +use sp_consensus::{error::Error as ConsensusError, BlockOrigin}; use sp_consensus_aura::{AuraApi, Slot, SlotDuration}; use sp_core::crypto::Pair; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; +use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor}; use std::{fmt::Debug, sync::Arc}; -/// A parachain block import verifier that checks the header slot and seal. -pub struct Verifier { +const LRU_WINDOW: u32 = 512; +const EQUIVOCATION_LIMIT: usize = 16; + +struct NaiveEquivocationDefender { + /// We distinguish blocks by `(Slot, BlockNumber, RelayParent)`. + cache: LruMap<(u64, N, RHash), usize>, +} + +impl Default for NaiveEquivocationDefender { + fn default() -> Self { + NaiveEquivocationDefender { cache: LruMap::new(ByLength::new(LRU_WINDOW)) } + } +} + +impl NaiveEquivocationDefender { + // Returns `true` if equivocation is beyond the limit. + fn insert_and_check(&mut self, slot: Slot, block_number: N, relay_chain_parent: RHash) -> bool { + let val = self + .cache + .get_or_insert((*slot, block_number, relay_chain_parent), || 0) + .expect("insertion with ByLength limiter always succeeds; qed"); + + if *val == EQUIVOCATION_LIMIT { + true + } else { + *val += 1; + false + } + } +} + +/// A parachain block import verifier that checks for equivocation limits within each slot. +pub struct Verifier { client: Arc, + create_inherent_data_providers: CIDP, + defender: Mutex>>, telemetry: Option, _phantom: std::marker::PhantomData (Block, P)>, } -impl Verifier +impl Verifier where P: Pair, P::Signature: Codec, @@ -51,16 +88,28 @@ where Block: BlockT, Client: ProvideRuntimeApi + Send + Sync, >::Api: BlockBuilderApi + AuraApi, + + CIDP: CreateInherentDataProviders, { /// Creates a new Verifier instance for handling parachain block import verification in Aura /// consensus. - pub fn new(client: Arc, telemetry: Option) -> Self { - Self { client, telemetry, _phantom: std::marker::PhantomData } + pub fn new( + client: Arc, + inherent_data_provider: CIDP, + telemetry: Option, + ) -> Self { + Self { + client, + create_inherent_data_providers: inherent_data_provider, + defender: Mutex::new(NaiveEquivocationDefender::default()), + telemetry, + _phantom: std::marker::PhantomData, + } } } #[async_trait::async_trait] -impl VerifierT for Verifier +impl VerifierT for Verifier where P: Pair, P::Signature: Codec, @@ -68,6 +117,8 @@ where Block: BlockT, Client: ProvideRuntimeApi + Send + Sync, >::Api: BlockBuilderApi + AuraApi, + + CIDP: CreateInherentDataProviders, { async fn verify( &self, @@ -106,7 +157,7 @@ where ); match res { - Ok((pre_header, _, seal_digest)) => { + Ok((pre_header, slot, seal_digest)) => { telemetry!( self.telemetry; CONSENSUS_TRACE; @@ -114,10 +165,38 @@ where "pre_header" => ?pre_header, ); + // We need some kind of identifier for the relay parent, in the worst case we + // take the all `0` hash. + let relay_parent = + cumulus_primitives_core::rpsr_digest::extract_relay_parent_storage_root( + pre_header.digest(), + ) + .map(|r| r.0) + .unwrap_or_else(|| { + cumulus_primitives_core::extract_relay_parent(pre_header.digest()) + .unwrap_or_default() + }); + block_params.header = pre_header; block_params.post_digests.push(seal_digest); block_params.fork_choice = Some(ForkChoiceStrategy::LongestChain); block_params.post_hash = Some(post_hash); + + // Check for and reject egregious amounts of equivocations. + // + // If the `origin` is `ConsensusBroadcast`, we ignore the result of the + // equivocation check. This `origin` is for example used by pov-recovery. + if self.defender.lock().insert_and_check( + slot, + *block_params.header.number(), + relay_parent, + ) && !matches!(block_params.origin, BlockOrigin::ConsensusBroadcast) + { + return Err(format!( + "Rejecting block {:?} due to excessive equivocations at slot", + post_hash, + )) + } }, Err(aura_internal::SealVerificationError::Deferred(hdr, slot)) => { telemetry!( @@ -142,6 +221,40 @@ where } } + // Check inherents. + if let Some(body) = block_params.body.clone() { + let block = Block::new(block_params.header.clone(), body); + let create_inherent_data_providers = self + .create_inherent_data_providers + .create_inherent_data_providers(parent_hash, ()) + .await + .map_err(|e| format!("Could not create inherent data {:?}", e))?; + + let inherent_data = create_inherent_data_providers + .create_inherent_data() + .await + .map_err(|e| format!("Could not create inherent data {:?}", e))?; + + let inherent_res = self + .client + .runtime_api() + .check_inherents(parent_hash, block, inherent_data) + .map_err(|e| format!("Unable to check block inherents {:?}", e))?; + + if !inherent_res.ok() { + for (i, e) in inherent_res.into_errors() { + match create_inherent_data_providers.try_handle_error(&i, &e).await { + Some(res) => res.map_err(|e| format!("Inherent Error {:?}", e))?, + None => + return Err(format!( + "Unknown inherent error, source {:?}", + String::from_utf8_lossy(&i[..]) + )), + } + } + } + } + Ok(block_params) } } @@ -151,18 +264,19 @@ fn slot_now(slot_duration: SlotDuration) -> Slot { Slot::from_timestamp(timestamp, slot_duration) } -/// Start an import queue for a Cumulus node which checks blocks' seals. +/// Start an import queue for a Cumulus node which checks blocks' seals and inherent data. /// /// Pass in only inherent data providers which don't include aura or parachain consensus inherents, /// e.g. things like timestamp and custom inherents for the runtime. /// /// The others are generated explicitly internally. /// -/// This should only be used for runtimes where the runtime does not check +/// This should only be used for runtimes where the runtime does not check all inherents and /// seals in `execute_block` (see ) -pub fn fully_verifying_import_queue( +pub fn fully_verifying_import_queue( client: Arc, block_import: I, + create_inherent_data_providers: CIDP, spawner: &impl sp_core::traits::SpawnEssentialNamed, registry: Option<&prometheus_endpoint::Registry>, telemetry: Option, @@ -178,7 +292,105 @@ where + 'static, Client: ProvideRuntimeApi + Send + Sync + 'static, >::Api: BlockBuilderApi + AuraApi, + CIDP: CreateInherentDataProviders + 'static, { - let verifier = Verifier:: { client, telemetry, _phantom: std::marker::PhantomData }; + let verifier = Verifier:: { + client, + create_inherent_data_providers, + defender: Mutex::new(NaiveEquivocationDefender::default()), + telemetry, + _phantom: std::marker::PhantomData, + }; + BasicQueue::new(verifier, Box::new(block_import), None, spawner, registry) } + +#[cfg(test)] +mod test { + use super::*; + use codec::Encode; + use cumulus_test_client::{ + runtime::Block, seal_block, Client, InitBlockBuilder, TestClientBuilder, + TestClientBuilderExt, + }; + use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; + use futures::FutureExt; + use polkadot_primitives::{HeadData, PersistedValidationData}; + use sc_client_api::HeaderBackend; + use sp_consensus_aura::sr25519; + use sp_tracing::try_init_simple; + use std::{collections::HashSet, sync::Arc}; + + #[test] + fn import_equivocated_blocks_from_recovery() { + try_init_simple(); + + let client = Arc::new(TestClientBuilder::default().build()); + + let verifier = Verifier:: { + client: client.clone(), + create_inherent_data_providers: |_, _| async move { + Ok(sp_timestamp::InherentDataProvider::from_system_time()) + }, + defender: Mutex::new(NaiveEquivocationDefender::default()), + telemetry: None, + _phantom: std::marker::PhantomData, + }; + + let genesis = client.info().best_hash; + let mut sproof = RelayStateSproofBuilder::default(); + sproof.included_para_head = Some(HeadData(client.header(genesis).unwrap().encode())); + sproof.para_id = cumulus_test_client::runtime::PARACHAIN_ID.into(); + + let validation_data = PersistedValidationData { + relay_parent_number: 1, + parent_head: client.header(genesis).unwrap().encode().into(), + ..Default::default() + }; + + let block_builder = client.init_block_builder(Some(validation_data), sproof); + let block = block_builder.block_builder.build().unwrap(); + + let mut blocks = Vec::new(); + for _ in 0..EQUIVOCATION_LIMIT + 1 { + blocks.push(seal_block(block.block.clone(), &client)) + } + + // sr25519 should generate a different signature every time you sign something and thus, all + // blocks get a different hash (even if they are the same block). + assert_eq!(blocks.iter().map(|b| b.hash()).collect::>().len(), blocks.len()); + + blocks.iter().take(EQUIVOCATION_LIMIT).for_each(|block| { + let mut params = + BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); + params.body = Some(block.extrinsics().to_vec()); + verifier.verify(params).now_or_never().unwrap().unwrap(); + }); + + // Now let's try some previously verified block and a block we have not verified yet. + // + // Verify should fail, because we are above the limit. However, when we change the origin to + // `ConsensusBroadcast`, it should work. + let extra_blocks = + vec![blocks[EQUIVOCATION_LIMIT / 2].clone(), blocks.last().unwrap().clone()]; + + extra_blocks.into_iter().for_each(|block| { + let mut params = + BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); + params.body = Some(block.extrinsics().to_vec()); + assert!(verifier + .verify(params) + .now_or_never() + .unwrap() + .map(drop) + .unwrap_err() + .contains("excessive equivocations at slot")); + + // When it comes from `pov-recovery`, we will accept it + let mut params = + BlockImportParams::new(BlockOrigin::ConsensusBroadcast, block.header().clone()); + params.body = Some(block.extrinsics().to_vec()); + assert!(verifier.verify(params).now_or_never().unwrap().is_ok()); + }); + } +} diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 30e0b2ed2854c..9e18a240dcfd3 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -60,24 +60,17 @@ use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, BlockCheckParams, BlockImport, BlockImportParams, DefaultImportQueue, ImportResult, }; -use sc_consensus_aura::find_pre_digest; use sc_service::{Configuration, Error, TaskManager}; use sc_telemetry::TelemetryHandle; use sc_transaction_pool::TransactionPoolHandle; -use schnellru::{ByLength, LruMap}; use sp_api::ProvideRuntimeApi; use sp_block_builder::BlockBuilder as BlockBuilderApi; -use sp_consensus::BlockOrigin; -use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, AuraApi, Slot}; +use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, AuraApi}; use sp_core::{traits::SpawnNamed, Pair}; use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; use sp_keystore::KeystorePtr; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor}; -use std::{ - marker::PhantomData, - sync::{Arc, Mutex}, - time::Duration, -}; +use std::{marker::PhantomData, sync::Arc, time::Duration}; struct Verifier { client: Arc, @@ -136,8 +129,9 @@ where let relay_chain_verifier = Box::new(RelayChainVerifier::new(client.clone(), |_, _| async { Ok(()) })); - let equivocation_aura_verifier = EquivocationVerifier::::new( + let equivocation_aura_verifier = EquivocationVerifier::::new( client.clone(), + create_inherent_data_providers, telemetry_handle, ); @@ -159,19 +153,12 @@ struct AuraBlockImport { inner: BI, client: Arc, create_inherent_data_providers: CIDP, - defender: Mutex>>, - _phantom: PhantomData, + _phantom: PhantomData<(Block, AuraId)>, } impl AuraBlockImport { fn new(inner: BI, client: Arc, create_inherent_data_providers: CIDP) -> Self { - Self { - inner, - client, - create_inherent_data_providers, - defender: Default::default(), - _phantom: Default::default(), - } + Self { inner, client, create_inherent_data_providers, _phantom: Default::default() } } } @@ -235,82 +222,10 @@ where block_params.body = Some(inner_body); } - if self.client.runtime_api().has_aura_api(*block_params.header.parent_hash()) { - let slot = find_pre_digest::::Signature>( - &block_params.header, - ) - .map_err(|e| sp_consensus::Error::Other(e.into()))?; - - // We need some kind of identifier for the relay parent, in the worst case we - // take the all `0` hash. - let relay_parent = - cumulus_primitives_core::rpsr_digest::extract_relay_parent_storage_root( - block_params.header.digest(), - ) - .map(|r| r.0) - .unwrap_or_else(|| { - cumulus_primitives_core::extract_relay_parent(block_params.header.digest()) - .unwrap_or_default() - }); - - // Check for and reject egregious amounts of equivocations. - // - // If the `origin` is `ConsensusBroadcast`, we ignore the result of the - // equivocation check. This `origin` is for example used by pov-recovery. - if self.defender.lock().unwrap().insert_and_check( - slot, - *block_params.header.number(), - relay_parent, - ) && !matches!(block_params.origin, BlockOrigin::ConsensusBroadcast) - { - return Err(sp_consensus::Error::Other( - format!("Rejecting block {slot:?} due to excessive equivocations at slot",) - .as_str() - .into(), - )) - } - } - self.inner.import_block(block_params).await } } -const LRU_WINDOW: u32 = 512; -const EQUIVOCATION_LIMIT: usize = 16; - -struct NaiveEquivocationDefender { - /// We distinguish blocks by `(Slot, BlockNumber, RelayParent)`. - cache: LruMap<(u64, N, polkadot_primitives::Hash), usize>, -} - -impl Default for NaiveEquivocationDefender { - fn default() -> Self { - NaiveEquivocationDefender { cache: LruMap::new(ByLength::new(LRU_WINDOW)) } - } -} - -impl NaiveEquivocationDefender { - // Returns `true` if equivocation is beyond the limit. - fn insert_and_check( - &mut self, - slot: Slot, - block_number: N, - relay_chain_parent: polkadot_primitives::Hash, - ) -> bool { - let val = self - .cache - .get_or_insert((*slot, block_number, relay_chain_parent), || 0) - .expect("insertion with ByLength limiter always succeeds; qed"); - - if *val == EQUIVOCATION_LIMIT { - true - } else { - *val += 1; - false - } - } -} - /// Uses the lookahead collator to support async backing. /// /// Start an aura powered parachain node. Some system chains use this. @@ -729,106 +644,3 @@ where Ok(()) } } - -#[cfg(test)] -mod test { - use super::*; - use codec::Encode; - use cumulus_test_client::{ - seal_block, InitBlockBuilder, TestClientBuilder, TestClientBuilderExt, - }; - use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; - use cumulus_test_runtime::AuraId; - use futures::FutureExt; - use polkadot_primitives::{HeadData, PersistedValidationData}; - use sc_client_api::HeaderBackend; - use std::{collections::HashSet, sync::Arc}; - - struct TestBlockImport; - - #[async_trait::async_trait] - impl BlockImport for TestBlockImport { - type Error = sp_consensus::Error; - - async fn check_block( - &self, - _: BlockCheckParams, - ) -> Result { - Ok(ImportResult::Imported(Default::default())) - } - - async fn import_block( - &self, - _: BlockImportParams, - ) -> Result { - Ok(ImportResult::Imported(Default::default())) - } - } - - #[test] - fn import_equivocated_blocks_from_recovery() { - let client = Arc::new(TestClientBuilder::default().build()); - - let block_import: AuraBlockImport<_, _, _, _, AuraId> = - AuraBlockImport::new(TestBlockImport, client.clone(), |_, _| async move { - Ok(sp_timestamp::InherentDataProvider::from_system_time()) - }); - - let genesis = client.info().best_hash; - let mut sproof = RelayStateSproofBuilder::default(); - sproof.included_para_head = Some(HeadData(client.header(genesis).unwrap().encode())); - sproof.para_id = cumulus_test_client::runtime::PARACHAIN_ID.into(); - - let validation_data = PersistedValidationData { - relay_parent_number: 1, - parent_head: client.header(genesis).unwrap().encode().into(), - ..Default::default() - }; - - let block_builder = client.init_block_builder(Some(validation_data), sproof); - let block = block_builder.block_builder.build().unwrap(); - - let mut blocks = Vec::new(); - for _ in 0..EQUIVOCATION_LIMIT + 1 { - blocks.push(seal_block(block.block.clone(), &client)) - } - - // sr25519 should generate a different signature every time you sign something and thus, all - // blocks get a different hash (even if they are the same block). - assert_eq!(blocks.iter().map(|b| b.hash()).collect::>().len(), blocks.len()); - - blocks.iter().take(EQUIVOCATION_LIMIT).for_each(|block| { - let mut params = - BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); - params.body = Some(block.extrinsics().to_vec()); - block_import.import_block(params).now_or_never().unwrap().unwrap(); - }); - - // Now let's try some previously verified block and a block we have not verified yet. - // - // Verify should fail, because we are above the limit. However, when we change the origin to - // `ConsensusBroadcast`, it should work. - let extra_blocks = - vec![blocks[EQUIVOCATION_LIMIT / 2].clone(), blocks.last().unwrap().clone()]; - - extra_blocks.into_iter().for_each(|block| { - let mut params = - BlockImportParams::new(BlockOrigin::NetworkBroadcast, block.header().clone()); - params.body = Some(block.extrinsics().to_vec()); - assert!(block_import - .import_block(params) - .now_or_never() - .unwrap() - .map(drop) - .unwrap_err() - .to_string() - .contains("excessive equivocations at slot")); - - // When it comes from `pov-recovery`, we will accept it - let mut params = - BlockImportParams::new(BlockOrigin::ConsensusBroadcast, block.header().clone()); - params.body = Some(block.extrinsics().to_vec()); - assert!(block_import.import_block(params).now_or_never().unwrap().is_ok()); - }); - } -} From 845db2b21fffca5198a50e8a30767563a9a7c821 Mon Sep 17 00:00:00 2001 From: sistemd Date: Mon, 30 Jun 2025 21:48:31 +0200 Subject: [PATCH 32/38] move AuraBlockImport to substrate --- .../polkadot-omni-node/lib/src/nodes/aura.rs | 85 +------------------ .../client/consensus/aura/src/import_queue.rs | 81 +++++++++++++++++- 2 files changed, 82 insertions(+), 84 deletions(-) diff --git a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs index 9e18a240dcfd3..ccf3b3a6fe198 100644 --- a/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs +++ b/cumulus/polkadot-omni-node/lib/src/nodes/aura.rs @@ -58,16 +58,15 @@ use sc_client_api::BlockchainEvents; use sc_client_db::DbHash; use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, - BlockCheckParams, BlockImport, BlockImportParams, DefaultImportQueue, ImportResult, + BlockImportParams, DefaultImportQueue, }; use sc_service::{Configuration, Error, TaskManager}; use sc_telemetry::TelemetryHandle; use sc_transaction_pool::TransactionPoolHandle; use sp_api::ProvideRuntimeApi; -use sp_block_builder::BlockBuilder as BlockBuilderApi; use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, AuraApi}; use sp_core::{traits::SpawnNamed, Pair}; -use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; +use sp_inherents::CreateInherentDataProviders; use sp_keystore::KeystorePtr; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor}; use std::{marker::PhantomData, sync::Arc, time::Duration}; @@ -142,90 +141,10 @@ where _phantom: Default::default(), }; - let block_import = - AuraBlockImport::new(block_import, client, create_inherent_data_providers); - Ok(BasicQueue::new(verifier, Box::new(block_import), None, &spawner, registry)) } } -struct AuraBlockImport { - inner: BI, - client: Arc, - create_inherent_data_providers: CIDP, - _phantom: PhantomData<(Block, AuraId)>, -} - -impl AuraBlockImport { - fn new(inner: BI, client: Arc, create_inherent_data_providers: CIDP) -> Self { - Self { inner, client, create_inherent_data_providers, _phantom: Default::default() } - } -} - -#[async_trait::async_trait] -impl BlockImport - for AuraBlockImport -where - Client: ProvideRuntimeApi + Send + Sync, - Client::Api: AuraRuntimeApi, - >::Api: BlockBuilderApi, - AuraId: AuraIdT + Sync, - BI: sc_consensus::BlockImport + Send + Sync, - CIDP: CreateInherentDataProviders + Sync, - AuraId::BoundedPair: Pair, - ::Signature: codec::Codec, -{ - type Error = sp_consensus::Error; - - async fn check_block( - &self, - block: BlockCheckParams, - ) -> Result { - self.inner.check_block(block).await - } - - async fn import_block( - &self, - mut block_params: BlockImportParams, - ) -> Result { - // Check inherents. - if let Some(inner_body) = block_params.body.take() { - let inherent_data_providers = self - .create_inherent_data_providers - .create_inherent_data_providers(*block_params.header.parent_hash(), ()) - .await - .map_err(sp_consensus::Error::Other)?; - - let inherent_data = inherent_data_providers - .create_inherent_data() - .await - .map_err(|e| sp_consensus::Error::Other(e.into()))?; - - let block = Block::new(block_params.header.clone(), inner_body); - - let inherent_res = self - .client - .runtime_api() - .check_inherents(*block.header().parent_hash(), block.clone(), inherent_data) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; - - if !inherent_res.ok() { - for (i, e) in inherent_res.into_errors() { - match inherent_data_providers.try_handle_error(&i, &e).await { - Some(res) => res.map_err(|e| sp_consensus::Error::InvalidInherents(e))?, - None => return Err(sp_consensus::Error::InvalidInherentsUnhandled(i)), - } - } - } - - let (_, inner_body) = block.deconstruct(); - block_params.body = Some(inner_body); - } - - self.inner.import_block(block_params).await - } -} - /// Uses the lookahead collator to support async backing. /// /// Start an aura powered parachain node. Some system chains use this. diff --git a/substrate/client/consensus/aura/src/import_queue.rs b/substrate/client/consensus/aura/src/import_queue.rs index e72efc80d636f..c3d96a682e4c5 100644 --- a/substrate/client/consensus/aura/src/import_queue.rs +++ b/substrate/client/consensus/aura/src/import_queue.rs @@ -29,6 +29,7 @@ use sc_client_api::{backend::AuxStore, BlockOf, UsageProvider}; use sc_consensus::{ block_import::{BlockImport, BlockImportParams, ForkChoiceStrategy}, import_queue::{BasicQueue, BoxJustificationImport, DefaultImportQueue, Verifier}, + BlockCheckParams, ImportResult, }; use sc_consensus_slots::CheckedHeader; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE}; @@ -39,6 +40,7 @@ use sp_consensus::Error as ConsensusError; use sp_consensus_aura::AuraApi; use sp_consensus_slots::Slot; use sp_core::crypto::Pair; +use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; use sp_runtime::{ traits::{Block as BlockT, Header, NumberFor}, DigestItem, @@ -228,12 +230,16 @@ where GetSlotFn: Fn(Block::Hash) -> sp_blockchain::Result + Send + Sync + 'static, { let verifier = build_verifier::(BuildVerifierParams { - client, + client: client.clone(), get_slot, telemetry, compatibility_mode, }); + let create_inherent_data_providers = + move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }; + let block_import = AuraBlockImport::new(block_import, client, create_inherent_data_providers); + Ok(BasicQueue::new(verifier, Box::new(block_import), justification_import, spawner, registry)) } @@ -261,3 +267,76 @@ pub fn build_verifier( ) -> AuraVerifier { AuraVerifier::<_, P, _, _>::new(client, get_slot, telemetry, compatibility_mode) } + +struct AuraBlockImport { + inner: BI, + client: Arc, + create_inherent_data_providers: CIDP, + _phantom: PhantomData, +} + +impl AuraBlockImport { + fn new(inner: BI, client: Arc, create_inherent_data_providers: CIDP) -> Self { + Self { inner, client, create_inherent_data_providers, _phantom: Default::default() } + } +} + +#[async_trait::async_trait] +impl BlockImport + for AuraBlockImport +where + Client: ProvideRuntimeApi + Send + Sync, + >::Api: BlockBuilderApi, + BI: sc_consensus::BlockImport + Send + Sync, + CIDP: CreateInherentDataProviders + Sync, +{ + type Error = sp_consensus::Error; + + async fn check_block( + &self, + block: BlockCheckParams, + ) -> Result { + self.inner.check_block(block).await + } + + async fn import_block( + &self, + mut block_params: BlockImportParams, + ) -> Result { + // Check inherents. + if let Some(inner_body) = block_params.body.take() { + let inherent_data_providers = self + .create_inherent_data_providers + .create_inherent_data_providers(*block_params.header.parent_hash(), ()) + .await + .map_err(sp_consensus::Error::Other)?; + + let inherent_data = inherent_data_providers + .create_inherent_data() + .await + .map_err(|e| sp_consensus::Error::Other(e.into()))?; + + let block = Block::new(block_params.header.clone(), inner_body); + + let inherent_res = self + .client + .runtime_api() + .check_inherents(*block.header().parent_hash(), block.clone(), inherent_data) + .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; + + if !inherent_res.ok() { + for (i, e) in inherent_res.into_errors() { + match inherent_data_providers.try_handle_error(&i, &e).await { + Some(res) => res.map_err(|e| sp_consensus::Error::InvalidInherents(e))?, + None => return Err(sp_consensus::Error::InvalidInherentsUnhandled(i)), + } + } + } + + let (_, inner_body) = block.deconstruct(); + block_params.body = Some(inner_body); + } + + self.inner.import_block(block_params).await + } +} From ef5895553e748cd94d3154c83e7b3bbbf39424a3 Mon Sep 17 00:00:00 2001 From: sistemd Date: Mon, 30 Jun 2025 23:28:16 +0200 Subject: [PATCH 33/38] strict equivocation check in AuraBlockImport --- .../client/consensus/aura/src/import_queue.rs | 2 +- .../client/consensus/aura/src/import_queue.rs | 121 +++++++++++++++--- 2 files changed, 104 insertions(+), 19 deletions(-) diff --git a/cumulus/client/consensus/aura/src/import_queue.rs b/cumulus/client/consensus/aura/src/import_queue.rs index c7107bc0f30c9..f3ead1f4b3d54 100644 --- a/cumulus/client/consensus/aura/src/import_queue.rs +++ b/cumulus/client/consensus/aura/src/import_queue.rs @@ -76,7 +76,7 @@ where + Send + Sync + 'static, - P: Pair + 'static, + P: Pair + Send + Sync + 'static, P::Public: Debug + Codec, P::Signature: Codec, S: sp_core::traits::SpawnEssentialNamed, diff --git a/substrate/client/consensus/aura/src/import_queue.rs b/substrate/client/consensus/aura/src/import_queue.rs index c3d96a682e4c5..58820d8f55faf 100644 --- a/substrate/client/consensus/aura/src/import_queue.rs +++ b/substrate/client/consensus/aura/src/import_queue.rs @@ -31,7 +31,7 @@ use sc_consensus::{ import_queue::{BasicQueue, BoxJustificationImport, DefaultImportQueue, Verifier}, BlockCheckParams, ImportResult, }; -use sc_consensus_slots::CheckedHeader; +use sc_consensus_slots::{check_equivocation, CheckedHeader, InherentDataProviderExt}; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_TRACE}; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_block_builder::BlockBuilder as BlockBuilderApi; @@ -223,7 +223,7 @@ where + UsageProvider + HeaderBackend, I: BlockImport + Send + Sync + 'static, - P: Pair + 'static, + P: Pair + Send + Sync + 'static, P::Public: Codec + Debug, P::Signature: Codec, S: sp_core::traits::SpawnEssentialNamed, @@ -236,9 +236,24 @@ where compatibility_mode, }); - let create_inherent_data_providers = - move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }; - let block_import = AuraBlockImport::new(block_import, client, create_inherent_data_providers); + let cidp_client = client.clone(); + let create_inherent_data_providers = move |parent_hash, _| { + let cidp_client = cidp_client.clone(); + async move { + let slot_duration = crate::standalone::slot_duration_at(&*cidp_client, parent_hash)?; + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + + let slot = + sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ); + + Ok((slot, timestamp)) + } + }; + let block_import = + AuraBlockImport::<_, _, _, _, P>::new(block_import, client, create_inherent_data_providers); Ok(BasicQueue::new(verifier, Box::new(block_import), justification_import, spawner, registry)) } @@ -268,27 +283,32 @@ pub fn build_verifier( AuraVerifier::<_, P, _, _>::new(client, get_slot, telemetry, compatibility_mode) } -struct AuraBlockImport { +struct AuraBlockImport { inner: BI, client: Arc, create_inherent_data_providers: CIDP, - _phantom: PhantomData, + _phantom: PhantomData<(Block, P)>, } -impl AuraBlockImport { +impl AuraBlockImport { fn new(inner: BI, client: Arc, create_inherent_data_providers: CIDP) -> Self { Self { inner, client, create_inherent_data_providers, _phantom: Default::default() } } } #[async_trait::async_trait] -impl BlockImport - for AuraBlockImport +impl BlockImport + for AuraBlockImport where - Client: ProvideRuntimeApi + Send + Sync, - >::Api: BlockBuilderApi, + Client: ProvideRuntimeApi + AuxStore + Send + Sync, + >::Api: + BlockBuilderApi + AuraApi>, BI: sc_consensus::BlockImport + Send + Sync, CIDP: CreateInherentDataProviders + Sync, + CIDP::InherentDataProviders: InherentDataProviderExt, + P: Pair + Send + Sync + 'static, + P::Public: Codec + Debug, + P::Signature: Codec, { type Error = sp_consensus::Error; @@ -303,14 +323,14 @@ where &self, mut block_params: BlockImportParams, ) -> Result { + let inherent_data_providers = self + .create_inherent_data_providers + .create_inherent_data_providers(*block_params.header.parent_hash(), ()) + .await + .map_err(sp_consensus::Error::Other)?; + // Check inherents. if let Some(inner_body) = block_params.body.take() { - let inherent_data_providers = self - .create_inherent_data_providers - .create_inherent_data_providers(*block_params.header.parent_hash(), ()) - .await - .map_err(sp_consensus::Error::Other)?; - let inherent_data = inherent_data_providers .create_inherent_data() .await @@ -337,6 +357,71 @@ where block_params.body = Some(inner_body); } + // Check equivocations. + let slot_now = inherent_data_providers.slot(); + let authorities = authorities( + self.client.as_ref(), + *block_params.header.parent_hash(), + *block_params.header.number(), + &Default::default(), + )?; + // we add one to allow for some small drift. + // FIXME #1019 in the future, alter this queue to allow deferring of + // headers + let hash = block_params.header.hash(); + block_params.header = check_header_equivocation::( + &self.client, + slot_now + 1, + block_params.header, + hash, + &authorities[..], + ) + .map_err(|e| Self::Error::Other(e.into()))?; + self.inner.import_block(block_params).await } } + +/// Check that the header is valid and not an equivocation. +fn check_header_equivocation( + client: &C, + slot_now: Slot, + header: B::Header, + hash: B::Hash, + authorities: &[AuthorityId

], +) -> Result> +where + P::Public: Codec, + P::Signature: Codec, + C: sc_client_api::backend::AuxStore, +{ + let check_result = + crate::standalone::check_header_slot_and_seal::(slot_now, header, authorities); + + match check_result { + Ok((header, slot, _)) => { + let expected_author = crate::standalone::slot_author::

(slot, &authorities); + if let Some(expected) = expected_author { + if let Some(equivocation_proof) = + check_equivocation(client, slot_now, slot, &header, expected) + .map_err(Error::Client)? + { + log::info!( + target: LOG_TARGET, + "Slot author is equivocating at slot {} with headers {:?} and {:?}", + slot, + equivocation_proof.first_header.hash(), + equivocation_proof.second_header.hash(), + ); + } + } + Ok(header) + }, + Err(SealVerificationError::Deferred(header, _)) => Ok(header), + Err(SealVerificationError::Unsealed) => Err(Error::HeaderUnsealed(hash)), + Err(SealVerificationError::BadSeal) => Err(Error::HeaderBadSeal(hash)), + Err(SealVerificationError::BadSignature) => Err(Error::BadSignature(hash)), + Err(SealVerificationError::SlotAuthorNotFound) => Err(Error::SlotAuthorNotFound), + Err(SealVerificationError::InvalidPreDigest(e)) => Err(Error::from(e)), + } +} From f90501fd1fe5791a22afb13a7982eb94ae42f338 Mon Sep 17 00:00:00 2001 From: sistemd Date: Tue, 1 Jul 2025 13:10:19 +0200 Subject: [PATCH 34/38] don't check equivocations in SlotBasedBlockImport --- .../src/collators/slot_based/block_import.rs | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs index a98c6be9cd58e..b3318181ae19a 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs @@ -194,39 +194,6 @@ where params.body = Some(inner_body); } - // Check for equivocation. - let authorities = sc_consensus_aura::authorities::<

::Public, _, _>( - self.client.as_ref(), - parent_hash, - *params.header.number(), - &self.compatibility_mode, - ) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))?; - let slot_now = create_inherent_data_providers.slot(); - let expected_author = sc_consensus_aura::standalone::slot_author::

(slot, &authorities); - if let (true, Some(expected)) = (self.check_for_equivocation, expected_author) { - if let Some(equivocation_proof) = sc_consensus_slots::check_equivocation( - self.client.as_ref(), - // we add one to allow for some small drift. - // FIXME #1019 in the future, alter this queue to allow deferring of - // headers - slot_now + 1, - slot, - ¶ms.header, - expected, - ) - .map_err(|e| sp_consensus::Error::Other(Box::new(e)))? - { - tracing::info!( - target: LOG_TARGET, - "Slot author is equivocating at slot {} with headers {:?} and {:?}", - slot, - equivocation_proof.first_header.hash(), - equivocation_proof.second_header.hash(), - ); - } - } - // If the channel exists and it is required to execute the block, we will execute the block // here. This is done to collect the storage proof and to prevent re-execution, we push // downwards the state changes. `StateAction::ApplyChanges` is ignored, because it either From 8a755d57af0dcc0fab2e57ed10c04782029edc4a Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 2 Jul 2025 12:06:26 +0200 Subject: [PATCH 35/38] use fully_verifying_import_queue in cumulus-test-client --- .../client/consensus/aura/src/import_queue.rs | 45 ------------------- cumulus/client/consensus/aura/src/lib.rs | 2 +- cumulus/test/service/src/lib.rs | 29 ++++++------ templates/parachain/node/src/service.rs | 2 + 4 files changed, 18 insertions(+), 60 deletions(-) diff --git a/cumulus/client/consensus/aura/src/import_queue.rs b/cumulus/client/consensus/aura/src/import_queue.rs index f3ead1f4b3d54..d95db16cef6ef 100644 --- a/cumulus/client/consensus/aura/src/import_queue.rs +++ b/cumulus/client/consensus/aura/src/import_queue.rs @@ -49,51 +49,6 @@ pub struct ImportQueueParams<'a, I, C, GetSlotFn, S> { pub telemetry: Option, } -/// Start an import queue for the Aura consensus algorithm. -pub fn import_queue( - ImportQueueParams { - block_import, - client, - get_slot, - spawner, - registry, - telemetry, - }: ImportQueueParams<'_, I, C, GetSlotFn, S>, -) -> Result, sp_consensus::Error> -where - Block: BlockT, - C::Api: BlockBuilderApi + AuraApi + ApiExt, - C: 'static - + ProvideRuntimeApi - + BlockOf - + Send - + Sync - + AuxStore - + UsageProvider - + HeaderBackend, - I: BlockImport - + ParachainBlockImportMarker - + Send - + Sync - + 'static, - P: Pair + Send + Sync + 'static, - P::Public: Debug + Codec, - P::Signature: Codec, - S: sp_core::traits::SpawnEssentialNamed, - GetSlotFn: Fn(Block::Hash) -> sp_blockchain::Result + Send + Sync + 'static, -{ - sc_consensus_aura::import_queue::(sc_consensus_aura::ImportQueueParams { - block_import, - justification_import: None, - client, - get_slot, - spawner, - registry, - telemetry, - compatibility_mode: CompatibilityMode::None, - }) -} - /// Parameters of [`build_verifier`]. pub struct BuildVerifierParams { /// The client to interact with the chain. diff --git a/cumulus/client/consensus/aura/src/lib.rs b/cumulus/client/consensus/aura/src/lib.rs index 422a593d91555..97035141d87dd 100644 --- a/cumulus/client/consensus/aura/src/lib.rs +++ b/cumulus/client/consensus/aura/src/lib.rs @@ -59,7 +59,7 @@ use std::{ mod import_queue; -pub use import_queue::{build_verifier, import_queue, BuildVerifierParams, ImportQueueParams}; +pub use import_queue::{build_verifier, BuildVerifierParams, ImportQueueParams}; use polkadot_node_primitives::PoV; pub use sc_consensus_aura::{ slot_duration, standalone::slot_duration_at, AuraVerifier, BuildAuraWorkerParams, diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index f70002fb1971d..7a7c60f743f05 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -258,20 +258,21 @@ pub fn new_partial( .build(), ); - let import_queue = cumulus_client_consensus_aura::import_queue::( - ImportQueueParams { - block_import: block_import.clone(), - client: client.clone(), - get_slot: move |_| { - let timestamp = sp_timestamp::Timestamp::current(); - let slot = Slot::from_timestamp(timestamp, slot_duration); - Ok(slot) - }, - spawner: &task_manager.spawn_essential_handle(), - registry: None, - telemetry: None, - }, - )?; + let import_queue = + cumulus_client_consensus_aura::equivocation_import_queue::fully_verifying_import_queue::< + AuthorityPair, + _, + _, + _, + _, + >( + client.clone(), + block_import.clone(), + move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }, + &task_manager.spawn_essential_handle(), + config.prometheus_registry(), + None, + )?; let params = PartialComponents { backend, diff --git a/templates/parachain/node/src/service.rs b/templates/parachain/node/src/service.rs index a0760fa3e1ec3..f09b8870c06fe 100644 --- a/templates/parachain/node/src/service.rs +++ b/templates/parachain/node/src/service.rs @@ -155,9 +155,11 @@ fn build_import_queue( _, _, _, + _, >( client, block_import, + move |_, _| async move { Ok(sp_timestamp::InherentDataProvider::from_system_time()) }, &task_manager.spawn_essential_handle(), config.prometheus_registry(), telemetry, From 5779e08bcc587cc5d0f054f3b55dbd820882fd97 Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 2 Jul 2025 13:42:50 +0200 Subject: [PATCH 36/38] fixing the build --- .../src/collators/slot_based/block_import.rs | 2 -- .../client/consensus/aura/src/import_queue.rs | 13 +------------ cumulus/test/service/src/lib.rs | 17 +++++++---------- 3 files changed, 8 insertions(+), 24 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs index b3318181ae19a..7368b73e1398f 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/block_import.rs @@ -30,8 +30,6 @@ use sp_runtime::traits::{Block as BlockT, Header as _, NumberFor}; use sp_trie::proof_size_extension::ProofSizeExt; use std::sync::Arc; -use crate::LOG_TARGET; - /// Handle for receiving the block and the storage proof from the [`SlotBasedBlockImport`]. /// /// This handle should be passed to [`Params`](super::Params) or can also be dropped if the node is diff --git a/cumulus/client/consensus/aura/src/import_queue.rs b/cumulus/client/consensus/aura/src/import_queue.rs index d95db16cef6ef..4589cd07cb100 100644 --- a/cumulus/client/consensus/aura/src/import_queue.rs +++ b/cumulus/client/consensus/aura/src/import_queue.rs @@ -17,21 +17,10 @@ //! Parachain specific wrapper for the AuRa import queue. -use codec::Codec; -use cumulus_client_consensus_common::ParachainBlockImportMarker; use prometheus_endpoint::Registry; -use sc_client_api::{backend::AuxStore, BlockOf, UsageProvider}; -use sc_consensus::{import_queue::DefaultImportQueue, BlockImport}; use sc_consensus_aura::{AuraVerifier, CompatibilityMode}; use sc_telemetry::TelemetryHandle; -use sp_api::{ApiExt, ProvideRuntimeApi}; -use sp_block_builder::BlockBuilder as BlockBuilderApi; -use sp_blockchain::HeaderBackend; -use sp_consensus::Error as ConsensusError; -use sp_consensus_aura::{AuraApi, Slot}; -use sp_core::crypto::Pair; -use sp_runtime::traits::Block as BlockT; -use std::{fmt::Debug, sync::Arc}; +use std::sync::Arc; /// Parameters for [`import_queue`]. pub struct ImportQueueParams<'a, I, C, GetSlotFn, S> { diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 9e001cff70f29..9ab2ab1dbff17 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -24,22 +24,19 @@ pub mod bench_utils; pub mod chain_spec; use cumulus_client_collator::service::CollatorService; -use cumulus_client_consensus_aura::{ - collators::{ - lookahead::{self as aura, Params as AuraParams}, - slot_based::{ - self as slot_based, Params as SlotBasedParams, SlotBasedBlockImport, - SlotBasedBlockImportHandle, - }, +use cumulus_client_consensus_aura::collators::{ + lookahead::{self as aura, Params as AuraParams}, + slot_based::{ + self as slot_based, Params as SlotBasedParams, SlotBasedBlockImport, + SlotBasedBlockImportHandle, }, - ImportQueueParams, }; use cumulus_client_consensus_proposer::Proposer; use cumulus_test_runtime::{Hash, Header, NodeBlock as Block, RuntimeApi}; use prometheus::Registry; use runtime::AccountId; use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY}; -use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, sr25519::AuthorityPair, Slot}; +use sp_consensus_aura::{inherents::AuraCreateInherentDataProviders, sr25519::AuthorityPair}; use std::{ collections::HashSet, future::Future, @@ -272,7 +269,7 @@ pub fn new_partial( &task_manager.spawn_essential_handle(), config.prometheus_registry(), None, - )?; + ); let params = PartialComponents { backend, From 78ab1cc1250ced9ad4dee2b0e30eb9170c52898c Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 2 Jul 2025 14:00:50 +0200 Subject: [PATCH 37/38] fix doc comment --- cumulus/client/consensus/aura/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/client/consensus/aura/src/lib.rs b/cumulus/client/consensus/aura/src/lib.rs index 97035141d87dd..ec49ea4285e75 100644 --- a/cumulus/client/consensus/aura/src/lib.rs +++ b/cumulus/client/consensus/aura/src/lib.rs @@ -19,7 +19,7 @@ //! //! This extends the Substrate provided AuRa consensus implementation to make it compatible for //! parachains. The main entry points for of this consensus algorithm are [`AuraConsensus::build`] -//! and [`fn@import_queue`]. +//! and [`fn@equivocation_import_queue::fully_verifying_import_queue`]. //! //! For more information about AuRa, the Substrate crate should be checked. From b7e565691c662e40daa7cf77b7c0180bd0a19b06 Mon Sep 17 00:00:00 2001 From: sistemd Date: Wed, 2 Jul 2025 14:35:55 +0200 Subject: [PATCH 38/38] remove ImportQueueParams --- .../client/consensus/aura/src/import_queue.rs | 17 ----------------- cumulus/client/consensus/aura/src/lib.rs | 2 +- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/cumulus/client/consensus/aura/src/import_queue.rs b/cumulus/client/consensus/aura/src/import_queue.rs index 4589cd07cb100..1e5c94db7eafa 100644 --- a/cumulus/client/consensus/aura/src/import_queue.rs +++ b/cumulus/client/consensus/aura/src/import_queue.rs @@ -17,27 +17,10 @@ //! Parachain specific wrapper for the AuRa import queue. -use prometheus_endpoint::Registry; use sc_consensus_aura::{AuraVerifier, CompatibilityMode}; use sc_telemetry::TelemetryHandle; use std::sync::Arc; -/// Parameters for [`import_queue`]. -pub struct ImportQueueParams<'a, I, C, GetSlotFn, S> { - /// The block import to use. - pub block_import: I, - /// The client to interact with the chain. - pub client: Arc, - /// Callback to get the current slot. - pub get_slot: GetSlotFn, - /// The spawner to spawn background tasks. - pub spawner: &'a S, - /// The prometheus registry. - pub registry: Option<&'a Registry>, - /// The telemetry handle. - pub telemetry: Option, -} - /// Parameters of [`build_verifier`]. pub struct BuildVerifierParams { /// The client to interact with the chain. diff --git a/cumulus/client/consensus/aura/src/lib.rs b/cumulus/client/consensus/aura/src/lib.rs index ec49ea4285e75..908b4c5e924d5 100644 --- a/cumulus/client/consensus/aura/src/lib.rs +++ b/cumulus/client/consensus/aura/src/lib.rs @@ -59,7 +59,7 @@ use std::{ mod import_queue; -pub use import_queue::{build_verifier, BuildVerifierParams, ImportQueueParams}; +pub use import_queue::{build_verifier, BuildVerifierParams}; use polkadot_node_primitives::PoV; pub use sc_consensus_aura::{ slot_duration, standalone::slot_duration_at, AuraVerifier, BuildAuraWorkerParams,