Skip to content

📝 docs(test): follow-up scope for finishing the Anvil-to-Tevm migration #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roninjin10 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

📝 docs(test): follow-up scope for finishing the Anvil-to-Tevm migration #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions test/FOLLOWUP-tevm-migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Follow-up scope for the Anvil-to-Tevm test migration

Companion to PR #2. Review findings are posted as comments on that PR; this file is
the durable plan for finishing the job.

## Follow-up scope: retiring the Anvil lane once tevm WS/IPC publish

evmts/tevm **#2084** (`createWebSocketServer`, JSON-RPC over WS on the same port as HTTP, `eth_subscribe`/`eth_unsubscribe` with real notification push) and **#2080** (`createIpcServer`, Unix domain socket, framing across fragmented/concatenated/newline-delimited chunks, `newHeads`/`newPendingTransactions`/`logs` subscriptions, cleanup on `eth_unsubscribe` and socket close) are both merged into `evmts/tevm` main. The stated blocker for this PR's Anvil lane is therefore gone in source — but **not on npm**: publishing is blocked on npm trusted-publisher configuration, and this fork pins `tevm@1.0.0-rc.151` from the registry. So this is sequenced, not immediately actionable.

### Phase 0 — do now, independent of publishing

None of this waits on tevm:

1. Fix `isUsed` so it tracks transport requests, not property reads (review comment #1). Biggest single win; likely removes most of the 429s and a large share of the 232 runtime errors.
2. Reset (or isolate) `tevmOptimism` / `tevmSepolia` / `tevmZksync` in `test/setup.ts` (#2).
3. Give the 19 files that call `reset()` their own harness, or make `harness.reset()` re-anchor when `evm_revert` returns false (#3).
4. Wire `VITE_TEVM_FORK_URL_OPTIMISM` / `_SEPOLIA` / `_ZKSYNC` secrets into `verify.yml` (#6).
5. Give Alto its own non-snapshot-reverted harness in `test/src/bundler.ts` (#8).
6. Re-record snapshots against the new `22263621n` anchor as a separate mechanical commit, then measure what behavioural drift actually remains.
7. Add a `contracts:check` CI job so `contracts/generated.ts` can't silently go stale (#7).
8. Fix `test/src/anvil-transports.ts`: pass an `ipcPath` to `Instance.anvil` and restore `noMining: true`, then actually run `TEST_ANVIL_TRANSPORTS=true` in a scheduled (non-blocking) CI job so the lane doesn't rot while it exists (#4).

### Phase 1 — when a tevm release containing #2084/#2080 is on npm

Bump `tevm`/`@tevm/*` off `1.0.0-rc.151` (also drop the `pnpm.packageExtensions` viem-injection hack in `package.json` if the new release ships a compatible viem peer).

Then extend `test/src/tevm.ts` with a `withServers()` variant that starts, on one harness:

- `createServer` (HTTP, already used by `test/src/bundler.ts`),
- `createWebSocketServer` on the same port,
- `createIpcServer` on a per-worker socket path (`/tmp/viem-tevm-${VITEST_POOL_ID}.ipc`),

exposing `rpcUrl.{http,ws,ipc}` with the same shape `anvil-transports.ts` fakes today, plus a real `close()`. Ports must be `VITEST_POOL_ID`-offset like `test/src/bundler.ts` already does; the IPC path must be unlinked in teardown.

### The four suites, and exactly what each needs

| Suite | What it needs from the new servers |
| --- | --- |
| `src/utils/rpc/webSocket.test.ts` | Lowest-level. Raw socket client behaviour: request/response correlation, batching, `eth_subscribe` → notification delivery, error frames, socket close/reconnect semantics, `keepAlive`. Needs `createWebSocketServer` plus the ability to kill the server mid-test to exercise reconnect. Verify tevm sends `eth_subscription` notifications with the exact envelope viem's socket client parses. |
| `src/clients/transports/webSocket.test.ts` | `webSocket()` transport over the above, plus the 3 `createPublicClient` `keepAlive & reconnect disabled` tests that were relocated here — those should move **back** to `src/clients/createPublicClient.test.ts` once a WS URL is available by default. Several cases call `mine()` and await `newHeads`, so tevm's `manual` mining config must still push `newHeads` on `tevm_mine`. |
| `src/utils/rpc/ipc.test.ts` | `createIpcServer` with a socket path. Exercises exactly the framing #2080 says it handles (fragmented / concatenated / newline-delimited), so it's the real acceptance test for that PR. Needs deterministic teardown to avoid stale sockets across workers. |
| `src/clients/transports/ipc.test.ts` | `ipc()` transport over the above; also imports `Instance` from prool today — that import goes away entirely. |

Once those four are on the tevm servers: delete `test/src/anvil-transports.ts`, `test/setup.anvil-transports.global.ts`, the `anvilTransportTests` array and the conditional `anvil-transports` project in `test/vitest.config.ts`, and the `TEST_ANVIL_TRANSPORTS` branch in `test/setup.ts`.

### Then restore the WebSocket CI matrix leg

The `transport-mode: ['http', 'webSocket']` axis and `VITE_NETWORK_TRANSPORT_MODE` deleted from `verify.yml` must come back pointed at the tevm WS server. This is the largest piece of coverage the migration dropped and it is only recoverable at this phase. Note `test/src/tevm.ts` currently has no `VITE_NETWORK_TRANSPORT_MODE` handling at all — the harness always returns the in-process `custom()` transport — so `getClient()` needs a mode switch that returns `webSocket(rpcUrl.ws)` when the env var says so.

### What's left before Anvil is fully gone

After the above, the residue is:

- `prool` — still needed for Alto (and the Tempo fixture), not for Anvil. It stays as a devDependency but no longer starts an Ethereum node. Removing the Alto dependency is a separate, larger project (an in-process bundler), not part of this.
- ~70 files with `anvil` in them that are **public API, not a node**: `src/chains/definitions/anvil.ts`, `mode: 'anvil'` in `createTestClient`, the `anvil_*` method spellings in `src/actions/test/*`, error mappings in `src/errors/node.ts`, docs. These are viem's supported surface and Tevm's test-RPC dialect. They should stay, and the PR description is right to separate them — but the migration would read better if the harness's `mode: 'anvil' as const` had a one-line comment at each site (it has one, in `getClient()`; the `createTestClient` call sites don't).
- Foundry: separate from Anvil, and this PR already removes it from `postinstall` and CI by checking in `contracts/generated.ts`. The remaining tie is the `lib/` submodule + `.sol` sources with no regeneration path (#7). Restoring a `contracts:build` script gated behind a `contracts:check` CI job is the right end state: Foundry needed only by contract authors, never by installers or CI test runs.

### Definition of done

- `pnpm test` green with **no** `TEST_ANVIL_TRANSPORTS` project defined and no `anvil` binary on `PATH`.
- CI matrix has both `http` and `webSocket` legs again.
- `grep -rl 'prool' test/` returns only Alto/Tempo fixtures.
- `test.skip`s reduced to the three genuine Tevm gaps (`eth_sendUnsignedTransaction`, `eth_signTypedData_v4`, EIP-4844 fork anchor), each linked to an upstream evmts/tevm issue.
Loading