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

refactor unincluded segment length into a ConsensusHook #2501

Merged
merged 19 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from 9 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
1 change: 1 addition & 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 client/collator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ polkadot-node-subsystem-test-helpers = { git = "https://github.com/paritytech/po
# Cumulus
cumulus-test-client = { path = "../../test/client" }
cumulus-test-runtime = { path = "../../test/runtime" }
cumulus-test-relay-sproof-builder = { path = "../../test/relay-sproof-builder" }
8 changes: 7 additions & 1 deletion client/collator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,11 @@ mod tests {
TestClientBuilder, TestClientBuilderExt,
};
use cumulus_test_runtime::{Block, Header};
use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder;
use futures::{channel::mpsc, executor::block_on, StreamExt};
use polkadot_node_subsystem_test_helpers::ForwardSubsystem;
use polkadot_overseer::{dummy::dummy_overseer_builder, HeadSupportsParachains};
use polkadot_primitives::HeadData;
use sp_consensus::BlockOrigin;
use sp_core::{testing::TaskExecutor, Pair};
use sp_runtime::traits::BlakeTwo256;
Expand Down Expand Up @@ -415,10 +417,14 @@ mod tests {
_: PHash,
validation_data: &PersistedValidationData,
) -> Option<ParachainCandidate<Block>> {
let mut sproof = RelayStateSproofBuilder::default();
sproof.included_para_head = Some(HeadData(parent.encode()));
sproof.para_id = cumulus_test_runtime::PARACHAIN_ID.into();

slumber marked this conversation as resolved.
Show resolved Hide resolved
let builder = self.client.init_block_builder_at(
parent.hash(),
Some(validation_data.clone()),
Default::default(),
sproof,
);

let (block, _, proof) = builder.build().expect("Creates block").into_inner();
Expand Down
102 changes: 102 additions & 0 deletions pallets/parachain-system/src/consensus_hook.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright 2023 Parity Technologies (UK) Ltd.
// This file is part of Cumulus.

// Cumulus is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Cumulus is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Cumulus. If not, see <http://www.gnu.org/licenses/>.

//! The definition of a [`ConsensusHook`] trait for consensus logic to manage the backlog
//! of parachain blocks ready to submit to the relay chain, as well as some basic implementations.

use super::relay_state_snapshot::RelayChainStateProof;
use sp_std::num::NonZeroU32;

/// The possible capacity of the unincluded segment.
#[derive(Clone)]
pub struct UnincludedSegmentCapacity(UnincludedSegmentCapacityInner);

impl UnincludedSegmentCapacity {
pub(crate) fn get(&self) -> u32 {
match self.0 {
UnincludedSegmentCapacityInner::ExpectParentIncluded => 1,
UnincludedSegmentCapacityInner::Value(v) => v.get(),
}
}

pub(crate) fn is_expecting_included_parent(&self) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Capacity expecting included parent seems confusing, or perhaps tangential?

Why isn't it is_set or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's more of a "is demanding" than "is expecting", true.

match self.0 {
UnincludedSegmentCapacityInner::ExpectParentIncluded => true,
UnincludedSegmentCapacityInner::Value(_) => false,
}
}
}

#[derive(Clone)]
pub(crate) enum UnincludedSegmentCapacityInner {
ExpectParentIncluded,
Value(NonZeroU32),
}

impl From<NonZeroU32> for UnincludedSegmentCapacity {
fn from(value: NonZeroU32) -> Self {
UnincludedSegmentCapacity(UnincludedSegmentCapacityInner::Value(value))
}
}

/// The consensus hook for dealing with the unincluded segment.
///
/// Higher-level and user-configurable consensus logic is more informed about the
/// desired unincluded segment length, as well as any rules for adapting it dynamically
/// according to the relay-chain state.
pub trait ConsensusHook {
/// This hook is called partway through the `set_validation_data` inherent in parachain-system.
///
/// The hook is allowed to panic if customized consensus rules aren't met and is required
/// to return a maximum capacity for the unincluded segment.
fn on_state_proof(state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity;
}

/// A special consensus hook for handling the migration to asynchronous backing gracefully,
/// even if collators haven't been updated to provide the last included parent in the state
/// proof yet.
///
/// This behaves as though the parent is included, even if the relay chain state proof doesn't contain
/// the included para head. If the para head is present in the state proof, this does ensure the
/// parent is included.
pub struct ExpectParentIncluded;

impl ConsensusHook for ExpectParentIncluded {
fn on_state_proof(_state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity {
UnincludedSegmentCapacity(UnincludedSegmentCapacityInner::ExpectParentIncluded)
}
}

/// A consensus hook for a fixed unincluded segment length. This hook does nothing but
/// set the capacity of the unincluded segment to the constant N.
///
/// Since it is illegal to provide an unincluded segment length of 0, this sets a minimum of
/// 1.
pub struct FixedCapacityUnincludedSegment<const N: u32>;

impl<const N: u32> ConsensusHook for FixedCapacityUnincludedSegment<N> {
fn on_state_proof(_state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity {
NonZeroU32::new(sp_std::cmp::max(N, 1))
.expect("1 is the minimum value and non-zero; qed")
.into()
}
}

/// A fixed-capacity unincluded segment hook, which requires that the parent block is
/// included prior to the current block being authored.
///
/// This is a simple type alias around a fixed-capacity unincluded segment with a size of 1.
pub type RequireParentIncluded = FixedCapacityUnincludedSegment<1>;
Loading