pvf-precheck: Strip PastCodeMeta

[pepyakin]

This PR is a part of
#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.

NOTE: This fixes a small bug related to parachain upgrades. Before, the acceptance function would allow the following behavior:

• a parachain upgrades the code.
• parachain does not produce blocks
• at or after expected_at block, the parachain produces a block that schedules an upgrade.

After this change this is not allowed. This is because we do not allow an upgrade until FutureCodeHash is cleared and FutureCodeHash is consumed by a enacting a candidate that has the relay-parent ≥ expected_at.

I say that this is a bug because even though such a candidate will pass the acceptance check, the upgrade it signals won't be enacted. This is because enacting consists of scheduling the upgrade first if any and only then calling note_new_head. schedule_code_upgrade bails if there is a pending upgrade and the pending upgrade only is enacted by note_new_head.

[pepyakin]

Current dependencies on/for this PR:

• master
  • PR pvf-precheck: Strip PastCodeMeta #4408 (this PR)
    • PR pvf-precheck: update configuration module #4420
      • PR pvf-precheck: Integrate PVF pre-checking into paras module #4457
        • PR paras: add log target #4478
        • PR pvf-precheck: update rustdoc for paras module #4459

This comment was auto-generated by Graphite and will continue to be automatically updated while this PR remains open.

[pepyakin]

This predicate seems to be equivalent to the previous condition, i.e.:

let valid_upgrade_attempt = <paras::Pallet<T>>::last_code_upgrade(para_id, true)
				.map_or(true, |last| {
					last <= self.relay_parent_number &&
						self.relay_parent_number.saturating_sub(last) >=
							self.config.validation_upgrade_frequency
				});

But needs double-checking

[rphmeier]

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.

polkadot/runtime/parachains/src/inclusion.rs

Lines 965 to 970 in 0043fe5

	let validation_code_hash = <paras::Pallet<T>>::validation_code_hash_at(para_id, now, None)
	// A candidate for a parachain without current validation code is not scheduled.
	.ok_or_else(|| Error::<T>::UnscheduledCandidate)?;
	ensure!(
	backed_candidate.descriptor().validation_code_hash == validation_code_hash,
	Error::<T>::InvalidValidationCodeHash,

We might want to keep it so this snippet can be altered as a part of asynchronous backing. Otherwise code changes would result in service interruptions for a few blocks. Worth noting.

[rphmeier]

i.e. this was previously future-proof changes to add asynchronous backing (old relay parent where code hadn't yet changed) but is not future-proof anymore

[rphmeier]

Probably fine, from a product perspective - if we build parachains 1 block in advance then a code upgrade block would lead to a 6 second service interruption. Workarounds possible but not needed for the moment

[pepyakin]

@rphmeier when I wrote this code that thought crossed my mind. I had to chase it away because I felt it was overthinking: it's not easy to predict the precise API that will actually be required by async execution and at the same time there is no benefit for that either: we can easily add/remove parameters and shuffle the code around when the time comes (and when we a better grasp of the actual needs). Then this change also does not alter any on-chain storage, so we can actually restore any code from git history and it will work. Although I had planned to remove the storage eventually as well.

I am also a bit confused since I was under impression that the context should be passed through a relay-chain state merkle proof. That's why here I thought it is acceptable to remove the historical data from the current state.

[rphmeier]

Yeah, it's fine as is. The latest version of contextual execution might not need merkle proofs

[pepyakin]

bot merge

[paritytech-processbot]

Bot will approve on the behalf of @pepyakin, since they are a team lead, in an attempt to reach the minimum approval count

[pepyakin]

(merged assuming this was an implicit approval and due to that this is a small change)