search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.83k stars 666 forks source link

PVF: Re-check file integrity before voting against; document #2399

Open mrcnski opened 11 months ago

mrcnski commented 11 months ago

Before voting against a candidate, re-check the file integrity.

Document it: add an # Artifact integrity section to artifacts.rs. It could explain that artifacts must remain valid to satisfy the SAFETY constraints of execute_artifact. Therefore we do two things: check file integrity and version compatibility on host startup, and also before voting against a candidate we re-check the file integrity.

_Originally posted by @mrcnski in https://github.com/paritytech/polkadot-sdk/pull/1918#discussion_r1390940354_

jpserrat commented 10 months ago

@mrcnski Is this issue about re-checking the file integrity and adding the # Artifact integrity? Why the re-checking is needed? Do we not vote against it in case of failure?

mrcnski commented 10 months ago

@mrcnski Is this issue about re-checking the file integrity and adding the # Artifact integrity?

Yes, exactly! We still need to do both, re-checking was not implemented yet.

Why the re-checking is needed? Do we not vote against it in case of failure?

We had the idea to re-check the file hash of the artifact before execution, to make sure it wasn't corrupted. However, this would add a few ms to each execution. As an optimization, we can only re-check after execution, if the execution failed. Reason being, that corruption should be very very rare, and if it happens then the execution will fail, so we only need to check it there. Hope that makes sense. Thanks for asking, for sure it's not obvious so should be documented. 👍

mrcnski commented 10 months ago

If we re-check and it's corrupted, we should return a specific error that indicates that the host should purge this artifact from the artifacts cache.

eagr commented 10 months ago

I'm about to take a shot at the integrity re-check. Wondering if you have started on this? @Jpserrat

jpserrat commented 10 months ago

@eagr I haven't, you can take it

eskimor commented 10 months ago

Do we not vote against it in case of failure?

We do and this is precisely the problem. If the file got corrupted on disk, we would currently vote invalid on the candidate and raise a dispute, despite the issue being our hard disk and not the candidate. We would get slashed and disabled for raising a dispute on a valid candidate. Obviously there should be a big fat warning in the logs if a corruption is detected.