Draft
Conversation
integration: better error handling
integration: simplify response comparison
integration: make tasks fully async
integration: refactor executeRequest implementation
Buffer out-of-order results and flush consecutively by index so parallel execution still runs concurrently but console output matches Python's deterministic ordering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Profile showed compress/bzip2 consuming 32.8s CPU (30.7%) on pure-Go decompression of 221 test archives. The dsnet library wraps libbzip2 via CGo and is already a project dependency. CPU user time drops from ~90s to ~73s. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add subcommand dispatch to main() that delegates to urfave/cli when os.Args[1] matches a known subcommand, preserving full backward compatibility with existing flag-based behavior. New subcommands: block-by-number, empty-blocks, filter-changes, latest-block-logs, subscriptions, graphql, replay-request, replay-tx, scan-block-receipts. New packages: internal/tools (subcommand registry + implementations), internal/rpc/wsconn.go (persistent WebSocket connection), internal/eth (pure-Go RLP encoding + MPT trie for receipt root verification). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ons shutdown
- Fix receipt encoding bug: logs were double-encoded via rlpEncodeBytes on
already-RLP-encoded log entries. Use rlpEncodeListFromRLP instead, which
correctly treats each encoded log as an already-RLP item. Verified against
100 recent sepolia blocks (up to 406 receipts each).
- Fix GraphQL content type: use application/json with {"query":"..."} body
instead of application/graphql.
- Fix subscriptions shutdown: close connection instead of sending
eth_unsubscribe (which races with the notification read loop). Signal done
channel first, then close connection to break RecvJSON cleanly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document the subcommand dispatch system, internal/tools/ and internal/eth/ packages, subcommand usage examples, the no-go-ethereum constraint, and key gotchas discovered during live verification (RLP double-encoding, Erigon old-block receipts, WebSocket shutdown, GraphQL content type). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove std-error-handling preset and os.Remove* exclusions from golangci config so unhandled file operation errors are flagged. Add explicit Close/Flush exclusion rule and set max-same-issues to 0 so no warnings are hidden. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This is a comprehensive v2 rewrite that ports the rpc-tests suite from Python to Go. The PR introduces a well-structured Go codebase with clear separation of concerns across internal packages for RPC client operations, test execution, filtering, comparison, and tooling.
Changes:
- Complete rewrite in Go 1.24 with new module structure
- New internal packages for RPC client, test runner, filtering, comparison, and Ethereum primitives
- Tool subcommands ported from Python scripts
- Comprehensive test coverage with unit tests and benchmarks
- Enhanced configuration and CLI framework using urfave/cli/v2
- JSON test fixture formatting improvements
Reviewed changes
Copilot reviewed 80 out of 95 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | New Go module with dependencies (websocket, JWT, JSON, vegeta, cli) |
| cmd/perf/main.go | Performance testing CLI entry point |
| internal/tools/*.go | Tool subcommands (block-by-number, subscriptions, replay-tx, etc.) |
| internal/rpc/*.go | HTTP/WebSocket RPC client with JWT auth and compression support |
| internal/runner/*.go | Parallel test execution framework with worker pools |
| internal/testdata/*.go | Test discovery and fixture loading |
| internal/config/config.go | Configuration and CLI flag management |
| internal/filter/*.go | Test filtering logic (API patterns, exclusions, latest block) |
| internal/compare/*.go | Response comparison with JSON diff support |
| internal/perf/*.go | Performance test configuration and execution |
| internal/eth/receipt_test.go | Ethereum primitives tests (RLP, Keccak256, MPT) |
| internal/archive/archive.go | Archive extraction (tar/gzip/bzip2) |
| integration/mainnet//test_.json | Test fixture formatting improvements and address case corrections |
| CLAUDE.md | Comprehensive documentation |
| .golangci.yml | Linter configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "response": { | ||
| "error": { | ||
| "code": -32000, | ||
| "message": "insufficient funds for gas * price + value: address 0x8A9d69Aa686fA0f9BbDec21294F67D4D9CFb4A3E have 1392684180000000000 want 2000126000000000000" |
There was a problem hiding this comment.
The error message changed the capitalization of the address from lowercase "0x8a9d69aa..." to mixed case "0x8A9d69Aa...". Ethereum addresses are case-insensitive (except for EIP-55 checksumming), but this change should match what the actual server returns. Verify this is not a test breaking change.
| "response": { | ||
| "error": { | ||
| "code": -32000, | ||
| "message": "eip-1559 transactions require London" |
There was a problem hiding this comment.
The error message changed "eip-1559 transactions require london" to "eip-1559 transactions require London" (capital L). This is a minor string difference but could cause test failures if the comparison is case-sensitive. Ensure this matches the actual error message returned by the server being tested.
Comment on lines
+60
to
+61
| "description": "", | ||
| "reference": "" |
There was a problem hiding this comment.
A new file debug_traceCallMany/test_02.json was added, but there's also a debug_traceCallMany/test_02.tar file (shown in file_contents but not in the diff). Having both a .json and .tar file with the same test number could cause confusion in test discovery. Clarify whether both files should exist or if one should be removed.
Suggested change
| "description": "", | |
| "reference": "" | |
| "description": "Canonical debug_traceCallMany test_02 JSON fixture (supersedes any legacy test_02.tar variant).", | |
| "reference": "See repository history for prior test_02.tar archive; this JSON file is the authoritative test_02 definition." |
JsonDiffGo handles unordered array comparison natively via SortArrays, eliminating the need for metadata.response.pathOptions in test fixtures and removing an external dependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
refactoring
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
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.
No description provided.