PVF: Document that preparation cannot lead to disputes (#6873)

* PVF: Document that preparation cannot lead to disputes

* Add warning for deterministic errors

* Fix warning

polkadot/node/core/candidate-validation/src/lib.rs

@@ -623,7 +623,7 @@ where
 		.await;
 
 	if let Err(ref error) = result {
-		gum::info!(target: LOG_TARGET, ?para_id, ?error, "Failed to validate candidate",);
+		gum::info!(target: LOG_TARGET, ?para_id, ?error, "Failed to validate candidate");
 	}
 
 	match result {
@@ -638,9 +638,16 @@ where
 			))),
 		Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::PrepareError(e))) => {
 			// In principle if preparation of the `WASM` fails, the current candidate can not be the
-			// reason for that. So we can't say whether it is invalid or not in addition with
+			// reason for that. So we can't say whether it is invalid or not. In addition, with
 			// pre-checking enabled only valid runtimes should ever get enacted, so we can be
-			// reasonably sure that this is some local problem on the current node.
+			// reasonably sure that this is some local problem on the current node. However, as this
+			// particular error *seems* to indicate a deterministic error, we raise a warning.
+			gum::warn!(
+				target: LOG_TARGET,
+				?para_id,
+				?e,
+				"Deterministic error occurred during preparation (should have been ruled out by pre-checking phase)",
+			);
 			Err(ValidationFailed(e))
 		},
 		Ok(res) =>

polkadot/node/core/pvf/src/error.rs

@@ -120,12 +120,6 @@ impl From<PrepareError> for ValidationError {
 	fn from(error: PrepareError) -> Self {
 		// Here we need to classify the errors into two errors: deterministic and non-deterministic.
 		// See [`PrepareError::is_deterministic`].
-		//
-		// We treat the deterministic errors as `InvalidCandidate`. Should those occur they could
-		// potentially trigger disputes.
-		//
-		// All non-deterministic errors are qualified as `InternalError`s and will not trigger
-		// disputes.
 		if error.is_deterministic() {
 			ValidationError::InvalidCandidate(InvalidCandidate::PrepareError(error.to_string()))
 		} else {

polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md

@@ -35,6 +35,12 @@ Rejecting instead of abstaining is better in several ways:
 
 Also, if we only abstain, an attacker can specially craft a PVF wasm blob so that it will fail on e.g. 50% of the validators. In that case a supermajority will never be reached and the vote will repeat multiple times, most likely with the same result (since all votes are cleared on a session change). This is avoided by rejecting failed PVFs, and by only requiring 1/3 of validators to reject a PVF to reach a decision.
 
+### Note on Disputes
+
+Having a pre-checking phase allows us to make certain assumptions later when preparing the PVF for execution. If a runtime passed pre-checking, then we know that the runtime should be valid, and therefore any issue during preparation for execution can be assumed to be a local problem on the current node.
+
+For this reason, even deterministic preparation errors should not trigger disputes. And since we do not dispute as a result of the pre-checking phase, as stated above, it should be impossible for preparation in general to result in disputes.
+
 [overview]: ../../pvf-prechecking.md
 [Runtime API]: runtime-api.md
 [PVF pre-checking runtime API]: ../../runtime-api/pvf-prechecking.md

polkadot/roadmap/implementers-guide/src/pvf-prechecking.md

@@ -46,7 +46,7 @@ The logic described above is implemented by the [paras] module.
 
 On the node-side, there is a PVF pre-checking [subsystem][pvf-prechecker-subsystem] that scans the chain for new PVFs via using [runtime APIs][pvf-runtime-api]. Upon finding a new PVF, the subsystem will initiate a PVF pre-checking request and wait for the result. Whenever the result is obtained, the subsystem will use the [runtime API][pvf-runtime-api] to submit a vote for the PVF. The vote is an unsigned transaction. The vote will be distributed via the gossip similarly to a normal transaction. Eventually a block producer will include the vote into the block where it will be handled by the [runtime][paras].
 
-## Pre-checking Summary
+## Summary
 
 Parachains' and parathreads' validation function is described by a wasm module that we refer to as a PVF.
 
@@ -54,7 +54,7 @@ In order to make the PVF usable for candidate validation it has to be registered
 
 As part of the registration process, it has to go through pre-checking. Pre-checking is a game of attempting preparation and reporting the results back on-chain.
 
-We define preparation as a process that: validates the consistency of the wasm binary (aka prevalidation) and the compilation of the wasm module into machine code (refered to as artifact).
+We define preparation as a process that: validates the consistency of the wasm binary (aka prevalidation) and the compilation of the wasm module into machine code (referred to as an artifact).
 
 Besides pre-checking, preparation can also be triggered by execution, since a compiled artifact is needed for the execution. If an artifact already exists, execution will skip preparation. If it does do preparation, execution uses a more lenient timeout than preparation, to avoid the situation where honest validators fail on valid, pre-checked PVFs.