From 7105a8f09443153e97be55de126e3e5c9bc67d65 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Mon, 14 Sep 2026 14:55:56 +0200 Subject: [PATCH 1/4] fix(ci): remove the three sources of flakiness in the weekly example runs - evm_block_explorer: query a post-Merge block. PublicNode routes across regional pools and some only retain history from block ~15500000, so the block 1 assertions failed with `pruned history unavailable` depending on which pool served the outcall. - unit_testable_rust_canister: prepare each test WASM once per test binary. Two tests downloaded the NNS governance WASM concurrently to the same path, and `fs::write` truncating under the peer's `fs::read` produced a partial gzip that PocketIC rejected as `CanisterInvalidWasm`. curl's exit status was also unchecked, so a failed transfer was cached as a valid artifact. - _run-example.yml: warm the mops toolchain cache. Projects with more than one Motoko canister run concurrent `mops build` invocations that race on the shared moc download and extraction, failing the build with no diagnostics. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/_run-example.yml | 11 +++ motoko/evm_block_explorer/README.md | 2 + motoko/evm_block_explorer/test.sh | 15 +++- rust/evm_block_explorer/README.md | 2 + rust/evm_block_explorer/test.sh | 15 +++- .../backend/tests/integration_tests.rs | 90 ++++++++++++++----- 6 files changed, 103 insertions(+), 32 deletions(-) diff --git a/.github/workflows/_run-example.yml b/.github/workflows/_run-example.yml index 6e41c68039..e80c66e02a 100644 --- a/.github/workflows/_run-example.yml +++ b/.github/workflows/_run-example.yml @@ -60,6 +60,17 @@ jobs: uses: dfinity/pocketic@a980c334fab1b21b0b8a6bba38e1a10836e7258b # main with: pocket-ic-server-version: ${{ inputs.pocketic-version }} + # Projects with more than one Motoko canister run one `mops build` per canister + # concurrently, and those invocations race on the shared moc toolchain download and + # extraction: whichever build loses reads a half-extracted moc and fails with no + # diagnostics. Warming the cache first makes the download happen exactly once. + # Reproduces with mops 3.2.0; remove once mops locks the toolchain cache itself. + - name: Warm the mops toolchain cache + working-directory: ${{ inputs.working-directory }} + run: | + if [ -f mops.toml ] && command -v mops > /dev/null; then + mops install + fi - name: Run working-directory: ${{ inputs.working-directory }} run: ${{ inputs.run }} diff --git a/motoko/evm_block_explorer/README.md b/motoko/evm_block_explorer/README.md index 5103e1e53f..229b2a05db 100644 --- a/motoko/evm_block_explorer/README.md +++ b/motoko/evm_block_explorer/README.md @@ -83,6 +83,8 @@ Both give the same interface as long as `icp.yaml` pins the release that is live The example uses [PublicNode](https://ethereum-rpc.publicnode.com) by default — a free, no-registration provider that works out of the box locally and on mainnet. This is sufficient for getting started and automated testing. +PublicNode routes requests across regional node pools, and not all of them keep the full chain history — some only retain blocks from shortly before the Merge (block ~15500000). Queries for earlier blocks therefore fail with `pruned history unavailable` depending on which pool serves the outcall. `test.sh` queries a post-Merge block for that reason; an archive provider is needed to read early history reliably. + For production deployments requiring premium providers (Alchemy, Ankr, BlockPi), refer to the [EVM RPC canister documentation](https://github.com/dfinity/evm-rpc-canister) for how to configure API keys. Once configured, change `#EthMainnet(?[#PublicNode])` in `backend/EvmRpcApi.mo` to `#EthMainnet(null)` to use all configured providers for better consensus. ## Security considerations and best practices diff --git a/motoko/evm_block_explorer/test.sh b/motoko/evm_block_explorer/test.sh index f6ce319654..e1cf2bbed8 100755 --- a/motoko/evm_block_explorer/test.sh +++ b/motoko/evm_block_explorer/test.sh @@ -1,9 +1,16 @@ #!/usr/bin/env bash set -e -echo "=== Test 1: get_evm_block returns correct data for Ethereum mainnet block 1 ===" -result=$(icp canister call backend get_evm_block '(1)') +# Block 20000000 is post-Merge. PublicNode serves several regional node pools and some of +# them only retain history from shortly before the Merge (block ~15500000), so querying +# early blocks such as block 1 fails depending on which pool the outcall lands in. +BLOCK=20000000 +BLOCK_HASH=0xd24fd73f794058a3807db926d8898c6481e902b7edb91ce0d479d6760f276183 +BLOCK_MINER=0x95222290dd7278aa3ddd389cc1e1d165cc4bafe5 + +echo "=== Test 1: get_evm_block returns correct data for Ethereum mainnet block $BLOCK ===" +result=$(icp canister call backend get_evm_block "($BLOCK)") echo "$result" echo "$result" | grep -q "Ok" && echo "PASS (Ok variant)" || (echo "FAIL (expected Ok)" && exit 1) -echo "$result" | grep -q "0x88e96d4537bea4d9c05d12549907b32561d3bf31f45aae734cdc119f13406cb6" && echo "PASS (hash)" || (echo "FAIL (wrong hash)" && exit 1) -echo "$result" | grep -q "0x05a56e2d52c817161883f50c441c3228cfe54d9f" && echo "PASS (miner)" || (echo "FAIL (wrong miner)" && exit 1) +echo "$result" | grep -q "$BLOCK_HASH" && echo "PASS (hash)" || (echo "FAIL (wrong hash)" && exit 1) +echo "$result" | grep -q "$BLOCK_MINER" && echo "PASS (miner)" || (echo "FAIL (wrong miner)" && exit 1) diff --git a/rust/evm_block_explorer/README.md b/rust/evm_block_explorer/README.md index 05dc00655e..82fd466417 100644 --- a/rust/evm_block_explorer/README.md +++ b/rust/evm_block_explorer/README.md @@ -64,6 +64,8 @@ icp build backend && candid-extractor target/wasm32-unknown-unknown/release/back The example uses [PublicNode](https://ethereum-rpc.publicnode.com) by default — a free, no-registration provider that works out of the box locally and on mainnet. This is sufficient for getting started and automated testing. +PublicNode routes requests across regional node pools, and not all of them keep the full chain history — some only retain blocks from shortly before the Merge (block ~15500000). Queries for earlier blocks therefore fail with `pruned history unavailable` depending on which pool serves the outcall. `test.sh` queries a post-Merge block for that reason; an archive provider is needed to read early history reliably. + For production deployments requiring premium providers (Alchemy, Ankr, BlockPi), refer to the [EVM RPC canister documentation](https://github.com/dfinity/evm-rpc-canister) for how to configure API keys. Once configured, change `RpcServices::EthMainnet(Some(vec![EthMainnetService::PublicNode]))` in `backend/src/lib.rs` to `RpcServices::EthMainnet(None)` to use all configured providers for better consensus. ## Security considerations and best practices diff --git a/rust/evm_block_explorer/test.sh b/rust/evm_block_explorer/test.sh index f6ce319654..e1cf2bbed8 100755 --- a/rust/evm_block_explorer/test.sh +++ b/rust/evm_block_explorer/test.sh @@ -1,9 +1,16 @@ #!/usr/bin/env bash set -e -echo "=== Test 1: get_evm_block returns correct data for Ethereum mainnet block 1 ===" -result=$(icp canister call backend get_evm_block '(1)') +# Block 20000000 is post-Merge. PublicNode serves several regional node pools and some of +# them only retain history from shortly before the Merge (block ~15500000), so querying +# early blocks such as block 1 fails depending on which pool the outcall lands in. +BLOCK=20000000 +BLOCK_HASH=0xd24fd73f794058a3807db926d8898c6481e902b7edb91ce0d479d6760f276183 +BLOCK_MINER=0x95222290dd7278aa3ddd389cc1e1d165cc4bafe5 + +echo "=== Test 1: get_evm_block returns correct data for Ethereum mainnet block $BLOCK ===" +result=$(icp canister call backend get_evm_block "($BLOCK)") echo "$result" echo "$result" | grep -q "Ok" && echo "PASS (Ok variant)" || (echo "FAIL (expected Ok)" && exit 1) -echo "$result" | grep -q "0x88e96d4537bea4d9c05d12549907b32561d3bf31f45aae734cdc119f13406cb6" && echo "PASS (hash)" || (echo "FAIL (wrong hash)" && exit 1) -echo "$result" | grep -q "0x05a56e2d52c817161883f50c441c3228cfe54d9f" && echo "PASS (miner)" || (echo "FAIL (wrong miner)" && exit 1) +echo "$result" | grep -q "$BLOCK_HASH" && echo "PASS (hash)" || (echo "FAIL (wrong hash)" && exit 1) +echo "$result" | grep -q "$BLOCK_MINER" && echo "PASS (miner)" || (echo "FAIL (wrong miner)" && exit 1) diff --git a/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs b/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs index 65008918e1..6de1eb675e 100644 --- a/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs +++ b/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs @@ -4,6 +4,7 @@ use serde::Deserialize; use std::io; use std::path::{Path, PathBuf}; use std::process::Command; +use std::sync::LazyLock; use std::time::SystemTime; use walkdir::WalkDir; @@ -21,11 +22,19 @@ const IC_COMMIT_FOR_PROPOSALS: &str = "4b7cde9a0e3b5ad4725e75cbc36ce635be6fa6a8" const NNS_GOVERNANCE_CANISTER_ID: &str = "rrkah-fqaaa-aaaaa-aaaaq-cai"; const NNS_ROOT_CANISTER_ID: &str = "r7inp-6aaaa-aaaaa-aaabq-cai"; -// WASM will be loaded dynamically with smart rebuilding -fn get_backend_wasm() -> Vec { +// Tests run concurrently on separate threads but share the files below, so each artifact is +// prepared exactly once per test binary. Without this, two tests building or downloading the +// same path at the same time can read a half-written file. +static BACKEND_WASM: LazyLock> = LazyLock::new(|| { let wasm_path = ensure_wasm_built(); std::fs::read(&wasm_path) .unwrap_or_else(|e| panic!("Failed to read WASM file at {:?}: {}", wasm_path, e)) +}); + +static GOVERNANCE_WASM: LazyLock> = LazyLock::new(load_governance_wasm); + +fn get_backend_wasm() -> Vec { + BACKEND_WASM.clone() } /// Ensures the WASM is built and up-to-date, returns path to the WASM file @@ -123,45 +132,78 @@ fn rebuild_wasm() { } } +/// Download to a sibling temporary file and rename into place, so an interrupted or failed +/// transfer never leaves a truncated file behind for the next run to pick up from the cache. fn download_wasm_to(url: String, wasm_path: &Path) { - // Ensure the target directory exists if let Some(parent) = wasm_path.parent() { std::fs::create_dir_all(parent) .unwrap_or_else(|e| panic!("Failed to create directory {:?}: {}", parent, e)); } - - let wasm = std::process::Command::new("curl") - .args(["-L", "-f", &url]) + let download_path = wasm_path.with_extension("gz.partial"); + + let output = Command::new("curl") + .args([ + "--location", + "--fail", + "--silent", + "--show-error", + "--retry", + "5", + "--retry-all-errors", + "--retry-delay", + "2", + "--output", + &download_path.to_string_lossy(), + &url, + ]) .output() - .expect("Failed to download NNS Governance WASM") - .stdout; + .expect("Failed to run curl"); + + if !output.status.success() { + let _ = std::fs::remove_file(&download_path); + panic!( + "Failed to download {url}: curl exited with {:?}: {}", + output.status.code(), + String::from_utf8_lossy(&output.stderr) + ); + } - std::fs::write(wasm_path, wasm).expect("Failed to write compressed WASM"); + std::fs::rename(&download_path, wasm_path).expect("Failed to move downloaded WASM into place"); } -/// Get the NNS Governance WASM binary, downloading if necessary -fn get_governance_wasm() -> Vec { +/// Reads the file only if it is a gzip archive. A missing prefix means a partial or failed +/// download, which would otherwise surface as an opaque `CanisterInvalidWasm` rejection. +fn read_gzip(wasm_path: &Path) -> Option> { + std::fs::read(wasm_path) + .ok() + .filter(|bytes| bytes.starts_with(&[0x1f, 0x8b])) +} + +/// Get the NNS Governance WASM binary, downloading if necessary. +/// Call [`get_governance_wasm`] instead — this runs once, behind `GOVERNANCE_WASM`. +fn load_governance_wasm() -> Vec { let wasm_path = PathBuf::from("../../target/ic/governance-canister.wasm.gz"); - // Check if we need to download - if !wasm_path.exists() { - let url = format!( - "https://download.dfinity.systems/ic/{}/canisters/governance-canister.wasm.gz", - IC_COMMIT_FOR_PROPOSALS - ); - download_wasm_to(url, &wasm_path); - } else { + if let Some(wasm) = read_gzip(&wasm_path) { println!("NNS Governance WASM already exists, skipping download"); + return wasm; } - std::fs::read(&wasm_path).unwrap_or_else(|e| { - panic!( - "Failed to read governance WASM file at {:?}: {}", - wasm_path, e - ) + let url = format!( + "https://download.dfinity.systems/ic/{}/canisters/governance-canister.wasm.gz", + IC_COMMIT_FOR_PROPOSALS + ); + download_wasm_to(url, &wasm_path); + + read_gzip(&wasm_path).unwrap_or_else(|| { + panic!("Downloaded governance WASM at {wasm_path:?} is not a readable gzip archive") }) } +fn get_governance_wasm() -> Vec { + GOVERNANCE_WASM.clone() +} + /// Sets up a minimal NNS Governance canister for testing. /// /// **Expected errors in test output (non-fatal):** From d9924051e97a86f001c22ac82b81e0b68b8f1de9 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Mon, 14 Sep 2026 15:12:48 +0200 Subject: [PATCH 2/4] docs(ci): point the mops workaround at the upstream issue Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/_run-example.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/_run-example.yml b/.github/workflows/_run-example.yml index e80c66e02a..3e68d0c155 100644 --- a/.github/workflows/_run-example.yml +++ b/.github/workflows/_run-example.yml @@ -64,7 +64,7 @@ jobs: # concurrently, and those invocations race on the shared moc toolchain download and # extraction: whichever build loses reads a half-extracted moc and fails with no # diagnostics. Warming the cache first makes the download happen exactly once. - # Reproduces with mops 3.2.0; remove once mops locks the toolchain cache itself. + # Remove once caffeinelabs/mops#818 is fixed and released. - name: Warm the mops toolchain cache working-directory: ${{ inputs.working-directory }} run: | From f344cca7f6c7f72c79e457d19baed1a73009ee6f Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Mon, 14 Sep 2026 15:21:45 +0200 Subject: [PATCH 3/4] fix(unit_testable_rust_canister): make the download temp name process-unique Keeps the atomic-rename guarantee if the tests are ever run per-process (cargo nextest) or in two checkouts at once, rather than relying on libtest running tests as threads within one process. Co-Authored-By: Claude Opus 5 (1M context) --- .../backend/tests/integration_tests.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs b/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs index 6de1eb675e..a532dfa0a7 100644 --- a/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs +++ b/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs @@ -132,14 +132,16 @@ fn rebuild_wasm() { } } -/// Download to a sibling temporary file and rename into place, so an interrupted or failed -/// transfer never leaves a truncated file behind for the next run to pick up from the cache. +/// Download to a temporary file and rename into place, so an interrupted or failed transfer +/// never leaves a truncated file behind for the next run to pick up from the cache. The +/// temporary name carries the process id so that concurrent runs of the test binary cannot +/// clobber each other's transfer either. fn download_wasm_to(url: String, wasm_path: &Path) { if let Some(parent) = wasm_path.parent() { std::fs::create_dir_all(parent) .unwrap_or_else(|e| panic!("Failed to create directory {:?}: {}", parent, e)); } - let download_path = wasm_path.with_extension("gz.partial"); + let download_path = wasm_path.with_extension(format!("gz.{}.partial", std::process::id())); let output = Command::new("curl") .args([ From 209028c394382ddd2f5bbb4a9185af788da82364 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Mon, 14 Sep 2026 15:55:01 +0200 Subject: [PATCH 4/4] fix(unit_testable_rust_canister): validate the whole cached gzip stream The magic-prefix check accepted a truncated archive left behind by an earlier interrupted download, so a corrupt cache entry survived and kept failing as CanisterInvalidWasm. Decoding the stream covers the trailing CRC and length, and a corrupt entry is now replaced rather than reused. flate2 is already built for the test profile via pocket-ic. Co-Authored-By: Claude Opus 5 (1M context) --- .../backend/Cargo.toml | 1 + .../backend/tests/integration_tests.rs | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/rust/unit_testable_rust_canister/backend/Cargo.toml b/rust/unit_testable_rust_canister/backend/Cargo.toml index eede2dd8e5..cb85b21e60 100644 --- a/rust/unit_testable_rust_canister/backend/Cargo.toml +++ b/rust/unit_testable_rust_canister/backend/Cargo.toml @@ -21,3 +21,4 @@ pocket-ic = "15.0.0" candid = "0.10" tokio = { version = "1.0", features = ["macros", "rt"] } walkdir = "2.0" +flate2 = "1.1" diff --git a/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs b/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs index a532dfa0a7..94ce44c386 100644 --- a/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs +++ b/rust/unit_testable_rust_canister/backend/tests/integration_tests.rs @@ -1,7 +1,9 @@ use candid::{decode_one, encode_one, CandidType, Principal}; +use flate2::read::GzDecoder; use pocket_ic::{PocketIc, PocketIcBuilder}; use serde::Deserialize; use std::io; +use std::io::Read; use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::LazyLock; @@ -173,12 +175,16 @@ fn download_wasm_to(url: String, wasm_path: &Path) { std::fs::rename(&download_path, wasm_path).expect("Failed to move downloaded WASM into place"); } -/// Reads the file only if it is a gzip archive. A missing prefix means a partial or failed -/// download, which would otherwise surface as an opaque `CanisterInvalidWasm` rejection. +/// Reads the archive only if the whole gzip stream decodes, which covers the trailing CRC and +/// length. Checking just the magic prefix would accept a truncated download and install it, +/// surfacing later as an opaque `CanisterInvalidWasm` rejection. fn read_gzip(wasm_path: &Path) -> Option> { - std::fs::read(wasm_path) - .ok() - .filter(|bytes| bytes.starts_with(&[0x1f, 0x8b])) + let bytes = std::fs::read(wasm_path).ok()?; + let mut decoded = Vec::new(); + GzDecoder::new(bytes.as_slice()) + .read_to_end(&mut decoded) + .ok()?; + Some(bytes) } /// Get the NNS Governance WASM binary, downloading if necessary. @@ -190,6 +196,9 @@ fn load_governance_wasm() -> Vec { println!("NNS Governance WASM already exists, skipping download"); return wasm; } + if wasm_path.exists() { + println!("Cached NNS Governance WASM is corrupt, downloading again"); + } let url = format!( "https://download.dfinity.systems/ic/{}/canisters/governance-canister.wasm.gz",