Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate code when scheduling upgrades from registrar #3232

Merged
merged 9 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions polkadot/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ pub use v6::{
UpgradeRestriction, UpwardMessage, ValidDisputeStatementKind, ValidationCode,
ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, ValidityAttestation,
ValidityError, ASSIGNMENT_KEY_TYPE_ID, LEGACY_MIN_BACKING_VOTES, LOWEST_PUBLIC_ID,
MAX_CODE_SIZE, MAX_HEAD_DATA_SIZE, MAX_POV_SIZE, ON_DEMAND_DEFAULT_QUEUE_MAX_SIZE,
PARACHAINS_INHERENT_IDENTIFIER, PARACHAIN_KEY_TYPE_ID,
MAX_CODE_SIZE, MAX_HEAD_DATA_SIZE, MAX_POV_SIZE, MIN_CODE_SIZE,
ON_DEMAND_DEFAULT_QUEUE_MAX_SIZE, PARACHAINS_INHERENT_IDENTIFIER, PARACHAIN_KEY_TYPE_ID,
};

#[cfg(feature = "std")]
Expand Down
3 changes: 3 additions & 0 deletions polkadot/primitives/src/v6/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,9 @@ pub const PARACHAINS_INHERENT_IDENTIFIER: InherentIdentifier = *b"parachn0";
/// The key type ID for parachain assignment key.
pub const ASSIGNMENT_KEY_TYPE_ID: KeyTypeId = KeyTypeId(*b"asgn");

/// Compressed or not the wasm blob can never be less than 9 bytes.
pub const MIN_CODE_SIZE: u32 = 9;

/// Maximum compressed code size we support right now.
/// At the moment we have runtime upgrade on chain, which restricts scalability severely. If we want
/// to have bigger values, we should fix that first.
Expand Down
2 changes: 1 addition & 1 deletion polkadot/primitives/test-helpers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub fn dummy_candidate_descriptor<H: AsRef<[u8]>>(relay_parent: H) -> CandidateD

/// Create meaningless validation code.
pub fn dummy_validation_code() -> ValidationCode {
ValidationCode(vec![1, 2, 3])
ValidationCode(vec![1, 2, 3, 4, 5, 6, 7, 8, 9])
}

/// Create meaningless head data.
Expand Down
3 changes: 2 additions & 1 deletion polkadot/runtime/common/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use pallet_identity::{self, legacy::IdentityInfo};
use parity_scale_codec::Encode;
use primitives::{
BlockNumber, HeadData, Id as ParaId, SessionIndex, ValidationCode, LOWEST_PUBLIC_ID,
MAX_CODE_SIZE,
};
use runtime_parachains::{
configuration, origin, paras, shared, Origin as ParaOrigin, ParaLifecycle,
Expand Down Expand Up @@ -315,7 +316,7 @@ pub fn new_test_ext() -> TestExternalities {
let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
configuration::GenesisConfig::<Test> {
config: configuration::HostConfiguration {
max_code_size: 2 * 1024 * 1024, // 2 MB
max_code_size: MAX_CODE_SIZE,
max_head_data_size: 1 * 1024 * 1024, // 1 MB
..Default::default()
},
Expand Down
63 changes: 54 additions & 9 deletions polkadot/runtime/common/src/paras_registrar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use frame_support::{
traits::{Currency, Get, ReservableCurrency},
};
use frame_system::{self, ensure_root, ensure_signed};
use primitives::{HeadData, Id as ParaId, ValidationCode, LOWEST_PUBLIC_ID};
use primitives::{HeadData, Id as ParaId, ValidationCode, LOWEST_PUBLIC_ID, MIN_CODE_SIZE};
use runtime_parachains::{
configuration, ensure_parachain,
paras::{self, ParaGenesisArgs, SetGoAhead},
Expand Down Expand Up @@ -182,8 +182,8 @@ pub mod pallet {
ParaLocked,
/// The ID given for registration has not been reserved.
NotReserved,
/// Registering parachain with empty code is not allowed.
EmptyCode,
/// The validation code is invalid.
InvalidCode,
/// Cannot perform a parachain slot / lifecycle swap. Check that the state of both paras
/// are correct for the swap to work.
CannotSwap,
Expand Down Expand Up @@ -657,7 +657,7 @@ impl<T: Config> Pallet<T> {
para_kind: ParaKind,
) -> Result<(ParaGenesisArgs, BalanceOf<T>), sp_runtime::DispatchError> {
let config = configuration::Pallet::<T>::config();
ensure!(validation_code.0.len() > 0, Error::<T>::EmptyCode);
ensure!(validation_code.0.len() >= MIN_CODE_SIZE as usize, Error::<T>::InvalidCode);
ensure!(validation_code.0.len() <= config.max_code_size as usize, Error::<T>::CodeTooLarge);
ensure!(
genesis_head.0.len() <= config.max_head_data_size as usize,
Expand Down Expand Up @@ -712,7 +712,7 @@ mod tests {
};
use frame_system::limits;
use pallet_balances::Error as BalancesError;
use primitives::{Balance, BlockNumber, SessionIndex};
use primitives::{Balance, BlockNumber, SessionIndex, MAX_CODE_SIZE};
use runtime_parachains::{configuration, origin, shared};
use sp_core::H256;
use sp_io::TestExternalities;
Expand Down Expand Up @@ -849,7 +849,7 @@ mod tests {

configuration::GenesisConfig::<Test> {
config: configuration::HostConfiguration {
max_code_size: 2 * 1024 * 1024, // 2 MB
max_code_size: MAX_CODE_SIZE,
max_head_data_size: 1 * 1024 * 1024, // 1 MB
..Default::default()
},
Expand Down Expand Up @@ -1032,6 +1032,51 @@ mod tests {
});
}

#[test]
fn schedule_code_upgrade_validates_code() {
new_test_ext().execute_with(|| {
const START_SESSION_INDEX: SessionIndex = 1;
run_to_session(START_SESSION_INDEX);

let para_id = LOWEST_PUBLIC_ID;
assert!(!Parachains::is_parathread(para_id));

let validation_code = test_validation_code(32);
assert_ok!(Registrar::reserve(RuntimeOrigin::signed(1)));
assert_eq!(Balances::reserved_balance(&1), <Test as Config>::ParaDeposit::get());
assert_ok!(Registrar::register(
RuntimeOrigin::signed(1),
para_id,
test_genesis_head(32),
validation_code.clone(),
));
conclude_pvf_checking::<Test>(&validation_code, VALIDATORS, START_SESSION_INDEX);

run_to_session(START_SESSION_INDEX + 2);
assert!(Parachains::is_parathread(para_id));

let new_code = test_validation_code(0);
assert_noop!(
Registrar::schedule_code_upgrade(
RuntimeOrigin::signed(1),
para_id,
new_code.clone(),
),
paras::Error::<Test>::InvalidCode
);

let new_code = test_validation_code(max_code_size() as usize + 1);
assert_noop!(
Registrar::schedule_code_upgrade(
RuntimeOrigin::signed(1),
para_id,
new_code.clone(),
),
paras::Error::<Test>::InvalidCode
);
});
}

#[test]
fn register_handles_basic_errors() {
new_test_ext().execute_with(|| {
Expand Down Expand Up @@ -1310,7 +1355,7 @@ mod tests {
RuntimeOrigin::signed(1),
para_id,
vec![1; 3].into(),
vec![1, 2, 3].into(),
test_validation_code(32)
));

assert_noop!(Registrar::add_lock(RuntimeOrigin::signed(2), para_id), BadOrigin);
Expand Down Expand Up @@ -1473,7 +1518,7 @@ mod benchmarking {
use crate::traits::Registrar as RegistrarT;
use frame_support::assert_ok;
use frame_system::RawOrigin;
use primitives::{MAX_CODE_SIZE, MAX_HEAD_DATA_SIZE};
use primitives::{MAX_CODE_SIZE, MAX_HEAD_DATA_SIZE, MIN_CODE_SIZE};
use runtime_parachains::{paras, shared, Origin as ParaOrigin};
use sp_runtime::traits::Bounded;

Expand Down Expand Up @@ -1604,7 +1649,7 @@ mod benchmarking {
}

schedule_code_upgrade {
let b in 1 .. MAX_CODE_SIZE;
let b in MIN_CODE_SIZE .. MAX_CODE_SIZE;
let new_code = ValidationCode(vec![0; b as usize]);
let para_id = ParaId::from(1000);
}: _(RawOrigin::Root, para_id, new_code)
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/parachains/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ impl<BlockNumber: Default + From<u32>> Default for HostConfiguration<BlockNumber
validation_upgrade_cooldown: Default::default(),
validation_upgrade_delay: 2u32.into(),
code_retention_period: Default::default(),
max_code_size: Default::default(),
max_code_size: MAX_CODE_SIZE,
max_pov_size: Default::default(),
max_head_data_size: Default::default(),
coretime_cores: Default::default(),
Expand Down
10 changes: 5 additions & 5 deletions polkadot/runtime/parachains/src/inclusion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,7 @@ fn candidate_checks() {
para_id: chain_a,
relay_parent: System::parent_hash(),
pov_hash: Hash::repeat_byte(1),
new_validation_code: Some(vec![5, 6, 7, 8].into()),
new_validation_code: Some(dummy_validation_code()),
persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(),
hrmp_watermark: RELAY_PARENT_NUM,
..Default::default()
Expand All @@ -1257,7 +1257,7 @@ fn candidate_checks() {
assert_eq!(expected_at, 12);
Paras::schedule_code_upgrade(
chain_a,
vec![1, 2, 3, 4].into(),
vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into(),
expected_at,
&cfg,
SetGoAhead::Yes,
Expand Down Expand Up @@ -1317,7 +1317,7 @@ fn candidate_checks() {
pov_hash: Hash::repeat_byte(1),
persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(),
hrmp_watermark: RELAY_PARENT_NUM,
validation_code: ValidationCode(vec![1]),
validation_code: ValidationCode(vec![9, 8, 7, 6, 5, 4, 3, 2, 1]),
..Default::default()
}
.build();
Expand Down Expand Up @@ -1726,7 +1726,7 @@ fn can_include_candidate_with_ok_code_upgrade() {
relay_parent: System::parent_hash(),
pov_hash: Hash::repeat_byte(1),
persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(),
new_validation_code: Some(vec![1, 2, 3].into()),
new_validation_code: Some(vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into()),
hrmp_watermark: RELAY_PARENT_NUM,
..Default::default()
}
Expand Down Expand Up @@ -2133,7 +2133,7 @@ fn para_upgrade_delay_scheduled_from_inclusion() {
shared::Pallet::<Test>::set_active_validators_ascending(validator_public.clone());
shared::Pallet::<Test>::set_session_index(5);

let new_validation_code: ValidationCode = vec![1, 2, 3, 4, 5].into();
let new_validation_code: ValidationCode = vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into();
let new_validation_code_hash = new_validation_code.hash();

// Otherwise upgrade is no-op.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ const SESSION_INDEX: SessionIndex = 1;
const VALIDATOR_NUM: usize = 800;
const CAUSES_NUM: usize = 100;
fn validation_code() -> ValidationCode {
ValidationCode(vec![0])
ValidationCode(vec![1, 2, 3, 4, 5, 6, 7, 8, 9])
}
fn old_validation_code() -> ValidationCode {
ValidationCode(vec![1])
ValidationCode(vec![9, 8, 7, 6, 5, 4, 3, 2, 1])
}

/// Prepares the PVF check statement and the validator signature to pass into
Expand Down
17 changes: 15 additions & 2 deletions polkadot/runtime/parachains/src/paras/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ use frame_system::pallet_prelude::*;
use parity_scale_codec::{Decode, Encode};
use primitives::{
ConsensusLog, HeadData, Id as ParaId, PvfCheckStatement, SessionIndex, UpgradeGoAhead,
UpgradeRestriction, ValidationCode, ValidationCodeHash, ValidatorSignature,
UpgradeRestriction, ValidationCode, ValidationCodeHash, ValidatorSignature, MIN_CODE_SIZE,
};
use scale_info::{Type, TypeInfo};
use sp_core::RuntimeDebug;
Expand Down Expand Up @@ -679,6 +679,8 @@ pub mod pallet {
PvfCheckSubjectInvalid,
/// Parachain cannot currently schedule a code upgrade.
CannotUpgradeCode,
/// Invalid validation code size.
InvalidCode,
}

/// All currently active PVF pre-checking votes.
Expand Down Expand Up @@ -1230,6 +1232,10 @@ impl<T: Config> Pallet<T> {
// Check that we can schedule an upgrade at all.
ensure!(Self::can_upgrade_validation_code(id), Error::<T>::CannotUpgradeCode);
let config = configuration::Pallet::<T>::config();
// Validation code sanity checks:
ensure!(new_code.0.len() >= MIN_CODE_SIZE as usize, Error::<T>::InvalidCode);
ensure!(new_code.0.len() <= config.max_code_size as usize, Error::<T>::InvalidCode);

let current_block = frame_system::Pallet::<T>::block_number();
// Schedule the upgrade with a delay just like if a parachain triggered the upgrade.
let upgrade_block = current_block.saturating_add(config.validation_upgrade_delay);
Expand Down Expand Up @@ -1890,7 +1896,14 @@ impl<T: Config> Pallet<T> {
) -> Weight {
let mut weight = T::DbWeight::get().reads(1);

// Enacting this should be prevented by the `can_schedule_upgrade`
// Should be prevented by checks in `schedule_code_upgrade_external`
let new_code_len = new_code.0.len();
if new_code_len < MIN_CODE_SIZE as usize || new_code_len > cfg.max_code_size as usize {
log::warn!(target: LOG_TARGET, "attempted to schedule an upgrade with invalid new validation code",);
return weight
}

// Enacting this should be prevented by the `can_upgrade_validation_code`
if FutureCodeHash::<T>::contains_key(&id) {
// This branch should never be reached. Signalling an upgrade is disallowed for a para
// that already has one upgrade scheduled.
Expand Down
Loading
Loading