feat: implement JSON-RPC 2.0 server using aiohttp#98
Conversation
WalkthroughAdds chain reorganization support to ChangesChain Reorg, JSON-RPC Server, P2P Sync, and Mempool Refactor
Sequence Diagram(s)sequenceDiagram
participant PeerNode
participant P2PNetwork
participant make_network_handler
participant Blockchain
participant Mempool
participant JSONRPCClient
PeerNode->>P2PNetwork: block message (ahead of local tip)
P2PNetwork->>make_network_handler: dispatch block
make_network_handler->>P2PNetwork: broadcast_chain_request()
P2PNetwork->>PeerNode: chain_request message
PeerNode->>P2PNetwork: chain_response{blocks:[...]}
P2PNetwork->>make_network_handler: dispatch chain_response
make_network_handler->>Blockchain: resolve_conflicts(new_chain_list)
Blockchain-->>make_network_handler: (True, orphaned_txs)
make_network_handler->>Mempool: add_transaction(orphan) for each orphan
JSONRPCClient->>JSONRPCServer: POST / mc_sendTransaction
JSONRPCServer->>Mempool: add_transaction(tx)
JSONRPCServer->>P2PNetwork: broadcast tx
JSONRPCServer-->>JSONRPCClient: JSON-RPC result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main.py`:
- Around line 169-173: The chain_request handler in main.py (lines 169-173)
currently broadcasts the chain response to all connected peers via
_broadcast_raw, which is wasteful and creates an amplification attack vector.
Instead of broadcasting, the handler should send the chain response only to the
peer that requested it. Modify the handler to access the writer object for the
requesting peer (either by having it passed through the callback or by
maintaining a peer address to writer mapping in the network module), and then
call a targeted send method (such as send_chain_response) that writes directly
to that specific peer's connection rather than using _broadcast_raw. This
ensures the response reaches only the requester.
- Around line 467-470: The rpc_task.cancel() call schedules cancellation but
does not await its completion, allowing network.stop() to execute while the RPC
server is still shutting down and causing race conditions. Replace the
non-awaited rpc_task.cancel() with a proper awaited cancellation that waits for
the task to complete before proceeding. This typically involves wrapping the
cancellation in a try-except block to handle CancelledError, then awaiting the
task completion. Additionally, consider implementing a proper rpc_server.stop()
method in minichain/rpc.py that cleanly shuts down aiohttp resources before
network.stop() is called.
- Around line 438-441: The RPC port calculation using `8545 + (port - 9000)` can
produce invalid port numbers when the P2P port argument is less than 9000,
resulting in negative or unexpectedly low port values. Fix this by adding
validation to ensure the calculated rpc_port value falls within the valid port
range (1-65535), either by clamping the result to a minimum valid port or by
implementing a separate --rpc-port command-line argument instead of deriving it
from the P2P port. At minimum, clamp rpc_port to ensure it stays within valid
bounds before passing it to rpc_server.start().
In `@minichain/chain.py`:
- Around line 233-234: The assignment of new_chain_list to self.chain at the
chain update location creates an alias that exposes the blockchain to external
mutations. Instead of directly assigning new_chain_list to self.chain, create a
defensive copy of the incoming list (for example, using list slicing or the copy
module) to ensure that any future modifications to new_chain_list by the caller
will not corrupt the internal blockchain state.
- Around line 188-190: The variables state_snapshot and original_chain are
created as defensive copies but are never referenced anywhere in the code.
Remove the two lines that create these snapshots (the assignments to
state_snapshot and original_chain) since they serve no purpose when temp_state
is a fresh instance and self.state/self.chain are only reassigned on the success
path.
In `@minichain/mempool.py`:
- Around line 48-51: Replace the nested `else: if` structure with `elif` to
reduce indentation nesting in the mempool full check logic. Specifically,
convert the `else:` statement followed by `if len(self._list) >= self.max_size:`
into a single `elif len(self._list) >= self.max_size:` block, keeping the
logger.warning and return False statements at the appropriate indentation level.
- Line 53: The line `i_min = min(i_min, i_max)` silently masks a broken
invariant in the per-sender nonce ordering of `self._list`. When `i_min >
i_max`, this indicates the invariant has already been violated, but clamping the
value hides this corruption and allows the code to continue in an invalid state.
Instead of silently clamping, add a check before this line to detect when `i_min
> i_max` and either raise an assertion error or log a warning to fail fast and
make the invariant violation visible during debugging. This ensures invariant
violations are caught and reported rather than silently masked.
In `@minichain/rpc.py`:
- Around line 76-80: The asyncio.create_task() call for
self.network.broadcast_transaction(tx) creates a fire-and-forget task without
storing a reference or handling potential exceptions, which means broadcast
failures will silently fail in the background without being tracked. Store a
reference to the task (for example, in an instance variable like a set of
pending tasks) or add a done callback to the task to capture and handle any
exceptions that occur during the broadcast operation. This will ensure broadcast
failures are properly tracked and logged rather than being silently swallowed by
the asyncio event loop.
- Around line 17-22: The JSON-RPC server lacks proper graceful shutdown, causing
potential resource leaks. At minichain/rpc.py lines 17-22, in the start()
method, store the AppRunner instance created as self._runner instead of using a
local variable, then add a new async stop() method to the JSONRPCServer class
that calls await self._runner.cleanup() to properly clean up aiohttp resources.
At main.py lines 467-470, replace the rpc_task.cancel() call with proper
shutdown by awaiting the task cancellation and then calling await
rpc_server.stop() before invoking network.stop() to ensure the RPC server shuts
down gracefully before the network is stopped.
In `@tests/test_reorg.py`:
- Around line 10-12: Remove the sys.path manipulation block at the start of the
test_reorg.py file (the lines containing import sys, import os, and
sys.path.insert). These manual path adjustments are unnecessary when tests run
from the project root with pytest, which will properly discover modules from the
standard project structure. If sys and os are not used elsewhere in the file,
remove those import statements as well.
In `@tests/test_rpc.py`:
- Line 41: In the unpacking assignment where rpc_server is destructured, prefix
the unused variables with an underscore to follow Python conventions. Keep only
the `port` variable without an underscore since it is actually used, and rename
`server`, `chain`, and `mempool` to `_server`, `_chain`, and `_mempool`
respectively.
- Line 29: In the tuple unpacking statement at the rpc_server assignment, only
the `port` variable is used in the subsequent code. Prefix the unused variables
(`server`, `chain`, and `mempool`) with an underscore (e.g., `_server`,
`_chain`, `_mempool`) to explicitly signal that these variables are
intentionally unused, which will satisfy linter requirements and improve code
clarity. Keep `port` without the underscore prefix since it is actively used.
- Line 53: In the unpacking assignment where `server, port, chain, mempool =
rpc_server`, only the `port` variable is used in the function. Prefix the unused
variables `server`, `chain`, and `mempool` with an underscore to follow Python
convention for unused variables. Change the line to `_server, port, _chain,
_mempool = rpc_server`.
- Around line 9-11: The anyio_backend fixture and `@pytest.mark.anyio` decorator
used in the test file require the pytest-anyio package, but this dependency is
not currently included in the project's dependencies file. Add pytest-anyio to
the project's dependency list (requirements.txt or equivalent dependency
management file) to ensure the test suite has access to the necessary
pytest-anyio plugin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a0100f89-ca57-4136-95e0-c35fd05e30c8
📒 Files selected for processing (8)
main.pyminichain/chain.pyminichain/mempool.pyminichain/p2p.pyminichain/rpc.pyminichain/state.pytests/test_reorg.pytests/test_rpc.py
| elif msg_type == "chain_request": | ||
| logger.info("📡 Peer requested chain sync. Broadcasting our chain...") | ||
| blocks_dicts = [b.to_dict() for b in chain.chain] | ||
| payload = {"type": "chain_response", "data": {"blocks": blocks_dicts}} | ||
| asyncio.create_task(network._broadcast_raw(payload)) |
There was a problem hiding this comment.
Chain response broadcasts to all peers instead of just the requester — amplification risk.
When handling chain_request, the code broadcasts the entire chain to all connected peers via _broadcast_raw, not just the peer who requested it. This is:
- Wasteful — peers who didn't request the chain receive it anyway
- Potentially exploitable — a malicious peer could trigger large broadcasts to all other peers (amplification attack)
The root cause is that the handler only receives _peer_addr (a string), not the writer object needed to call send_chain_response. Consider passing the writer through the handler callback or storing a peer address → writer mapping.
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 173-173: Store a reference to the return value of asyncio.create_task
(RUF006)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 169 - 173, The chain_request handler in main.py (lines
169-173) currently broadcasts the chain response to all connected peers via
_broadcast_raw, which is wasteful and creates an amplification attack vector.
Instead of broadcasting, the handler should send the chain response only to the
peer that requested it. Modify the handler to access the writer object for the
requesting peer (either by having it passed through the callback or by
maintaining a peer address to writer mapping in the network module), and then
call a targeted send method (such as send_chain_response) that writes directly
to that specific peer's connection rather than using _broadcast_raw. This
ensures the response reaches only the requester.
|
|
||
| # Start RPC server on a port correlated to the node port (e.g. 8545 if P2P is 9000) | ||
| rpc_port = 8545 + (port - 9000) | ||
| rpc_task = asyncio.create_task(rpc_server.start(host="127.0.0.1", port=rpc_port)) |
There was a problem hiding this comment.
RPC port calculation can produce invalid or unexpected ports.
The formula 8545 + (port - 9000) assumes P2P port is ≥9000. If a user runs with --port 100, the RPC port becomes -355, which will fail. Even valid-but-low ports like --port 8000 yield 7545, which may not match user expectations.
Consider adding a separate --rpc-port argument or at minimum clamping the port to a valid range.
Proposed fix with validation
# Start RPC server on a port correlated to the node port (e.g. 8545 if P2P is 9000)
rpc_port = 8545 + (port - 9000)
+ if rpc_port < 1 or rpc_port > 65535:
+ rpc_port = 8545 # fallback to default
+ logger.warning("Computed RPC port out of range, using default %d", rpc_port)
rpc_task = asyncio.create_task(rpc_server.start(host="127.0.0.1", port=rpc_port))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 438 - 441, The RPC port calculation using `8545 + (port
- 9000)` can produce invalid port numbers when the P2P port argument is less
than 9000, resulting in negative or unexpectedly low port values. Fix this by
adding validation to ensure the calculated rpc_port value falls within the valid
port range (1-65535), either by clamping the result to a minimum valid port or
by implementing a separate --rpc-port command-line argument instead of deriving
it from the P2P port. At minimum, clamp rpc_port to ensure it stays within valid
bounds before passing it to rpc_server.start().
|
|
||
| if rpc_task: | ||
| rpc_task.cancel() | ||
| await network.stop() |
There was a problem hiding this comment.
RPC task cancellation is not awaited — incomplete cleanup.
Calling rpc_task.cancel() schedules cancellation but doesn't wait for it to complete. The network.stop() call may execute while the RPC server is still running, leading to race conditions during shutdown.
Proposed fix
if rpc_task:
rpc_task.cancel()
+ try:
+ await rpc_task
+ except asyncio.CancelledError:
+ pass
await network.stop()Ideally, combine this with a proper rpc_server.stop() method that cleans up aiohttp resources (see related comment in minichain/rpc.py).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if rpc_task: | |
| rpc_task.cancel() | |
| await network.stop() | |
| if rpc_task: | |
| rpc_task.cancel() | |
| try: | |
| await rpc_task | |
| except asyncio.CancelledError: | |
| pass | |
| await network.stop() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 467 - 470, The rpc_task.cancel() call schedules
cancellation but does not await its completion, allowing network.stop() to
execute while the RPC server is still shutting down and causing race conditions.
Replace the non-awaited rpc_task.cancel() with a proper awaited cancellation
that waits for the task to complete before proceeding. This typically involves
wrapping the cancellation in a try-except block to handle CancelledError, then
awaiting the task completion. Additionally, consider implementing a proper
rpc_server.stop() method in minichain/rpc.py that cleanly shuts down aiohttp
resources before network.stop() is called.
| # 2. Snapshot current state and chain in case reorg fails validation | ||
| state_snapshot = self.state.snapshot() | ||
| original_chain = list(self.chain) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Unused snapshots can be removed.
state_snapshot and original_chain are captured but never used. Since temp_state is a fresh instance and self.state/self.chain are only reassigned on the success path (lines 233-234), these defensive copies serve no purpose.
🧹 Remove dead code
logger.info("Incoming chain is heavier (%s > %s). Attempting reorg...", new_work, current_work)
- # 2. Snapshot current state and chain in case reorg fails validation
- state_snapshot = self.state.snapshot()
- original_chain = list(self.chain)
-
- # 3. Rebuild state entirely from genesis using the new chain
+ # 2. Rebuild state entirely from genesis using the new chain
temp_state = State()
temp_state.restore(self._genesis_state_snapshot)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # 2. Snapshot current state and chain in case reorg fails validation | |
| state_snapshot = self.state.snapshot() | |
| original_chain = list(self.chain) | |
| # 2. Rebuild state entirely from genesis using the new chain | |
| temp_state = State() | |
| temp_state.restore(self._genesis_state_snapshot) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minichain/chain.py` around lines 188 - 190, The variables state_snapshot and
original_chain are created as defensive copies but are never referenced anywhere
in the code. Remove the two lines that create these snapshots (the assignments
to state_snapshot and original_chain) since they serve no purpose when
temp_state is a fresh instance and self.state/self.chain are only reassigned on
the success path.
| self.chain = new_chain_list | ||
| self.state = temp_state |
There was a problem hiding this comment.
Consider copying the incoming chain to avoid aliasing.
Directly assigning new_chain_list to self.chain creates a shared reference. If the caller retains and later mutates new_chain_list, it would corrupt the blockchain state. A defensive copy is cheap and eliminates this risk.
🛡️ Proposed fix
- self.chain = new_chain_list
+ self.chain = list(new_chain_list)
self.state = temp_state📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.chain = new_chain_list | |
| self.state = temp_state | |
| self.chain = list(new_chain_list) | |
| self.state = temp_state |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minichain/chain.py` around lines 233 - 234, The assignment of new_chain_list
to self.chain at the chain update location creates an alias that exposes the
blockchain to external mutations. Instead of directly assigning new_chain_list
to self.chain, create a defensive copy of the incoming list (for example, using
list slicing or the copy module) to ensure that any future modifications to
new_chain_list by the caller will not corrupt the internal blockchain state.
| import sys | ||
| import os | ||
| sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Remove sys.path manipulation; rely on proper project structure.
Manual path manipulation is unnecessary when tests run from the project root. Pytest will discover main.py if the project follows standard Python package structure.
♻️ Proposed fix
-import sys
-import os
-sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..')))
from main import mine_and_process_block🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_reorg.py` around lines 10 - 12, Remove the sys.path manipulation
block at the start of the test_reorg.py file (the lines containing import sys,
import os, and sys.path.insert). These manual path adjustments are unnecessary
when tests run from the project root with pytest, which will properly discover
modules from the standard project structure. If sys and os are not used
elsewhere in the file, remove those import statements as well.
| @pytest.fixture | ||
| def anyio_backend(): | ||
| return 'asyncio' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if pytest-anyio is declared in project dependencies
# Search for pytest-anyio in common dependency files
rg -i 'pytest-anyio' -g 'requirements*.txt' -g 'pyproject.toml' -g 'setup.py' -g 'setup.cfg' -g 'Pipfile'Repository: StabilityNexus/MiniChain
Length of output: 50
🏁 Script executed:
# Check what dependencies are actually declared
fd -e 'pyproject.toml' -o -e 'setup.py' -o -e 'requirements.txt' | head -20Repository: StabilityNexus/MiniChain
Length of output: 185
🏁 Script executed:
# Look for content of main dependency files
cat pyproject.toml 2>/dev/null || cat setup.py 2>/dev/null || cat requirements.txt 2>/dev/nullRepository: StabilityNexus/MiniChain
Length of output: 94
🏁 Script executed:
# Search for anyio imports and pytest_anyio references in the codebase
rg 'import.*anyio|from.*anyio|pytest.mark.anyio|pytest_anyio' --type python -iRepository: StabilityNexus/MiniChain
Length of output: 99
🏁 Script executed:
# Check if conftest.py exists and what's in it
find . -name 'conftest.py' -exec head -30 {} \;Repository: StabilityNexus/MiniChain
Length of output: 152
🏁 Script executed:
# Search for anyio-related imports correctly
rg 'import.*anyio|from.*anyio|pytest\.mark\.anyio' --type-listRepository: StabilityNexus/MiniChain
Length of output: 458
🏁 Script executed:
# Check test_rpc.py content
cat tests/test_rpc.py | head -50Repository: StabilityNexus/MiniChain
Length of output: 1650
🏁 Script executed:
# Search for anyio or pytest_anyio in all files
rg 'anyio|pytest_anyio' -iRepository: StabilityNexus/MiniChain
Length of output: 218
🏁 Script executed:
# Check if anyio is in any dependency file
rg 'anyio' requirements*.txt pyproject.toml setup.py setup.cfg 2>/dev/nullRepository: StabilityNexus/MiniChain
Length of output: 50
Add pytest-anyio to project dependencies.
The @pytest.mark.anyio decorator and anyio_backend fixture in test_rpc.py require the pytest-anyio package, which is not listed in requirements.txt. These tests will fail without it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_rpc.py` around lines 9 - 11, The anyio_backend fixture and
`@pytest.mark.anyio` decorator used in the test file require the pytest-anyio
package, but this dependency is not currently included in the project's
dependencies file. Add pytest-anyio to the project's dependency list
(requirements.txt or equivalent dependency management file) to ensure the test
suite has access to the necessary pytest-anyio plugin.
|
|
||
| @pytest.mark.anyio | ||
| async def test_rpc_blockNumber(rpc_server): | ||
| server, port, chain, mempool = rpc_server |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use underscore prefix for unused unpacked variables.
Only port is used; prefix unused variables with _ to signal intent and silence linter warnings.
♻️ Proposed fix
- server, port, chain, mempool = rpc_server
+ _server, port, _chain, _mempool = rpc_server📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| server, port, chain, mempool = rpc_server | |
| _server, port, _chain, _mempool = rpc_server |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 29-29: Unpacked variable server is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 29-29: Unpacked variable chain is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 29-29: Unpacked variable mempool is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_rpc.py` at line 29, In the tuple unpacking statement at the
rpc_server assignment, only the `port` variable is used in the subsequent code.
Prefix the unused variables (`server`, `chain`, and `mempool`) with an
underscore (e.g., `_server`, `_chain`, `_mempool`) to explicitly signal that
these variables are intentionally unused, which will satisfy linter requirements
and improve code clarity. Keep `port` without the underscore prefix since it is
actively used.
Source: Linters/SAST tools
|
|
||
| @pytest.mark.anyio | ||
| async def test_rpc_getBlockByNumber(rpc_server): | ||
| server, port, chain, mempool = rpc_server |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use underscore prefix for unused unpacked variables.
Only port is used; prefix unused variables with _.
♻️ Proposed fix
- server, port, chain, mempool = rpc_server
+ _server, port, _chain, _mempool = rpc_server📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| server, port, chain, mempool = rpc_server | |
| _server, port, _chain, _mempool = rpc_server |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 41-41: Unpacked variable server is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 41-41: Unpacked variable chain is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 41-41: Unpacked variable mempool is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_rpc.py` at line 41, In the unpacking assignment where rpc_server
is destructured, prefix the unused variables with an underscore to follow Python
conventions. Keep only the `port` variable without an underscore since it is
actually used, and rename `server`, `chain`, and `mempool` to `_server`,
`_chain`, and `_mempool` respectively.
Source: Linters/SAST tools
|
|
||
| @pytest.mark.anyio | ||
| async def test_rpc_invalid_method(rpc_server): | ||
| server, port, chain, mempool = rpc_server |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use underscore prefix for unused unpacked variables.
Only port is used; prefix unused variables with _.
♻️ Proposed fix
- server, port, chain, mempool = rpc_server
+ _server, port, _chain, _mempool = rpc_server🧰 Tools
🪛 Ruff (0.15.15)
[warning] 53-53: Unpacked variable server is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 53-53: Unpacked variable chain is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
[warning] 53-53: Unpacked variable mempool is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_rpc.py` at line 53, In the unpacking assignment where `server,
port, chain, mempool = rpc_server`, only the `port` variable is used in the
function. Prefix the unused variables `server`, `chain`, and `mempool` with an
underscore to follow Python convention for unused variables. Change the line to
`_server, port, _chain, _mempool = rpc_server`.
Source: Linters/SAST tools
1 participant
Addressed Issues:
minichain/rpc.pyconfiguring an asynchronous HTTP server powered byaiohttp.mc_blockNumber,mc_getBlockByNumber,mc_getBalance, andmc_sendTransactionwith proper JSON-RPC error codes.JSONRPCServerinto the mainasyncioevent loop inmain.py, bound dynamically to port 8545 (or offset by base P2P port).mc_sendTransactiondeeply into theMempoolqueue, automatically broadcasting valid payloads to the P2P network.tests/test_rpc.pyenforcing correct error handling and JSON parsing.Screenshots/Recordings:
TODO: If applicable, add screenshots or recordings that demonstrate the interface before and after the changes.
Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Checklist
Summary by CodeRabbit
Release Notes
New Features
Tests