paritytech / polkadot-sdk

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

Discussion: PVF Compilation(Interpretation) time testing #592

Open vikinatora opened 1 year ago

vikinatora commented 1 year ago

Hi everyone, I would like to initiate a discussion regarding our ongoing efforts in developing the PVF Conformance testing suite for the Hosts as part of a RFP. The purpose of this discussion is to seek input from the community and the maintainers, to share our approach and address a specific challenge.

Current Progress

Our team is actively working on the conformance testing suite with a focus on injecting malicious instructions into the validate_block function. This allows us to simulate various scenarios of bad PVFs and assess the behaviour of the relay chain and parachain using Zombienet. So far, we have covered test cases such as compilation time, execution time, stack overflow, and memory overflow. All but the first one have had substantial progress towards completing them.

Challenge

The team has encountered an issue when attempting to cover the compilation time scenario. Our current approach involves injecting millions of no-op instructions to potentially increase the interpretation time of the PVF and trigger a timeout after 60 seconds (DEFAULT_PRECHECK_PREPARATION_TIMEOUT). However, this method has not been effective due to existing security measures (validations).

Existing Limitations:

The first limitation is the VALIDATION_CODE_BOMB_LIMIT, which restricts the number of instructions we can insert into the PVF. Another one, not directly related to the Polkadot Host, is a JavaScript error during the chain spec generation in Zombienet: Error: Cannot create a string longer than 0x1fffffe8 characters. This is another limit that limits us on the number of instructions that we can insert.

Request for Suggestions:

We believe the maintainers’ expertise could help us overcome this challenge. We are open to any suggestions or alternative approaches to increase the interpretation time of the PVF successfully without bloating the size of the PVF.

rphmeier commented 1 year ago

Well, it seems we either need (a) a small payload which exploits the compiler or (b) a large payload which just takes a long time to compile.

(a) is the realistic test scenario of malicious code, but it seems error-prone and not future-proof and will need updating when Wasmtime gets updated.

For (b), we could bypass the VALIDATION_CODE_BOMB_LIMIT. That isn't something I'd feel very comfortable committing to master as a CLI flag, as it's unsafe and would be specifically for this one test scenario.

bkchr commented 1 year ago

For (b), we could bypass the VALIDATION_CODE_BOMB_LIMIT. That isn't something I'd feel very comfortable committing to master as a CLI flag, as it's unsafe and would be specifically for this one test scenario.

TBH, I still find it a little bit weird that we can define these values in the HostConfig on chain and still have them hard coded in the code. (Yeah I know, this limit is not in the host config, but it is a multiple of the validation code size). Maybe a proper way forward would be to stop depending on these hard coded constants and to fetch them from on chain. Updates of these values should only be accepted when they are in a finalized block.

In the meantime the best is probably to have a patch that decreases/increases the values to make it possible to test this stuff.