From 8581bd8bd92c3cf903aa4a3a3aa29fcf8f6e7d50 Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Wed, 10 May 2023 19:32:58 +0400 Subject: [PATCH 1/4] aura_ext consensus hook --- Cargo.lock | 1 + pallets/aura-ext/src/consensus_hook.rs | 44 +++++++++++++++++++ pallets/aura-ext/src/lib.rs | 25 ++++++++++- pallets/parachain-system/Cargo.toml | 2 + .../parachain-system/src/consensus_hook.rs | 14 ++++++ 5 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 pallets/aura-ext/src/consensus_hook.rs diff --git a/Cargo.lock b/Cargo.lock index 93b69a38be1..1470ba34cce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2154,6 +2154,7 @@ version = "0.1.0" dependencies = [ "assert_matches", "bytes", + "cumulus-pallet-aura-ext", "cumulus-pallet-parachain-system-proc-macro", "cumulus-primitives-core", "cumulus-primitives-parachain-inherent", diff --git a/pallets/aura-ext/src/consensus_hook.rs b/pallets/aura-ext/src/consensus_hook.rs new file mode 100644 index 00000000000..032cc700405 --- /dev/null +++ b/pallets/aura-ext/src/consensus_hook.rs @@ -0,0 +1,44 @@ +// 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 . + +//! The definition of a [`FixedVelocityConsensusHook`] for consensus logic to manage +//! block velocity. +//! +//! The velocity `V` refers to the rate of block processing by the relay chain. + +use super::pallet; +use frame_support::pallet_prelude::*; +use sp_std::marker::PhantomData; + +/// A consensus hook for a fixed block processing velocity. +pub struct FixedVelocityConsensusHook(PhantomData); + +impl FixedVelocityConsensusHook { + /// Validates the number of authored blocks within the slot with respect to the `V + 1` limit. + pub fn validate_slot() -> Weight { + // Ensure velocity is non-zero. + let velocity = V.max(1); + + let authored = pallet::Pallet::::slot_info() + .map(|(_slot, authored)| authored) + .expect("slot info is inserted on block initialization"); + if authored > velocity + 1 { + panic!("authored blocks limit is reached for the slot") + } + + T::DbWeight::get().reads(1) + } +} diff --git a/pallets/aura-ext/src/lib.rs b/pallets/aura-ext/src/lib.rs index 15e82edeefe..5605e2f2ac5 100644 --- a/pallets/aura-ext/src/lib.rs +++ b/pallets/aura-ext/src/lib.rs @@ -37,9 +37,12 @@ use frame_support::traits::{ExecuteBlock, FindAuthor}; use sp_application_crypto::RuntimeAppPublic; -use sp_consensus_aura::digests::CompatibleDigestItem; +use sp_consensus_aura::{digests::CompatibleDigestItem, Slot}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +pub mod consensus_hook; +pub use consensus_hook::FixedVelocityConsensusHook; + type Aura = pallet_aura::Pallet; pub use pallet::*; @@ -68,6 +71,19 @@ pub mod pallet { // Fetch the authorities once to get them into the storage proof of the PoV. Authorities::::get(); + let new_slot = Aura::::current_slot(); + + let (new_slot, authored) = match SlotInfo::::get() { + Some((slot, authored)) if slot == new_slot => (slot, authored + 1), + Some((slot, _)) if slot < new_slot => (new_slot, 1), + Some(..) => { + panic!("slot moved backwards") + }, + None => (new_slot, 1), + }; + + SlotInfo::::put((new_slot, authored)); + T::DbWeight::get().reads_writes(2, 1) } } @@ -84,6 +100,13 @@ pub mod pallet { ValueQuery, >; + /// Current slot paired with a number of authored blocks. + /// + /// Updated on each block initialization. + #[pallet::storage] + #[pallet::getter(fn slot_info)] + pub(crate) type SlotInfo = StorageValue<_, (Slot, u32), OptionQuery>; + #[pallet::genesis_config] #[derive(Default)] pub struct GenesisConfig; diff --git a/pallets/parachain-system/Cargo.toml b/pallets/parachain-system/Cargo.toml index 0d6f1954b9e..bfe9e88ebea 100644 --- a/pallets/parachain-system/Cargo.toml +++ b/pallets/parachain-system/Cargo.toml @@ -34,6 +34,7 @@ xcm = { git = "https://github.com/paritytech/polkadot", default-features = false cumulus-pallet-parachain-system-proc-macro = { path = "proc-macro", default-features = false } cumulus-primitives-core = { path = "../../primitives/core", default-features = false } cumulus-primitives-parachain-inherent = { path = "../../primitives/parachain-inherent", default-features = false } +cumulus-pallet-aura-ext = { path = "../aura-ext", default-features = false } [dev-dependencies] assert_matches = "1.5" @@ -60,6 +61,7 @@ std = [ "cumulus-pallet-parachain-system-proc-macro/std", "cumulus-primitives-core/std", "cumulus-primitives-parachain-inherent/std", + "cumulus-pallet-aura-ext/std", "frame-support/std", "frame-system/std", "sp-core/std", diff --git a/pallets/parachain-system/src/consensus_hook.rs b/pallets/parachain-system/src/consensus_hook.rs index 8d3606d14b8..7b99b88c6f9 100644 --- a/pallets/parachain-system/src/consensus_hook.rs +++ b/pallets/parachain-system/src/consensus_hook.rs @@ -20,6 +20,8 @@ use super::relay_state_snapshot::RelayChainStateProof; use sp_std::num::NonZeroU32; +use cumulus_pallet_aura_ext::{consensus_hook as aura_consensus_hook, pallet as pallet_aura_ext}; + /// The possible capacity of the unincluded segment. #[derive(Clone)] pub struct UnincludedSegmentCapacity(UnincludedSegmentCapacityInner); @@ -100,3 +102,15 @@ impl ConsensusHook for FixedCapacityUnincludedSegment { /// /// This is a simple type alias around a fixed-capacity unincluded segment with a size of 1. pub type RequireParentIncluded = FixedCapacityUnincludedSegment<1>; + +impl ConsensusHook + for aura_consensus_hook::FixedVelocityConsensusHook +{ + fn on_state_proof(_state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity { + Self::validate_slot(); + + NonZeroU32::new(sp_std::cmp::max(C, 1)) + .expect("1 is the minimum value and non-zero; qed") + .into() + } +} From a021217b6a5efb9040d0303f6b2a68397d89d380 Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Thu, 11 May 2023 02:37:39 +0400 Subject: [PATCH 2/4] reverse dependency --- Cargo.lock | 1 - pallets/aura-ext/Cargo.toml | 4 ++++ pallets/aura-ext/src/consensus_hook.rs | 21 ++++++++++++------- pallets/parachain-system/Cargo.toml | 2 -- .../parachain-system/src/consensus_hook.rs | 14 ------------- pallets/parachain-system/src/lib.rs | 2 +- 6 files changed, 19 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1470ba34cce..93b69a38be1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2154,7 +2154,6 @@ version = "0.1.0" dependencies = [ "assert_matches", "bytes", - "cumulus-pallet-aura-ext", "cumulus-pallet-parachain-system-proc-macro", "cumulus-primitives-core", "cumulus-primitives-parachain-inherent", diff --git a/pallets/aura-ext/Cargo.toml b/pallets/aura-ext/Cargo.toml index 1db43697511..df145aad522 100644 --- a/pallets/aura-ext/Cargo.toml +++ b/pallets/aura-ext/Cargo.toml @@ -18,6 +18,9 @@ sp-consensus-aura = { git = "https://github.com/paritytech/substrate", default-f sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } sp-std = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } +# Cumulus +cumulus-pallet-parachain-system = { path = "../parachain-system", default-features = false } + [dev-dependencies] # Cumulus @@ -35,5 +38,6 @@ std = [ "sp-consensus-aura/std", "sp-runtime/std", "sp-std/std", + "cumulus-pallet-parachain-system/std", ] try-runtime = ["frame-support/try-runtime"] diff --git a/pallets/aura-ext/src/consensus_hook.rs b/pallets/aura-ext/src/consensus_hook.rs index 032cc700405..cfb869a73a8 100644 --- a/pallets/aura-ext/src/consensus_hook.rs +++ b/pallets/aura-ext/src/consensus_hook.rs @@ -20,15 +20,20 @@ //! The velocity `V` refers to the rate of block processing by the relay chain. use super::pallet; -use frame_support::pallet_prelude::*; -use sp_std::marker::PhantomData; +use cumulus_pallet_parachain_system::{ + consensus_hook::{ConsensusHook, UnincludedSegmentCapacity}, + relay_state_snapshot::RelayChainStateProof, +}; +use sp_std::{marker::PhantomData, num::NonZeroU32}; -/// A consensus hook for a fixed block processing velocity. +/// A consensus hook for a fixed block processing velocity and unincluded segment capacity. pub struct FixedVelocityConsensusHook(PhantomData); -impl FixedVelocityConsensusHook { - /// Validates the number of authored blocks within the slot with respect to the `V + 1` limit. - pub fn validate_slot() -> Weight { +impl ConsensusHook + for FixedVelocityConsensusHook +{ + // Validates the number of authored blocks within the slot with respect to the `V + 1` limit. + fn on_state_proof(_state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity { // Ensure velocity is non-zero. let velocity = V.max(1); @@ -39,6 +44,8 @@ impl FixedVelocityConsensusHook ConsensusHook for FixedCapacityUnincludedSegment { /// /// This is a simple type alias around a fixed-capacity unincluded segment with a size of 1. pub type RequireParentIncluded = FixedCapacityUnincludedSegment<1>; - -impl ConsensusHook - for aura_consensus_hook::FixedVelocityConsensusHook -{ - fn on_state_proof(_state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity { - Self::validate_slot(); - - NonZeroU32::new(sp_std::cmp::max(C, 1)) - .expect("1 is the minimum value and non-zero; qed") - .into() - } -} diff --git a/pallets/parachain-system/src/lib.rs b/pallets/parachain-system/src/lib.rs index 50df5e8ba59..8edeec448af 100644 --- a/pallets/parachain-system/src/lib.rs +++ b/pallets/parachain-system/src/lib.rs @@ -58,7 +58,7 @@ use sp_std::{cmp, collections::btree_map::BTreeMap, prelude::*}; use xcm::latest::XcmHash; mod migration; -mod relay_state_snapshot; +pub mod relay_state_snapshot; #[cfg(test)] mod tests; mod unincluded_segment; From ae8908b951a878e25adb3ac2cbbc6be484534a93 Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Thu, 11 May 2023 02:46:16 +0400 Subject: [PATCH 3/4] include weight into hook --- pallets/aura-ext/src/consensus_hook.rs | 15 ++++++++---- .../parachain-system/src/consensus_hook.rs | 23 ++++++++++++------- pallets/parachain-system/src/lib.rs | 5 ++-- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/pallets/aura-ext/src/consensus_hook.rs b/pallets/aura-ext/src/consensus_hook.rs index cfb869a73a8..c8806b1f4cb 100644 --- a/pallets/aura-ext/src/consensus_hook.rs +++ b/pallets/aura-ext/src/consensus_hook.rs @@ -24,6 +24,7 @@ use cumulus_pallet_parachain_system::{ consensus_hook::{ConsensusHook, UnincludedSegmentCapacity}, relay_state_snapshot::RelayChainStateProof, }; +use frame_support::pallet_prelude::*; use sp_std::{marker::PhantomData, num::NonZeroU32}; /// A consensus hook for a fixed block processing velocity and unincluded segment capacity. @@ -33,7 +34,7 @@ impl ConsensusHook for FixedVelocityConsensusHook { // Validates the number of authored blocks within the slot with respect to the `V + 1` limit. - fn on_state_proof(_state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity { + fn on_state_proof(_state_proof: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity) { // Ensure velocity is non-zero. let velocity = V.max(1); @@ -43,9 +44,13 @@ impl ConsensusHook if authored > velocity + 1 { panic!("authored blocks limit is reached for the slot") } - - NonZeroU32::new(sp_std::cmp::max(C, 1)) - .expect("1 is the minimum value and non-zero; qed") - .into() + let weight = T::DbWeight::get().reads(1); + + ( + weight, + NonZeroU32::new(sp_std::cmp::max(C, 1)) + .expect("1 is the minimum value and non-zero; qed") + .into(), + ) } } diff --git a/pallets/parachain-system/src/consensus_hook.rs b/pallets/parachain-system/src/consensus_hook.rs index 8d3606d14b8..bdf590a0fd5 100644 --- a/pallets/parachain-system/src/consensus_hook.rs +++ b/pallets/parachain-system/src/consensus_hook.rs @@ -18,6 +18,7 @@ //! of parachain blocks ready to submit to the relay chain, as well as some basic implementations. use super::relay_state_snapshot::RelayChainStateProof; +use frame_support::weights::Weight; use sp_std::num::NonZeroU32; /// The possible capacity of the unincluded segment. @@ -61,8 +62,8 @@ 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; + /// to return a maximum capacity for the unincluded segment with weight consumed. + fn on_state_proof(state_proof: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity); } /// A special consensus hook for handling the migration to asynchronous backing gracefully, @@ -75,8 +76,11 @@ pub trait ConsensusHook { pub struct ExpectParentIncluded; impl ConsensusHook for ExpectParentIncluded { - fn on_state_proof(_state_proof: &RelayChainStateProof) -> UnincludedSegmentCapacity { - UnincludedSegmentCapacity(UnincludedSegmentCapacityInner::ExpectParentIncluded) + fn on_state_proof(_state_proof: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity) { + ( + Weight::zero(), + UnincludedSegmentCapacity(UnincludedSegmentCapacityInner::ExpectParentIncluded), + ) } } @@ -88,10 +92,13 @@ impl ConsensusHook for ExpectParentIncluded { pub struct FixedCapacityUnincludedSegment; impl ConsensusHook for FixedCapacityUnincludedSegment { - 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() + fn on_state_proof(_state_proof: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity) { + ( + Weight::zero(), + NonZeroU32::new(sp_std::cmp::max(N, 1)) + .expect("1 is the minimum value and non-zero; qed") + .into(), + ) } } diff --git a/pallets/parachain-system/src/lib.rs b/pallets/parachain-system/src/lib.rs index 8edeec448af..a5f29cfdea4 100644 --- a/pallets/parachain-system/src/lib.rs +++ b/pallets/parachain-system/src/lib.rs @@ -484,7 +484,8 @@ pub mod pallet { .expect("Invalid relay chain state proof"); // Update the desired maximum capacity according to the consensus hook. - let capacity = T::ConsensusHook::on_state_proof(&relay_state_proof); + let (consensus_hook_weight, capacity) = + T::ConsensusHook::on_state_proof(&relay_state_proof); // initialization logic: we know that this runs exactly once every block, // which means we can put the initialization logic here to remove the @@ -543,7 +544,7 @@ pub mod pallet { // ancestor was included, the MQC heads wouldn't match and the block would be invalid. // // - let mut total_weight = Weight::zero(); + let mut total_weight = consensus_hook_weight; total_weight += Self::process_inbound_downward_messages( relevant_messaging_state.dmq_mqc_head, downward_messages, From 94d4d2bb572e4c9ddf3a66f47a7e3805b7fc3a1d Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Thu, 11 May 2023 16:40:39 +0400 Subject: [PATCH 4/4] fix tests --- pallets/parachain-system/src/lib.rs | 2 +- .../parachain-system/src/relay_state_snapshot.rs | 2 ++ pallets/parachain-system/src/tests.rs | 14 +++++++++----- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pallets/parachain-system/src/lib.rs b/pallets/parachain-system/src/lib.rs index a5f29cfdea4..5601bcdd17d 100644 --- a/pallets/parachain-system/src/lib.rs +++ b/pallets/parachain-system/src/lib.rs @@ -58,12 +58,12 @@ use sp_std::{cmp, collections::btree_map::BTreeMap, prelude::*}; use xcm::latest::XcmHash; mod migration; -pub mod relay_state_snapshot; #[cfg(test)] mod tests; mod unincluded_segment; pub mod consensus_hook; +pub mod relay_state_snapshot; #[macro_use] pub mod validate_block; diff --git a/pallets/parachain-system/src/relay_state_snapshot.rs b/pallets/parachain-system/src/relay_state_snapshot.rs index 9da5a03ce83..ead077f527d 100644 --- a/pallets/parachain-system/src/relay_state_snapshot.rs +++ b/pallets/parachain-system/src/relay_state_snapshot.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Cumulus. If not, see . +//! Relay chain state proof provides means for accessing part of relay chain storage for reads. + use codec::{Decode, Encode}; use cumulus_primitives_core::{ relay_chain, AbridgedHostConfiguration, AbridgedHrmpChannel, ParaId, diff --git a/pallets/parachain-system/src/tests.rs b/pallets/parachain-system/src/tests.rs index e1010f89be0..574ab43078d 100755 --- a/pallets/parachain-system/src/tests.rs +++ b/pallets/parachain-system/src/tests.rs @@ -121,14 +121,14 @@ std::thread_local! { static HANDLED_DMP_MESSAGES: RefCell)>> = RefCell::new(Vec::new()); static HANDLED_XCMP_MESSAGES: RefCell)>> = RefCell::new(Vec::new()); static SENT_MESSAGES: RefCell)>> = RefCell::new(Vec::new()); - static CONSENSUS_HOOK: RefCell UnincludedSegmentCapacity>> - = RefCell::new(Box::new(|_| NonZeroU32::new(1).unwrap().into())); + static CONSENSUS_HOOK: RefCell (Weight, UnincludedSegmentCapacity)>> + = RefCell::new(Box::new(|_| (Weight::zero(), NonZeroU32::new(1).unwrap().into()))); } pub struct TestConsensusHook; impl ConsensusHook for TestConsensusHook { - fn on_state_proof(s: &RelayChainStateProof) -> UnincludedSegmentCapacity { + fn on_state_proof(s: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity) { CONSENSUS_HOOK.with(|f| f.borrow_mut()(s)) } } @@ -440,7 +440,9 @@ fn block_tests_run_on_drop() { #[test] fn unincluded_segment_works() { - CONSENSUS_HOOK.with(|c| *c.borrow_mut() = Box::new(|_| NonZeroU32::new(10).unwrap().into())); + CONSENSUS_HOOK.with(|c| { + *c.borrow_mut() = Box::new(|_| (Weight::zero(), NonZeroU32::new(10).unwrap().into())) + }); BlockTests::new() .with_inclusion_delay(1) @@ -475,7 +477,9 @@ fn unincluded_segment_works() { #[test] #[should_panic = "no space left for the block in the unincluded segment"] fn unincluded_segment_is_limited() { - CONSENSUS_HOOK.with(|c| *c.borrow_mut() = Box::new(|_| NonZeroU32::new(1).unwrap().into())); + CONSENSUS_HOOK.with(|c| { + *c.borrow_mut() = Box::new(|_| (Weight::zero(), NonZeroU32::new(1).unwrap().into())) + }); BlockTests::new() .with_inclusion_delay(2)