From ba29d06334a763042ebad6614bb9643cdf216955 Mon Sep 17 00:00:00 2001
From: Marcin S <marcin@realemail.net>
Date: Wed, 15 Mar 2023 01:35:38 +0100
Subject: [PATCH] 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 | 13 ++++++++++---
polkadot/node/core/pvf/src/error.rs | 6 ------
.../src/node/utility/pvf-prechecker.md | 6 ++++++
.../implementers-guide/src/pvf-prechecking.md | 4 ++--
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs
index fdd6ff002dd..a1c939b236a 100644
--- a/polkadot/node/core/candidate-validation/src/lib.rs
+++ b/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) =>
diff --git a/polkadot/node/core/pvf/src/error.rs b/polkadot/node/core/pvf/src/error.rs
index 3f642cd6ed2..5edf435e719 100644
--- a/polkadot/node/core/pvf/src/error.rs
+++ b/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 {
diff --git a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md b/polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md
index 90193ec00e1..3cae12e65f3 100644
--- a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md
+++ b/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
diff --git a/polkadot/roadmap/implementers-guide/src/pvf-prechecking.md b/polkadot/roadmap/implementers-guide/src/pvf-prechecking.md
index fd2ca12330b..155d32d5289 100644
--- a/polkadot/roadmap/implementers-guide/src/pvf-prechecking.md
+++ b/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.
--
GitLab