pvf-precheck: Strip PastCodeMeta (#4408)

This PR is a part of
https://github.com/paritytech/polkadot/issues/3211.

This PR prepares ground for the following runtime changes required for
PVF pre-checking. Specifically, we do several changes here:

1. We remove `validation_code_at` and `validation_code_hash_at`. Those
   functions are not used. They were added in the early days with intent
   to use it later but turned out that we do not need them.
2. We replace `validation_code_hash_at` with just `current_code_hash`
   for the case of inclusion and candidate checking.
3. We also replace `last_code_upgrade` with a direct query into
   `FutureCodeHash` and `UpgradeRestrictionSignal`. Those in conjunction
   should replace the logic that was used for allowing/disallowing
   upgrades. This requires special attention of the reviewers.
4. Then we remove the machinery required to support those queries.
   Specifically the code related to `UseCodeAt`. We do not need it since
   we do not answer the historical queries. However, we still leave all
   the data on-chain. At some point we may clean it up, but that would
   be needed to be done with a dedicated migration which can be done as
   follow-up.
5. Some now irrelevant tests were removed and/or adapted.

polkadot/roadmap/implementers-guide/src/runtime/paras.md

@@ -217,13 +217,6 @@ CodeByHash: map ValidationCodeHash => Option<ValidationCode>
 * `note_new_head(ParaId, HeadData, BlockNumber)`: note that a para has progressed to a new head,
   where the new head was executed in the context of a relay-chain block with given number. This will
   apply pending code upgrades based on the block number provided. If an upgrade took place it will clear the `UpgradeGoAheadSignal`.
-* `validation_code_at(ParaId, at: BlockNumber, assume_intermediate: Option<BlockNumber>)`: Fetches
-  the validation code to be used when validating a block in the context of the given relay-chain
-  height. A second block number parameter may be used to tell the lookup to proceed as if an
-  intermediate parablock has been included at the given relay-chain height. This may return past,
-  current, or (with certain choices of `assume_intermediate`) future code. `assume_intermediate`, if
-  provided, must be before `at`. If the validation code has been pruned, this will return `None`.
-* `validation_code_hash_at(ParaId, at: BlockNumber, assume_intermediate: Option<BlockNumber>)`: Just like `validation_code_at`, but returns the code hash.
 * `lifecycle(ParaId) -> Option<ParaLifecycle>`: Return the `ParaLifecycle` of a para.
 * `is_parachain(ParaId) -> bool`: Returns true if the para ID references any live parachain,
   including those which may be transitioning to a parathread in the future.
@@ -231,9 +224,7 @@ CodeByHash: map ValidationCodeHash => Option<ValidationCode>
   including those which may be transitioning to a parachain in the future.
 * `is_valid_para(ParaId) -> bool`: Returns true if the para ID references either a live parathread
   or live parachain.
-* `last_code_upgrade(id: ParaId, include_future: bool) -> Option<BlockNumber>`: The block number of
+* `can_upgrade_validation_code(ParaId) -> bool`: Returns true if the given para can signal code upgrade right now.
-  the last scheduled upgrade of the requested para. Includes future upgrades if the flag is set.
-  This is the `expected_at` number, not the `activated_at` number.
 
 ## Finalization
 

polkadot/runtime/parachains/src/inclusion/mod.rs

@@ -36,10 +36,7 @@ use primitives::v1::{
 	ValidatorIndex, ValidityAttestation,
 };
 use scale_info::TypeInfo;
-use sp_runtime::{
+use sp_runtime::{traits::One, DispatchError};
-	traits::{One, Saturating},
-	DispatchError,
-};
 use sp_std::{collections::btree_set::BTreeSet, prelude::*};
 
 pub use pallet::*;
@@ -953,7 +950,6 @@ impl<T: Config> CandidateCheckContext<T> {
 		backed_candidate: &BackedCandidate<<T as frame_system::Config>::Hash>,
 	) -> Result<(), Error<T>> {
 		let para_id = backed_candidate.descriptor().para_id;
-		let now = self.now;
 
 		// we require that the candidate is in the context of the parent block.
 		ensure!(
@@ -965,7 +961,7 @@ impl<T: Config> CandidateCheckContext<T> {
 			Error::<T>::NotCollatorSigned,
 		);
 
-		let validation_code_hash = <paras::Pallet<T>>::validation_code_hash_at(para_id, now, None)
+		let validation_code_hash = <paras::Pallet<T>>::current_code_hash(para_id)
 			// A candidate for a parachain without current validation code is not scheduled.
 			.ok_or_else(|| Error::<T>::UnscheduledCandidate)?;
 		ensure!(
@@ -1019,13 +1015,10 @@ impl<T: Config> CandidateCheckContext<T> {
 
 		// if any, the code upgrade attempt is allowed.
 		if let Some(new_validation_code) = new_validation_code {
-			let valid_upgrade_attempt = <paras::Pallet<T>>::last_code_upgrade(para_id, true)
+			ensure!(
-				.map_or(true, |last| {
+				<paras::Pallet<T>>::can_upgrade_validation_code(para_id),
-					last <= self.relay_parent_number &&
+				AcceptanceCheckErr::PrematureCodeUpgrade,
-						self.relay_parent_number.saturating_sub(last) >=
+			);
-							self.config.validation_upgrade_frequency
-				});
-			ensure!(valid_upgrade_attempt, AcceptanceCheckErr::PrematureCodeUpgrade);
 			ensure!(
 				new_validation_code.0.len() <= self.config.max_code_size as _,
 				AcceptanceCheckErr::NewCodeTooLarge,

polkadot/runtime/parachains/src/inclusion/tests.rs

@@ -1281,8 +1281,6 @@ fn candidate_checks() {
 				let expected_at = 10 + cfg.validation_upgrade_delay;
 				assert_eq!(expected_at, 10);
 				Paras::schedule_code_upgrade(chain_a, vec![1, 2, 3, 4].into(), expected_at, &cfg);
-
-				assert_eq!(Paras::last_code_upgrade(chain_a, true), Some(expected_at));
 			}
 
 			assert_noop!(

polkadot/runtime/parachains/src/paras.rs

@@ -76,16 +76,6 @@ pub struct ParaPastCodeMeta<N> {
 	last_pruned: Option<N>,
 }
 
-#[cfg_attr(test, derive(Debug, PartialEq))]
-enum UseCodeAt<N> {
-	/// Use the current code.
-	Current,
-	/// Use the code that was replaced at the given block number.
-	/// This is an inclusive endpoint - a parablock in the context of a relay-chain block on this fork
-	/// with number N should use the code that is replaced at N.
-	ReplacedAt(N),
-}
-
 /// The possible states of a para, to take into account delayed lifecycle changes.
 ///
 /// If the para is in a "transition state", it is expected that the parachain is
@@ -164,71 +154,14 @@ impl<N: Ord + Copy + PartialEq> ParaPastCodeMeta<N> {
 		self.upgrade_times.push(ReplacementTimes { expected_at, activated_at })
 	}
 
-	// Yields an identifier that should be used for validating a
+	/// Returns `true` if the upgrade logs list is empty.
-	// parablock in the context of a particular relay-chain block number in this chain.
+	fn is_empty(&self) -> bool {
-	//
+		self.upgrade_times.is_empty()
-	// a return value of `None` means that there is no code we are aware of that
-	// should be used to validate at the given height.
-	fn code_at(&self, para_at: N) -> Option<UseCodeAt<N>> {
-		// Find out
-		// a) if there is a point where code was replaced in the current chain after the context
-		//    we are finding out code for.
-		// b) what the index of that point is.
-		//
-		// The reason we use `activated_at` instead of `expected_at` is that a gap may occur
-		// between expectation and actual activation. Any block executed in a context from
-		// `expected_at..activated_at` is expected to activate the code upgrade and therefore should
-		// use the previous code.
-		//
-		// A block executed in the context of `activated_at` should use the new code.
-		//
-		// Cases where `expected_at` and `activated_at` are the same, that is, zero-delay code upgrades
-		// are also handled by this rule correctly.
-		let replaced_after_pos = self.upgrade_times.iter().position(|t| {
-			// example: code replaced at (5, 5)
-			//
-			// context #4 should use old code
-			// context #5 should use new code
-			//
-			// example: code replaced at (10, 20)
-			// context #9 should use the old code
-			// context #10 should use the old code
-			// context #19 should use the old code
-			// context #20 should use the new code
-			para_at < t.activated_at
-		});
-
-		if let Some(replaced_after_pos) = replaced_after_pos {
-			// The earliest stored code replacement needs to be special-cased, since we need to check
-			// against the pruning state to see if this replacement represents the correct code, or
-			// is simply after a replacement that actually represents the correct code, but has been pruned.
-			let was_pruned =
-				replaced_after_pos == 0 && self.last_pruned.map_or(false, |t| t >= para_at);
-
-			if was_pruned {
-				None
-			} else {
-				Some(UseCodeAt::ReplacedAt(self.upgrade_times[replaced_after_pos].expected_at))
-			}
-		} else {
-			// No code replacements after this context.
-			// This means either that the current code is valid, or `para_at` is so old that
-			// we don't know the code necessary anymore. Compare against `last_pruned` to determine.
-			self.last_pruned.as_ref().map_or(
-				Some(UseCodeAt::Current), // nothing pruned, use current
-				|earliest_activation| {
-					if &para_at < earliest_activation {
-						None
-					} else {
-						Some(UseCodeAt::Current)
-					}
-				},
-			)
-		}
 	}
 
 	// The block at which the most recently tracked code change occurred, from the perspective
 	// of the para.
+	#[cfg(test)]
 	fn most_recent_change(&self) -> Option<N> {
 		self.upgrade_times.last().map(|x| x.expected_at.clone())
 	}
@@ -802,7 +735,7 @@ impl<T: Config> Pallet<T> {
 							}
 						}
 
-						meta.most_recent_change().is_none() && Self::para_head(&para_id).is_none()
+						meta.is_empty() && Self::para_head(&para_id).is_none()
 					});
 
 					// This parachain has been removed and now the vestigial code
@@ -1048,43 +981,6 @@ impl<T: Config> Pallet<T> {
 		}
 	}
 
-	/// Fetches the validation code hash for the validation code to be used when validating a block
-	/// in the context of the given relay-chain height. A second block number parameter may be used
-	/// to tell the lookup to proceed as if an intermediate parablock has been with the given
-	/// relay-chain height as its context. This may return the hash for the past, current, or
-	/// (with certain choices of `assume_intermediate`) future code.
-	///
-	/// `assume_intermediate`, if provided, must be before `at`. This will return `None` if the validation
-	/// code has been pruned.
-	///
-	/// To get associated code see [`Self::validation_code_at`].
-	pub(crate) fn validation_code_hash_at(
-		id: ParaId,
-		at: T::BlockNumber,
-		assume_intermediate: Option<T::BlockNumber>,
-	) -> Option<ValidationCodeHash> {
-		if assume_intermediate.as_ref().map_or(false, |i| &at <= i) {
-			return None
-		}
-
-		let planned_upgrade = <Self as Store>::FutureCodeUpgrades::get(&id);
-		let upgrade_applied_intermediate = match assume_intermediate {
-			Some(a) => planned_upgrade.as_ref().map_or(false, |u| u <= &a),
-			None => false,
-		};
-
-		if upgrade_applied_intermediate {
-			FutureCodeHash::<T>::get(&id)
-		} else {
-			match Self::past_code_meta(&id).code_at(at) {
-				None => None,
-				Some(UseCodeAt::Current) => CurrentCodeHash::<T>::get(&id),
-				Some(UseCodeAt::ReplacedAt(replaced)) =>
-					<Self as Store>::PastCodeHash::get(&(id, replaced)),
-			}
-		}
-	}
-
 	/// Returns the current lifecycle state of the para.
 	pub fn lifecycle(id: ParaId) -> Option<ParaLifecycle> {
 		ParaLifecycles::<T>::get(&id)
@@ -1123,16 +1019,10 @@ impl<T: Config> Pallet<T> {
 		}
 	}
 
-	/// The block number of the last scheduled upgrade of the requested para. Includes future upgrades
+	/// If a candidate from the specified parachain were submitted at the current block, this
-	/// if the flag is set. This is the `expected_at` number, not the `activated_at` number.
+	/// function returns if that candidate passes the acceptance criteria.
-	pub(crate) fn last_code_upgrade(id: ParaId, include_future: bool) -> Option<T::BlockNumber> {
+	pub(crate) fn can_upgrade_validation_code(id: ParaId) -> bool {
-		if include_future {
+		FutureCodeHash::<T>::get(&id).is_none() && UpgradeRestrictionSignal::<T>::get(&id).is_none()
-			if let Some(at) = Self::future_code_upgrade_at(id) {
-				return Some(at)
-			}
-		}
-
-		Self::past_code_meta(&id).most_recent_change()
 	}
 
 	/// Return the session index that should be used for any future scheduled changes.
@@ -1237,52 +1127,6 @@ mod tests {
 		assert!(!<Paras as Store>::CodeByHash::contains_key(validation_code.hash()));
 	}
 
-	fn fetch_validation_code_at(
-		para_id: ParaId,
-		at: BlockNumber,
-		assume_intermediate: Option<BlockNumber>,
-	) -> Option<ValidationCode> {
-		Paras::validation_code_hash_at(para_id, at, assume_intermediate)
-			.and_then(Paras::code_by_hash)
-	}
-
-	#[test]
-	fn para_past_code_meta_gives_right_code() {
-		let mut past_code = ParaPastCodeMeta::default();
-		assert_eq!(past_code.code_at(0u32), Some(UseCodeAt::Current));
-
-		past_code.note_replacement(10, 12);
-		assert_eq!(past_code.code_at(0), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(10), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(11), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(12), Some(UseCodeAt::Current));
-
-		past_code.note_replacement(20, 25);
-		assert_eq!(past_code.code_at(1), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(10), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(11), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(12), Some(UseCodeAt::ReplacedAt(20)));
-		assert_eq!(past_code.code_at(24), Some(UseCodeAt::ReplacedAt(20)));
-		assert_eq!(past_code.code_at(25), Some(UseCodeAt::Current));
-
-		past_code.note_replacement(30, 30);
-		assert_eq!(past_code.code_at(1), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(10), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(11), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(12), Some(UseCodeAt::ReplacedAt(20)));
-		assert_eq!(past_code.code_at(24), Some(UseCodeAt::ReplacedAt(20)));
-		assert_eq!(past_code.code_at(25), Some(UseCodeAt::ReplacedAt(30)));
-		assert_eq!(past_code.code_at(30), Some(UseCodeAt::Current));
-
-		past_code.last_pruned = Some(5);
-		assert_eq!(past_code.code_at(1), None);
-		assert_eq!(past_code.code_at(5), None);
-		assert_eq!(past_code.code_at(6), Some(UseCodeAt::ReplacedAt(10)));
-		assert_eq!(past_code.code_at(24), Some(UseCodeAt::ReplacedAt(20)));
-		assert_eq!(past_code.code_at(25), Some(UseCodeAt::ReplacedAt(30)));
-		assert_eq!(past_code.code_at(30), Some(UseCodeAt::Current));
-	}
-
 	#[test]
 	fn para_past_code_pruning_works_correctly() {
 		let mut past_code = ParaPastCodeMeta::default();
@@ -1705,27 +1549,6 @@ mod tests {
 
 				assert_eq!(Paras::past_code_meta(&para_id).most_recent_change(), Some(expected_at));
 
-				// Some hypothetical block which would have triggered the code change
-				// should still use the old code.
-				assert_eq!(
-					Paras::past_code_meta(&para_id).code_at(expected_at),
-					Some(UseCodeAt::ReplacedAt(expected_at)),
-				);
-
-				// Some hypothetical block at the context which actually triggered the
-				// code change should still use the old code.
-				assert_eq!(
-					Paras::past_code_meta(&para_id).code_at(expected_at + 4),
-					Some(UseCodeAt::ReplacedAt(expected_at)),
-				);
-
-				// Some hypothetical block at the context after the code was upgraded
-				// should use the new code.
-				assert_eq!(
-					Paras::past_code_meta(&para_id).code_at(expected_at + 4 + 1),
-					Some(UseCodeAt::Current),
-				);
-
 				assert_eq!(
 					<Paras as Store>::PastCodeHash::get(&(para_id, expected_at)),
 					Some(original_code.hash()),
@@ -1976,64 +1799,6 @@ mod tests {
 		})
 	}
 
-	#[test]
-	fn code_hash_at_with_intermediate() {
-		let code_retention_period = 10;
-		let validation_upgrade_delay = 10;
-
-		let paras = vec![(
-			0u32.into(),
-			ParaGenesisArgs {
-				parachain: true,
-				genesis_head: Default::default(),
-				validation_code: vec![1, 2, 3].into(),
-			},
-		)];
-
-		let genesis_config = MockGenesisConfig {
-			paras: GenesisConfig { paras, ..Default::default() },
-			configuration: crate::configuration::GenesisConfig {
-				config: HostConfiguration {
-					code_retention_period,
-					validation_upgrade_delay,
-					..Default::default()
-				},
-				..Default::default()
-			},
-			..Default::default()
-		};
-
-		new_test_ext(genesis_config).execute_with(|| {
-			let para_id = ParaId::from(0);
-			let old_code: ValidationCode = vec![1, 2, 3].into();
-			let new_code: ValidationCode = vec![4, 5, 6].into();
-
-			// expected_at = 10 = 0 + validation_upgrade_delay = 0 + 10
-			Paras::schedule_code_upgrade(para_id, new_code.clone(), 0, &Configuration::config());
-			assert_eq!(<Paras as Store>::FutureCodeUpgrades::get(&para_id), Some(10));
-
-			// no intermediate, falls back on current/past.
-			assert_eq!(fetch_validation_code_at(para_id, 1, None), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 10, None), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 100, None), Some(old_code.clone()));
-
-			// intermediate before upgrade meant to be applied, falls back on current.
-			assert_eq!(fetch_validation_code_at(para_id, 9, Some(8)), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 10, Some(9)), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 11, Some(9)), Some(old_code.clone()));
-
-			// intermediate at or after upgrade applied
-			assert_eq!(fetch_validation_code_at(para_id, 11, Some(10)), Some(new_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 100, Some(11)), Some(new_code.clone()));
-
-			run_to_block(code_retention_period + 5, None);
-
-			// at <= intermediate not allowed
-			assert_eq!(fetch_validation_code_at(para_id, 10, Some(10)), None);
-			assert_eq!(fetch_validation_code_at(para_id, 9, Some(10)), None);
-		});
-	}
-
 	#[test]
 	fn code_hash_at_returns_up_to_end_of_code_retention_period() {
 		let code_retention_period = 10;
@@ -2071,18 +1836,12 @@ mod tests {
 			Paras::note_new_head(para_id, Default::default(), 7);
 
 			assert_eq!(Paras::past_code_meta(&para_id).upgrade_times, vec![upgrade_at(2, 10)]);
+			assert_eq!(Paras::current_code(&para_id), Some(new_code.clone()));
 
-			assert_eq!(fetch_validation_code_at(para_id, 2, None), Some(old_code.clone()));
+			// Make sure that the old code is available **before** the code retion period passes.
-			assert_eq!(fetch_validation_code_at(para_id, 3, None), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 9, None), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 10, None), Some(new_code.clone()));
-
 			run_to_block(10 + code_retention_period, None);
-
+			assert_eq!(Paras::code_by_hash(&old_code.hash()), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 2, None), Some(old_code.clone()));
+			assert_eq!(Paras::code_by_hash(&new_code.hash()), Some(new_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 3, None), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 9, None), Some(old_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 10, None), Some(new_code.clone()));
 
 			run_to_block(10 + code_retention_period + 1, None);
 
@@ -2093,10 +1852,8 @@ mod tests {
 				ParaPastCodeMeta { upgrade_times: Vec::new(), last_pruned: Some(10) },
 			);
 
-			assert_eq!(fetch_validation_code_at(para_id, 2, None), None); // pruned :(
+			assert_eq!(Paras::code_by_hash(&old_code.hash()), None); // pruned :(
-			assert_eq!(fetch_validation_code_at(para_id, 9, None), None);
+			assert_eq!(Paras::code_by_hash(&new_code.hash()), Some(new_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 10, None), Some(new_code.clone()));
-			assert_eq!(fetch_validation_code_at(para_id, 11, None), Some(new_code.clone()));
 		});
 	}