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

Feature Flagging Consensus Hook Type Parameter #2911

Merged
merged 6 commits into from
Jul 20, 2023
Merged

Feature Flagging Consensus Hook Type Parameter #2911

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5f0980a
First pass
BradleyOlson64 Jul 19, 2023
e155997
fmt
BradleyOlson64 Jul 19, 2023
7afe68b
Added as default feature in tomls
BradleyOlson64 Jul 20, 2023
d1238b0
Changed to direct dependency feature
BradleyOlson64 Jul 20, 2023
6ff8923
Dealing with clippy error
BradleyOlson64 Jul 20, 2023
eec072d
Update pallets/parachain-system/src/lib.rs
BradleyOlson64 Jul 20, 2023
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 pallets/parachain-system/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,5 @@ runtime-benchmarks = [
]

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

parameterized-consensus-hook = []
Copy link
Contributor

Choose a reason for hiding this comment

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

missing endlines though

9 changes: 6 additions & 3 deletions pallets/parachain-system/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ use unincluded_segment::{
UsedBandwidth,
};

pub use consensus_hook::ConsensusHook;
pub use consensus_hook::{ConsensusHook, ExpectParentIncluded};
/// Register the `validate_block` function that is used by parachains to validate blocks on a
/// validator.
///
Expand Down Expand Up @@ -212,6 +212,7 @@ pub mod pallet {
/// [`consensus_hook::ExpectParentIncluded`] here. This is only necessary in the case
/// that collators aren't expected to have node versions that supply the included block
/// in the relay-chain state proof.
#[cfg(feature = "parameterized-consensus-hook")]
BradleyOlson64 marked this conversation as resolved.
Show resolved Hide resolved
type ConsensusHook: ConsensusHook;
}

Expand Down Expand Up @@ -490,8 +491,10 @@ pub mod pallet {
.expect("Invalid relay chain state proof");

// Update the desired maximum capacity according to the consensus hook.
let (consensus_hook_weight, capacity) =
T::ConsensusHook::on_state_proof(&relay_state_proof);
#[cfg(feature = "parameterized-consensus-hook")]
let (consensus_hook_weight, capacity) = T::ConsensusHook::on_state_proof(&relay_state_proof);
#[cfg(not(feature = "parameterized-consensus-hook"))]
let (consensus_hook_weight, capacity) = ExpectParentIncluded::on_state_proof(&relay_state_proof);
total_weight += consensus_hook_weight;
total_weight += Self::maybe_drop_included_ancestors(&relay_state_proof, capacity);
// Deposit a log indicating the relay-parent storage root.
Expand Down
8 changes: 5 additions & 3 deletions pallets/parachain-system/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ use frame_support::{
traits::{OnFinalize, OnInitialize},
weights::Weight,
};
use frame_system::{pallet_prelude::{BlockNumberFor, HeaderFor}, RawOrigin};
use frame_system::{
pallet_prelude::{BlockNumberFor, HeaderFor},
RawOrigin,
};
use hex_literal::hex;
use relay_chain::HrmpChannelId;
use sp_core::{blake2_256, H256};
Expand Down Expand Up @@ -238,8 +241,7 @@ struct BlockTests {
inherent_data_hook:
Option<Box<dyn Fn(&BlockTests, RelayChainBlockNumber, &mut ParachainInherentData)>>,
inclusion_delay: Option<usize>,
relay_block_number:
Option<Box<dyn Fn(&BlockNumberFor<Test>) -> RelayChainBlockNumber>>,
relay_block_number: Option<Box<dyn Fn(&BlockNumberFor<Test>) -> RelayChainBlockNumber>>,

included_para_head: Option<relay_chain::HeadData>,
pending_blocks: VecDeque<relay_chain::HeadData>,
Expand Down
4 changes: 4 additions & 0 deletions parachain-template/runtime/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,7 @@ try-runtime = [
"pallet-xcm/try-runtime",
"parachain-info/try-runtime",
]

parameterized-consensus-hook = [
"cumulus-pallet-parachain-system/parameterized-consensus-hook",
]
4 changes: 4 additions & 0 deletions parachain-template/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,14 @@ const MAXIMUM_BLOCK_WEIGHT: Weight = Weight::from_parts(

/// Maximum number of blocks simultaneously accepted by the Runtime, not yet included
/// into the relay chain.
#[cfg(feature = "parameterized-consensus-hook")]
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't actually need feature-flags in the parachain-template, as it's meant to be example code. So it should use the most recently available logic.

const UNINCLUDED_SEGMENT_CAPACITY: u32 = 1;
/// How many parachain blocks are processed by the relay chain per parent. Limits the
/// number of blocks authored per slot.
#[cfg(feature = "parameterized-consensus-hook")]
const BLOCK_PROCESSING_VELOCITY: u32 = 1;
/// Relay chain slot duration, in seconds.
#[cfg(feature = "parameterized-consensus-hook")]
const RELAY_CHAIN_SLOT_DURATION: u32 = 6;

/// The version information used to identify this runtime when compiled natively.
Expand Down Expand Up @@ -388,6 +391,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/assets/asset-hub-kusama/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

use codec::{Decode, Encode, MaxEncodedLen};
use constants::{consensus::*, currency::*, fee::WeightToFee};
#[cfg(feature = "parameterized-consensus-hook")]
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need the feature flag in system-parachain runtimes either, as they're binaries, not libraries.

use constants::consensus::*;
use constants::{currency::*, fee::WeightToFee};
use frame_support::{
construct_runtime,
dispatch::DispatchClass,
Expand Down Expand Up @@ -537,6 +539,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/assets/asset-hub-polkadot/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

use codec::{Decode, Encode, MaxEncodedLen};
use constants::{consensus::*, currency::*, fee::WeightToFee};
#[cfg(feature = "parameterized-consensus-hook")]
use constants::consensus::*;
use constants::{currency::*, fee::WeightToFee};
use frame_support::{
construct_runtime,
dispatch::DispatchClass,
Expand Down Expand Up @@ -550,6 +552,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/assets/asset-hub-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ pub mod xcm_config;
use crate::xcm_config::{TrustBackedAssetsPalletLocation, UniversalLocation};
use assets_common::local_and_foreign_assets::{LocalAndForeignAssets, MultiLocationConverter};
use codec::{Decode, Encode, MaxEncodedLen};
use constants::{consensus::*, currency::*, fee::WeightToFee};
#[cfg(feature = "parameterized-consensus-hook")]
use constants::consensus::*;
use constants::{currency::*, fee::WeightToFee};
use cumulus_pallet_parachain_system::RelayNumberStrictlyIncreases;
use frame_support::{
construct_runtime,
Expand Down Expand Up @@ -584,6 +586,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ use sp_std::prelude::*;
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

use constants::{consensus::*, currency::*, fee::WeightToFee};
#[cfg(feature = "parameterized-consensus-hook")]
use constants::consensus::*;
use constants::{currency::*, fee::WeightToFee};
use frame_support::{
construct_runtime,
dispatch::DispatchClass,
Expand Down Expand Up @@ -284,6 +286,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ use sp_std::prelude::*;
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

use constants::{consensus::*, currency::*, fee::WeightToFee};
#[cfg(feature = "parameterized-consensus-hook")]
use constants::consensus::*;
use constants::{currency::*, fee::WeightToFee};
use frame_support::{
construct_runtime,
dispatch::DispatchClass,
Expand Down Expand Up @@ -284,6 +286,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ pub mod constants;
mod weights;
pub mod xcm_config;

use constants::{consensus::*, currency::*};
#[cfg(feature = "parameterized-consensus-hook")]
use constants::consensus::*;
use constants::currency::*;
use cumulus_pallet_parachain_system::RelayNumberStrictlyIncreases;
use sp_api::impl_runtime_apis;
use sp_core::{crypto::KeyTypeId, OpaqueMetadata};
Expand Down Expand Up @@ -299,6 +301,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/collectives/collectives-polkadot/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

use codec::{Decode, Encode, MaxEncodedLen};
use constants::{consensus::*, currency::*, fee::WeightToFee};
#[cfg(feature = "parameterized-consensus-hook")]
use constants::consensus::*;
use constants::{currency::*, fee::WeightToFee};
use frame_support::{
construct_runtime,
dispatch::DispatchClass,
Expand Down Expand Up @@ -369,6 +371,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
5 changes: 4 additions & 1 deletion parachains/runtimes/contracts/contracts-rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ use sp_std::prelude::*;
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

use constants::{consensus::*, currency::*, fee::WeightToFee};
#[cfg(feature = "parameterized-consensus-hook")]
use constants::consensus::*;
use constants::{currency::*, fee::WeightToFee};
use frame_support::{
construct_runtime,
dispatch::DispatchClass,
Expand Down Expand Up @@ -274,6 +276,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
1 change: 1 addition & 0 deletions parachains/runtimes/glutton/glutton-kusama/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = ();
type ReservedXcmpWeight = ();
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_parachain_system::consensus_hook::ExpectParentIncluded;
}

Expand Down
1 change: 1 addition & 0 deletions parachains/runtimes/starters/seedling/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = ();
type ReservedXcmpWeight = ();
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_parachain_system::consensus_hook::ExpectParentIncluded;
}

Expand Down
1 change: 1 addition & 0 deletions parachains/runtimes/starters/shell/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = ();
type ReservedXcmpWeight = ();
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_parachain_system::consensus_hook::ExpectParentIncluded;
}

Expand Down
4 changes: 4 additions & 0 deletions parachains/runtimes/testing/penpal/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,14 @@ const MAXIMUM_BLOCK_WEIGHT: Weight = Weight::from_parts(

/// Maximum number of blocks simultaneously accepted by the Runtime, not yet included
/// into the relay chain.
#[cfg(feature = "parameterized-consensus-hook")]
const UNINCLUDED_SEGMENT_CAPACITY: u32 = 1;
/// How many parachain blocks are processed by the relay chain per parent. Limits the
/// number of blocks authored per slot.
#[cfg(feature = "parameterized-consensus-hook")]
const BLOCK_PROCESSING_VELOCITY: u32 = 1;
/// Relay chain slot duration, in seconds.
#[cfg(feature = "parameterized-consensus-hook")]
const RELAY_CHAIN_SLOT_DURATION: u32 = 6;

/// The version information used to identify this runtime when compiled natively.
Expand Down Expand Up @@ -466,6 +469,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
4 changes: 4 additions & 0 deletions parachains/runtimes/testing/rococo-parachain/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,14 @@ const MAXIMUM_BLOCK_WEIGHT: Weight = Weight::from_parts(

/// Maximum number of blocks simultaneously accepted by the Runtime, not yet included
/// into the relay chain.
#[cfg(feature = "parameterized-consensus-hook")]
const UNINCLUDED_SEGMENT_CAPACITY: u32 = 1;
/// How many parachain blocks are processed by the relay chain per parent. Limits the
/// number of blocks authored per slot.
#[cfg(feature = "parameterized-consensus-hook")]
const BLOCK_PROCESSING_VELOCITY: u32 = 1;
/// Relay chain slot duration, in seconds.
#[cfg(feature = "parameterized-consensus-hook")]
const RELAY_CHAIN_SLOT_DURATION: u32 = 6;

parameter_types! {
Expand Down Expand Up @@ -280,6 +283,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = XcmpQueue;
type ReservedXcmpWeight = ReservedXcmpWeight;
type CheckAssociatedRelayNumber = RelayNumberStrictlyIncreases;
#[cfg(feature = "parameterized-consensus-hook")]
type ConsensusHook = cumulus_pallet_aura_ext::FixedVelocityConsensusHook<
Runtime,
RELAY_CHAIN_SLOT_DURATION,
Expand Down
4 changes: 3 additions & 1 deletion test/relay-sproof-builder/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ impl RelayStateSproofBuilder {
if let Some(para_head) = self.included_para_head {
insert(relay_chain::well_known_keys::para_head(self.para_id), para_head.encode());
}
if let Some(relay_dispatch_queue_remaining_capacity) = self.relay_dispatch_queue_remaining_capacity {
if let Some(relay_dispatch_queue_remaining_capacity) =
self.relay_dispatch_queue_remaining_capacity
{
insert(
relay_chain::well_known_keys::relay_dispatch_queue_remaining_capacity(
self.para_id,
Expand Down
1 change: 1 addition & 0 deletions test/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
type XcmpMessageHandler = ();
type ReservedXcmpWeight = ();
type CheckAssociatedRelayNumber = cumulus_pallet_parachain_system::AnyRelayNumber;
#[cfg(feature = "parameterized-consensus-hook")]
Copy link
Contributor

Choose a reason for hiding this comment

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

likewise here - it seems fine for the test runtime (which is effectively a binary crate used internally, not a library) to explicitly use the RequireParentIncluded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which way is best to get this to compile without the feature flag here?

Screenshot 2023-07-19 at 5 02 57 PM

I expect some change is needed around line 215 of pallets/parachain-system/src/lib.rs such that it can still accept a ConsensusHook type even without the feature flag enabled. Just haven't seen an example of what that would look like.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just have it unconditionally enable the crate feature in its Cargo.toml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does make sense 👍 Will be back on to try it this evening.

Copy link
Contributor

@slumber slumber Jul 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Robert probably meant doing

cumulus-pallet-parachain-system = { path = "../../../../pallets/parachain-system", default-features = false, features = [
    "parameterized-consensus-hook",
] }

instead of introducing a feature to test-crates.

type ConsensusHook = cumulus_pallet_parachain_system::consensus_hook::RequireParentIncluded;
}

Expand Down