Skip to content

initial commit#1

Merged
j4ys0n merged 1 commit into
mainfrom
initial
Jun 6, 2026
Merged

initial commit#1
j4ys0n merged 1 commit into
mainfrom
initial

Conversation

@j4ys0n

@j4ys0n j4ys0n commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

MCP Etherscan
A read-only MCP server for the Etherscan API. The base URL and chain ID are configurable, so the same server can target Etherscan V2 or a compatible explorer such as Blockscout, Routescan, or a self-hosted deployment.

@j4ys0n

j4ys0n commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Automated review 🤖

Summary of Changes

This is an initial commit introducing a complete, production-ready read-only MCP server for the Etherscan V2 API and compatible EVM explorers. The PR adds 30+ files spanning source code, tests, CI/CD, scripts, and configuration. The server exposes 26 tools covering accounts, contracts, transactions, logs, blocks, proxy (JSON-RPC), and stats, with configurable base URL, chain ID, and API key injected as hidden per-request secrets.

Architecture & Organization

  • Clean separation: src/config.ts (env loading), src/resolver.ts (per-request secret resolution), src/client.ts (HTTP), src/cache.ts (LRU), src/format.ts (output bounding), src/schemas.ts (shared Zod validators), src/tools/ (one file per module). Each layer has a single responsibility.
  • src/tools/index.ts is a pure registration aggregator — adding a new module requires only writing its register function and calling it here, which is a good extensibility pattern.
  • scripts/schema-audit.mjs and scripts/smoke.mjs use raw MCP stdio protocol rather than a test harness, providing genuine integration coverage that unit tests cannot replicate.
  • AGENTS.md and CLAUDE.md are byte-for-byte identical — one of them is redundant and should be removed or differentiated.

Key Changes & Positives

  • 🟢 Context-bounded output by default: buildBoundedResponse and buildListEnvelope in src/format.ts use a binary-search truncation strategy to fit within a 25,000-character budget, with fullOutput opt-in. This is a well-considered design for LLM context management.
  • 🟢 Streaming body cap: src/client.ts readBoundedJson checks Content-Length before reading and enforces a 10 MiB hard cap during streaming, preventing memory exhaustion from malicious or oversized responses.
  • 🟢 Request coalescing: src/cache.ts inFlight map prevents thundering-herd on cache misses — concurrent identical requests share one upstream fetch.
  • 🟢 API key excluded from cache keys: createCacheKey hashes endpoint + params but not apiKey, so different users sharing the same explorer endpoint share cached results correctly.
  • 🟢 Schema audit enforces fullOutput contract: scripts/schema-audit.mjs verifies that every large-output tool has fullOutput defaulting to false and mentions it in the description — this is a strong regression guard.
  • 🟢 resolveRequestConfig validates before use: URL protocol, embedded credentials, and chain ID format are all validated with user-facing UserError before any network call.
  • 🟢 98%+ test coverage with a real FastMCP registration harness that exercises all 26 tools against deterministic mocked responses.

Potential Issues & Recommendations

  1. Issue / Risk: superlru pulls in redis as a runtime dependency (yarn.lock shows redis@^4.0.4 resolved under superlru@1.4.0), yet the cache is described as "in-memory mode with compression disabled." If superlru conditionally connects to Redis, a missing Redis instance could cause startup failures or silent fallback behavior in production.

    • Impact: Unexpected Redis connection attempts or errors in environments where Redis is not available, potentially causing the server to fail or log noise.
    • Recommendation: Verify superlru 1.4.0's behavior when no Redis is configured. If it attempts a connection, either pin to a version that is purely in-memory, or document the Redis dependency explicitly and add a startup check. Consider replacing with a simpler in-memory LRU (e.g., lru-cache) to eliminate the transitive Redis dependency entirely.
    • Status: 🔴 Problem
  2. Issue / Risk: src/cache.ts getSharedResponseCache uses a module-level Map<string, ExplorerResponseCache> keyed on config.enabled:maxEntries:maxEntryBytes. Two callers with identical config share one cache instance, but if AppConfig cache settings change at runtime (e.g., via env reload), stale shared instances are never evicted.

    • Impact: Low risk in practice since loadAppConfig is called once at startup, but the pattern is fragile if the server is ever extended to support dynamic reconfiguration.
    • Recommendation: Document that getSharedResponseCache is process-lifetime and that config changes require a restart, or replace the shared map with a single module-level singleton initialized from loadAppConfig.
    • Status: 🟡 Needs review
  3. Issue / Risk: src/format.ts buildListEnvelope computes has_more as results.length >= offset (line ~52). This is a heuristic: a full page implies more pages may exist. However, if the API returns exactly offset results on the last page, has_more will be true when it should be false.

    • Impact: LLM agents may make unnecessary follow-up pagination requests, wasting API quota and adding latency.
    • Recommendation: Document this heuristic explicitly in the field description (e.g., "has_more": true means a full page was returned; a subsequent request may return zero results), or change to results.length > offset if the intent is "strictly more than requested."
    • Status: 🟡 Needs review
  4. Issue / Risk: missionsquad.server.json uses "inputType": "password" for chainId, which is a non-secret numeric identifier. Treating it as a password field obscures it unnecessarily in UIs and may confuse users.

    • Impact: Minor UX friction; users may not realize they can enter a plain integer.
    • Recommendation: Change chainId's inputType to "text" since it is not sensitive.
    • Status: 🟡 Needs review
  5. Issue / Risk: src/index.ts hardcodes version: "1.0.0" in the FastMCP constructor rather than reading from package.json. These will drift as the package version is bumped.

    • Impact: The MCP serverInfo.version reported to clients will always be 1.0.0 regardless of the published npm version.
    • Recommendation: Import version from package.json (using assert { type: "json" } with NodeNext module resolution, or via a build-time constant) and pass it to FastMCP.
    • Status: 🟡 Needs review

Language/Framework Checks

  • src/schemas.ts documents that tool schemas must NOT use .strict() or top-level .refine() due to FastMCP's hidden-value injection behavior. This is a non-obvious constraint that could cause subtle bugs if contributors add new tools without reading the comment. Consider adding a lint rule or a test that asserts no registered tool schema is a ZodEffects instance.
  • src/cache.ts uses await this.cache.get(key) and checks !== null for a miss. If superlru returns undefined for a miss (common in LRU libraries), the null check will incorrectly treat misses as hits. Verify the exact miss sentinel from superlru 1.4.0's .d.ts.
  • src/client.ts fetchJson appends apikey after all other params, which is correct for security (avoids it appearing in logs mid-URL), but the URL is constructed with new URL(this.baseUrl) which will preserve any existing query params in baseUrl — this is tested in test/client.test.ts and works correctly.
  • publish.yaml uses npm publish after installing with yarn --frozen-lockfile. This is intentional (npm publish reads package.json files), but mixing package managers in CI can cause subtle issues if npm and yarn resolve differently. Consider using yarn publish or npm ci consistently.

Security & Privacy

  • scripts/schema-audit.mjs explicitly checks that apiKey, baseUrl, and chainId do not appear as tool input schema properties, preventing accidental secret exposure in tool definitions. This is a strong guard.
  • src/resolver.ts validateBaseUrl rejects embedded credentials (user:password@host) in the base URL, preventing credential leakage via URL logging.
  • The API key is appended last in fetchJson and excluded from cache keys — both correct. However, it will appear in server-side HTTP access logs on the explorer's end; this is unavoidable but worth noting in documentation.

Build/CI & Ops

  • build.yaml triggers on PRs only; publish.yaml triggers on pushes to main excluding markdown changes. The publish workflow runs yarn format (check only) and yarn test before publishing — correct ordering.
  • package.json prepublishOnly runs yarn format && yarn test && yarn build, which means build runs twice during publish (once in prepublishOnly, once in the CI workflow). This is harmless but redundant.
  • vitest.config.ts excludes src/index.ts from coverage, relying on yarn smoke for entry-point coverage. This is documented and intentional, but src/index.ts is included in the npm files array — ensure the smoke test is always run before publishing (it is, via test:schema which calls yarn build && node scripts/schema-audit.mjs).

Tests

  • test/cache.test.ts tests TTL expiry using vi.useFakeTimers() and vi.advanceTimersByTimeAsync — correct approach. The "retains immutable and fixed historical responses" test advances by 10,001ms and expects only 2 fetch calls, but the immutable TTL is 6 hours — this test only verifies that live TTL (10s) does not evict immutable/historical entries, which is the right assertion.
  • test/tools.test.ts verifies all 26 tools are registered and produce valid JSON. The requestedActions set assertion at the end is a strong regression guard against accidental action name changes.
  • Missing: no test for getSharedResponseCache identity/sharing behavior, and no test for the ETHERSCAN_CACHE_ENABLED=false path through EtherscanClient (only ExplorerResponseCache disabled is tested directly).

Approval Recommendation

Approve with caveats

  • Investigate and document the superlruredis transitive dependency; confirm in-memory-only behavior or replace with a pure in-memory LRU to avoid unexpected Redis connection attempts in production.
  • Verify superlru miss sentinel (null vs undefined) against installed .d.ts to ensure cache miss detection in src/cache.ts is correct.
  • Change chainId inputType from "password" to "text" in missionsquad.server.json.
  • Remove the duplicate AGENTS.md / CLAUDE.md or differentiate their content.
  • Read version from package.json in src/index.ts rather than hardcoding "1.0.0".

@j4ys0n j4ys0n merged commit 9735a46 into main Jun 6, 2026
2 checks passed
@j4ys0n j4ys0n deleted the initial branch June 6, 2026 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant