Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

(async backing) restore CheckInherents for backwards-compatibility #2977

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
17 changes: 17 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ members = [
"parachain-template/runtime",
"primitives/core",
"primitives/parachain-inherent",
"primitives/timestamp",
"primitives/utility",
"polkadot-parachain",
"parachains/common",
Expand Down
4 changes: 3 additions & 1 deletion pallets/parachain-system/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,6 @@ runtime-benchmarks = [

try-runtime = ["frame-support/try-runtime"]

parameterized-consensus-hook = []
parameterized-consensus-hook = [
"cumulus-pallet-parachain-system-proc-macro/parameterized-consensus-hook",
]
1 change: 1 addition & 0 deletions pallets/parachain-system/proc-macro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ proc-macro-crate = "1.3.1"
[features]
default = [ "std" ]
std = []
parameterized-consensus-hook = []
40 changes: 32 additions & 8 deletions pallets/parachain-system/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Cumulus. If not, see <http://www.gnu.org/licenses/>.

use proc_macro2::{Span, TokenStream};
use proc_macro2::Span;
use proc_macro_crate::{crate_name, FoundCrate};
use syn::{
parse::{Parse, ParseStream},
Expand All @@ -25,17 +25,20 @@ use syn::{
mod keywords {
syn::custom_keyword!(Runtime);
syn::custom_keyword!(BlockExecutor);
syn::custom_keyword!(CheckInherents);
}

struct Input {
runtime: Path,
block_executor: Path,
check_inherents: Option<Path>,
}

impl Parse for Input {
fn parse(input: ParseStream) -> Result<Self, Error> {
let mut runtime = None;
let mut block_executor = None;
let mut check_inherents = None;

fn parse_inner<KW: Parse + Spanned>(
input: ParseStream,
Expand All @@ -56,26 +59,24 @@ impl Parse for Input {
}
}

while runtime.is_none() || block_executor.is_none() {
while !input.is_empty() || runtime.is_none() || block_executor.is_none() {
let lookahead = input.lookahead1();

if lookahead.peek(keywords::Runtime) {
parse_inner::<keywords::Runtime>(input, &mut runtime)?;
} else if lookahead.peek(keywords::BlockExecutor) {
parse_inner::<keywords::BlockExecutor>(input, &mut block_executor)?;
} else if lookahead.peek(keywords::CheckInherents) {
parse_inner::<keywords::CheckInherents>(input, &mut check_inherents)?;
} else {
return Err(lookahead.error())
}
}

let rest = input.parse::<TokenStream>()?;
if !rest.is_empty() {
return Err(Error::new(rest.span(), "Unexpected input data"))
}

Ok(Self {
runtime: runtime.expect("Everything is parsed before; qed"),
block_executor: block_executor.expect("Everything is parsed before; qed"),
check_inherents,
})
}
}
Expand All @@ -91,7 +92,7 @@ fn crate_() -> Result<Ident, Error> {

#[proc_macro]
pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
let Input { runtime, block_executor } = match syn::parse(input) {
let Input { runtime, block_executor, check_inherents } = match syn::parse(input) {
Ok(t) => t,
Err(e) => return e.into_compile_error().into(),
};
Expand All @@ -101,6 +102,28 @@ pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::To
Err(e) => return e.into_compile_error().into(),
};

let check_inherents = match check_inherents {
Some(_check_inherents) => {
#[cfg(not(feature = "parameterized-consensus-hook"))]
quote::quote! { #_check_inherents }
#[cfg(feature = "parameterized-consensus-hook")]
return Error::new(
Span::call_site(),
"CheckInherents is incompatible with consensus hook feature",
slumber marked this conversation as resolved.
Show resolved Hide resolved
)
.into_compile_error()
.into()
},
None => {
#[cfg(feature = "parameterized-consensus-hook")]
quote::quote! { #crate_::DummyCheckInherents<<#runtime as #crate_::validate_block::GetRuntimeBlockType>::RuntimeBlock> }
#[cfg(not(feature = "parameterized-consensus-hook"))]
return Error::new(Span::call_site(), "missing CheckInherents input")
.into_compile_error()
.into()
},
};

if cfg!(not(feature = "std")) {
quote::quote! {
#[doc(hidden)]
Expand All @@ -127,6 +150,7 @@ pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::To
<#runtime as #crate_::validate_block::GetRuntimeBlockType>::RuntimeBlock,
#block_executor,
#runtime,
#check_inherents,
>(params);

#crate_::validate_block::polkadot_parachain::write_result(&res)
Expand Down
29 changes: 28 additions & 1 deletion pallets/parachain-system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use frame_system::{ensure_none, ensure_root, pallet_prelude::HeaderFor};
use polkadot_parachain::primitives::RelayChainBlockNumber;
use scale_info::TypeInfo;
use sp_runtime::{
traits::{BlockNumberProvider, Hash},
traits::{Block as BlockT, BlockNumberProvider, Hash},
transaction_validity::{
InvalidTransaction, TransactionLongevity, TransactionSource, TransactionValidity,
ValidTransaction,
Expand Down Expand Up @@ -1497,6 +1497,33 @@ impl<T: Config> UpwardMessageSender for Pallet<T> {
}
}

/// Something that can check the inherents of a block.
#[deprecated = "use `cumulus-pallet-aura-ext::FixedVelocityConsensusHook` instead"]
Copy link
Contributor

@rphmeier rphmeier Aug 7, 2023

Choose a reason for hiding this comment

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

A couple nits:

  1. type ConsensusHook in parachain-system currently lives under a feature flag - therefore we are deprecating and asking users to enable a feature flag simultaneously. I'd recommend just adding a comment here for now rather than a deprecation notice.
  2. The comment should be general and not point directly to aura_ext.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[deprecated = "use `cumulus-pallet-aura-ext::FixedVelocityConsensusHook` instead"]
#[cfg_attr(feature = "parameterized-consensus-hook", deprecated = "Perform these checks using a `ConsensusHook` instead"]

Something like this?

pub trait CheckInherents<Block: BlockT> {
/// Check all inherents of the block.
///
/// This function gets passed all the extrinsics of the block, so it is up to the callee to
/// identify the inherents. The `validation_data` can be used to access the
fn check_inherents(
block: &Block,
validation_data: &RelayChainStateProof,
) -> frame_support::inherent::CheckInherentsResult;
}

/// Struct that always returns `Ok` on inherents check, needed for backwards-compatibility.
#[doc(hidden)]
pub struct DummyCheckInherents<Block>(sp_std::marker::PhantomData<Block>);

#[allow(deprecated)]
impl<Block: BlockT> CheckInherents<Block> for DummyCheckInherents<Block> {
fn check_inherents(
_: &Block,
_: &RelayChainStateProof,
) -> frame_support::inherent::CheckInherentsResult {
sp_inherents::CheckInherentsResult::new()
}
}

/// Something that should be informed about system related events.
///
/// This includes events like [`on_validation_data`](Self::on_validation_data) that is being
Expand Down
29 changes: 28 additions & 1 deletion pallets/parachain-system/src/validate_block/implementation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ fn with_externalities<F: FnOnce(&mut dyn Externalities) -> R, R>(f: F) -> R {
/// ensuring that the final storage root matches the storage root in the header of the block. In the
/// end we return back the [`ValidationResult`] with all the required information for the validator.
#[doc(hidden)]
pub fn validate_block<B: BlockT, E: ExecuteBlock<B>, PSC: crate::Config>(
pub fn validate_block<
B: BlockT,
E: ExecuteBlock<B>,
PSC: crate::Config,
CI: crate::CheckInherents<B>,
>(
MemoryOptimizedValidationParams {
block_data,
parent_head,
Expand Down Expand Up @@ -159,6 +164,28 @@ where
sp_io::offchain_index::host_clear.replace_implementation(host_offchain_index_clear),
);

#[cfg(not(feature = "parameterized-consensus-hook"))]
run_with_externalities::<B, _, _>(&backend, || {
let relay_chain_proof = crate::RelayChainStateProof::new(
PSC::SelfParaId::get(),
inherent_data.validation_data.relay_parent_storage_root,
inherent_data.relay_chain_state.clone(),
)
.expect("Invalid relay chain state proof");

let res = CI::check_inherents(&block, &relay_chain_proof);

if !res.ok() {
if log::log_enabled!(log::Level::Error) {
res.into_errors().for_each(|e| {
log::error!("Checking inherent with identifier `{:?}` failed", e.0)
});
}

panic!("Checking inherents failed");
}
});

run_with_externalities::<B, _, _>(&backend, || {
let head_data = HeadData(block.header().encode());

Expand Down
39 changes: 39 additions & 0 deletions primitives/timestamp/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
[package]
name = "cumulus-primitives-timestamp"
version = "0.1.0"
authors = ["Parity Technologies <[email protected]>"]
edition = "2021"
description = "Provides timestamp related functionality for parachains."

[dependencies]
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = [ "derive" ] }
futures = "0.3.28"

# Substrate
sp-inherents = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
sp-std = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
sp-timestamp = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }

# Cumulus
cumulus-primitives-core = { path = "../core", default-features = false }

[dev-dependencies]

# Substrate
sp-consensus = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }

# Cumulus
cumulus-test-client = { path = "../../test/client" }
cumulus-test-relay-sproof-builder = { path = "../../test/relay-sproof-builder" }


[features]
default = [ "std" ]
std = [
"sp-inherents/std",
"sp-std/std",
"sp-timestamp/std",
"cumulus-primitives-core/std",
]
Loading