test(cketh): migrate integration tests to PocketIC

[gregorydemay]

📚 PR stack

1. test(ledger-suite-orchestrator): migrate integration tests to PocketIC #10949 — test(ledger-suite-orchestrator): migrate integration tests to PocketIC (merged)
2. test(cketh): deduplicate signed-transaction and transaction-hash literals in integration tests #10950 — test(cketh): deduplicate signed-transaction and transaction-hash literals (merged)
3. test(cketh): migrate integration tests to PocketIC #10955 — test(cketh): migrate integration tests to PocketIC 👈 you are here
4. test(ledger-suite-orchestrator): drop the StateMachine test fixture #10956 — test(ledger-suite-orchestrator): drop the StateMachine test fixture — its content is part of this PR, see below

Follow-up, based on this PR: #11123 (PocketIC server lifetime).

Summary

Flips rs/ethereum/cketh/test_utils and the minter's cketh.rs/ckerc20.rs integration tests from ic-state-machine-tests onto the sync pocket_ic client, following the pattern established for the ledger-suite-orchestrator in #10949.

The minter is now initialized against the fiduciary subnet's "key_1" ECDSA key (StateMachine's "master_ecdsa_public_key" isn't available on PocketIC's fiduciary subnet), which changes the minter's derived Ethereum address. Every signature-derived constant is regenerated as a consequence: MINTER_ADDRESS, DEFAULT_WITHDRAWAL_TRANSACTION(_HASH), DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION(_HASH), the r/s of the default ERC20 signed transaction, the per-account deposit address in should_record_address_to_deposit, and the resubmitted-transaction literal in should_resubmit_new_transaction_with_same_max_fee_per_gas_when_price_increased.

Canister-http mocking is rebuilt around PocketIC's get_canister_http() / mock_canister_http_response(), preserving the legacy "Http body exceeds size limit of {n} bytes." reject that the minter's response-size check matches on.

tests/dump_stable_memory.rs and tests/deposit_from_cex_demo.rs are untouched.

Also included: dropping the StateMachine fixture from the orchestrator

This was meant to be a separate PR (#10956) reviewed on top of this one. It was marked merged by mistake — see the note there — and since GitHub cannot reopen a merged PR, its content stays here rather than being shuffled again.

With the ckETH minter's tests on PocketIC, nothing in rs/ethereum uses the orchestrator's StateMachine-based fixture, so it is deleted along with the now-unused ic-state-machine-tests, ic-types, ic-management-canister-types-private and assert_matches dependencies. The environment-agnostic helpers the pocket_ic module and cketh still import (wasm loaders, init-arg builders, usdc()/usdt(), constants) stay at their existing paths. After this PR, no Rust target under rs/ethereum compiles against ic-state-machine-tests.

Behavioural differences PocketIC forces, and how they are handled

PocketIC differs from StateMachine in two ways the tests are sensitive to. Both are handled in the fixture, in one place each, rather than worked around per test.

Time advances kill the outcalls in flight. PocketIC fails canister-http requests once they cross CANISTER_HTTP_TIMEOUT_INTERVAL; StateMachine left them pending indefinitely. The requests also stay listed until a round processes the failures, so a stub could bind to a request that was already dead and see its response discarded. Worse, a scraping cycle awaiting such a request keeps holding its TimerGuard, so the firing due at the new time is dropped as AlreadyProcessing and the query the test expects is never issued. CkEthSetup::advance_time therefore fails those outcalls up front and deterministically, doing what the advance would have done anyway. It is a no-op when nothing is in flight, because a fixture that has executed no round is expected to trigger the first scraping cycle itself.

A stop request takes effect only in the following round. The outcalls the minter's timers issue in that last running round become visible after it, so stop_minter ticks once before draining them.

The block number stubs also now match on ["finalized", false], which is what the scraping cycle actually queries — unconstrained, they could be consumed by the block height refresh timer's query for the latest block.

Runtime

Measured against master with identical in-test instrumentation on both sides: +5% (ckETH) and +11% (ckERC20) of wall-clock at default parallelism (+15%/+20% single-threaded). Fixture-light tests are indistinguishable from master; the difference tracks the number of IC rounds a test drives, since PocketIC runs out of process. The dominant cost in both harnesses is tests that rebuild the whole fixture several times within one #[test].

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Migrates ckETH/ckERC20 integration test utilities and minter integration tests from ic-state-machine-tests to the synchronous pocket_ic client, aligning the cketh test harness with the PocketIC-based approach introduced earlier for the ledger-suite-orchestrator.

Changes:

• Replaced StateMachine-based fixtures and ingress/query plumbing with PocketIC equivalents (including manual ingress-status polling where needed).
• Rebuilt canister-http mocking around PocketIC’s get_canister_http() / mock_canister_http_response() and regenerated signature-derived test constants due to ECDSA key differences.
• Updated Rust and Bazel dependency graphs to drop ic-state-machine-tests usage in these test crates and add PocketIC inputs/binaries for test execution.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
rs/ethereum/cketh/test_utils/src/response.rs	Regenerates signature components for default signed tx fixtures under PocketIC-derived minter identity.
rs/ethereum/cketh/test_utils/src/mock.rs	Ports JSON-RPC outcall matching/mocking to PocketIC’s canister-http APIs and adds retry logic for timer-cap interactions.
rs/ethereum/cketh/test_utils/src/lib.rs	Replaces StateMachine fixture with PocketIC fixture, updates canister lifecycle helpers, and adds manual await/drain helpers for outcalls.
rs/ethereum/cketh/test_utils/src/flow.rs	Switches async call tracking from MessageId to PocketIC RawMessageId and uses the new await helper.
rs/ethereum/cketh/test_utils/src/ckerc20.rs	Ports ckERC20 test setup/flows to PocketIC and updates orchestrator fixture usage to pocket-ic module.
rs/ethereum/cketh/test_utils/Cargo.toml	Swaps StateMachine-related deps for PocketIC + public management-canister-types + metrics assert pocket_ic feature.
rs/ethereum/cketh/test_utils/BUILD.bazel	Updates Bazel deps to PocketIC + ic-metrics-assert pocket_ic target; drops state-machine deps.
rs/ethereum/cketh/minter/tests/cketh.rs	Updates tests to PocketIC APIs and new regenerated constants; adjusts assertions around outcall queues.
rs/ethereum/cketh/minter/tests/ckerc20.rs	Updates ckERC20 integration tests to PocketIC fixtures/APIs and regenerated deposit address constant.
rs/ethereum/cketh/minter/Cargo.toml	Drops ic-state-machine-tests and adds PocketIC + public management-canister-types for tests.
rs/ethereum/cketh/minter/BUILD.bazel	Adds PocketIC server binary to test data and switches deps to PocketIC.
Cargo.lock	Reflects dependency changes (adds pocket-ic, removes ic-state-machine-tests from these crates, adds ic-management-canister-types).

Comments suppressed due to low confidence (3)

rs/ethereum/cketh/test_utils/src/lib.rs:574

• stop_ongoing_https_outcalls returns even if it never manages to drain pending canister-http requests. Callers (e.g. tests using PocketIc::await_call) can then hang/flap because outcalls remain open; it’s better to fail loudly with diagnostics when draining doesn’t succeed.

    pub fn stop_ongoing_https_outcalls(&self) {
        for _ in 0..MAX_TICKS {
            if self.drain_pending_https_outcalls() {
                return;
            }

rs/ethereum/cketh/test_utils/src/lib.rs:841

• drain_startup_http_outcalls registers mocked responses but never ticks afterwards in the final loop iteration, so the last batch of startup outcalls may remain pending (defeating the purpose of draining them before long advance_time jumps). It should tick after mocking and stop once the queue is empty.

fn drain_startup_http_outcalls(env: &PocketIc) {
    for _ in 0..MAX_TICKS {
        env.tick();
        for request in env.get_canister_http() {
            // Reject rather than answer: these are the startup timers' one-shot log-scraping and

rs/ethereum/cketh/test_utils/src/lib.rs:857

• drain_stray_latest_block_refresh_calls unconditionally unwrap()s JSON parsing of every pending outcall body; if any non-JSON/JSON-RPC request is present, tests will panic while trying to drain. Also, like the startup drain, it should tick after registering mocks to actually clear requests.

        env.tick();
        for request in env.get_canister_http() {
            let json_request: serde_json::Value = serde_json::from_slice(&request.body).unwrap();
            if json_request["method"] == "eth_getBlockByNumber"
                && json_request["params"] == serde_json::json!(["latest", false])

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gregorydemay]

🤖🧐 VERDICT: CHANGES_REQUESTED — 0 blockers, 6 mediums, 7 nits; CI pending (24 pass / 4 pending / 0 fail). Local: 2 consecutive uncached green runs of //rs/ethereum/cketh/minter:integration_tests.

⚠️ Posted as a comment rather than a formal “Request changes” review: GitHub refuses --request-changes on a PR owned by the same account. The verdict below is still CHANGES_REQUESTED.

Review details

Bottom line

The port itself is excellent. The #[test] sets in tests/cketh.rs (26) and tests/ckerc20.rs (39) are identical to the PR-2 base — no test dropped, renamed, #[ignore]d or #[should_panic]ed — and every assertion change is accounted for by the documented type/constant mapping. No loosened asserts, no println!/dbg!/commented-out asserts survived the constant-regeneration pass. Senders are preserved exactly (execute_ingress/query → Principal::anonymous(), execute_ingress_as/query_as → pid.0). mock.rs keeps the raw-body max_response_bytes check with the verbatim legacy message and never invokes cleanup_response manually. Component boundary is clean: the delta touches only rs/ethereum/cketh/** + Cargo.lock; dump_stable_memory.rs and deposit_from_cex_demo.rs are untouched. Everything I flag is in the new fixture plumbing, not in the tests.

The three declared deviations

1. find_rpc_call_retrying (positive-only) — the asymmetry is sound. expect_no_matching_rpc_call / expect_no_refresh_gas_fee_estimate still go through find_rpc_call → poll_for_request, whose detection window (10 × tick() + 1ns) is byte-for-byte the StateMachine tick_until_next_http_request it replaces — it has not silently shrunk. The mechanism is deterministic, not a flakiness source. I found no simpler in-repo precedent: rs/bitcoin/checker's tick_until_next_request is the same plain 10-tick loop, but the checker has no competing periodic timer, so it doesn't face this. One concrete concern remains (see the inline note on mock.rs:152): the retry budget can burn 300s of simulated time, more than SCRAPING_ETH_LOGS_INTERVAL (180s), so it can cross a scrape boundary.
2. Custom await_call — justified: pocket-ic's own await_call ticks internally with no injection point, and await_call_no_ticks would deadlock in a sync client that must drive the rounds itself. It is bounded (1000 ticks) and panics with the message id. Two issues inline: the 100× bound relaxation is written as 100 * MAX_TICKS, and the helper's implicit "500 every pending outcall" policy is only wanted by stop_minter, not by minter_response/expect_trap.
3. new_pocket_ic() made pub — sound and effectively forced. CkErc20Setup::new_without_ckerc20_active installs the minter, and CkEthSetup::new calls minter_address(), which needs the fiduciary key_1; LSO's system-subnet builder cannot serve it. The only semantic delta is that cycles are now charged, and neither should_trap_when_ckerc20_feature_not_active asserts a balance — they assert a trap. pub is the minimum surface needed by the two call sites.

Maintainability accounting

• Duplication — found. The latest-block-refresh predicate exists in three places, two of them different implementations (mock.rs::matches, mock.rs::is_latest_block_refresh, lib.rs::drain_stray_latest_block_refresh_calls). The 500-reply block and the transient-reject block are each duplicated across two sites. Inline on mock.rs:187, lib.rs:851, lib.rs:584.
• Divergent invariant handling — found, same predicate: a malformed request body is .expect(..) in one copy, Err(_) => false in the second, bare .unwrap() in the third.
• Silent fallbacks — found: stop_ongoing_https_outcalls gives up after 10×10 attempts and returns as though it succeeded (lib.rs:570). The two drain_* helpers likewise never assert they drained anything.
• Unused derives — none; the PR introduces no new types.
• Primitive-obsession parameters — none of substance; the only primitives are the raw reject_code literals (nit, mock.rs:125).
• Test-only code in production modules — none.
• Redundant / derivable parameters, caller-owned decisions — one instance: await_call's drain policy is decided implicitly for all three call sites (lib.rs:885).

Coverage / test-pyramid

No new tests are warranted — this is a harness swap, and the existing suite is the regression oracle. The regenerated constants (MINTER_ADDRESS, both DEFAULT_*_WITHDRAWAL_TRANSACTION(_HASH), both response.rs r/s pairs) are cross-checked by test_utils:lib_tests::should_use_meaningful_constants (encoding + keccak hash) and, for MINTER_ADDRESS, by the assert_eq! in CkEthSetup::new. Note that the per-account deposit address at tests/ckerc20.rs:182 is a pure golden value with no independent oracle — unavoidable, just worth knowing.

Not blocking, FYI

drain_startup_http_outcalls in CkEthSetup::new has no StateMachine counterpart: it rejects the genesis scrape and block-refresh outcalls rather than leaving them pending. I traced this and it does not weaken anything (a rejection leaves last_observed_block_number None exactly as a pending call did, and the scrape timer retries on the next interval), but it is a behavioural delta the PR description does not mention — worth a sentence there. Also: the description says 40 ckerc20 tests; there are 39.

Verification run

• bazel test //rs/ethereum/cketh/test_utils:lib_tests //rs/ethereum/cketh/minter:lib_tests //rs/ethereum/cketh/minter:integration_tests --nocache_test_results → 4/4 PASSED (ckerc20 143.9s, cketh 71.3s)
• second uncached run of //rs/ethereum/cketh/minter:integration_tests → 2/2 PASSED (141.3s / 71.0s) — no timing drift between runs, consistent with the retry mechanism being deterministic
• cargo check --all-targets --all-features -p ic-cketh-test-utils -p ic-cketh-minter → clean
• timeout = "long" on the suite is still appropriate (bazel reports the declared size as too big, not too small).

CI is not yet green (4 jobs still pending, none failing), which is an independent gate on a READY verdict.

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 2615f60.

Security Overview

• 🔎 Scanned files: 17 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/783a9bd4-fb4e-4e10-ab3c-c2fb2ddfb92f?scanId=f304eb4a-6720-4bbf-850c-569acf11bcb9&codeScanTypes=PrScan&tab=issues

Detected Code Changes

The diff is too large to display a summary of code changes.

[gregorydemay]

🤖 Applied your review from #10956 here, since that PR's content ended up in this one — 2615f601d4 drops ic-http-types, proptest (both from Cargo.toml and BUILD.bazel) and ic-test-utilities-load-wasm (Cargo.toml only, it was never in BUILD.bazel). Confirmed none of the three is referenced anywhere in the crate.

Also removed the note repeating what the stub parameters already say (3b6dff3259).

ledger-suite-orchestrator:integration_tests and cketh_test pass, buildifier clean.