From 5c65fa5634c87a2181d0e534c65be64d7caaa859 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 25 Nov 2021 17:25:11 +0000 Subject: [PATCH] pvf-precheck: Integrate PVF pre-checking into paras module Closes #4009 This is the most of the runtime-side change needed for #3211. Here is how it works. The PVF pre-checking can be triggered either by an upgrade or by onboarding (i.e. calling `schedule_para_initialize`). The PVF pre-checking process is identified by the PVF code hash that is being voted on. If there is already PVF pre-checking process running, then no new PVF pre-checking process will be started. Instead, we just subscribe to the existing one. If there is no PVF pre-checking process running but the PVF code hash was already saved in the storage, that necessarily means (I invite the reviewers to double-check this invariant) that the PVF already passed pre-checking. This is equivalent to instant approving of the PVF. The pre-checking process can be concluded either by obtaining a supermajority or if it expires. Each validator checks the list of PVFs available for voting. The vote is binary, i.e. accept or reject a given PVF. As soon as the supermajority of votes are collected for one of the sides of the vote, the voting is concluded in that direction and the effects of the voting are enacted. Only validators from the active set can participate in the vote. The set of active validators can change each session. That's why we reset the votes each session. A voting that observed a certain number of sessions will be rejected. The effects of the PVF accepting depend on the operations requested it: 1. All onboardings subscribed to the approved PVF pre-checking process will get scheduled and after passing 2 session boundaries they will be onboarded. 2. All upgrades subscribed to the approved PVF pre-checking process will get scheduled very similarly to the existing process. Upgrades with pre-checking are really the same process that is just delayed by the time required for pre-checking voting. In case of instant approval the mechanism is exactly the same. This is important from parachains compatibility standpoint since following the delayed upgrade requires the parachain to implement https://github.com/paritytech/cumulus/pull/517. In case, PVF pre-checking process was concluded with rejection, then all the requesting operations get cancelled. For onboarding it means it gets without movement: the lifecycle of such parachain is terminated on the `Onboarding` state and after rejection the lifecycle is none. That in turn means that the caller can attempt registering the parachain once more. For upgrading it means that the upgrade process is aborted: that flashes go-ahead signal with `Abort` flag. Rejection leads to removing the allegedly bad validation code from the chain storage. Among other things, this implies that the operation can be re-requested. That allows for retrying an operation in case there was some bug. At the same time it does not look as a DoS vector due to the caching performed by the nodes. PVF pre-checking can be enabled and disabled. Initially, according to the changes in #4420, this mechanism is disabled. Triggering the PVF pre-checking when it is disabled just means that we insta approve the requesting operation. This should lead to the behavior being unchanged. Follow-ups: - expose runtime APIs --- runtime/common/src/assigned_slots.rs | 15 + runtime/common/src/integration_tests.rs | 19 +- runtime/common/src/mock.rs | 25 +- runtime/common/src/paras_registrar.rs | 15 + runtime/kusama/src/lib.rs | 6 + runtime/parachains/src/configuration.rs | 3 + runtime/parachains/src/hrmp.rs | 1 + runtime/parachains/src/mock.rs | 35 +- runtime/parachains/src/paras.rs | 1248 +++++++++++++++-- runtime/parachains/src/runtime_api_impl/v1.rs | 18 +- runtime/parachains/src/scheduler.rs | 1 + runtime/polkadot/src/lib.rs | 6 + runtime/rococo/src/lib.rs | 6 + runtime/test-runtime/src/lib.rs | 8 +- runtime/westend/src/lib.rs | 6 + 15 files changed, 1322 insertions(+), 90 deletions(-) diff --git a/runtime/common/src/assigned_slots.rs b/runtime/common/src/assigned_slots.rs index e0981a63d8c0..60633f1954e9 100644 --- a/runtime/common/src/assigned_slots.rs +++ b/runtime/common/src/assigned_slots.rs @@ -554,6 +554,7 @@ mod tests { use sp_core::H256; use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, + transaction_validity::TransactionPriority, DispatchError::BadOrigin, }; @@ -576,6 +577,14 @@ mod tests { } ); + impl frame_system::offchain::SendTransactionTypes for Test + where + Call: From, + { + type Extrinsic = UncheckedExtrinsic; + type OverarchingCall = Call; + } + parameter_types! { pub const BlockHashCount: u32 = 250; } @@ -625,9 +634,15 @@ mod tests { type WeightInfo = parachains_configuration::TestWeightInfo; } + parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); + } + impl parachains_paras::Config for Test { type Event = Event; type WeightInfo = parachains_paras::TestWeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = crate::mock::TestNextSessionRotation; } impl parachains_shared::Config for Test {} diff --git a/runtime/common/src/integration_tests.rs b/runtime/common/src/integration_tests.rs index aa5feee11b30..bd29d9aec0a2 100644 --- a/runtime/common/src/integration_tests.rs +++ b/runtime/common/src/integration_tests.rs @@ -38,7 +38,10 @@ use runtime_parachains::{ use sp_core::{crypto::KeyTypeId, H256}; use sp_io::TestExternalities; use sp_keystore::{testing::KeyStore, KeystoreExt}; -use sp_runtime::traits::{BlakeTwo256, IdentityLookup, One}; +use sp_runtime::{ + traits::{BlakeTwo256, IdentityLookup, One}, + transaction_validity::TransactionPriority, +}; use sp_std::sync::Arc; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -73,6 +76,14 @@ frame_support::construct_runtime!( } ); +impl frame_system::offchain::SendTransactionTypes for Test +where + Call: From, +{ + type Extrinsic = UncheckedExtrinsic; + type OverarchingCall = Call; +} + use crate::{auctions::Error as AuctionsError, crowdloan::Error as CrowdloanError}; parameter_types! { @@ -169,9 +180,15 @@ impl shared::Config for Test {} impl origin::Config for Test {} +parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); +} + impl paras::Config for Test { type Event = Event; type WeightInfo = paras::TestWeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = crate::mock::TestNextSessionRotation; } parameter_types! { diff --git a/runtime/common/src/mock.rs b/runtime/common/src/mock.rs index 523f0c766689..ae664e25efd4 100644 --- a/runtime/common/src/mock.rs +++ b/runtime/common/src/mock.rs @@ -17,10 +17,13 @@ //! Mocking utilities for testing. use crate::traits::Registrar; -use frame_support::dispatch::{DispatchError, DispatchResult}; +use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + weights::Weight, +}; use parity_scale_codec::{Decode, Encode}; use primitives::v1::{HeadData, Id as ParaId, ValidationCode}; -use sp_runtime::traits::SaturatedConversion; +use sp_runtime::{traits::SaturatedConversion, Permill}; use std::{cell::RefCell, collections::HashMap}; thread_local! { @@ -210,3 +213,21 @@ impl TestRegistrar { MANAGERS.with(|x| x.borrow_mut().clear()); } } + +/// A very dumb implementation of `EstimateNextSessionRotation`. At the moment of writing, this +/// is more to satisfy type requirements rather than to test anything. +pub struct TestNextSessionRotation; + +impl frame_support::traits::EstimateNextSessionRotation for TestNextSessionRotation { + fn average_session_length() -> u32 { + 10 + } + + fn estimate_current_session_progress(_now: u32) -> (Option, Weight) { + (None, 0) + } + + fn estimate_next_session_rotation(_now: u32) -> (Option, Weight) { + (None, 0) + } +} diff --git a/runtime/common/src/paras_registrar.rs b/runtime/common/src/paras_registrar.rs index 778292dddf1d..6635711496fd 100644 --- a/runtime/common/src/paras_registrar.rs +++ b/runtime/common/src/paras_registrar.rs @@ -583,6 +583,7 @@ mod tests { use sp_io::TestExternalities; use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, + transaction_validity::TransactionPriority, Perbill, }; @@ -605,6 +606,14 @@ mod tests { } ); + impl frame_system::offchain::SendTransactionTypes for Test + where + Call: From, + { + type Extrinsic = UncheckedExtrinsic; + type OverarchingCall = Call; + } + const NORMAL_RATIO: Perbill = Perbill::from_percent(75); parameter_types! { pub const BlockHashCount: u32 = 250; @@ -660,9 +669,15 @@ mod tests { impl origin::Config for Test {} + parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); + } + impl paras::Config for Test { type Event = Event; type WeightInfo = paras::TestWeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = crate::mock::TestNextSessionRotation; } impl configuration::Config for Test { diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index efc1f29e2c77..ae3915c6f956 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1170,9 +1170,15 @@ impl parachains_inclusion::Config for Runtime { type RewardValidators = parachains_reward_points::RewardValidatorsWithEraPoints; } +parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); +} + impl parachains_paras::Config for Runtime { type Event = Event; type WeightInfo = weights::runtime_parachains_paras::WeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = Babe; } parameter_types! { diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index b91d581785e0..5e869f0a915f 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -73,6 +73,9 @@ pub struct HostConfiguration { /// This parameter affects the upper bound of size of `CandidateCommitments`. pub hrmp_max_message_num_per_candidate: u32, /// The minimum frequency at which parachains can update their validation code. + /// + /// If PVF pre-checking is enabled this should be greater than the maximum number of blocks + /// PVF pre-checking can take. pub validation_upgrade_frequency: BlockNumber, /// The delay, in blocks, before a validation upgrade is applied. pub validation_upgrade_delay: BlockNumber, diff --git a/runtime/parachains/src/hrmp.rs b/runtime/parachains/src/hrmp.rs index 87ba4ad861b8..447b2b98a92c 100644 --- a/runtime/parachains/src/hrmp.rs +++ b/runtime/parachains/src/hrmp.rs @@ -1370,6 +1370,7 @@ mod tests { configuration: crate::configuration::GenesisConfig { config: crate::configuration::HostConfiguration { max_downward_message_size: 1024, + pvf_checking_enabled: false, ..Default::default() }, }, diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index b7af99305ba3..4194a8dfabbc 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -38,7 +38,8 @@ use sp_core::H256; use sp_io::TestExternalities; use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, - KeyTypeId, + transaction_validity::TransactionPriority, + KeyTypeId, Permill, }; use std::{cell::RefCell, collections::HashMap}; @@ -70,6 +71,14 @@ frame_support::construct_runtime!( } ); +impl frame_system::offchain::SendTransactionTypes for Test +where + Call: From, +{ + type Extrinsic = UncheckedExtrinsic; + type OverarchingCall = Call; +} + parameter_types! { pub const BlockHashCount: u32 = 250; pub BlockWeights: frame_system::limits::BlockWeights = @@ -180,9 +189,33 @@ impl crate::shared::Config for Test {} impl origin::Config for Test {} +parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); +} + +/// A very dumb implementation of `EstimateNextSessionRotation`. At the moment of writing, this +/// is more to satisfy type requirements rather than to test anything. +pub struct TestNextSessionRotation; + +impl frame_support::traits::EstimateNextSessionRotation for TestNextSessionRotation { + fn average_session_length() -> u32 { + 10 + } + + fn estimate_current_session_progress(_now: u32) -> (Option, Weight) { + (None, 0) + } + + fn estimate_next_session_rotation(_now: u32) -> (Option, Weight) { + (None, 0) + } +} + impl crate::paras::Config for Test { type Event = Event; type WeightInfo = crate::paras::TestWeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = TestNextSessionRotation; } impl crate::dmp::Config for Test {} diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index a173d5cd06cb..4e32e1d4004e 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -24,17 +24,21 @@ //! only occur at session boundaries. use crate::{configuration, initializer::SessionChangeNotification, shared}; -use frame_support::pallet_prelude::*; +use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec}; +use frame_support::{pallet_prelude::*, traits::EstimateNextSessionRotation}; use frame_system::pallet_prelude::*; use parity_scale_codec::{Decode, Encode}; use primitives::v1::{ - ConsensusLog, HeadData, Id as ParaId, SessionIndex, UpgradeGoAhead, UpgradeRestriction, - ValidationCode, ValidationCodeHash, + ConsensusLog, HeadData, Id as ParaId, PvfCheckStatement, SessionIndex, UpgradeGoAhead, + UpgradeRestriction, ValidationCode, ValidationCodeHash, ValidatorSignature, }; use scale_info::TypeInfo; use sp_core::RuntimeDebug; -use sp_runtime::{traits::One, DispatchResult, SaturatedConversion}; -use sp_std::prelude::*; +use sp_runtime::{ + traits::{AppVerify, One}, + DispatchResult, SaturatedConversion, +}; +use sp_std::{cmp, convert::TryInto, mem, prelude::*}; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; @@ -203,6 +207,96 @@ pub struct ParaGenesisArgs { pub parachain: bool, } +/// This enum describes a reason why a particular PVF pre-checking vote was initiated. When the +/// PVF vote in question is concluded, this enum indicates what changes should be performed. +#[derive(Encode, Decode, TypeInfo)] +enum PvfCheckCause { + /// PVF vote was initiated by the initial onboarding process of the given para. + Onboarding(ParaId), + /// PVF vote was initiated by signalling of an upgrade by the given para. + Upgrade { + /// The ID of the parachain that initiated or is waiting for the conclusion of pre-checking. + id: ParaId, + /// The relay-chain block number that was used as the relay-parent for the parablock that + /// initiated the upgrade. + relay_parent_number: BlockNumber, + }, +} + +/// Specifies what was the outcome of a PVF pre-checking vote. +#[derive(Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] +enum PvfCheckOutcome { + Accepted, + Rejected, +} + +/// This struct describes the current state of an in-progress PVF pre-checking vote. +#[derive(Encode, Decode, TypeInfo)] +struct PvfCheckActiveVoteState { + // The two following vectors have their length equal to the number of validators in the active + // set. They start with all zeroes. A 1 is set at an index when the validator at the that index + // makes a vote. Once a 1 is set for either of the vectors, that validator cannot vote anymore. + // Since the active validator set changes each session, the bit vectors are reinitialized as + // well: zeroed and resized so that each validator gets its own bit. + votes_accept: BitVec, + votes_reject: BitVec, + + /// The number of session changes this PVF vote has observed. Therefore, this number is + /// increased at each session boundary. When created, it is initialized with 0. + age: SessionIndex, + /// The block number at which this PVF vote was created. + created_at: BlockNumber, + /// A list of causes for this PVF pre-checking. Has at least one. + causes: Vec>, +} + +impl PvfCheckActiveVoteState { + /// Returns a new instance of vote state, started at the specified block `now`, with the + /// number of validators in the current session `n_validators` and the originating `cause`. + fn new(now: BlockNumber, n_validators: usize, cause: PvfCheckCause) -> Self { + let mut causes = Vec::with_capacity(1); + causes.push(cause); + Self { + created_at: now, + votes_accept: bitvec::bitvec![BitOrderLsb0, u8; 0; n_validators], + votes_reject: bitvec::bitvec![BitOrderLsb0, u8; 0; n_validators], + age: 0, + causes, + } + } + + /// Resets all votes and resizes the votes vectors corresponding to the number of validators + /// in the new session. + fn reinitialize_ballots(&mut self, n_validators: usize) { + let clear_and_resize = |v: &mut BitVec<_, _>| { + v.clear(); + v.resize(n_validators, false); + }; + clear_and_resize(&mut self.votes_accept); + clear_and_resize(&mut self.votes_reject); + } + + /// Returns `Some(true)` if the validator at the given index has already cast their vote within + /// the ongoing session. Returns `None` in case the index is out of bounds. + fn has_vote(&self, validator_index: usize) -> Option { + let accept_vote = self.votes_accept.get(validator_index)?; + let reject_vote = self.votes_reject.get(validator_index)?; + Some(*accept_vote || *reject_vote) + } + + /// Returns `None` if the quorum is not reached, or the direction of the decision. + fn quorum(&self, n_validators: usize) -> Option { + let q_threshold = primitives::v1::supermajority_threshold(n_validators); + if self.votes_accept.count_ones() >= q_threshold { + Some(PvfCheckOutcome::Accepted) + } else if self.votes_reject.count_ones() >= q_threshold { + Some(PvfCheckOutcome::Rejected) + } else { + None + } + } +} + pub trait WeightInfo { fn force_set_current_code(c: u32) -> Weight; fn force_set_current_head(s: u32) -> Weight; @@ -233,15 +327,29 @@ impl WeightInfo for TestWeightInfo { #[frame_support::pallet] pub mod pallet { use super::*; + use sp_runtime::transaction_validity::{ + InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, + ValidTransaction, + }; #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] pub struct Pallet(_); #[pallet::config] - pub trait Config: frame_system::Config + configuration::Config + shared::Config { + pub trait Config: + frame_system::Config + + configuration::Config + + shared::Config + + frame_system::offchain::SendTransactionTypes> + { type Event: From + IsType<::Event>; + #[pallet::constant] + type UnsignedPriority: Get; + + type NextSessionRotation: EstimateNextSessionRotation; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -273,7 +381,35 @@ pub mod pallet { CannotUpgrade, /// Para cannot be downgraded to a parathread. CannotDowngrade, - } + /// The statement for PVF pre-checking is stale. + PvfCheckStatementStale, + /// Claimed validator index is out of bounds. + PvfCheckValidatorIndexOutOfBounds, + /// The signature for the PVF pre-checking is invalid. + PvfCheckInvalidSignature, + /// The given validator already has cast a vote. + PvfCheckDoubleVote, + /// The given PVF does not exist at the moment of process a vote. + PvfCheckSubjectInvalid, + } + + /// All currently active PVF pre-checking votes. + /// + /// Invariant: + /// - There are no PVF pre-checking votes that exists in list but not in the set and vice versa. + #[pallet::storage] + pub(super) type PvfActiveVoteMap = StorageMap< + _, + Twox64Concat, + ValidationCodeHash, + PvfCheckActiveVoteState, + OptionQuery, + >; + + /// The list of all currently active PVF votes. Auxiliary to `PvfActiveVoteMap`. + #[pallet::storage] + pub(super) type PvfActiveVoteList = + StorageValue<_, Vec, ValueQuery>; /// All parachains. Ordered ascending by `ParaId`. Parathreads are not included. #[pallet::storage] @@ -524,9 +660,166 @@ pub mod pallet { Self::deposit_event(Event::ActionQueued(para, next_session)); Ok(()) } + + /// Includes a statement for a PVF pre-checking vote. Potentially, finalizes the vote and + /// enacts the results if that was the last vote before achieving the supermajority. + #[pallet::weight(0)] + pub fn include_pvf_check_statement( + origin: OriginFor, + stmt: PvfCheckStatement, + signature: ValidatorSignature, + ) -> DispatchResult { + ensure_none(origin)?; + let validators = shared::Pallet::::active_validator_keys(); + let current_session = shared::Pallet::::session_index(); + ensure!(stmt.session_index == current_session, Error::::PvfCheckStatementStale); + let validator_index = stmt.validator_index.0 as usize; + let validator_public = validators + .get(validator_index) + .ok_or(Error::::PvfCheckValidatorIndexOutOfBounds)?; + + let signing_payload = stmt.signing_payload(); + ensure!( + signature.verify(&signing_payload[..], &validator_public), + Error::::PvfCheckInvalidSignature, + ); + + let mut active_vote = PvfActiveVoteMap::::get(&stmt.subject) + .ok_or(Error::::PvfCheckSubjectInvalid)?; + + ensure!( + !active_vote + .has_vote(validator_index) + .ok_or(Error::::PvfCheckValidatorIndexOutOfBounds)?, + Error::::PvfCheckDoubleVote, + ); + + // Finally, cast the vote and persist. + if stmt.accept { + active_vote.votes_accept.set(validator_index, true); + } else { + active_vote.votes_reject.set(validator_index, true); + } + + if let Some(outcome) = active_vote.quorum(validators.len()) { + // The supermajority quorum has been achieved. + // + // Remove the PVF vote from the active map and finalize the PVF checking according + // to the outcome. + PvfActiveVoteMap::::remove(&stmt.subject); + PvfActiveVoteList::::mutate(|l| { + if let Ok(i) = l.binary_search(&stmt.subject) { + l.remove(i); + } + }); + match outcome { + PvfCheckOutcome::Accepted => { + let cfg = configuration::Pallet::::config(); + Self::enact_pvf_accepted( + >::block_number(), + &stmt.subject, + &active_vote.causes, + active_vote.age, + &cfg, + ); + }, + PvfCheckOutcome::Rejected => { + Self::enact_pvf_rejected(&stmt.subject, active_vote.causes); + }, + } + } else { + // No quorum has been achieved. So just store the updated state back into the + // storage. + PvfActiveVoteMap::::insert(&stmt.subject, active_vote); + } + + Ok(()) + } + } + + #[pallet::validate_unsigned] + impl ValidateUnsigned for Pallet { + type Call = Call; + + fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { + let (stmt, signature) = match call { + Call::include_pvf_check_statement { stmt, signature } => (stmt, signature), + _ => return InvalidTransaction::Call.into(), + }; + + let current_session = shared::Pallet::::session_index(); + if stmt.session_index != current_session { + return InvalidTransaction::Stale.into() + } + + let validator_index = stmt.validator_index.0 as usize; + let validators = shared::Pallet::::active_validator_keys(); + let validator_public = match validators.get(validator_index) { + Some(pk) => pk, + None => return InvalidTransaction::Custom(INVALID_TX_BAD_VALIDATOR_IDX).into(), + }; + + let signing_payload = stmt.signing_payload(); + if !signature.verify(&signing_payload[..], &validator_public) { + return InvalidTransaction::BadProof.into() + } + + let active_vote = match PvfActiveVoteMap::::get(&stmt.subject) { + Some(v) => v, + None => return InvalidTransaction::Custom(INVALID_TX_BAD_SUBJECT).into(), + }; + + match active_vote.has_vote(validator_index) { + Some(false) => (), + Some(true) => return InvalidTransaction::Custom(INVALID_TX_DOUBLE_VOTE).into(), + None => return InvalidTransaction::Custom(INVALID_TX_BAD_VALIDATOR_IDX).into(), + } + + ValidTransaction::with_tag_prefix("PvfPreCheckingVote") + .priority(T::UnsignedPriority::get()) + .longevity( + TryInto::::try_into( + T::NextSessionRotation::average_session_length() / 2u32.into(), + ) + .unwrap_or(64_u64), + ) + .and_provides((stmt.session_index, stmt.validator_index, stmt.subject)) + .propagate(true) + .build() + } + + fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> { + let stmt = match call { + Call::include_pvf_check_statement { stmt, .. } => stmt, + _ => return Err(InvalidTransaction::Call.into()), + }; + + let current_session = shared::Pallet::::session_index(); + if stmt.session_index != current_session { + return Err(InvalidTransaction::Stale.into()) + } + + let active_vote = match PvfActiveVoteMap::::get(&stmt.subject) { + Some(v) => v, + None => return Err(InvalidTransaction::Custom(INVALID_TX_BAD_SUBJECT).into()), + }; + + match active_vote.has_vote(stmt.validator_index.0 as usize) { + Some(false) => (), + Some(true) => return Err(InvalidTransaction::Custom(INVALID_TX_DOUBLE_VOTE).into()), + None => return Err(InvalidTransaction::Custom(INVALID_TX_BAD_VALIDATOR_IDX).into()), + } + + Ok(()) + } } } +// custom transaction error codes +const INVALID_TX_BAD_VALIDATOR_IDX: u8 = 1; +const INVALID_TX_BAD_SUBJECT: u8 = 2; +const INVALID_TX_DOUBLE_VOTE: u8 = 3; + impl Pallet { /// Called by the initializer to initialize the configuration pallet. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { @@ -544,6 +837,7 @@ impl Pallet { notification: &SessionChangeNotification, ) -> Vec { let outgoing_paras = Self::apply_actions_queue(notification.session_index); + Self::groom_ongoing_pvf_votes(¬ification.new_config, notification.validators.len()); outgoing_paras } @@ -581,7 +875,6 @@ impl Pallet { match lifecycle { None | Some(ParaLifecycle::Parathread) | Some(ParaLifecycle::Parachain) => { /* Nothing to do... */ }, - // Onboard a new parathread or parachain. Some(ParaLifecycle::Onboarding) => { if let Some(genesis_data) = ::UpcomingParasGenesis::take(¶) { if genesis_data.parachain { @@ -593,10 +886,18 @@ impl Pallet { ParaLifecycles::::insert(¶, ParaLifecycle::Parathread); } - let code_hash = genesis_data.validation_code.hash(); + // HACK: see the notice in `schedule_para_initialize`. + // + // Apparently, this is left over from a prior version of the runtime. + // To handle this we just insert the code and link the current code hash + // to it. + if !genesis_data.validation_code.0.is_empty() { + let code_hash = genesis_data.validation_code.hash(); + Self::increase_code_ref(&code_hash, &genesis_data.validation_code); + ::CurrentCodeHash::insert(¶, code_hash); + } + ::Heads::insert(¶, genesis_data.genesis_head); - Self::increase_code_ref(&code_hash, &genesis_data.validation_code); - ::CurrentCodeHash::insert(¶, code_hash); } }, // Upgrade a parathread to a parachain @@ -648,13 +949,13 @@ impl Pallet { // NOTE both of those iterates over the list and the outgoing. We do not expect either // of these to be large. Thus should be fine. ::UpcomingUpgrades::mutate(|upcoming_upgrades| { - *upcoming_upgrades = sp_std::mem::take(upcoming_upgrades) + *upcoming_upgrades = mem::take(upcoming_upgrades) .into_iter() .filter(|&(ref para, _)| !outgoing.contains(para)) .collect(); }); ::UpgradeCooldowns::mutate(|upgrade_cooldowns| { - *upgrade_cooldowns = sp_std::mem::take(upgrade_cooldowns) + *upgrade_cooldowns = mem::take(upgrade_cooldowns) .into_iter() .filter(|&(ref para, _)| !outgoing.contains(para)) .collect(); @@ -689,7 +990,7 @@ impl Pallet { // exits the slashing window. ::PastCodePruning::mutate(|pruning| { let insert_idx = - pruning.binary_search_by_key(&at, |&(_, b)| b).unwrap_or_else(|idx| idx); + pruning.binary_search_by_key(&now, |&(_, b)| b).unwrap_or_else(|idx| idx); pruning.insert(insert_idx, (id, now)); }); @@ -781,32 +1082,248 @@ impl Pallet { T::DbWeight::get().reads_writes(2, upgrades_signaled as u64 + cooldowns_expired as u64) } + /// Goes over all PVF votes in progress, reinitializes ballots, increments ages and prunes the + /// active votes that reached their time-to-live. + fn groom_ongoing_pvf_votes( + cfg: &configuration::HostConfiguration, + new_n_validators: usize, + ) -> Weight { + let mut weight = T::DbWeight::get().reads(1); + + let potentially_active_votes = PvfActiveVoteList::::get(); + + // Initially empty list which contains all the PVF active votes that made it through this + // session change. + // + // **Ordered** as well as `PvfActiveVoteList`. + let mut actually_active_votes = Vec::with_capacity(potentially_active_votes.len()); + + for vote_subject in potentially_active_votes { + let mut vote_state = match PvfActiveVoteMap::::take(&vote_subject) { + Some(v) => v, + None => { + // This branch should never be reached. This is due to the fact that the set of + // `PvfActiveVoteMap`'s keys is always equal to the set of items found in + // `PvfActiveVoteList`. + log::warn!( + target: "runtime::paras", + "The PvfActiveVoteMap is out of sync with PvfActiveVoteList!", + ); + debug_assert!(false); + continue + }, + }; + + vote_state.age += 1; + if vote_state.age < cfg.pvf_voting_ttl { + weight += T::DbWeight::get().writes(1); + vote_state.reinitialize_ballots(new_n_validators); + PvfActiveVoteMap::::insert(&vote_subject, vote_state); + + // push maintaining the original order. + actually_active_votes.push(vote_subject); + } else { + // TTL is reached. Reject. + weight += Self::enact_pvf_rejected(&vote_subject, vote_state.causes); + } + } + + weight += T::DbWeight::get().writes(1); + PvfActiveVoteList::::put(actually_active_votes); + + weight + } + + fn enact_pvf_accepted( + now: T::BlockNumber, + code_hash: &ValidationCodeHash, + causes: &[PvfCheckCause], + sessions_observed: SessionIndex, + cfg: &configuration::HostConfiguration, + ) -> Weight { + let mut weight = 0; + for cause in causes { + match cause { + PvfCheckCause::Onboarding(id) => { + weight += Self::proceed_with_onboarding(*id, sessions_observed); + }, + PvfCheckCause::Upgrade { id, relay_parent_number } => { + weight += + Self::proceed_with_upgrade(*id, code_hash, now, *relay_parent_number, cfg); + }, + } + } + weight + } + + fn proceed_with_onboarding(id: ParaId, sessions_observed: SessionIndex) -> Weight { + let weight = T::DbWeight::get().reads_writes(2, 1); + + // we should onboard only after `SESSION_DELAY` sessions but we should take + // into account the number of sessions the PVF pre-checking occupied. + // + // we cannot onboard at the current session, so it must be at least one + // session ahead. + let onboard_at: SessionIndex = shared::Pallet::::session_index() + + cmp::max(shared::SESSION_DELAY.saturating_sub(sessions_observed), 1); + + ActionsQueue::::mutate(onboard_at, |v| { + if let Err(i) = v.binary_search(&id) { + v.insert(i, id); + } + }); + + weight + } + + fn proceed_with_upgrade( + id: ParaId, + code_hash: &ValidationCodeHash, + now: T::BlockNumber, + relay_parent_number: T::BlockNumber, + cfg: &configuration::HostConfiguration, + ) -> Weight { + let mut weight = 0; + + // Compute the relay-chain block number starting at which the code upgrade is ready to be + // applied. + // + // The first parablock that has a relay-parent higher or at the same height of `expected_at` + // will trigger the code upgrade. The parablock that comes after that will be validated + // against the new validation code. + // + // Here we are trying to choose the block number that will have `validation_upgrade_delay` + // blocks from the relay-parent of the block that schedule code upgrade but no less than + // `minimum_validation_upgrade_delay`. We want this delay out of caution so that when + // the last vote for pre-checking comes the parachain will have some time until the upgrade + // finally takes place. + let expected_at = cmp::max( + relay_parent_number + cfg.validation_upgrade_delay, + now + cfg.minimum_validation_upgrade_delay, + ); + + weight += T::DbWeight::get().reads_writes(1, 4); + FutureCodeUpgrades::::insert(&id, expected_at); + + ::UpcomingUpgrades::mutate(|upcoming_upgrades| { + let insert_idx = upcoming_upgrades + .binary_search_by_key(&expected_at, |&(_, b)| b) + .unwrap_or_else(|idx| idx); + upcoming_upgrades.insert(insert_idx, (id, expected_at)); + }); + + let expected_at = expected_at.saturated_into(); + let log = ConsensusLog::ParaScheduleUpgradeCode(id, *code_hash, expected_at); + >::deposit_log(log.into()); + + weight + } + + fn enact_pvf_rejected( + code_hash: &ValidationCodeHash, + causes: Vec>, + ) -> Weight { + let mut weight = T::DbWeight::get().writes(1); + + for cause in causes { + // Whenever PVF pre-checking is started or a new cause is added to it, the RC is bumped. + // Now we need to unbump it. + weight += Self::decrease_code_ref(code_hash); + + match cause { + PvfCheckCause::Onboarding(id) => { + // Here we need to undo everything that was done during `schedule_para_initialize`. + // Essentially, the logic is similar to offboarding, with exception that before + // actual onboarding the parachain did not have a chance to reach to upgrades. + // Therefore we can skip all the upgrade related storage items here. + weight += T::DbWeight::get().writes(3); + UpcomingParasGenesis::::remove(&id); + CurrentCodeHash::::remove(&id); + ParaLifecycles::::remove(&id); + }, + PvfCheckCause::Upgrade { id, .. } => { + weight += T::DbWeight::get().writes(2); + UpgradeGoAheadSignal::::insert(&id, UpgradeGoAhead::Abort); + FutureCodeHash::::remove(&id); + }, + } + } + + weight + } + /// Verify that `schedule_para_initialize` can be called successfully. /// /// Returns false if para is already registered in the system. - pub fn can_schedule_para_initialize(id: &ParaId, _: &ParaGenesisArgs) -> bool { - let lifecycle = ParaLifecycles::::get(id); - lifecycle.is_none() + pub fn can_schedule_para_initialize(id: &ParaId) -> bool { + ParaLifecycles::::get(id).is_none() } - /// Schedule a para to be initialized at the start of the next session. + /// Schedule a para to be initialized. If the validation code is not already stored in the + /// code storage, then a PVF pre-checking process will be initiated. /// - /// Will return error if para is already registered in the system. - pub(crate) fn schedule_para_initialize(id: ParaId, genesis: ParaGenesisArgs) -> DispatchResult { - let scheduled_session = Self::scheduled_session(); - + /// Only after the PVF pre-checking succeeds can the para be onboarded. Note, that calling this + /// does not guarantee that the parachain will eventually be onboarded. This can happen in case + /// the PVF does not pass PVF pre-checking. + /// + /// The Para ID should be not activated in this module. The validation code supplied in + /// `genesis_data` should not be empty. If those conditions are not met, then the para cannot + /// be onboarded. + pub(crate) fn schedule_para_initialize( + id: ParaId, + mut genesis_data: ParaGenesisArgs, + ) -> DispatchResult { // Make sure parachain isn't already in our system and that the onboarding parameters are // valid. - ensure!(Self::can_schedule_para_initialize(&id, &genesis), Error::::CannotOnboard); - ensure!(!genesis.validation_code.0.is_empty(), Error::::CannotOnboard); - + ensure!(Self::can_schedule_para_initialize(&id), Error::::CannotOnboard); + ensure!(!genesis_data.validation_code.0.is_empty(), Error::::CannotOnboard); ParaLifecycles::::insert(&id, ParaLifecycle::Onboarding); - UpcomingParasGenesis::::insert(&id, genesis); - ActionsQueue::::mutate(scheduled_session, |v| { - if let Err(i) = v.binary_search(&id) { - v.insert(i, id); - } - }); + + // HACK: here we are doing something nasty. + // + // In order to fix the [soaking issue] we insert the code eagerly here. When the onboarding + // is finally enacted, we do not need to insert the code anymore. Therefore, there is no + // reason for the validation code to be copied into the `ParaGenesisArgs`. We also do not + // want to risk it by copying the validation code needlessly to not risk adding more + // memory pressure. + // + // That said, we also want to preserve `ParaGenesisArgs` as it is, for now. There are two + // reasons: + // + // - Doing it within the context of the PR that introduces this change is undesirable, since + // it is already a big change, and that change would require a migration. Moreover, if we + // run the new version of the runtime, there will be less things to worry about during + // the eventual proper migration. + // + // - This data type already is used for generating genesis, and changing it will probably + // introduce some unnecessary burden. + // + // So instead of going through it right now, we will do something sneaky. Specifically: + // + // - Insert the `CurrentCodeHash` now, instead during the onboarding. That would allow to + // get rid of hashing of the validation code when onboarding. + // + // - Replace `validation_code` with a sentinel value: an empty vector. This should be fine + // as long we do not allow registering parachains with empty code. At the moment of writing + // this should already be the case. + // + // - Empty value is treated as the current code is already inserted during the onboarding. + // + // This is only an intermediate solution and should be fixed in foreseable future. + // + // [soaking issue]: https://github.com/paritytech/polkadot/issues/3918 + let validation_code = mem::take(&mut genesis_data.validation_code); + UpcomingParasGenesis::::insert(&id, genesis_data); + let validation_code_hash = validation_code.hash(); + ::CurrentCodeHash::insert(&id, validation_code_hash); + + let cfg = configuration::Pallet::::config(); + Self::kick_off_pvf_check( + PvfCheckCause::Onboarding(id), + validation_code_hash, + validation_code, + &cfg, + ); Ok(()) } @@ -824,7 +1341,7 @@ impl Pallet { // // This is not a fundamential limitation but rather simplification: it allows us to get // away without introducing additional logic for pruning and, more importantly, enacting - // ongoing PVF pre-checking votings. It also removes some nasty edge cases. + // ongoing PVF pre-checking votes. It also removes some nasty edge cases. // // This implicitly assumes that the given para exists, i.e. it's lifecycle != None. if FutureCodeHash::::contains_key(&id) { @@ -892,10 +1409,20 @@ impl Pallet { Ok(()) } - /// Schedule a future code upgrade of the given parachain, to be applied after inclusion - /// of a block of the same parachain executed in the context of a relay-chain block - /// with number >= `expected_at` + /// Schedule a future code upgrade of the given parachain. /// + /// If the new code is not known, then the PVF pre-checking will be started for that validation + /// code. In case the validation code does not pass the PVF pre-checking process, the + /// upgrade will be aborted. + /// + /// Only after the code is approved by the process, the upgrade can be scheduled. Specifically, + /// the relay-chain block number will be determined at which the upgrade will take place. We + /// call that block `expected_at`. + /// + /// Once the candidate with the relay-parent >= `expected_at` is enacted, the new validation code + /// will be applied. Therefore, the new code will be used to validate the next candidate. + /// + /// The new code should not be equal to the current one, otherwise the upgrade will be aborted. /// If there is already a scheduled code upgrade for the para, this is a no-op. pub(crate) fn schedule_code_upgrade( id: ParaId, @@ -903,44 +1430,135 @@ impl Pallet { relay_parent_number: T::BlockNumber, cfg: &configuration::HostConfiguration, ) -> Weight { - ::FutureCodeUpgrades::mutate(&id, |up| { - if up.is_some() { - T::DbWeight::get().reads_writes(1, 0) - } else { - let expected_at = relay_parent_number + cfg.validation_upgrade_delay; - let next_possible_upgrade_at = - relay_parent_number + cfg.validation_upgrade_frequency; + let mut weight = T::DbWeight::get().reads(1); + + // Enacting this should be prevented by the `can_schedule_upgrade` + if FutureCodeHash::::contains_key(&id) { + // This branch should never be reached. Signalling an upgrade is disallowed for a para + // that already has one upgrade scheduled. + // + // Any candidate that attempts to do that should be rejected by + // `can_upgrade_validation_code`. + UpgradeGoAheadSignal::::insert(&id, UpgradeGoAhead::Abort); + log::warn!( + target: "runtime::paras", + "ended up scheduling an upgrade while one is pending", + ); + return weight + } - *up = Some(expected_at); + let code_hash = new_code.hash(); - ::UpcomingUpgrades::mutate(|upcoming_upgrades| { - let insert_idx = upcoming_upgrades - .binary_search_by_key(&expected_at, |&(_, b)| b) - .unwrap_or_else(|idx| idx); - upcoming_upgrades.insert(insert_idx, (id, expected_at)); - }); + // para signals an update to the same code? This does not make a lot of sense, so abort the + // process right away. + weight += T::DbWeight::get().reads(1); + if CurrentCodeHash::::get(&id) == Some(code_hash) { + UpgradeGoAheadSignal::::insert(&id, UpgradeGoAhead::Abort); + return weight + } - // From the moment of signalling of the upgrade until the cooldown expires, the - // parachain is disallowed to make further upgrades. Therefore set the upgrade - // permission signal to disallowed and activate the cooldown timer. - ::UpgradeRestrictionSignal::insert(&id, UpgradeRestriction::Present); - ::UpgradeCooldowns::mutate(|upgrade_cooldowns| { - let insert_idx = upgrade_cooldowns - .binary_search_by_key(&next_possible_upgrade_at, |&(_, b)| b) - .unwrap_or_else(|idx| idx); - upgrade_cooldowns.insert(insert_idx, (id, next_possible_upgrade_at)); - }); + // This is the start of the upgrade process. Prevent any further attempts at upgrading. + weight += T::DbWeight::get().writes(2); + FutureCodeHash::::insert(&id, &code_hash); + UpgradeRestrictionSignal::::insert(&id, UpgradeRestriction::Present); + + weight += T::DbWeight::get().reads_writes(1, 1); + let next_possible_upgrade_at = relay_parent_number + cfg.validation_upgrade_frequency; + ::UpgradeCooldowns::mutate(|upgrade_cooldowns| { + let insert_idx = upgrade_cooldowns + .binary_search_by_key(&next_possible_upgrade_at, |&(_, b)| b) + .unwrap_or_else(|idx| idx); + upgrade_cooldowns.insert(insert_idx, (id, next_possible_upgrade_at)); + }); - let new_code_hash = new_code.hash(); - let expected_at_u32 = expected_at.saturated_into(); - let log = ConsensusLog::ParaScheduleUpgradeCode(id, new_code_hash, expected_at_u32); - >::deposit_log(log.into()); + weight += Self::kick_off_pvf_check( + PvfCheckCause::Upgrade { id, relay_parent_number }, + code_hash, + new_code, + cfg, + ); - let (reads, writes) = Self::increase_code_ref(&new_code_hash, &new_code); - FutureCodeHash::::insert(&id, new_code_hash); - T::DbWeight::get().reads_writes(2 + reads, 3 + writes) - } - }) + weight + } + + /// Makes sure that the given code hash has passed pre-checking. + /// + /// If the given code hash has already passed pre-checking, then the approval happens + /// immediately. Similarly, if the pre-checking is turned off, the update is scheduled immediately + /// as well. In this case, the behavior is similar to the previous, i.e. the upgrade sequence + /// is purely time-based. + /// + /// If the code is unknown, but the pre-checking for that PVF is already running then we perform + /// "coalescing". We save the cause for this PVF pre-check request and just add it to the + /// existing active PVF vote. + /// + /// And finally, if the code is unknown and pre-checking is not running, we start the + /// pre-checking process anew. + /// + /// Unconditionally increases the reference count for the passed `code`. + fn kick_off_pvf_check( + cause: PvfCheckCause, + code_hash: ValidationCodeHash, + code: ValidationCode, + cfg: &configuration::HostConfiguration, + ) -> Weight { + let mut weight = 0; + + weight += T::DbWeight::get().reads(1); + match PvfActiveVoteMap::::get(&code_hash) { + None => { + let known_code = CodeByHash::::contains_key(&code_hash); + weight += T::DbWeight::get().reads(1); + + if !cfg.pvf_checking_enabled || known_code { + // Either: + // - the code is known and there is no active PVF vote for it meaning it is + // already checked, or + // - the PVF checking is diabled + // In any case: fast track the PVF checking into the accepted state + weight += T::DbWeight::get().reads(1); + let now = >::block_number(); + weight += Self::enact_pvf_accepted(now, &code_hash, &[cause], 0, cfg); + } else { + // PVF is not being pre-checked and it is not known. Start a new pre-checking + // process. + weight += T::DbWeight::get().reads_writes(3, 2); + let now = >::block_number(); + let n_validators = shared::Pallet::::active_validator_keys().len(); + PvfActiveVoteMap::::insert( + &code_hash, + PvfCheckActiveVoteState::new(now, n_validators, cause), + ); + PvfActiveVoteList::::mutate(|l| { + if let Err(idx) = l.binary_search(&code_hash) { + l.insert(idx, code_hash); + } + }); + } + }, + Some(mut vote_state) => { + // Coalescing: the PVF is already being pre-checked so we just need to piggy back + // on it. + weight += T::DbWeight::get().writes(1); + vote_state.causes.push(cause); + PvfActiveVoteMap::::insert(&code_hash, vote_state); + }, + } + + // We increase the code RC here in any case. Intuitively the parachain that requested this + // action is now a user of that PVF. + // + // If the result of the pre-checking is reject, then we would decrease the RC for each cause, + // including the current. + // + // If the result of the pre-checking is accept, then we do nothing to the RC because the PVF + // will continue be used by the same users. + // + // If the PVF was fast-tracked (i.e. there is already non zero RC) and there is no + // pre-checking, we also do not change the RC then. + weight += Self::increase_code_ref(&code_hash, &code); + + weight } /// Note that a para has progressed to a new head, where the new head was executed in the context @@ -977,10 +1595,44 @@ impl Pallet { T::DbWeight::get().reads_writes(1, 1 + 0) } } else { + // This means there is no upgrade scheduled. + // + // In case the upgrade was aborted by the relay-chain we should reset + // the `Abort` signal. + UpgradeGoAheadSignal::::remove(&id); T::DbWeight::get().reads_writes(1, 1) } } + /// Returns the list of PVFs (aka validation code) that require casting a vote by a validator in + /// the active validator set. + pub(crate) fn pvfs_require_precheck() -> Vec { + PvfActiveVoteList::::get() + } + + /// Submits a given PVF check statement with corresponding signature as an unsigned transaction + /// into the memory pool. Ultimately, that disseminates the transaction accross the network. + /// + /// This function expects an offchain context and cannot be callable from the on-chain logic. + /// + /// The signature assumed to pertain to `stmt`. + pub(crate) fn submit_pvf_check_statement( + stmt: PvfCheckStatement, + signature: ValidatorSignature, + ) { + use frame_system::offchain::SubmitTransaction; + + if let Err(e) = SubmitTransaction::>::submit_unsigned_transaction( + Call::include_pvf_check_statement { stmt, signature }.into(), + ) { + log::error!( + target: "runtime::paras", + "Error submitting pvf check statement: {:?}", + e, + ); + } + } + /// Returns the current lifecycle state of the para. pub fn lifecycle(id: ParaId) -> Option { ParaLifecycles::::get(&id) @@ -1032,30 +1684,36 @@ impl Pallet { /// Store the validation code if not already stored, and increase the number of reference. /// - /// Returns the number of storage reads and number of storage writes. - fn increase_code_ref(code_hash: &ValidationCodeHash, code: &ValidationCode) -> (u64, u64) { - let reads = 1; - let mut writes = 1; + /// Returns the weight consumed. + fn increase_code_ref(code_hash: &ValidationCodeHash, code: &ValidationCode) -> Weight { + let mut weight = T::DbWeight::get().reads_writes(1, 1); ::CodeByHashRefs::mutate(code_hash, |refs| { if *refs == 0 { - writes += 1; + weight += T::DbWeight::get().writes(1); ::CodeByHash::insert(code_hash, code); } *refs += 1; }); - (reads, writes) + weight } - /// Decrease the number of reference ofthe validation code and remove it from storage if zero + /// Decrease the number of reference of the validation code and remove it from storage if zero /// is reached. - fn decrease_code_ref(code_hash: &ValidationCodeHash) { + /// + /// Returns the weight consumed. + fn decrease_code_ref(code_hash: &ValidationCodeHash) -> Weight { + let mut weight = T::DbWeight::get().reads(1); let refs = ::CodeByHashRefs::get(code_hash); + debug_assert!(refs != 0); if refs <= 1 { + weight += T::DbWeight::get().writes(2); ::CodeByHash::remove(code_hash); ::CodeByHashRefs::remove(code_hash); } else { + weight += T::DbWeight::get().writes(1); ::CodeByHashRefs::insert(code_hash, refs - 1); } + weight } /// Test function for triggering a new session in this pallet. @@ -1077,14 +1735,56 @@ impl Pallet { mod tests { use super::*; use frame_support::{assert_err, assert_ok}; - use primitives::v1::BlockNumber; + use keyring::Sr25519Keyring; + use primitives::{ + v0::PARACHAIN_KEY_TYPE_ID, + v1::{BlockNumber, ValidatorId}, + }; + use sc_keystore::LocalKeystore; + use sp_keystore::{SyncCryptoStore, SyncCryptoStorePtr}; + use std::sync::Arc; use crate::{ configuration::HostConfiguration, mock::{new_test_ext, Configuration, MockGenesisConfig, Paras, ParasShared, System, Test}, }; + static VALIDATORS: &[Sr25519Keyring] = &[ + Sr25519Keyring::Alice, + Sr25519Keyring::Bob, + Sr25519Keyring::Charlie, + Sr25519Keyring::Dave, + Sr25519Keyring::Ferdie, + ]; + + fn validator_pubkeys(val_ids: &[Sr25519Keyring]) -> Vec { + val_ids.iter().map(|v| v.public().into()).collect() + } + + fn sign_and_include_pvf_check_statement(stmt: PvfCheckStatement) { + let validators = &[ + Sr25519Keyring::Alice, + Sr25519Keyring::Bob, + Sr25519Keyring::Charlie, + Sr25519Keyring::Dave, + Sr25519Keyring::Ferdie, + ]; + let signature = validators[stmt.validator_index.0 as usize].sign(&stmt.signing_payload()); + Paras::include_pvf_check_statement(None.into(), stmt, signature.into()).unwrap(); + } + fn run_to_block(to: BlockNumber, new_session: Option>) { + let keystore: SyncCryptoStorePtr = Arc::new(LocalKeystore::in_memory()); + for validator in VALIDATORS.iter() { + SyncCryptoStore::sr25519_generate_new( + &*keystore, + PARACHAIN_KEY_TYPE_ID, + Some(&validator.to_seed()), + ) + .unwrap(); + } + let validator_pubkeys = validator_pubkeys(VALIDATORS); + while System::block_number() < to { let b = System::block_number(); Paras::initializer_finalize(); @@ -1092,12 +1792,14 @@ mod tests { if new_session.as_ref().map_or(false, |v| v.contains(&(b + 1))) { let mut session_change_notification = SessionChangeNotification::default(); session_change_notification.session_index = ParasShared::session_index() + 1; + session_change_notification.validators = validator_pubkeys.clone(); ParasShared::initializer_on_new_session( session_change_notification.session_index, session_change_notification.random_seed, &session_change_notification.new_config, session_change_notification.validators.clone(), ); + ParasShared::set_active_validators_ascending(validator_pubkeys.clone()); Paras::initializer_on_new_session(&session_change_notification); } System::on_finalize(b); @@ -1387,6 +2089,7 @@ mod tests { code_retention_period, validation_upgrade_delay, validation_upgrade_frequency, + pvf_checking_enabled: false, ..Default::default() }, ..Default::default() @@ -1494,6 +2197,7 @@ mod tests { code_retention_period, validation_upgrade_delay, validation_upgrade_frequency, + pvf_checking_enabled: false, ..Default::default() }, ..Default::default() @@ -1565,6 +2269,7 @@ mod tests { fn submit_code_change_when_not_allowed_is_err() { let code_retention_period = 10; let validation_upgrade_delay = 7; + let validation_upgrade_frequency = 100; let paras = vec![( 0u32.into(), @@ -1581,6 +2286,8 @@ mod tests { config: HostConfiguration { code_retention_period, validation_upgrade_delay, + validation_upgrade_frequency, + pvf_checking_enabled: false, ..Default::default() }, ..Default::default() @@ -1617,7 +2324,7 @@ mod tests { #[test] fn full_parachain_cleanup_storage() { - let code_retention_period = 10; + let code_retention_period = 20; let validation_upgrade_delay = 1 + 5; let original_code = ValidationCode(vec![1, 2, 3]); @@ -1636,6 +2343,8 @@ mod tests { config: HostConfiguration { code_retention_period, validation_upgrade_delay, + pvf_checking_enabled: false, + minimum_validation_upgrade_delay: 0, ..Default::default() }, ..Default::default() @@ -1724,8 +2433,20 @@ mod tests { #[test] fn para_incoming_at_session() { - new_test_ext(Default::default()).execute_with(|| { - run_to_block(1, None); + let code_a = ValidationCode(vec![2]); + let code_b = ValidationCode(vec![1]); + let code_c = ValidationCode(vec![3]); + + let genesis_config = MockGenesisConfig { + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { pvf_checking_enabled: true, ..Default::default() }, + ..Default::default() + }, + ..Default::default() + }; + + new_test_ext(genesis_config).execute_with(|| { + run_to_block(1, Some(vec![1])); let b = ParaId::from(525); let a = ParaId::from(999); @@ -1736,7 +2457,7 @@ mod tests { ParaGenesisArgs { parachain: true, genesis_head: vec![1].into(), - validation_code: vec![1].into(), + validation_code: code_b.clone(), }, )); @@ -1745,7 +2466,7 @@ mod tests { ParaGenesisArgs { parachain: false, genesis_head: vec![2].into(), - validation_code: vec![2].into(), + validation_code: code_a.clone(), }, )); @@ -1754,10 +2475,37 @@ mod tests { ParaGenesisArgs { parachain: true, genesis_head: vec![3].into(), - validation_code: vec![3].into(), + validation_code: code_c.clone(), }, )); + IntoIterator::into_iter([0, 1, 2, 3]) + .map(|i| PvfCheckStatement { + accept: true, + subject: code_a.hash(), + session_index: 1, + validator_index: i.into(), + }) + .for_each(sign_and_include_pvf_check_statement); + + IntoIterator::into_iter([1, 2, 3, 4]) + .map(|i| PvfCheckStatement { + accept: true, + subject: code_b.hash(), + session_index: 1, + validator_index: i.into(), + }) + .for_each(sign_and_include_pvf_check_statement); + + IntoIterator::into_iter([0, 2, 3, 4]) + .map(|i| PvfCheckStatement { + accept: true, + subject: code_c.hash(), + session_index: 1, + validator_index: i.into(), + }) + .for_each(sign_and_include_pvf_check_statement); + assert_eq!( ::ActionsQueue::get(Paras::scheduled_session()), vec![c, b, a], @@ -1819,6 +2567,7 @@ mod tests { config: HostConfiguration { code_retention_period, validation_upgrade_delay, + pvf_checking_enabled: false, ..Default::default() }, ..Default::default() @@ -1832,6 +2581,11 @@ mod tests { let new_code: ValidationCode = vec![4, 5, 6].into(); Paras::schedule_code_upgrade(para_id, new_code.clone(), 0, &Configuration::config()); + // The new validation code can be applied but a new parablock hasn't gotten in yet, + // so the old code should still be current. + run_to_block(3, None); + assert_eq!(Paras::current_code(¶_id), Some(old_code.clone())); + run_to_block(10, None); Paras::note_new_head(para_id, Default::default(), 7); @@ -1879,6 +2633,334 @@ mod tests { }); } + #[test] + fn pvf_check_coalescing_onboarding_and_upgrade() { + let validation_upgrade_delay = 5; + + let a = ParaId::from(111); + let b = ParaId::from(222); + let validation_code: ValidationCode = vec![3, 2, 1].into(); + + let paras = vec![( + a, + ParaGenesisArgs { + parachain: true, + genesis_head: Default::default(), + validation_code: ValidationCode(vec![]), // valid since in genesis + }, + )]; + + let genesis_config = MockGenesisConfig { + paras: GenesisConfig { paras, ..Default::default() }, + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { + pvf_checking_enabled: true, + validation_upgrade_delay, + ..Default::default() + }, + ..Default::default() + }, + ..Default::default() + }; + + new_test_ext(genesis_config).execute_with(|| { + // At this point `a` is already onboarded. Run to block 1 performing session change at + // the end of block #0. + run_to_block(2, Some(vec![1])); + + // Expected current session index. + const EXPECTED_SESSION: SessionIndex = 1; + // Relay parent of the parablock that schedules the upgrade. + const RELAY_PARENT: BlockNumber = 1; + + // Now we register `b` with `validation_code` + assert_ok!(Paras::schedule_para_initialize( + b, + ParaGenesisArgs { + parachain: true, + genesis_head: vec![2].into(), + validation_code: validation_code.clone(), + }, + )); + + // And now at the same time upgrade `a` to `validation_code` + Paras::schedule_code_upgrade( + a, + validation_code.clone(), + RELAY_PARENT, + &Configuration::config(), + ); + assert!(!Paras::pvfs_require_precheck().is_empty()); + + // Supermajority of validators vote for `validation_code`. It should be approved. + IntoIterator::into_iter([0, 1, 2, 3]) + .map(|i| PvfCheckStatement { + accept: true, + subject: validation_code.hash(), + session_index: EXPECTED_SESSION, + validator_index: i.into(), + }) + .for_each(sign_and_include_pvf_check_statement); + + // Check that `b` actually onboards. + assert_eq!(::ActionsQueue::get(EXPECTED_SESSION + 2), vec![b]); + + // Check that the upgrade got scheduled. + assert_eq!( + ::FutureCodeUpgrades::get(&a), + Some(RELAY_PARENT + validation_upgrade_delay), + ); + }); + } + + #[test] + fn pvf_check_onboarding_reject_on_expiry() { + let pvf_voting_ttl = 2; + let a = ParaId::from(111); + let validation_code: ValidationCode = vec![3, 2, 1].into(); + + let genesis_config = MockGenesisConfig { + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { + pvf_checking_enabled: true, + pvf_voting_ttl, + ..Default::default() + }, + ..Default::default() + }, + ..Default::default() + }; + + new_test_ext(genesis_config).execute_with(|| { + run_to_block(1, Some(vec![1])); + + assert_ok!(Paras::schedule_para_initialize( + a, + ParaGenesisArgs { + parachain: false, + genesis_head: vec![2].into(), + validation_code: validation_code.clone(), + }, + )); + + // Make sure that we kicked off the PVF vote for this validation code and that the + // validation code is stored. + assert!(::PvfActiveVoteMap::get(&validation_code.hash()).is_some()); + check_code_is_stored(&validation_code); + + // Skip 2 sessions (i.e. `pvf_voting_ttl`) verifying that the code is still stored in + // the intermediate session. + assert_eq!(pvf_voting_ttl, 2); + run_to_block(2, Some(vec![2])); + check_code_is_stored(&validation_code); + run_to_block(3, Some(vec![3])); + + // --- At this point the PVF vote for onboarding should be rejected. + + // Verify that the PVF is no longer stored and there is no active PVF vote. + check_code_is_not_stored(&validation_code); + assert!(::PvfActiveVoteMap::get(&validation_code.hash()).is_none()); + assert!(Paras::pvfs_require_precheck().is_empty()); + + // Verify that at this point we can again try to initialize the same para. + assert!(Paras::can_schedule_para_initialize(&a)); + }); + } + + #[test] + fn pvf_check_upgrade_reject() { + let a = ParaId::from(111); + let new_code: ValidationCode = vec![3, 2, 1].into(); + + let paras = vec![( + a, + ParaGenesisArgs { + parachain: false, + genesis_head: Default::default(), + validation_code: ValidationCode(vec![]), // valid since in genesis + }, + )]; + + let genesis_config = MockGenesisConfig { + paras: GenesisConfig { paras, ..Default::default() }, + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { pvf_checking_enabled: true, ..Default::default() }, + ..Default::default() + }, + ..Default::default() + }; + + new_test_ext(genesis_config).execute_with(|| { + // At this point `a` is already onboarded. Run to block 1 performing session change at + // the end of block #0. + run_to_block(2, Some(vec![1])); + + // Relay parent of the block that schedules the upgrade. + const RELAY_PARENT: BlockNumber = 1; + // Expected current session index. + const EXPECTED_SESSION: SessionIndex = 1; + + Paras::schedule_code_upgrade( + a, + new_code.clone(), + RELAY_PARENT, + &Configuration::config(), + ); + check_code_is_stored(&new_code); + + // Supermajority of validators vote against `new_code`. PVF should be rejected. + IntoIterator::into_iter([0, 1, 2, 3]) + .map(|i| PvfCheckStatement { + accept: false, + subject: new_code.hash(), + session_index: EXPECTED_SESSION, + validator_index: i.into(), + }) + .for_each(sign_and_include_pvf_check_statement); + + // Verify that the new code is discarded. + check_code_is_not_stored(&new_code); + + assert!(::PvfActiveVoteMap::get(&new_code.hash()).is_none()); + assert!(Paras::pvfs_require_precheck().is_empty()); + assert!(::FutureCodeHash::get(&a).is_none()); + }); + } + + #[test] + fn pvf_check_submit_vote() { + let code_a: ValidationCode = vec![3, 2, 1].into(); + let code_b: ValidationCode = vec![1, 2, 3].into(); + + let check = |stmt: PvfCheckStatement| -> (Result<_, _>, Result<_, _>, Result<_, _>) { + let validators = &[ + Sr25519Keyring::Alice, + Sr25519Keyring::Bob, + Sr25519Keyring::Charlie, + Sr25519Keyring::Dave, + Sr25519Keyring::Ferdie, + Sr25519Keyring::Eve, // <- this validator is not in the set + ]; + let signature: ValidatorSignature = + validators[stmt.validator_index.0 as usize].sign(&stmt.signing_payload()).into(); + + let call = Call::include_pvf_check_statement { + stmt: stmt.clone(), + signature: signature.clone(), + }; + let validate_unsigned = + ::validate_unsigned(TransactionSource::InBlock, &call) + .map(|_| ()); + let pre_dispatch = ::pre_dispatch(&call); + let dispatch_result = + Paras::include_pvf_check_statement(None.into(), stmt.clone(), signature.clone()); + + (validate_unsigned, pre_dispatch, dispatch_result) + }; + + let genesis_config = MockGenesisConfig { + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { pvf_checking_enabled: true, ..Default::default() }, + ..Default::default() + }, + ..Default::default() + }; + + new_test_ext(genesis_config).execute_with(|| { + // Important to run this to seed the validators. + run_to_block(1, Some(vec![1])); + + assert_ok!(Paras::schedule_para_initialize( + 1000.into(), + ParaGenesisArgs { + parachain: false, + genesis_head: vec![2].into(), + validation_code: code_a.clone(), + }, + )); + + assert_eq!( + check(PvfCheckStatement { + accept: false, + subject: code_a.hash(), + session_index: 1, + validator_index: 1.into(), + }), + (Ok(()), Ok(()), Ok(())), + ); + + // A vote in the same direction. + let (unsigned, pre_dispatch, dispatch) = check(PvfCheckStatement { + accept: false, + subject: code_a.hash(), + session_index: 1, + validator_index: 1.into(), + }); + assert_eq!(unsigned, Err(InvalidTransaction::Custom(INVALID_TX_DOUBLE_VOTE).into())); + assert_eq!( + pre_dispatch, + Err(InvalidTransaction::Custom(INVALID_TX_DOUBLE_VOTE).into()) + ); + assert_err!(dispatch, Error::::PvfCheckDoubleVote); + + // Equivocation + let (unsigned, pre_dispatch, dispatch) = check(PvfCheckStatement { + accept: true, + subject: code_a.hash(), + session_index: 1, + validator_index: 1.into(), + }); + assert_eq!(unsigned, Err(InvalidTransaction::Custom(INVALID_TX_DOUBLE_VOTE).into())); + assert_eq!( + pre_dispatch, + Err(InvalidTransaction::Custom(INVALID_TX_DOUBLE_VOTE).into()) + ); + assert_err!(dispatch, Error::::PvfCheckDoubleVote); + + // Vote for an earlier session. + let (unsigned, pre_dispatch, dispatch) = check(PvfCheckStatement { + accept: false, + subject: code_a.hash(), + session_index: 0, + validator_index: 1.into(), + }); + assert_eq!(unsigned, Err(InvalidTransaction::Stale.into())); + assert_eq!(pre_dispatch, Err(InvalidTransaction::Stale.into())); + assert_err!(dispatch, Error::::PvfCheckStatementStale); + + // Validator not in the set. + let (unsigned, pre_dispatch, dispatch) = check(PvfCheckStatement { + accept: false, + subject: code_a.hash(), + session_index: 1, + validator_index: 5.into(), + }); + assert_eq!( + unsigned, + Err(InvalidTransaction::Custom(INVALID_TX_BAD_VALIDATOR_IDX).into()) + ); + assert_eq!( + pre_dispatch, + Err(InvalidTransaction::Custom(INVALID_TX_BAD_VALIDATOR_IDX).into()) + ); + assert_err!(dispatch, Error::::PvfCheckValidatorIndexOutOfBounds); + + // Bad subject (code_b) + let (unsigned, pre_dispatch, dispatch) = check(PvfCheckStatement { + accept: false, + subject: code_b.hash(), + session_index: 1, + validator_index: 1.into(), + }); + assert_eq!(unsigned, Err(InvalidTransaction::Custom(INVALID_TX_BAD_SUBJECT).into())); + assert_eq!( + pre_dispatch, + Err(InvalidTransaction::Custom(INVALID_TX_BAD_SUBJECT).into()) + ); + assert_err!(dispatch, Error::::PvfCheckSubjectInvalid); + }); + } + #[test] fn verify_upgrade_go_ahead_signal_is_externally_accessible() { use primitives::v1::well_known_keys; diff --git a/runtime/parachains/src/runtime_api_impl/v1.rs b/runtime/parachains/src/runtime_api_impl/v1.rs index d544ec4ed646..3ae0e0162dc5 100644 --- a/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/runtime/parachains/src/runtime_api_impl/v1.rs @@ -25,8 +25,8 @@ use primitives::v1::{ AuthorityDiscoveryId, CandidateEvent, CommittedCandidateReceipt, CoreIndex, CoreOccupied, CoreState, GroupIndex, GroupRotationInfo, Hash, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, OccupiedCore, OccupiedCoreAssumption, PersistedValidationData, - ScheduledCore, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, - ValidationCodeHash, ValidatorId, ValidatorIndex, + PvfCheckStatement, ScheduledCore, ScrapedOnChainVotes, SessionIndex, SessionInfo, + ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, }; use sp_runtime::traits::One; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; @@ -371,3 +371,17 @@ pub fn validation_code_by_hash( pub fn on_chain_votes() -> Option> { >::on_chain_votes() } + +/// Submits an PVF pre-checking vote. See [`paras::Pallet::submit_pvf_check_statement`]. +pub fn submit_pvf_check_statement( + stmt: PvfCheckStatement, + signature: ValidatorSignature, +) { + >::submit_pvf_check_statement(stmt, signature) +} + +/// Returns the list of all PVF code hashes that require pre-checking. See +/// [`paras::Pallet::pvfs_require_precheck`]. +pub fn pvfs_require_precheck() -> Vec { + >::pvfs_require_precheck() +} diff --git a/runtime/parachains/src/scheduler.rs b/runtime/parachains/src/scheduler.rs index 706fd844b316..483ae011d207 100644 --- a/runtime/parachains/src/scheduler.rs +++ b/runtime/parachains/src/scheduler.rs @@ -843,6 +843,7 @@ mod tests { thread_availability_period: 5, scheduling_lookahead: 2, parathread_retries: 1, + pvf_checking_enabled: false, ..Default::default() } } diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 4b883ddb2e79..82dd94fb5f2b 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1158,9 +1158,15 @@ impl parachains_inclusion::Config for Runtime { type RewardValidators = parachains_reward_points::RewardValidatorsWithEraPoints; } +parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); +} + impl parachains_paras::Config for Runtime { type Event = Event; type WeightInfo = weights::runtime_parachains_paras::WeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = Babe; } parameter_types! { diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 996e0042bddc..5fe8c3360d2a 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -617,9 +617,15 @@ impl parachains_inclusion::Config for Runtime { type RewardValidators = RewardValidators; } +parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); +} + impl parachains_paras::Config for Runtime { type Event = Event; type WeightInfo = weights::runtime_parachains_paras::WeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = Babe; } parameter_types! { diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 7c20da6d9e19..311e602275d8 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -63,7 +63,7 @@ use sp_runtime::{ BlakeTwo256, Block as BlockT, ConvertInto, Extrinsic as ExtrinsicT, OpaqueKeys, SaturatedConversion, StaticLookup, Verify, }, - transaction_validity::{TransactionSource, TransactionValidity}, + transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity}, ApplyExtrinsicResult, KeyTypeId, Perbill, }; use sp_staking::SessionIndex; @@ -489,9 +489,15 @@ impl parachains_initializer::Config for Runtime { impl parachains_session_info::Config for Runtime {} +parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); +} + impl parachains_paras::Config for Runtime { type Event = Event; type WeightInfo = parachains_paras::TestWeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = Babe; } impl parachains_dmp::Config for Runtime {} diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 1d9b9a25a1ae..b879de0f7abf 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -825,9 +825,15 @@ impl parachains_inclusion::Config for Runtime { type RewardValidators = parachains_reward_points::RewardValidatorsWithEraPoints; } +parameter_types! { + pub const ParasUnsignedPriority: TransactionPriority = TransactionPriority::max_value(); +} + impl parachains_paras::Config for Runtime { type Event = Event; type WeightInfo = weights::runtime_parachains_paras::WeightInfo; + type UnsignedPriority = ParasUnsignedPriority; + type NextSessionRotation = Babe; } parameter_types! {