diff --git a/polkadot/doc/testing.md b/polkadot/doc/testing.md index 1045303baf0df1e757cefa374ca2396b99984504..76703b1b4398a0cac8423183a5d2febfabab0e6c 100644 --- a/polkadot/doc/testing.md +++ b/polkadot/doc/testing.md @@ -1,6 +1,7 @@ # Testing -Automated testing is an essential tool to assure correctness. +Testing is an essential tool to assure correctness. This document describes how we test the Polkadot code, whether +locally, at scale, and/or automatically in CI. ## Scopes @@ -8,27 +9,57 @@ The testing strategy for Polkadot is 4-fold: ### Unit testing (1) -Boring, small scale correctness tests of individual functions. +Boring, small scale correctness tests of individual functions. It is usually +enough to run `cargo test` in the crate you are testing. + +For full coverage you may have to pass some additional features. For example: + +```sh +cargo test --features ci-only-tests +``` ### Integration tests -There are two variants of integration tests: +There are the following variants of integration tests: #### Subsystem tests (2) One particular subsystem (subsystem under test) interacts with a mocked overseer that is made to assert incoming and -outgoing messages of the subsystem under test. This is largely present today, but has some fragmentation in the evolved -integration test implementation. A `proc-macro`/`macro_rules` would allow for more consistent implementation and -structure. +outgoing messages of the subsystem under test. See e.g. the `statement-distribution` tests. #### Behavior tests (3) -Launching small scale networks, with multiple adversarial nodes without any further tooling required. This should -include tests around the thresholds in order to evaluate the error handling once certain assumed invariants fail. +Launching small scale networks, with multiple adversarial nodes. This should include tests around the thresholds in +order to evaluate the error handling once certain assumed invariants fail. + +Currently, we commonly use **zombienet** to run mini test-networks, whether locally or in CI. To run on your machine: + +- First, make sure you have [zombienet][zombienet] installed. + +- Now, all the required binaries must be installed in your $PATH. You must run the following from the `polkadot/` +directory in order to test your changes. (Not `zombienet setup`, or you will get the released binaries without your +local changes!) -For this purpose based on `AllSubsystems` and `proc-macro` `AllSubsystemsGen`. +```sh +cargo install --path . --locked +``` + +- You will also need to install whatever binaries are required for your specific tests. For example, to install +`undying-collator`, from `polkadot/`, run: + +```sh +cargo install --path ./parachain/test-parachains/undying/collator --locked +``` -This assumes a simplistic test runtime. +- Finally, run the zombienet test from the `polkadot` directory: + +```sh +RUST_LOG=parachain::pvf=trace zombienet --provider=native spawn zombienet_tests/functional/0001-parachains-pvf.toml +``` + +- You can pick a validator node like `alice` from the output and view its logs +(`tail -f <log_file>`) or metrics. Make sure there is nothing funny in the logs +(try `grep WARN <log_file>`). #### Testing at scale (4) @@ -41,13 +72,27 @@ addition prometheus avoiding additional Polkadot source changes. _Behavior tests_ and _testing at scale_ have naturally soft boundary. The most significant difference is the presence of a real network and the number of nodes, since a single host often not capable to run multiple nodes at once. ---- +## Observing Logs + +To verify expected behavior it's often useful to observe logs. To avoid too many +logs at once, you can run one test at a time: + +1. Add `sp_tracing::try_init_simple();` to the beginning of a test +2. Specify `RUST_LOG=<target>::<subtarget>=trace` before the cargo command. + +For example: + +```sh +RUST_LOG=parachain::pvf=trace cargo test execute_can_run_serially +``` + +For more info on how our logs work, check [the docs][logs]. ## Coverage Coverage gives a _hint_ of the actually covered source lines by tests and test applications. -The state of the art is currently [tarpaulin][tarpaulin] which unfortunately yields a lot of false negatives. Lines that +The state of the art is currently tarpaulin which unfortunately yields a lot of false negatives. Lines that are in fact covered, marked as uncovered due to a mere linebreak in a statement can cause these artifacts. This leads to lower coverage percentages than there actually is. @@ -102,7 +147,7 @@ Fuzzing is an approach to verify correctness against arbitrary or partially stru Currently implemented fuzzing targets: -* `erasure-coding` +- `erasure-coding` The tooling of choice here is `honggfuzz-rs` as it allows _fastest_ coverage according to "some paper" which is a positive feature when run as part of PRs. @@ -113,16 +158,16 @@ hence simply not feasible due to the amount of state that is required. Other candidates to implement fuzzing are: -* `rpc` -* ... +- `rpc` +- ... ## Performance metrics There are various ways of performance metrics. -* timing with `criterion` -* cache hits/misses w/ `iai` harness or `criterion-perf` -* `coz` a performance based compiler +- timing with `criterion` +- cache hits/misses w/ `iai` harness or `criterion-perf` +- `coz` a performance based compiler Most of them are standard tools to aid in the creation of statistical tests regarding change in time of certain unit tests. @@ -140,10 +185,10 @@ pursued at the current time. Requirements: -* spawn nodes with preconfigured behaviors -* allow multiple types of configuration to be specified -* allow extendability via external crates -* ... +- spawn nodes with preconfigured behaviors +- allow multiple types of configuration to be specified +- allow extendability via external crates +- ... --- @@ -251,5 +296,7 @@ behavior_testcase!{ } ``` +[zombienet]: https://github.com/paritytech/zombienet [Gurke]: https://github.com/paritytech/gurke [simnet]: https://github.com/paritytech/simnet_scripts +[logs]: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/node/gum/src/lib.rs diff --git a/polkadot/node/core/pvf/README.md b/polkadot/node/core/pvf/README.md new file mode 100644 index 0000000000000000000000000000000000000000..796e17c05faa47ceec455125ae29f7943ffa5740 --- /dev/null +++ b/polkadot/node/core/pvf/README.md @@ -0,0 +1,47 @@ +# PVF Host + +This is the PVF host, responsible for responding to requests from Candidate +Validation and spawning worker tasks to fulfill those requests. + +See also: + +- for more information: [the Implementer's Guide][impl-guide] +- for an explanation of terminology: [the Glossary][glossary] + +## Running basic tests + +Running `cargo test` in the `pvf/` directory will run unit and integration +tests. + +**Note:** some tests run only under Linux, amd64, and/or with the +`ci-only-tests` feature enabled. + +See the general [Testing][testing] instructions for more information on +**running tests** and **observing logs**. + +## Running a test-network with zombienet + +Since this crate is consensus-critical, for major changes it is highly +recommended to run a test-network. See the "Behavior tests" section of the +[Testing][testing] docs for full instructions. + +To run the PVF-specific zombienet test: + +```sh +RUST_LOG=parachain::pvf=trace zombienet --provider=native spawn zombienet_tests/functional/0001-parachains-pvf.toml +``` + +## Testing on Linux + +Some of the PVF functionality, especially related to security, is Linux-only, +and some is amd64-only. If you touch anything security-related, make sure to +test on Linux amd64! If you're on a Mac, you can either run a VM or you can hire +a VPS and use the open-source tool [EternalTerminal][et] to connect to it.[^et] + +[^et]: Unlike ssh, ET preserves your session across disconnects, and unlike +another popular persistent shell, mosh, it allows scrollback. + +[impl-guide]: https://paritytech.github.io/polkadot-sdk/book/pvf-prechecking.html#summary +[glossary]: https://paritytech.github.io/polkadot-sdk/book/glossary.html +[testing]: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/doc/testing.md +[et]: https://github.com/MisterTea/EternalTerminal diff --git a/polkadot/node/core/pvf/common/src/executor_intf.rs b/polkadot/node/core/pvf/common/src/executor_interface.rs similarity index 98% rename from polkadot/node/core/pvf/common/src/executor_intf.rs rename to polkadot/node/core/pvf/common/src/executor_interface.rs index 4bafc3f4291a60a1fd30750176035493c0c4eb0c..e634940dbe65458d08d143978c9e45fd087f2e32 100644 --- a/polkadot/node/core/pvf/common/src/executor_intf.rs +++ b/polkadot/node/core/pvf/common/src/executor_interface.rs @@ -175,11 +175,9 @@ pub fn params_to_wasmtime_semantics(par: &ExecutorParams) -> Semantics { /// Runs the prevalidation on the given code. Returns a [`RuntimeBlob`] if it succeeds. pub fn prevalidate(code: &[u8]) -> Result<RuntimeBlob, sc_executor_common::error::WasmError> { + // Construct the runtime blob and do some basic checks for consistency. let blob = RuntimeBlob::new(code)?; - // It's assumed this function will take care of any prevalidation logic - // that needs to be done. - // - // Do nothing for now. + // In the future this function should take care of any further prevalidation logic. Ok(blob) } diff --git a/polkadot/node/core/pvf/common/src/lib.rs b/polkadot/node/core/pvf/common/src/lib.rs index c041c60aaf9c766cc5cfbc5c84d88091e0dda5a4..dced43ef21340379d9ea4fc8d766d6e986c242a7 100644 --- a/polkadot/node/core/pvf/common/src/lib.rs +++ b/polkadot/node/core/pvf/common/src/lib.rs @@ -18,7 +18,7 @@ pub mod error; pub mod execute; -pub mod executor_intf; +pub mod executor_interface; pub mod prepare; pub mod pvf; pub mod worker; diff --git a/polkadot/node/core/pvf/execute-worker/src/lib.rs b/polkadot/node/core/pvf/execute-worker/src/lib.rs index d82a8fca65d34c73c085b2ad2e2b87eb91fa3d47..8c985fc7996ea28417fff0776625232dd40c15cb 100644 --- a/polkadot/node/core/pvf/execute-worker/src/lib.rs +++ b/polkadot/node/core/pvf/execute-worker/src/lib.rs @@ -17,7 +17,7 @@ //! Contains the logic for executing PVFs. Used by the polkadot-execute-worker binary. pub use polkadot_node_core_pvf_common::{ - executor_intf::execute_artifact, worker_dir, SecurityStatus, + executor_interface::execute_artifact, worker_dir, SecurityStatus, }; // NOTE: Initializing logging in e.g. tests will not have an effect in the workers, as they are @@ -236,7 +236,7 @@ fn validate_using_artifact( let descriptor_bytes = match unsafe { // SAFETY: this should be safe since the compiled artifact passed here comes from the // file created by the prepare workers. These files are obtained by calling - // [`executor_intf::prepare`]. + // [`executor_interface::prepare`]. execute_artifact(compiled_artifact_blob, executor_params, params) } { Err(err) => return JobResponse::format_invalid("execute", &err), diff --git a/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs b/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs index ba2568cd80cc634c16e43065a9ffd1fd31137e6c..d531c90b64b578e31f42a13f2399b4343469fa6d 100644 --- a/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs +++ b/polkadot/node/core/pvf/prepare-worker/benches/prepare_rococo_runtime.rs @@ -16,7 +16,7 @@ use criterion::{criterion_group, criterion_main, Criterion, SamplingMode}; use polkadot_node_core_pvf_common::{ - executor_intf::{prepare, prevalidate}, + executor_interface::{prepare, prevalidate}, prepare::PrepareJobKind, pvf::PvfPrepData, }; diff --git a/polkadot/node/core/pvf/prepare-worker/src/lib.rs b/polkadot/node/core/pvf/prepare-worker/src/lib.rs index 499e87c6e207998f9fcf583f37b243ccea1a7921..b3f8a6b11ba43b5e9e9f27679f2a35464f7857ec 100644 --- a/polkadot/node/core/pvf/prepare-worker/src/lib.rs +++ b/polkadot/node/core/pvf/prepare-worker/src/lib.rs @@ -18,7 +18,7 @@ mod memory_stats; -use polkadot_node_core_pvf_common::executor_intf::{prepare, prevalidate}; +use polkadot_node_core_pvf_common::executor_interface::{prepare, prevalidate}; // NOTE: Initializing logging in e.g. tests will not have an effect in the workers, as they are // separate spawned processes. Run with e.g. `RUST_LOG=parachain::pvf-prepare-worker=trace`. @@ -41,7 +41,7 @@ use os_pipe::{self, PipeReader, PipeWriter}; use parity_scale_codec::{Decode, Encode}; use polkadot_node_core_pvf_common::{ error::{PrepareError, PrepareWorkerResult}, - executor_intf::create_runtime_from_artifact_bytes, + executor_interface::create_runtime_from_artifact_bytes, framed_recv_blocking, framed_send_blocking, prepare::{MemoryStats, PrepareJobKind, PrepareStats, PrepareWorkerSuccess}, pvf::PvfPrepData, diff --git a/polkadot/node/core/pvf/src/execute/mod.rs b/polkadot/node/core/pvf/src/execute/mod.rs index 669b9dc04d7c518d06791536462b2c8bdbcd91ea..c6d9cf90fa289601f89b0aad307483a002f89427 100644 --- a/polkadot/node/core/pvf/src/execute/mod.rs +++ b/polkadot/node/core/pvf/src/execute/mod.rs @@ -21,6 +21,6 @@ //! `polkadot_node_core_pvf_worker::execute_worker_entrypoint`. mod queue; -mod worker_intf; +mod worker_interface; pub use queue::{start, PendingExecutionRequest, ToQueue}; diff --git a/polkadot/node/core/pvf/src/execute/queue.rs b/polkadot/node/core/pvf/src/execute/queue.rs index a0c24fd44323037ebd1572a30767be349a26a7a7..be607fe1c20b06659765776e90b29bf939f4aed7 100644 --- a/polkadot/node/core/pvf/src/execute/queue.rs +++ b/polkadot/node/core/pvf/src/execute/queue.rs @@ -16,12 +16,12 @@ //! A queue that handles requests for PVF execution. -use super::worker_intf::Outcome; +use super::worker_interface::Outcome; use crate::{ artifacts::{ArtifactId, ArtifactPathId}, host::ResultSender, metrics::Metrics, - worker_intf::{IdleWorker, WorkerHandle}, + worker_interface::{IdleWorker, WorkerHandle}, InvalidCandidate, PossiblyInvalidError, ValidationError, LOG_TARGET, }; use futures::{ @@ -448,7 +448,7 @@ async fn spawn_worker_task( use futures_timer::Delay; loop { - match super::worker_intf::spawn( + match super::worker_interface::spawn( &program_path, &cache_path, job.executor_params.clone(), @@ -500,7 +500,7 @@ fn assign(queue: &mut Queue, worker: Worker, job: ExecuteJob) { queue.mux.push( async move { let _timer = execution_timer; - let outcome = super::worker_intf::start_work( + let outcome = super::worker_interface::start_work( idle, job.artifact.clone(), job.exec_timeout, diff --git a/polkadot/node/core/pvf/src/execute/worker_intf.rs b/polkadot/node/core/pvf/src/execute/worker_interface.rs similarity index 99% rename from polkadot/node/core/pvf/src/execute/worker_intf.rs rename to polkadot/node/core/pvf/src/execute/worker_interface.rs index 16a7c290b5289ee4c9500cb0d9061b2e58a1ab84..7864ce548c24da58ca6ffc3ce397abc8a6ce96f6 100644 --- a/polkadot/node/core/pvf/src/execute/worker_intf.rs +++ b/polkadot/node/core/pvf/src/execute/worker_interface.rs @@ -18,7 +18,7 @@ use crate::{ artifacts::ArtifactPathId, - worker_intf::{ + worker_interface::{ clear_worker_dir_path, framed_recv, framed_send, spawn_with_program_path, IdleWorker, SpawnErr, WorkerDir, WorkerHandle, JOB_TIMEOUT_WALL_CLOCK_FACTOR, }, diff --git a/polkadot/node/core/pvf/src/lib.rs b/polkadot/node/core/pvf/src/lib.rs index 3306a2461c155cad4384a73a7993712b00b3a1d7..79391630b2d324e40f4f42bd802997392cb69b9d 100644 --- a/polkadot/node/core/pvf/src/lib.rs +++ b/polkadot/node/core/pvf/src/lib.rs @@ -98,7 +98,7 @@ mod metrics; mod prepare; mod priority; mod security; -mod worker_intf; +mod worker_interface; #[cfg(feature = "test-utils")] pub mod testing; @@ -107,7 +107,7 @@ pub use error::{InvalidCandidate, PossiblyInvalidError, ValidationError}; pub use host::{start, Config, ValidationHost, EXECUTE_BINARY_NAME, PREPARE_BINARY_NAME}; pub use metrics::Metrics; pub use priority::Priority; -pub use worker_intf::{framed_recv, framed_send, JOB_TIMEOUT_WALL_CLOCK_FACTOR}; +pub use worker_interface::{framed_recv, framed_send, JOB_TIMEOUT_WALL_CLOCK_FACTOR}; // Re-export some common types. pub use polkadot_node_core_pvf_common::{ diff --git a/polkadot/node/core/pvf/src/prepare/mod.rs b/polkadot/node/core/pvf/src/prepare/mod.rs index 580f67f73fa0c126994395ce5a2129234fca51a3..eb88070c2bab253092c9f8a37637702546a6b686 100644 --- a/polkadot/node/core/pvf/src/prepare/mod.rs +++ b/polkadot/node/core/pvf/src/prepare/mod.rs @@ -24,7 +24,7 @@ mod pool; mod queue; -mod worker_intf; +mod worker_interface; pub use pool::start as start_pool; pub use queue::{start as start_queue, FromQueue, ToQueue}; diff --git a/polkadot/node/core/pvf/src/prepare/pool.rs b/polkadot/node/core/pvf/src/prepare/pool.rs index 4901be9fe1b7d8a41e6390f4b9b9eaa4fff398ae..4e11f977c9e7d82527111cc9b751e15d1dc6c7a8 100644 --- a/polkadot/node/core/pvf/src/prepare/pool.rs +++ b/polkadot/node/core/pvf/src/prepare/pool.rs @@ -14,10 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see <http://www.gnu.org/licenses/>. -use super::worker_intf::{self, Outcome}; +use super::worker_interface::{self, Outcome}; use crate::{ metrics::Metrics, - worker_intf::{IdleWorker, WorkerHandle}, + worker_interface::{IdleWorker, WorkerHandle}, LOG_TARGET, }; use always_assert::never; @@ -278,7 +278,7 @@ async fn spawn_worker_task( use futures_timer::Delay; loop { - match worker_intf::spawn( + match worker_interface::spawn( &program_path, &cache_path, spawn_timeout, @@ -306,7 +306,7 @@ async fn start_work_task<Timer>( cache_path: PathBuf, _preparation_timer: Option<Timer>, ) -> PoolEvent { - let outcome = worker_intf::start_work(&metrics, idle, pvf, cache_path).await; + let outcome = worker_interface::start_work(&metrics, idle, pvf, cache_path).await; PoolEvent::StartWork(worker, outcome) } diff --git a/polkadot/node/core/pvf/src/prepare/worker_intf.rs b/polkadot/node/core/pvf/src/prepare/worker_interface.rs similarity index 99% rename from polkadot/node/core/pvf/src/prepare/worker_intf.rs rename to polkadot/node/core/pvf/src/prepare/worker_interface.rs index a393e9baa9e53b7060ae35cc96413bae0cd4a3cc..984a87ce5c9bd745b43c60979a73c4c19c6fddb4 100644 --- a/polkadot/node/core/pvf/src/prepare/worker_intf.rs +++ b/polkadot/node/core/pvf/src/prepare/worker_interface.rs @@ -19,7 +19,7 @@ use crate::{ artifacts::ArtifactId, metrics::Metrics, - worker_intf::{ + worker_interface::{ clear_worker_dir_path, framed_recv, framed_send, spawn_with_program_path, IdleWorker, SpawnErr, WorkerDir, WorkerHandle, JOB_TIMEOUT_WALL_CLOCK_FACTOR, }, diff --git a/polkadot/node/core/pvf/src/testing.rs b/polkadot/node/core/pvf/src/testing.rs index c7c885c434211f05db9527e34f36568f20ce5ff7..60b0b4b8d3d0c49199800194570d8ef2783fc67f 100644 --- a/polkadot/node/core/pvf/src/testing.rs +++ b/polkadot/node/core/pvf/src/testing.rs @@ -18,7 +18,7 @@ pub use crate::{ host::{EXECUTE_BINARY_NAME, PREPARE_BINARY_NAME}, - worker_intf::{spawn_with_program_path, SpawnErr}, + worker_interface::{spawn_with_program_path, SpawnErr}, }; use crate::get_worker_version; @@ -36,7 +36,7 @@ pub fn validate_candidate( code: &[u8], params: &[u8], ) -> Result<Vec<u8>, Box<dyn std::error::Error>> { - use polkadot_node_core_pvf_common::executor_intf::{prepare, prevalidate}; + use polkadot_node_core_pvf_common::executor_interface::{prepare, prevalidate}; use polkadot_node_core_pvf_execute_worker::execute_artifact; let code = sp_maybe_compressed_blob::decompress(code, 10 * 1024 * 1024) diff --git a/polkadot/node/core/pvf/src/worker_intf.rs b/polkadot/node/core/pvf/src/worker_interface.rs similarity index 100% rename from polkadot/node/core/pvf/src/worker_intf.rs rename to polkadot/node/core/pvf/src/worker_interface.rs diff --git a/polkadot/node/core/pvf/tests/README.md b/polkadot/node/core/pvf/tests/README.md deleted file mode 100644 index 27385e190250df9baa0d14cea7afc9e00f973a98..0000000000000000000000000000000000000000 --- a/polkadot/node/core/pvf/tests/README.md +++ /dev/null @@ -1,9 +0,0 @@ -# PVF host integration tests - -## Testing - -Before running these tests, make sure the worker binaries are built first. This can be done with: - -```sh -cargo build --bin polkadot-execute-worker --bin polkadot-prepare-worker -``` diff --git a/polkadot/roadmap/implementers-guide/src/glossary.md b/polkadot/roadmap/implementers-guide/src/glossary.md index b2365ba51c5ce80fd0d60b53c592b1037f540a52..ac2392b14d2ae1a75a6690567f32407ebc3a1637 100644 --- a/polkadot/roadmap/implementers-guide/src/glossary.md +++ b/polkadot/roadmap/implementers-guide/src/glossary.md @@ -48,10 +48,13 @@ has exactly one downward message queue. - **Proof-of-Validity (PoV):** A stateless-client proof that a parachain candidate is valid, with respect to some validation function. - **PVF:** Parachain Validation Function. The validation code that is run by validators on parachains. -- **PVF Prechecking:** This is the process of initially checking the PVF when it is first added. We attempt preparation - of the PVF and make sure it succeeds within a given timeout, plus some additional checks. +- **PVF Prechecking:** This is the process of checking a PVF when it appears + on-chain, either when the parachain is onboarded or when it signalled an + upgrade of its validation code. We attempt preparation of the PVF and make + sure it that succeeds within a given timeout, plus some additional checks. - **PVF Preparation:** This is the process of preparing the WASM blob and includes both prevalidation and compilation. - As there is no prevalidation right now, preparation just consists of compilation. +- **PVF Prevalidation:** Some basic checks for correctness of the PVF blob. The + first step of PVF preparation, before compilation. - **Relay Parent:** A block in the relay chain, referred to in a context where work is being done in the context of the state at this block. - **Runtime:** The relay-chain state machine.