From c301b648943dc555a86b0335b7d9e2433b5dc3b8 Mon Sep 17 00:00:00 2001
From: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Date: Thu, 2 Mar 2023 16:48:14 +0100
Subject: [PATCH] Add Version Checks on Para Upgrade (#2261)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* add version checks on para ugprade
* Apply suggestions from code review
Co-authored-by: Bastian Köcher <git@kchr.de>
* remove unneeded imports and errors
* fix test error path
---------
Co-authored-by: Bastian Köcher <git@kchr.de>
---
cumulus/pallets/parachain-system/src/lib.rs | 82 +++++++++++++++----
cumulus/pallets/parachain-system/src/tests.rs | 37 ++++++++-
2 files changed, 100 insertions(+), 19 deletions(-)
diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs
index 456ae5bf578..3a693b0911a 100644
--- a/cumulus/pallets/parachain-system/src/lib.rs
+++ b/cumulus/pallets/parachain-system/src/lib.rs
@@ -16,18 +16,18 @@
#![cfg_attr(not(feature = "std"), no_std)]
-//! cumulus-pallet-parachain-system is a base pallet for cumulus-based parachains.
+//! `cumulus-pallet-parachain-system` is a base pallet for Cumulus-based parachains.
//!
-//! This pallet handles low-level details of being a parachain. It's responsibilities include:
+//! This pallet handles low-level details of being a parachain. Its responsibilities include:
//!
-//! - ingestion of the parachain validation data
-//! - ingestion of incoming downward and lateral messages and dispatching them
-//! - coordinating upgrades with the relay-chain
-//! - communication of parachain outputs, such as sent messages, signalling an upgrade, etc.
+//! - ingestion of the parachain validation data;
+//! - ingestion and dispatch of incoming downward and lateral messages;
+//! - coordinating upgrades with the Relay Chain; and
+//! - communication of parachain outputs, such as sent messages, signaling an upgrade, etc.
//!
//! Users must ensure that they register this pallet as an inherent provider.
-use codec::Encode;
+use codec::{Decode, Encode, MaxEncodedLen};
use cumulus_primitives_core::{
relay_chain, AbridgedHostConfiguration, ChannelStatus, CollationInfo, DmpMessageHandler,
GetChannelInfo, InboundDownwardMessage, InboundHrmpMessage, MessageSendError,
@@ -45,6 +45,7 @@ use frame_support::{
};
use frame_system::{ensure_none, ensure_root};
use polkadot_parachain::primitives::RelayChainBlockNumber;
+use scale_info::TypeInfo;
use sp_runtime::{
traits::{Block as BlockT, BlockNumberProvider, Hash},
transaction_validity::{
@@ -134,6 +135,20 @@ impl CheckAssociatedRelayNumber for AnyRelayNumber {
fn check_associated_relay_number(_: RelayChainBlockNumber, _: RelayChainBlockNumber) {}
}
+/// Information needed when a new runtime binary is submitted and needs to be authorized before
+/// replacing the current runtime.
+#[derive(Decode, Encode, Default, PartialEq, Eq, MaxEncodedLen, TypeInfo)]
+#[scale_info(skip_type_params(T))]
+struct CodeUpgradeAuthorization<T>
+where
+ T: Config,
+{
+ /// Hash of the new runtime binary.
+ code_hash: T::Hash,
+ /// Whether or not to carry out version checks.
+ check_version: bool,
+}
+
#[frame_support::pallet]
pub mod pallet {
use super::*;
@@ -442,17 +457,40 @@ pub mod pallet {
Ok(())
}
+ /// Authorize an upgrade to a given `code_hash` for the runtime. The runtime can be supplied
+ /// later.
+ ///
+ /// The `check_version` parameter sets a boolean flag for whether or not the runtime's spec
+ /// version and name should be verified on upgrade. Since the authorization only has a hash,
+ /// it cannot actually perform the verification.
+ ///
+ /// This call requires Root origin.
#[pallet::call_index(2)]
#[pallet::weight((1_000_000, DispatchClass::Operational))]
- pub fn authorize_upgrade(origin: OriginFor<T>, code_hash: T::Hash) -> DispatchResult {
+ pub fn authorize_upgrade(
+ origin: OriginFor<T>,
+ code_hash: T::Hash,
+ check_version: bool,
+ ) -> DispatchResult {
ensure_root(origin)?;
-
- AuthorizedUpgrade::<T>::put(&code_hash);
+ AuthorizedUpgrade::<T>::put(CodeUpgradeAuthorization {
+ code_hash: code_hash.clone(),
+ check_version,
+ });
Self::deposit_event(Event::UpgradeAuthorized { code_hash });
Ok(())
}
+ /// Provide the preimage (runtime binary) `code` for an upgrade that has been authorized.
+ ///
+ /// If the authorization required a version check, this call will ensure the spec name
+ /// remains unchanged and that the spec version has increased.
+ ///
+ /// Note that this function will not apply the new `code`, but only attempt to schedule the
+ /// upgrade with the Relay Chain.
+ ///
+ /// All origins are allowed.
#[pallet::call_index(3)]
#[pallet::weight(1_000_000)]
pub fn enact_authorized_upgrade(
@@ -487,16 +525,16 @@ pub mod pallet {
#[pallet::error]
pub enum Error<T> {
- /// Attempt to upgrade validation function while existing upgrade pending
+ /// Attempt to upgrade validation function while existing upgrade pending.
OverlappingUpgrades,
- /// Polkadot currently prohibits this parachain from upgrading its validation function
+ /// Polkadot currently prohibits this parachain from upgrading its validation function.
ProhibitedByPolkadot,
/// The supplied validation function has compiled into a blob larger than Polkadot is
- /// willing to run
+ /// willing to run.
TooBig,
- /// The inherent which supplies the validation data did not run this block
+ /// The inherent which supplies the validation data did not run this block.
ValidationDataNotAvailable,
- /// The inherent which supplies the host configuration did not run this block
+ /// The inherent which supplies the host configuration did not run this block.
HostConfigurationNotAvailable,
/// No validation function upgrade is currently scheduled.
NotScheduled,
@@ -645,7 +683,7 @@ pub mod pallet {
/// The next authorized upgrade, if there is one.
#[pallet::storage]
- pub(super) type AuthorizedUpgrade<T: Config> = StorageValue<_, T::Hash>;
+ pub(super) type AuthorizedUpgrade<T: Config> = StorageValue<_, CodeUpgradeAuthorization<T>>;
/// A custom head data that should be returned as result of `validate_block`.
///
@@ -712,9 +750,17 @@ pub mod pallet {
impl<T: Config> Pallet<T> {
fn validate_authorized_upgrade(code: &[u8]) -> Result<T::Hash, DispatchError> {
- let required_hash = AuthorizedUpgrade::<T>::get().ok_or(Error::<T>::NothingAuthorized)?;
+ let authorization = AuthorizedUpgrade::<T>::get().ok_or(Error::<T>::NothingAuthorized)?;
+
+ // ensure that the actual hash matches the authorized hash
let actual_hash = T::Hashing::hash(&code[..]);
- ensure!(actual_hash == required_hash, Error::<T>::Unauthorized);
+ ensure!(actual_hash == authorization.code_hash, Error::<T>::Unauthorized);
+
+ // check versions if required as part of the authorization
+ if authorization.check_version {
+ frame_system::Pallet::<T>::can_set_code(code)?;
+ }
+
Ok(actual_hash)
}
}
diff --git a/cumulus/pallets/parachain-system/src/tests.rs b/cumulus/pallets/parachain-system/src/tests.rs
index 871596ae8cb..70e4c106bf2 100755
--- a/cumulus/pallets/parachain-system/src/tests.rs
+++ b/cumulus/pallets/parachain-system/src/tests.rs
@@ -32,10 +32,11 @@ use frame_support::{
use frame_system::RawOrigin;
use hex_literal::hex;
use relay_chain::HrmpChannelId;
-use sp_core::H256;
+use sp_core::{blake2_256, H256};
use sp_runtime::{
testing::Header,
traits::{BlakeTwo256, IdentityLookup},
+ DispatchErrorWithPostInfo,
};
use sp_version::RuntimeVersion;
use std::cell::RefCell;
@@ -974,3 +975,37 @@ fn test() {
.add(1, || {})
.add(2, || {});
}
+
+#[test]
+fn upgrade_version_checks_should_work() {
+ let test_data = vec![
+ ("test", 0, 1, Err(frame_system::Error::<Test>::SpecVersionNeedsToIncrease)),
+ ("test", 1, 0, Err(frame_system::Error::<Test>::SpecVersionNeedsToIncrease)),
+ ("test", 1, 1, Err(frame_system::Error::<Test>::SpecVersionNeedsToIncrease)),
+ ("test", 1, 2, Err(frame_system::Error::<Test>::SpecVersionNeedsToIncrease)),
+ ("test2", 1, 1, Err(frame_system::Error::<Test>::InvalidSpecName)),
+ ];
+
+ for (spec_name, spec_version, impl_version, expected) in test_data.into_iter() {
+ let version = RuntimeVersion {
+ spec_name: spec_name.into(),
+ spec_version,
+ impl_version,
+ ..Default::default()
+ };
+ let read_runtime_version = ReadRuntimeVersion(version.encode());
+
+ let mut ext = new_test_ext();
+ ext.register_extension(sp_core::traits::ReadRuntimeVersionExt::new(read_runtime_version));
+ ext.execute_with(|| {
+ let new_code = vec![1, 2, 3, 4];
+ let new_code_hash = sp_core::H256(blake2_256(&new_code));
+
+ let _authorize =
+ ParachainSystem::authorize_upgrade(RawOrigin::Root.into(), new_code_hash, true);
+ let res = ParachainSystem::enact_authorized_upgrade(RawOrigin::None.into(), new_code);
+
+ assert_eq!(expected.map_err(DispatchErrorWithPostInfo::from), res);
+ });
+ }
+}
--
GitLab