Skip to content

Part 5 - jsonrpc: replace jsonrpcserver/jsonrpcclient with custom implementation#971

Open
ping-ke wants to merge 13 commits intoupgrade/py313-baselinefrom
upgrade/jsonrpc
Open

Part 5 - jsonrpc: replace jsonrpcserver/jsonrpcclient with custom implementation#971
ping-ke wants to merge 13 commits intoupgrade/py313-baselinefrom
upgrade/jsonrpc

Conversation

@ping-ke
Copy link
Copy Markdown
Contributor

@ping-ke ping-ke commented Mar 15, 2026

Summary

jsonrpcserver 3.x and jsonrpcclient 2.x are no longer maintained
and are incompatible with Python 3.13.

  • Add quarkchain/cluster/jsonrpcserver.py: a lightweight JSON-RPC 2.0
    server dispatcher built on aiohttp (RpcMethods, JsonRpcError hierarchy,
    positional and named parameter dispatch)
  • Add quarkchain/jsonrpc_client.py: JsonRpcClient (synchronous, httpx-based)
    and AsyncJsonRpcClient (async, aiohttp-based) replacing the old clients
  • Update jsonrpc.py to use the new RpcMethods class and make
    start_*_server() classmethods async
  • Replace jsonrpcclient calls in all tools and prom.py with JsonRpcClient.call()
  • Rename quarkchain/tools/statsquarkchain/tools/stats.py (was missing extension)
  • Update test_jsonrpc.py to use AsyncJsonRpcClient

Test plan

  • pytest quarkchain/cluster/tests/test_jsonrpc.py passes
  • Public and private JSON-RPC endpoints respond correctly
  • external_miner.py can fetch work and submit results

QuarkChain Dev and others added 2 commits March 13, 2026 04:09
The jsonrpcserver (3.x) and jsonrpcclient (2.x) packages are
incompatible with Python 3.13 and are no longer maintained.

Changes:
- Add quarkchain/cluster/jsonrpcserver.py: lightweight custom
  JSON-RPC 2.0 server built on aiohttp (RpcMethods, JsonRpcError
  hierarchy, request dispatch with positional/named params)
- Add quarkchain/jsonrpc_client.py: synchronous JsonRpcClient
  (uses httpx) and asynchronous AsyncJsonRpcClient (uses aiohttp)
  replacing jsonrpcclient.HTTPClient / aiohttpClient
- Update quarkchain/cluster/jsonrpc.py to use the new RpcMethods
  class; make start_*_server() classmethod async; use get_running_loop()
- Replace jsonrpcclient calls in all tools with JsonRpcClient.call()
- Rename quarkchain/tools/stats → quarkchain/tools/stats.py
- Remove jsonrpcserver config log suppression (no longer needed)
- Update test_jsonrpc.py to use AsyncJsonRpcClient

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- adjust_difficulty.py: replace bare `import monitoring` with
  `from quarkchain.tools import monitoring` (breaks when run from
  outside the tools directory)
- adjust_difficulty.py: replace jsonrpc_async.Server with
  AsyncJsonRpcClient and use .call() method
- monitoring.py: replace jsonrpc_async.Server with AsyncJsonRpcClient
  and use .call() method; close session via .close()
- jsonrpc_client.py: fix AsyncJsonRpcClient.call signature to use
  *params (variadic) to match JsonRpcClient.call, fixing callers
  that pass positional arguments

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ping-ke ping-ke changed the title jsonrpc: replace jsonrpcserver/jsonrpcclient with custom implementation Part 5 - jsonrpc: replace jsonrpcserver/jsonrpcclient with custom implementation Mar 15, 2026
ping-ke and others added 3 commits March 16, 2026 23:25
- Fix send_request in test_jsonrpc.py to unpack list params correctly
  instead of double-wrapping them (e.g. [["0x..."]] → ["0x..."])
- Add call_with_dict_params to AsyncJsonRpcClient for named params
- Implement JSONRPCWebsocketServer.shutdown() to actually close the
  server, fixing test isolation hangs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ping-ke ping-ke marked this pull request as draft March 17, 2026 18:04
@ping-ke ping-ke marked this pull request as ready for review March 18, 2026 16:42
@ping-ke ping-ke requested review from qizhou, qzhodl and syntrust March 18, 2026 16:43
@qzhodl
Copy link
Copy Markdown
Contributor

qzhodl commented Mar 31, 2026

Code review

Found 2 issues:

  1. Typo: client.client() instead of client.call() in stats.py -- basic() calls client.client("getStats") but JsonRpcClient.client is the internal httpx.Client instance, not a callable RPC method. This will raise TypeError at runtime. The stats() function on line 35 correctly uses client.call("getStats").

def basic(client: JsonRpcClient, ip):
s = client.client("getStats")
msg = "QuarkChain Cluster Stats\n\n"

  1. asyncio.get_running_loop() called from synchronous __init__ -- Both JSONRPCHttpServer.__init__ (line 509) and JSONRPCWebsocketServer.__init__ (line 1462) call asyncio.get_running_loop(). These constructors are invoked from synchronous main() in master.py and slave.py before the event loop is running, which will raise RuntimeError: no running event loop. The previous asyncio.get_event_loop() works in both sync and async contexts.

def __init__(
self, env, master_server: MasterServer, port, host, methods: RpcMethods
):
self.loop = asyncio.get_running_loop()
self.port = port

def __init__(
self, env, slave_server: SlaveServer, port, host, methods: RpcMethods
):
self.loop = asyncio.get_running_loop()
self.port = port

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…sues

- Rename jsonrpcserver.py to jsonrpc_server.py for consistent naming
- Replace armor with asyncio.shield for cancellation protection
- Fix get_running_loop to get_event_loop for compatibility
- Fix subscription.py import from old jsonrpcserver package
- Fix indentation in test_jsonrpc.py and method name in stats.py
- Fix snake_case field names to camelCase for JSON-RPC responses
  (network_id->networkId, shard_size->chainSize, block_height->blockHeight,
   contract_address->contractAddress)
- Fix resp.data.result access pattern to direct dict access
- Add exception handling and cli.close() to prevent connection leaks
- Add 0x prefix for address in balance_watcher query_balance
@ping-ke ping-ke requested a review from qzhodl March 31, 2026 16:35
@ping-ke ping-ke changed the base branch from upgrade/py313-baseline to master April 5, 2026 02:43
@ping-ke ping-ke changed the base branch from master to upgrade/py313-baseline April 5, 2026 02:44
@ping-ke ping-ke requested a review from syntrust April 6, 2026 12:14
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.

3 participants