Skip to content
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
9ebf20c
check inherents in import_block
sistemd May 20, 2025
4431bb1
empty prdoc
sistemd May 20, 2025
0f9533b
Update from github-actions[bot] running command 'prdoc --audience nod…
github-actions[bot] May 20, 2025
e3e5846
fix clippies
sistemd May 20, 2025
dc09e64
toml formatting
sistemd May 20, 2025
610fd40
update test
sistemd May 20, 2025
09a0896
fix semver
sistemd May 20, 2025
73a9b0a
remove .vim
sistemd May 21, 2025
4eb7197
use Arc instead of custom CreateInherentDataProvider impls
sistemd May 22, 2025
cc36b4a
remove CheckForEquivocation
sistemd May 23, 2025
eb14482
typedefs
sistemd May 23, 2025
8966cbf
remove redundant type parameter
sistemd May 23, 2025
484579a
improvements and oversights
sistemd May 27, 2025
ea74fa2
Merge remote-tracking branch 'origin/master' into sistemd/check-inher…
sistemd May 27, 2025
9bc4f69
improve logs
sistemd May 30, 2025
d59dc15
Merge remote-tracking branch 'origin/master' into sistemd/check-inher…
sistemd May 30, 2025
f9c046a
fix
sistemd May 30, 2025
fc83d6b
remove ValidatingBlockImport
sistemd Jun 2, 2025
d3f0bc7
remove outdated comment
sistemd Jun 6, 2025
a171009
remove unneeded BlockBuilder trait
sistemd Jun 6, 2025
2130887
remove unnecessary InherentDataProviderExt constraint
sistemd Jun 6, 2025
4e9a465
use simple callback for AuraVerifier instead of CIDP
sistemd Jun 6, 2025
ab5abde
Merge remote-tracking branch 'origin/master' into sistemd/check-inher…
sistemd Jun 6, 2025
c1c9e97
fix compiler errors
sistemd Jun 6, 2025
40fd334
update prdoc
sistemd Jun 6, 2025
ac54928
remove BlockBuilder constraint
sistemd Jun 9, 2025
b8b9b72
remove doc hidden
sistemd Jun 9, 2025
2d800c0
update prdoc
sistemd Jun 9, 2025
50c8e9c
remove unneeded constraints
sistemd Jun 11, 2025
81d7d39
Merge branch 'master' into sistemd/check-inherents-when-importing-block
sistemd Jun 11, 2025
8566346
fix warning
sistemd Jun 11, 2025
7a28d33
add AuraBlockImport for polkadot-omni-node
sistemd Jun 12, 2025
89642dc
Merge remote-tracking branch 'origin/master' into sistemd/check-inher…
sistemd Jun 12, 2025
6147815
fixing ci
sistemd Jun 13, 2025
07df2ee
Merge branch 'master' into sistemd/check-inherents-when-importing-block
sistemd Jun 13, 2025
55dcf7b
Merge branch 'master' into sistemd/check-inherents-when-importing-block
sistemd Jun 13, 2025
500d87a
Merge branch 'master' into sistemd/check-inherents-when-importing-block
sistemd Jun 24, 2025
171156c
Merge branch 'master' into sistemd/check-inherents-when-importing-block
sistemd Jun 25, 2025
4a77f23
remove some unneeded constraints
sistemd Jun 25, 2025
8aae9f1
move NaiveEquivocationDefender back into import queue
sistemd Jun 30, 2025
845db2b
move AuraBlockImport to substrate
sistemd Jun 30, 2025
ef58955
strict equivocation check in AuraBlockImport
sistemd Jun 30, 2025
f90501f
don't check equivocations in SlotBasedBlockImport
sistemd Jul 1, 2025
8a755d5
use fully_verifying_import_queue in cumulus-test-client
sistemd Jul 2, 2025
507f3b2
Merge remote-tracking branch 'origin/master' into sistemd/check-inher…
sistemd Jul 2, 2025
5779e08
fixing the build
sistemd Jul 2, 2025
78ab1cc
fix doc comment
sistemd Jul 2, 2025
b7e5656
remove ImportQueueParams
sistemd Jul 2, 2025
2721f74
Merge branch 'master' into sistemd/check-inherents-when-importing-block
sistemd Jul 9, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 16 additions & 6 deletions cumulus/client/consensus/aura/src/collators/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -101,14 +104,15 @@ where
+ Send
+ Sync
+ 'static,
Client::Api: AuraApi<Block, P::Public> + CollectCollationInfo<Block>,
Client::Api: BlockBuilder<Block> + AuraApi<Block, P::Public> + CollectCollationInfo<Block>,
RClient: RelayChainInterface + Send + Clone + 'static,
CIDP: CreateInherentDataProviders<Block, ()> + Send + 'static,
CIDP::InherentDataProviders: Send,
CIDP: CreateInherentDataProviders<Block, ()> + Clone + Send + 'static,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CIDP: CreateInherentDataProviders<Block, ()> + Clone + Send + 'static,
CIDP: CreateInherentDataProviders<Block, ()> + Send + 'static,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4a77f23 (and other constraints-related changes)

CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync,
BI: BlockImport<Block> + ParachainBlockImportMarker + Send + Sync + 'static,
BI::Error: Into<sp_consensus::Error>,
Proposer: ProposerInterface<Block> + Send + Sync + 'static,
CS: CollatorServiceInterface<Block> + Send + Sync + 'static,
P: Pair,
P: Pair + Sync + Send + 'static,
P::Public: AppPublic + Member + Codec,
P::Signature: TryFrom<Vec<u8>> + Member + Codec,
{
Expand All @@ -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(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
create_inherent_data_providers: params.create_inherent_data_providers.clone(),
create_inherent_data_providers: params.create_inherent_data_providers,

The changes to this file seem unnecessary.

block_import: ValidatingBlockImport::<_, _, _, _, P>::new(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said below, this is not needed and can be deleted.

params.block_import,
params.para_client.clone(),
params.create_inherent_data_providers.clone(),
true,
Default::default(),
),
relay_client: params.relay_client.clone(),
keystore: params.keystore.clone(),
para_id: params.para_id,
Expand Down
39 changes: 27 additions & 12 deletions cumulus/client/consensus/aura/src/collators/lookahead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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::*;
Expand Down Expand Up @@ -114,17 +117,20 @@ where
+ Send
+ Sync
+ 'static,
Client::Api:
AuraApi<Block, P::Public> + CollectCollationInfo<Block> + AuraUnincludedSegmentApi<Block>,
Client::Api: BlockBuilder<Block>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't need the BlockBuilder API here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ AuraApi<Block, P::Public>
+ CollectCollationInfo<Block>
+ AuraUnincludedSegmentApi<Block>,
Backend: sc_client_api::Backend<Block> + 'static,
RClient: RelayChainInterface + Clone + 'static,
CIDP: CreateInherentDataProviders<Block, ()> + 'static,
CIDP::InherentDataProviders: Send,
CIDP: CreateInherentDataProviders<Block, ()> + Clone + 'static,
CIDP::InherentDataProviders: InherentDataProviderExt + Send,
BI: BlockImport<Block> + ParachainBlockImportMarker + Send + Sync + 'static,
BI::Error: Into<sp_consensus::Error>,
Proposer: ProposerInterface<Block> + Send + Sync + 'static,
CS: CollatorServiceInterface<Block> + Send + Sync + 'static,
CHP: consensus_common::ValidationCodeHashProvider<Block::Hash> + Send + 'static,
P: Pair,
P: Pair + Send + Sync + 'static,
P::Public: AppPublic + Member + Codec,
P::Signature: TryFrom<Vec<u8>> + Member + Codec,
{
Expand Down Expand Up @@ -166,17 +172,20 @@ where
+ Send
+ Sync
+ 'static,
Client::Api:
AuraApi<Block, P::Public> + CollectCollationInfo<Block> + AuraUnincludedSegmentApi<Block>,
Client::Api: BlockBuilder<Block>
+ AuraApi<Block, P::Public>
+ CollectCollationInfo<Block>
+ AuraUnincludedSegmentApi<Block>,
Backend: sc_client_api::Backend<Block> + 'static,
RClient: RelayChainInterface + Clone + 'static,
CIDP: CreateInherentDataProviders<Block, ()> + 'static,
CIDP::InherentDataProviders: Send,
CIDP: CreateInherentDataProviders<Block, ()> + Clone + 'static,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as in basic.rs, bound not needes and clone call below too.

CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync,
BI: BlockImport<Block> + ParachainBlockImportMarker + Send + Sync + 'static,
BI::Error: Into<sp_consensus::Error>,
Proposer: ProposerInterface<Block> + Send + Sync + 'static,
CS: CollatorServiceInterface<Block> + Send + Sync + 'static,
CHP: consensus_common::ValidationCodeHashProvider<Block::Hash> + Send + 'static,
P: Pair,
P: Pair + Send + Sync + 'static,
P::Public: AppPublic + Member + Codec,
P::Signature: TryFrom<Vec<u8>> + Member + Codec,
{
Expand Down Expand Up @@ -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(),
true,
Default::default(),
),
relay_client: params.relay_client.clone(),
keystore: params.keystore.clone(),
para_id: params.para_id,
Expand Down
187 changes: 183 additions & 4 deletions cumulus/client/consensus/aura/src/collators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -270,6 +281,174 @@ where
.map(|parent| (included_block, parent))
}

struct ValidatingBlockImport<BI, Client, CIDP, N, P> {
inner: BI,
client: Arc<Client>,
create_inherent_data_providers: CIDP,
check_for_equivocation: bool,
compatibility_mode: CompatibilityMode<N>,
_phantom: std::marker::PhantomData<P>,
}

impl<BI, Client, CIDP, N, P> ValidatingBlockImport<BI, Client, CIDP, N, P> {
pub fn new(
inner: BI,
client: Arc<Client>,
create_inherent_data_providers: CIDP,
check_for_equivocation: bool,
compatibility_mode: CompatibilityMode<N>,
) -> Self {
Self {
inner,
client,
create_inherent_data_providers,
check_for_equivocation,
compatibility_mode,
_phantom: Default::default(),
}
}
}

#[async_trait]
impl<Block, BI, Client, CIDP, P> BlockImport<Block>
for ValidatingBlockImport<BI, Client, CIDP, NumberFor<Block>, P>
where
Block: BlockT,
BI: BlockImport<Block> + Sync,
BI::Error: Into<sp_consensus::Error>,
Client: ProvideRuntimeApi<Block> + sc_client_api::backend::AuxStore + Send + Sync,
Client::Api: Core<Block> + BlockBuilder<Block> + AuraApi<Block, <P as Pair>::Public>,
P: Pair + Sync,
P::Public: Codec + std::fmt::Debug,
P::Signature: Codec,
CIDP: sp_inherents::CreateInherentDataProviders<Block, ()> + Send,
CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync,
{
type Error = sp_consensus::Error;

async fn check_block(
&self,
block: BlockCheckParams<Block>,
) -> Result<ImportResult, Self::Error> {
self.inner.check_block(block).await.map_err(Into::into)
}

async fn import_block(
&self,
mut block: BlockImportParams<Block>,
) -> Result<ImportResult, Self::Error> {
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<BI, Client, CIDP, N, P> ParachainBlockImportMarker
for ValidatingBlockImport<BI, Client, CIDP, N, P>
{
}

async fn validate_block_import<Block, Client, P, CIDP>(
params: &mut sc_consensus::BlockImportParams<Block>,
client: &Client,
create_inherent_data_providers: &CIDP,
check_for_equivocation: bool,
compatibility_mode: &CompatibilityMode<NumberFor<Block>>,
) -> Result<(), sp_consensus::Error>
where
Block: BlockT,
Client: ProvideRuntimeApi<Block> + sc_client_api::backend::AuxStore + Send + Sync,
Client::Api: Core<Block> + BlockBuilder<Block> + AuraApi<Block, <P as Pair>::Public>,
P: Pair + Sync,
P::Public: Codec + std::fmt::Debug,
P::Signature: Codec,
CIDP: sp_inherents::CreateInherentDataProviders<Block, ()> + Send,
CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync,
{
let slot = find_pre_digest::<Block, P::Signature>(&params.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::<dyn BlockBuilder<Block>, _>(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::<<P as Pair>::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::<P>(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,
&params.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;
Expand Down
Loading
Loading