From a42fdf19197523627ac56a218678a51b29547d3e Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Mon, 18 Sep 2023 14:18:38 +0200 Subject: [PATCH] ROLLUP-286: add code coverage (#94) * chore: ignore temp files used to generate code coverage report * chore: add pull request description template * ci:remove prover test because already included in server-rust * test: allow the possibility to measure unit test code coverage * doc: describe how to measure code coverage and generate reports * ci: rename workflows * style: zk fmt * style: fix markdown style issue * style: zk fmt * ci: remove unused command * docs: include required tools to generate code coverage * ci: add code coverage tools to zk image * ci: rename docker workflow * ci: measure unit test code coverage * refactor: avoid code duplication * ci: fix yaml syntax * ci: replace code coverage action * ci: create intermediate directories for coverage report * ci: debug prover tests * chore: debug lint issues * build: add cargo.lock * Revert "build: add cargo.lock" This reverts commit 29942d644cd786947d9db027dd34208d8acf3f94. * ci: fix rust version used for lint * ci: install clippy using rustup * ci: install rustfmt with rustup * ci: debug unit test failing * chore: update tests and docs according to PR suggestions * ci: debug ci fails * ci(debug): debug failing test * test: fix failing test * build: restore Cargo.lock * build: restore Cargo.lock * ci: enable all the tests * ci: debug coverage file size * ci: debug coverage file size * ci: debug coverage file size * ci: debug coverage file size * ci: test lcov file size * ci: test lcov file size * ci: test cobertura file size * Revert "ci: test cobertura file size" This reverts commit d59991090bc9613b68d06e6039a0f6944d4ea88b. * ci: enable all the tests * test: increate prover timeout * test: increate prover timeout * ci: enable ci to run only when the PR is ready --- .github/pull_request_template.md | 13 +++ .github/workflows/ci-iov-lint.yml | 10 +- .github/workflows/ci-iov-sim-tool.yml | 4 +- .github/workflows/ci-iov.yml | 26 ++++- .github/workflows/convert-draft.yml | 2 +- .github/workflows/docker.yml | 2 +- .gitignore | 3 + Cargo.lock | 160 +++++++++++++++++++++----- core/bin/prover/tests/tests.rs | 5 +- docker-compose-runner.yml | 3 +- docker/environment/Dockerfile | 4 + docs/development.md | 27 +++++ infrastructure/zk/src/test/test.ts | 32 ++++-- 13 files changed, 233 insertions(+), 58 deletions(-) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000000..ec9909d080 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,13 @@ + + +## What + +- (describe the changes) + +## Why + +- (the reason these changes are wanted) + +## Refs + +- (optional: include links to other issues, PRs, tickets, etc) diff --git a/.github/workflows/ci-iov-lint.yml b/.github/workflows/ci-iov-lint.yml index e801efd3bd..bc639f682c 100644 --- a/.github/workflows/ci-iov-lint.yml +++ b/.github/workflows/ci-iov-lint.yml @@ -1,4 +1,4 @@ -name: rif-rollup CI lint +name: Lint on: [push] jobs: @@ -14,13 +14,17 @@ jobs: node-version: "16.x" - name: Use rust - run: rustup install 1.69.0 + run: | + rustup install 1.69.0 + rustup default 1.69.0 + rustup component add clippy + rustup component add rustfmt - name: Toolchain info run: | node --version rustc --version - cargo clippy --version + cargo clippy --version - name: setup-env run: | diff --git a/.github/workflows/ci-iov-sim-tool.yml b/.github/workflows/ci-iov-sim-tool.yml index 7b2d85ba3a..6a2af2dfa0 100644 --- a/.github/workflows/ci-iov-sim-tool.yml +++ b/.github/workflows/ci-iov-sim-tool.yml @@ -1,4 +1,4 @@ -name: rif-rollup CI unit/integration tests +name: Simulation tool tests on: push: paths: @@ -20,9 +20,7 @@ jobs: - name: start-services run: | - which ci_run docker-compose -f docker-compose-runner.yml up --build -d zk - which ci_run - name: init run: | diff --git a/.github/workflows/ci-iov.yml b/.github/workflows/ci-iov.yml index 48f54bfeb7..607f0f48e7 100644 --- a/.github/workflows/ci-iov.yml +++ b/.github/workflows/ci-iov.yml @@ -1,5 +1,5 @@ -name: rif-rollup CI unit/integration tests -on: +name: Unit/integration tests +on: pull_request: types: [ready_for_review] @@ -47,12 +47,15 @@ jobs: - name: restart dev-ticker run: docker-compose -f docker-compose-runner.yml restart dev-ticker + - name: Toolchain info + run: | + node --version + ci_run rustc --version + ci_run cargo clippy --version + - name: contracts-unit-tests run: ci_run zk test contracts - - name: prover-unit-tests - run: ci_run zk test prover - - name: witness-generator-unit-tests run: ci_run zk test witness-generator @@ -107,4 +110,15 @@ jobs: run: | ci_run cat server.log ci_run cat api.log - ci_run cat dummy_prover.log \ No newline at end of file + ci_run cat dummy_prover.log + ci_run mkdir -p ./target/release/coverage/ + ci_run "grcov . --binary-path ./target/release/deps/ -s . -t lcov --branch --ignore-not-existing --ignore '../*' --ignore '/*' -o ./target/release/coverage/lcov.info" + docker compose -f docker-compose-runner.yml cp zk:/usr/src/zksync/target/release/coverage/lcov.info lcov.info + du lcov.info + + - uses: codecov/codecov-action@v3 + with: + files: ./lcov.info + flags: unit-tests + name: codecov-umbrella # optional + verbose: true # optional (default = false) diff --git a/.github/workflows/convert-draft.yml b/.github/workflows/convert-draft.yml index 7e7da006f8..ab73f2c092 100644 --- a/.github/workflows/convert-draft.yml +++ b/.github/workflows/convert-draft.yml @@ -1,4 +1,4 @@ -name: rif-rollup Mark as draft +name: Mark as draft on: pull_request_review: types: [submitted] diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 82d5b634cc..de2cfa2ab8 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -1,4 +1,4 @@ -name: publish server docker image +name: Publish docker images on: push: diff --git a/.gitignore b/.gitignore index 9a316c0b74..60f816df8c 100644 --- a/.gitignore +++ b/.gitignore @@ -60,3 +60,6 @@ flamegraph.svg bin/mkcert docker/rskj/data + +# Files used to generate code-coverage reports +*.profraw diff --git a/Cargo.lock b/Cargo.lock index 87eb2656ef..c515e67d3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -822,10 +822,10 @@ name = "block_revert" version = "1.0.0" dependencies = [ "anyhow", - "ethabi", + "ethabi 16.0.0", "structopt", "tokio", - "web3", + "web3 0.18.0", "zksync_config", "zksync_eth_client", "zksync_storage", @@ -898,6 +898,12 @@ dependencies = [ "iovec", ] +[[package]] +name = "bytes" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e4cec68f03f32e44924783795810fa50a7035d8c8ebe78580ad7e6c703fba38" + [[package]] name = "bytes" version = "1.1.0" @@ -1498,6 +1504,22 @@ dependencies = [ "serde", ] +[[package]] +name = "ethabi" +version = "14.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a01317735d563b3bad2d5f90d2e1799f414165408251abb762510f40e790e69a" +dependencies = [ + "anyhow", + "ethereum-types 0.11.0", + "hex", + "serde", + "serde_json", + "sha3", + "thiserror", + "uint", +] + [[package]] name = "ethabi" version = "16.0.0" @@ -4000,10 +4022,10 @@ name = "remove_proofs" version = "1.0.0" dependencies = [ "anyhow", - "ethabi", + "ethabi 16.0.0", "structopt", "tokio", - "web3", + "web3 0.18.0", "zksync_config", "zksync_eth_client", "zksync_storage", @@ -4060,6 +4082,33 @@ dependencies = [ "sha3", ] +[[package]] +name = "rif_rollup_wallet_generator" +version = "1.0.0" +dependencies = [ + "anyhow", + "async-trait", + "ethabi 14.1.0", + "hex", + "jsonrpc-core 17.1.0", + "num", + "reqwest", + "serde", + "serde_json", + "sha2 0.8.2", + "thiserror", + "tokio", + "web3 0.16.0", + "zksync", + "zksync_config", + "zksync_crypto", + "zksync_eth_client", + "zksync_eth_signer", + "zksync_test_account", + "zksync_types", + "zksync_utils", +] + [[package]] name = "ring" version = "0.16.20" @@ -4615,6 +4664,21 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "soketto" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5c71ed3d54db0a699f4948e1bb3e45b450fa31fe602621dee6680361d569c88" +dependencies = [ + "base64 0.12.3", + "bytes 0.5.6", + "futures 0.3.17", + "httparse", + "log 0.4.14", + "rand 0.7.3", + "sha-1 0.9.8", +] + [[package]] name = "soketto" version = "0.7.1" @@ -5618,6 +5682,40 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "web3" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc4c18ae15621f764fab919f7e4a83d87163494cbc3460884debef7c6bc1bc6b" +dependencies = [ + "arrayvec 0.5.2", + "base64 0.13.0", + "bytes 1.1.0", + "derive_more", + "ethabi 14.1.0", + "ethereum-types 0.11.0", + "futures 0.3.17", + "futures-timer", + "headers", + "hex", + "jsonrpc-core 17.1.0", + "log 0.4.14", + "parking_lot 0.11.2", + "pin-project", + "reqwest", + "rlp", + "secp256k1 0.20.3", + "serde", + "serde_json", + "soketto 0.4.2", + "tiny-keccak 2.0.2", + "tokio", + "tokio-stream", + "tokio-util", + "url 2.2.2", + "web3-async-native-tls", +] + [[package]] name = "web3" version = "0.18.0" @@ -5628,7 +5726,7 @@ dependencies = [ "base64 0.13.0", "bytes 1.1.0", "derive_more", - "ethabi", + "ethabi 16.0.0", "ethereum-types 0.12.1", "futures 0.3.17", "futures-timer", @@ -5645,7 +5743,7 @@ dependencies = [ "secp256k1 0.21.3", "serde", "serde_json", - "soketto", + "soketto 0.7.1", "tiny-keccak 2.0.2", "tokio", "tokio-stream", @@ -5840,7 +5938,7 @@ version = "0.3.0" dependencies = [ "anyhow", "async-trait", - "ethabi", + "ethabi 16.0.0", "hex", "jsonrpc-core 17.1.0", "num", @@ -5850,7 +5948,7 @@ dependencies = [ "sha2 0.8.2", "thiserror", "tokio", - "web3", + "web3 0.18.0", "zksync_config", "zksync_crypto", "zksync_eth_client", @@ -5874,7 +5972,7 @@ dependencies = [ "chrono", "criterion", "ctrlc", - "ethabi", + "ethabi 16.0.0", "futures 0.3.17", "hex", "hyper 0.14.14", @@ -5903,7 +6001,7 @@ dependencies = [ "tiny-keccak 1.5.0", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_api_client", "zksync_api_types", "zksync_balancer", @@ -5971,7 +6069,7 @@ name = "zksync_basic_types" version = "1.0.0" dependencies = [ "serde", - "web3", + "web3 0.18.0", ] [[package]] @@ -6010,7 +6108,7 @@ dependencies = [ name = "zksync_contracts" version = "1.0.0" dependencies = [ - "ethabi", + "ethabi 16.0.0", "serde_json", ] @@ -6024,7 +6122,7 @@ dependencies = [ "async-trait", "chrono", "ctrlc", - "ethabi", + "ethabi 16.0.0", "futures 0.3.17", "itertools 0.9.0", "metrics", @@ -6036,7 +6134,7 @@ dependencies = [ "tiny-keccak 1.5.0", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_api_types", "zksync_balancer", "zksync_config", @@ -6061,7 +6159,7 @@ version = "1.0.0" dependencies = [ "base64 0.13.0", "bincode", - "ethabi", + "ethabi 16.0.0", "fnv", "franklin-crypto", "hex", @@ -6085,7 +6183,7 @@ dependencies = [ "async-trait", "chrono", "db_test_macro", - "ethabi", + "ethabi 16.0.0", "futures 0.3.17", "hex", "jsonrpc-core 18.0.0", @@ -6096,7 +6194,7 @@ dependencies = [ "tiny-keccak 1.5.0", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_config", "zksync_contracts", "zksync_crypto", @@ -6111,7 +6209,7 @@ name = "zksync_eth_client" version = "1.0.0" dependencies = [ "anyhow", - "ethabi", + "ethabi 16.0.0", "hex", "metrics", "parity-crypto 0.8.0", @@ -6119,7 +6217,7 @@ dependencies = [ "sha3", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_config", "zksync_contracts", "zksync_eth_signer", @@ -6134,7 +6232,7 @@ dependencies = [ "async-trait", "chrono", "ctrlc", - "ethabi", + "ethabi 16.0.0", "futures 0.3.17", "hex", "lazy_static", @@ -6144,7 +6242,7 @@ dependencies = [ "serde_json", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_basic_types", "zksync_config", "zksync_contracts", @@ -6175,7 +6273,7 @@ dependencies = [ "serde_json", "thiserror", "tokio", - "web3", + "web3 0.18.0", "zksync_types", ] @@ -6204,7 +6302,7 @@ dependencies = [ "anyhow", "async-trait", "chrono", - "ethabi", + "ethabi 16.0.0", "futures 0.3.17", "hex", "log 0.4.14", @@ -6212,7 +6310,7 @@ dependencies = [ "num", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_api", "zksync_config", "zksync_contracts", @@ -6235,7 +6333,7 @@ dependencies = [ "tokio", "tokio-stream", "vlog", - "web3", + "web3 0.18.0", "zksync_config", "zksync_eth_client", "zksync_utils", @@ -6316,7 +6414,7 @@ dependencies = [ "structopt", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_circuit", "zksync_config", "zksync_crypto", @@ -6389,7 +6487,7 @@ dependencies = [ "serde_json", "thiserror", "vlog", - "web3", + "web3 0.18.0", "zksync_crypto", "zksync_types", "zksync_utils", @@ -6441,7 +6539,7 @@ name = "zksync_testkit" version = "1.0.0" dependencies = [ "anyhow", - "ethabi", + "ethabi 16.0.0", "futures 0.3.17", "itertools 0.9.0", "num", @@ -6450,7 +6548,7 @@ dependencies = [ "structopt", "tokio", "vlog", - "web3", + "web3 0.18.0", "zksync_circuit", "zksync_config", "zksync_contracts", @@ -6485,7 +6583,7 @@ dependencies = [ "bigdecimal", "chrono", "criterion", - "ethabi", + "ethabi 16.0.0", "hex", "itertools 0.9.0", "num", @@ -6498,7 +6596,7 @@ dependencies = [ "thiserror", "tiny-keccak 1.5.0", "vlog", - "web3", + "web3 0.18.0", "zksync_basic_types", "zksync_crypto", "zksync_utils", diff --git a/core/bin/prover/tests/tests.rs b/core/bin/prover/tests/tests.rs index 70a586830f..db49a26577 100644 --- a/core/bin/prover/tests/tests.rs +++ b/core/bin/prover/tests/tests.rs @@ -204,7 +204,8 @@ async fn test_receiving_heartbeats() { &prover_name, ) .fuse(); - let timeout = tokio::time::sleep(Duration::from_secs(10)).fuse(); + // We need to set an higher timeout value to avoid the test to fail + let timeout = tokio::time::sleep(Duration::from_secs(30)).fuse(); pin_mut!(prover_work_cycle, timeout); @@ -241,7 +242,7 @@ async fn test_publishing_proof() { &prover_name, ) .fuse(); - let timeout = tokio::time::sleep(Duration::from_secs(10)).fuse(); + let timeout = tokio::time::sleep(Duration::from_secs(30)).fuse(); pin_mut!(prover_work_cycle, timeout); diff --git a/docker-compose-runner.yml b/docker-compose-runner.yml index 540d7d0974..d42a9c6e80 100644 --- a/docker-compose-runner.yml +++ b/docker-compose-runner.yml @@ -24,7 +24,7 @@ services: - ./etc/tokens/:/etc/tokens zk: - image: "rsksmart/rollup-environment:1.0.0-beta" + image: "rsksmart/rollup-environment:1.1.0-beta" depends_on: - postgres - rskj @@ -42,3 +42,4 @@ services: - CI=1 - CAROOT=/usr/src/zksync - NODE_EXTRA_CA_CERTS=/usr/src/zksync/rootCA.pem + - CODE_COVERAGE=true diff --git a/docker/environment/Dockerfile b/docker/environment/Dockerfile index 4ab78fa440..9951808b02 100644 --- a/docker/environment/Dockerfile +++ b/docker/environment/Dockerfile @@ -36,6 +36,10 @@ RUN chmod +x solc-static-linux RUN mv solc-static-linux /usr/local/bin/solc RUN apt-get install -y axel postgresql +# Install tools used to measure code coverage +RUN cargo install grcov +RUN rustup component add llvm-tools-preview + # Setup the environment ENV ZKSYNC_HOME=/usr/src/zksync ENV PATH="${ZKSYNC_HOME}/bin:${PATH}" diff --git a/docs/development.md b/docs/development.md index 0f17abe306..6bc7ffd5be 100644 --- a/docs/development.md +++ b/docs/development.md @@ -181,6 +181,33 @@ zk test db **Note**. If you have compilation issues with `sqlx`, then make sure to run `zk up` before running the tests. Also, if you see some tests fail, you might need to call `zk db reset` and restart the tests. +### Code coverage + +**Note**. Code coverage measurement requires `grcov` and `llvm-tools-preview` to be installed. + +```bash +cargo install grcov +rustup component add llvm-tools-preview +``` + +To measure code coverage of unit tests, just set the environment variable `CODE_COVERAGE` to `true` or `1`. + +```bash +CODE_COVERAGE=true zk test prover +``` + +Code coverage is measured using the following tests: + +- `zk test witness-generator` +- `zk test server-rust` +- `zk test crypto-rust --no-circuit` + +Reports can be generated using [grcov](https://github.com/mozilla/grcov): + +```bash +grcov . --binary-path ./target/release/deps/ -s . -t html --branch --ignore-not-existing --ignore '../*' --ignore "/*" -o target/release/coverage/html +``` + ## Developing circuit - To generate proofs one must have the universal setup files (which are downloaded during the first initialization). diff --git a/infrastructure/zk/src/test/test.ts b/infrastructure/zk/src/test/test.ts index 9a118e7a47..59accede93 100644 --- a/infrastructure/zk/src/test/test.ts +++ b/infrastructure/zk/src/test/test.ts @@ -4,6 +4,10 @@ import * as utils from '../utils'; import * as integration from './integration'; export { integration }; +const CODE_COVERAGE_FLAGS = `CARGO_INCREMENTAL=0 RUSTFLAGS='-C instrument-coverage' LLVM_PROFILE_FILE='cargo-test-%p-%m.profraw'`; +const IS_CODE_COVERAGE_ENABLED = process.env.CODE_COVERAGE === 'true' || process.env.CODE_COVERAGE === '1'; +const CARGO_FLAGS = IS_CODE_COVERAGE_ENABLED ? CODE_COVERAGE_FLAGS : ''; + async function runOnTestDb(reset: boolean, dir: string, command: string) { const databaseUrl = process.env.DATABASE_URL as string; process.env.DATABASE_URL = databaseUrl.replace(/plasma/g, 'plasma_test'); @@ -26,7 +30,7 @@ export async function db(reset: boolean, ...args: string[]) { await runOnTestDb( reset, 'core/lib/storage', - `cargo test --release -p zksync_storage --lib -- --ignored --nocapture --test-threads=1 + `${CARGO_FLAGS} cargo test --release -p zksync_storage --lib -- --ignored --nocapture --test-threads=1 ${args.join(' ')}` ); } @@ -37,7 +41,7 @@ export async function rustApi(reset: boolean, ...args: string[]) { await runOnTestDb( reset, 'core/bin/zksync_api', - `cargo test --release -p zksync_api --lib -- --ignored --nocapture --test-threads=1 api_server + `${CARGO_FLAGS} cargo test --release -p zksync_api --lib -- --ignored --nocapture --test-threads=1 api_server ${args.join(' ')}` ); } @@ -48,17 +52,17 @@ export async function contracts() { export async function circuit(threads: number = 1, testName?: string, ...args: string[]) { await utils.spawn( - `cargo test --no-fail-fast --release -p zksync_circuit ${testName || ''} + `${CARGO_FLAGS} cargo test --no-fail-fast --release -p zksync_circuit ${testName || ''} -- --ignored --test-threads ${threads} ${args.join(' ')}` ); } export async function prover() { - await utils.spawn('cargo test -p zksync_prover --release'); + await utils.spawn(`${CARGO_FLAGS} cargo test -p zksync_prover --release`); } export async function witness_generator() { - await utils.spawn('cargo test -p zksync_witness_generator --release'); + await utils.spawn(`${CARGO_FLAGS} cargo test -p zksync_witness_generator --release`); } export async function js() { @@ -67,19 +71,21 @@ export async function js() { async function rustCryptoTests() { process.chdir('sdk/zksync-crypto'); - await utils.spawn('cargo test --release'); + await utils.spawn(`${CARGO_FLAGS} cargo test --release`); process.chdir(process.env.ZKSYNC_HOME as string); } export async function serverRust() { - await utils.spawn('cargo test --release'); + await utils.spawn(`${CARGO_FLAGS} cargo test --release`); await db(true); await rustApi(true); await prover(); } -export async function cryptoRust() { - await circuit(25); +export async function cryptoRust(runCircuit = true) { + if (runCircuit) { + await circuit(25); + } await rustCryptoTests(); } @@ -101,7 +107,13 @@ command.command('witness-generator').description('run unit-tests for the witness command.command('contracts').description('run unit-tests for the contracts').action(contracts); command.command('rust').description('run unit-tests for all rust binaries and libraries').action(rust); command.command('server-rust').description('run unit-tests for server binaries and libraries').action(serverRust); -command.command('crypto-rust').description('run unit-tests for rust crypto binaries and libraries').action(cryptoRust); +command + .command('crypto-rust') + .description('run unit-tests for rust crypto binaries and libraries') + .option('--no-circuit', 'do not run the circuit tests') + .action(async (cmd: Command) => { + await cryptoRust(cmd.circuit); + }); command .command('wallet-generator') .description('run unit-tests for the wallet-generator library')