hardening(viem-chain): anchor wallet/public clients to chainId (SSOT 7.4) - #4
Merged
Conversation
…tion
SSOT 7.4 hardening: every viem WalletClient / PublicClient built by the
CLI now carries an explicit `chain` parameter pinned to the chains.toml
entry, instead of relying on viem's auto-fetch of `eth_chainId` from
the RPC. This defends against:
- MITM RPCs that lie about eth_chainId (returning a chainId for a fork
the user did not intend to broadcast on).
- Offline-signing flows that need a deterministic chainId baked into
the signature without an extra RPC round-trip.
- RPC drift / endpoint reassignment between client construction and
the actual broadcast.
Changes:
1. defi-core/src/registry/chain.ts
- ChainConfig.viemChain() builds a viem-compatible Chain from the
toml entry: id (chain_id), name, nativeCurrency (native_token,
18 decimals), rpcUrls.default.http (effectiveRpcUrl()), optional
blockExplorers (explorer_url) and contracts.multicall3.
- Returns a local ViemChainShape interface so defi-core stays
viem-agnostic at the package boundary; consumers cast to viem's
Chain when they wire the result.
2. defi-core/src/provider.ts
- getProvider(rpcUrl, chain?) — when chain is provided, the cache
key becomes `${rpcUrl}@${chain.id}` so two callers with the same
RPC but different anchors don't collide on cached client.
3. defi-cli/src/executor.ts
- Executor constructor accepts an optional 4th `chain: Chain` arg,
stored as readonly. Backwards-compatible — existing callers
(executor.test.ts, scripts) don't pass it and continue to work.
- All five createPublicClient / createWalletClient call sites now
spread chainOpt() so the anchor flows through fee fetching,
simulation, allowance probing, and broadcast.
4. defi-cli/src/cli.ts
- makeExecutor() pulls the ChainConfig from the registry and threads
`chain.viemChain()` into the new Executor 4th arg.
5. defi-core/src/registry/chain.test.ts (new, 4 tests)
- viemChain() returns full viem Chain shape with all required fields.
- viemChain() omits optional explorer / multicall3 when missing.
- viemChain() honors env-var RPC overrides via effectiveRpcUrl().
- chainId is preserved verbatim — a config typo (chain_id = 0)
surfaces immediately rather than auto-fetched at runtime.
Verified: monorepo build/test/lint all clean — 82/82 tests pass
(defi-core 32 + defi-protocols 21 + defi-cli 29).
Refs: F3 in docs/qa-reports/2026-05-05-test-foundation.md.
This was referenced May 5, 2026
4 tasks
Hiksang
added a commit
that referenced
this pull request
May 6, 2026
PR #3 (feat/slippage-protection) closed 14 of the 15 sites that the 2026-05-05 baseline catalogued in qa/slippage.test.ts:KNOWN_INFINITE_SLIPPAGE. The line numbers also shifted when PR #4 (viem-chain anchor) refactored uniswap_v3.ts, so every entry in the original snapshot became stale on 2026-05-06 main and the test went red on every new PR (e.g. #7). This commit re-grounds the snapshot against current main: Adapter scan (regex unchanged): exactly 1 hit remains in the entire defi-protocols/src tree — dex/uniswap_v3.ts:262 amountOutMinimum: 0n (eth_call simulation inside quote() fallback; never broadcasts a tx) PR #3 hardened the other 14 sites: algebra_v3.ts:86,273,278,304 -> applyMinSlippage / explicit min args balancer_v3.ts:37 -> applyMinSlippage thena_cl.ts:78,165,190 -> applyMinSlippage / explicit min args uniswap_v3.ts:90,242,343,344, -> applyMinSlippage / explicit min args 363,364,403 In addition to refreshing the set, this commit fixes a self-trigger bug: uniswap_v3.ts:94 carries a documentation comment that warns against `amountOutMinimum: 0n`, and that comment matched the regex on its own — driving a false-positive "novel site" failure. Adding a `commentLinePattern` skip in both `it` blocks keeps the guard's intent (catch real broadcast paths) while letting docs reference the forbidden literal. Verified: - pnpm -C ts -r build — synced + dist rebuilt clean. - pnpm -C ts -r lint — 3 packages, tsc --noEmit clean. - pnpm -C ts -r test — 64/64 passing (was 62/64 before this fix). Refs: PR #7 CI failure (3x Build & Test red) was the smoke; root cause was the line-number drift, not anything Monad-related.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
F3 follow-up tracked in PR #2
(qa/2026-05-05-test-foundation report). Anchors every viem
WalletClient/PublicClientbuilt by the CLI to an explicitchainIdat construction time, instead of relying on viem's auto-fetchof
eth_chainIdfrom the RPC.Threat model: a MITM (or simply mis-pointed) RPC can return a
chainIdfor a fork the user did not intend to broadcast on. Withno anchor at the client, viem's signed payload uses whatever the RPC
returned. With this PR, the chainId is baked at
new Executor(...)time from the bundled
chains.toml, so an RPC that drifts no longercauses a tx to be signed for the wrong chain.
Changes
packages/defi-core/src/registry/chain.tsChainConfig.viemChain()returns a viem-compatible Chain from the toml entry (id, name, native currency, rpc, optional explorer + multicall3). LocalViemChainShapeinterface keeps defi-core viem-agnostic.packages/defi-core/src/provider.tsgetProvider(rpcUrl, chain?)accepts an optional anchor; cache key becomes${rpcUrl}@${chain.id}to prevent collisions across anchors.packages/defi-cli/src/executor.tsExecutorconstructor takes an optional 4thchainarg. All 5createPublicClient/createWalletClientsites spreadthis.chainOpt()so the anchor flows through fee fetching, simulation, allowance probing, and broadcast. Backwards-compatible.packages/defi-cli/src/cli.tsmakeExecutor()threadschain.viemChain()into the Executor.packages/defi-core/src/registry/chain.test.tsTest plan
pnpm -C ts -r buildclean.pnpm -C ts -r test→ 82/82 pass (defi-core 32 + defi-protocols 21 + defi-cli 29).pnpm -C ts -r lintclean.Coordination with sibling PRs
qa/2026-05-05-v1-0-12-baseline) — establishes the SSOTworkflow + Dockerfile + snapshot tests. Merge first.
feat/slippage-protection) — F1 follow-up. Independentof this PR; can merge in either order after PR QA: SSOT establishment + test foundation (v1.0.12 baseline) #2.
🤖 Generated with Claude Code