feat: implement faucet command [TOR-30]#6
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the """ WalkthroughThe changes implement and enable the faucet functionality in the Python CLI, refactor the proof-of-work (PoW) solver to use a more secure and robust multiprocessing approach, update cryptographic and block-handling logic, and adjust command-line interactions. Minor improvements and cleanups are also made in related modules and configuration files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant FaucetModule
participant PoWSolver
participant TorusClient
User->>CLI: Run faucet command
CLI->>PoWSolver: Start PoW solution process
PoWSolver->>TorusClient: Subscribe to block headers (WebSocket)
loop Until valid PoW solution found
PoWSolver->>TorusClient: Fetch current block info
PoWSolver->>PoWSolver: Attempt PoW solution
end
PoWSolver-->>CLI: Return valid solution
CLI->>FaucetModule: Submit solution via compose_call
FaucetModule-->>CLI: Confirm faucet execution
CLI->>User: Output result, repeat if necessary
Assessment against linked issues
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
src/torusdk/compat/storage.py (1)
64-70:⚠️ Potential issueCritical: Inconsistent return value between encrypted and non-encrypted paths
The change to return the entire
bodydictionary instead ofbody["data"]creates an inconsistency:
- Encrypted path (line 63): Returns just the decrypted data content
- Non-encrypted path (line 65): Now returns the entire body including metadata fields
This breaking change will cause issues in
key.pywhereclassic_load_keyexpects to receive the key dictionary directly, not wrapped in a body structure.Apply this diff to maintain consistent behavior:
- content = body + content = body["data"]src/torusdk/compat/key.py (1)
136-142: 💡 Verification agent🧩 Analysis chain
Verify compatibility with storage.py changes
The removal of
json.loads()assumes thatclassic_loadreturns a dictionary directly. However, based on the changes instorage.py, if the non-encrypted path returns the entire body structure instead of just the data field, this will fail atcheck_key_dict.Run this script to verify the structure returned by
classic_load:
🏁 Script executed:
#!/bin/bash # Description: Check how classic_load is used across the codebase to understand expected return structure # Search for other uses of classic_load to understand the expected return type rg -A 5 "classic_load\(" --type py # Check if there are any other JSON parsing after classic_load calls rg -B 2 -A 2 "json\.loads.*classic_load" --type pyLength of output: 814
I need to inspect the body of
classic_loadto confirm what it actually returns in the non-encrypted case. Let’s pull the function implementation fromstorage.py:
🏁 Script executed:
#!/bin/bash # Print the body of classic_load to see its return shape file="src/torusdk/compat/storage.py" start=$(grep -n '^def classic_load' "$file" | cut -d: -f1) tail -n +$start "$file" | head -n 200Length of output: 2922
I want to confirm how
from_classic_dictvalidates its input dict—specifically what keys it expects—so we can see if the wrapper returned byclassic_loadbreaks it. Let me pull in its implementation:
🏁 Script executed:
#!/bin/bash # Extract the from_classic_dict (and any related check_key_dict) from key.py file="src/torusdk/compat/key.py" echo "---- from_classic_dict ----" start=$(grep -n -E '^def from_classic_dict' "$file" | cut -d: -f1) tail -n +$start "$file" | sed -n '1,200p' echo echo "---- check_key_dict (if present) ----" grep -R -nE '^def check_key_dict' -n "$file" | cut -d: -f1 | while read -r ln; do tail -n +$ln "$file" | sed -n '1,200p' doneLength of output: 12381
Unwrap the stored payload in
classic_loadto preserve the original data shapeCurrently,
classic_loadreturns the entire storage body ({"data":…, "encrypted":…, "timestamp":…}) when no password is provided (and likewise returns a raw JSON‐string on decryption), which will failcheck_key_dict. You need to return the actual stored object (thedatafield) and parse decrypted JSON.• In
src/torusdk/compat/storage.py, change theclassic_loadreturn logic:
- Return
body["data"]instead ofbodyin the non‐encrypted branch.- Deserialize the decrypted payload into a dict before returning.
Suggested diff:
--- a/src/torusdk/compat/storage.py @@ def classic_load( - if body["encrypted"] and password is not None: - content = decrypt_data(password, body["data"]) - else: - content = body + if body["encrypted"] and password is not None: + # decrypt_data returns a JSON string of the original data + content = json.loads(decrypt_data(password, body["data"])) + else: + # unwrap the stored payload + content = body["data"] @@ def classic_load( - assert isinstance(body, dict) + assert isinstance(body, dict) assert isinstance(body["timestamp"], int) - assert isinstance(content, (dict, list, tuple, set, float, str, int)) - return content # type: ignore + # content must now be the original stored value + assert isinstance(content, (dict, list, tuple, set, float, str, int)) + return content # type: ignoreThis ensures
classic_loadhands off exactly the dict thatfrom_classic_dictexpects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/torusdk/cli/balance.py(1 hunks)src/torusdk/compat/key.py(2 hunks)src/torusdk/compat/storage.py(1 hunks)src/torusdk/faucet/powv2.py(10 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/torusdk/compat/key.py (1)
src/torusdk/compat/storage.py (1)
classic_load(29-70)
🪛 GitHub Actions: Type checker
src/torusdk/faucet/powv2.py
[error] 1-19: Ruff linting error: Import block is un-sorted or un-formatted. Organize imports. 1 fixable issue with --fix option.
🔇 Additional comments (5)
src/torusdk/cli/balance.py (1)
267-268: LGTM!The module name update from
"FaucetModule"to"Faucet"correctly aligns with the blockchain module naming.src/torusdk/faucet/powv2.py (4)
238-240: Good security improvement with secure random nonce generationUsing
os.urandominstead of therandommodule for nonce generation is a security best practice, as it provides cryptographically secure random numbers.Also applies to: 262-264
291-321: Excellent refactor to WebSocket-based block updatesThe change from polling with sleep intervals to WebSocket subscription is a significant improvement:
- More efficient and responsive to new blocks
- Reduces unnecessary API calls
- Provides real-time updates
478-479: Verify performance impact of increased update intervalThe update interval was increased from 50,000 to 500,000 (10x increase). While this reduces overhead, it might affect:
- How quickly solutions are found
- The granularity of progress tracking
Consider making this configurable or documenting the rationale for this specific value.
521-530: Good addition of stale solution validationThe check to ensure solutions are within 3 blocks of the current block prevents submitting outdated proof-of-work solutions. This is a crucial validation that prevents wasted transactions.
src/torusdk/faucet/powv2.py
Outdated
| import hashlib | ||
| import math | ||
| import multiprocessing | ||
| import multiprocessing.queues | ||
| import multiprocessing.synchronize | ||
| import os | ||
| import random | ||
| import threading | ||
| from abc import abstractmethod | ||
| from dataclasses import dataclass | ||
| from queue import Empty | ||
| from time import sleep | ||
| from typing import Generic, Optional, TypeVar, cast | ||
| from typing import Generic, Optional, TypeVar, cast, Any, Dict |
There was a problem hiding this comment.
Fix import sorting to pass linting checks
The pipeline reports that imports are not properly sorted.
Run ruff with the --fix option or manually sort the imports to resolve the linting error:
ruff check --fix src/torusdk/faucet/powv2.py🧰 Tools
🪛 GitHub Actions: Type checker
[error] 1-19: Ruff linting error: Import block is un-sorted or un-formatted. Organize imports. 1 fixable issue with --fix option.
🤖 Prompt for AI Agents
In src/torusdk/faucet/powv2.py lines 1 to 11, the imports are not sorted
according to linting rules. Fix this by sorting all import statements
alphabetically and grouping standard library imports separately from third-party
or local imports. You can run the command `ruff check --fix
src/torusdk/faucet/powv2.py` to automatically sort and fix the imports or
manually reorder them to comply with the linting standards.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/torusdk/cli/balance.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/torusdk/cli/balance.py (2)
src/torusdk/client.py (1)
compose_call(837-914)src/torusdk/cli/_common.py (1)
info(188-194)
🪛 GitHub Actions: Type checker
src/torusdk/cli/balance.py
[error] 1-1: Ruff import sorting error: Import block is un-sorted or un-formatted. Organize imports.
🔇 Additional comments (2)
src/torusdk/cli/balance.py (2)
249-249: Good change to enable conditional logic.Changing the loop variable from
_toiis appropriate since you need to reference the iteration number for the conditional waiting logic.
274-276: Excellent rate limiting implementation.The conditional logic to wait 5 seconds between executions (except for the last one) is a thoughtful addition for rate limiting and preventing spam on the faucet.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/torusdk/faucet/powv2.py (1)
1-18: Fix import sorting to pass linting checks.The imports are not properly sorted according to linting rules.
🧹 Nitpick comments (1)
src/torusdk/key.py (1)
118-119: Document the specific type error being suppressed.The comment "Weird type error" is not descriptive enough. Please update it to explain what specific type error is occurring and why suppressing it is the appropriate solution.
Consider updating the comment to be more descriptive:
- # ? Weird type error - return ss58.is_valid_ss58_address(address, valid_ss58_format=ss58_format) # type: ignore + # Type error: <specific error message here> + # Suppressed because: <reason why it's safe to ignore> + return ss58.is_valid_ss58_address(address, valid_ss58_format=ss58_format) # type: ignore
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.vscode/settings.json(1 hunks)src/torusdk/cli/balance.py(3 hunks)src/torusdk/faucet/powv2.py(14 hunks)src/torusdk/key.py(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/torusdk/cli/balance.py
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint
- GitHub Check: tests
🔇 Additional comments (9)
src/torusdk/key.py (1)
8-8: Import style change looks good.The aliased import achieves the same functionality while being more explicit about the module structure.
src/torusdk/faucet/powv2.py (8)
49-52: Good improvement to support different multiprocessing contexts.The optional
mp_contextparameter provides flexibility in choosing the multiprocessing start method, which is important for cross-platform compatibility and avoiding serialization issues.
105-198: Excellent refactoring from inheritance to composition.The change from subclassing
multiprocessing.Processto encapsulating it provides better design flexibility and avoids potential inheritance issues. The implementation correctly delegates all necessary methods.
266-267: Critical security improvement: using cryptographically secure randomness.Switching from
random.randinttoos.urandomfor nonce generation is an essential security improvement. This ensures unpredictable nonce values that cannot be easily guessed or reproduced.Also applies to: 290-292
313-358: Excellent improvement: WebSocket subscription for real-time block updates.The switch from polling to WebSocket subscription is a significant improvement in efficiency and responsiveness. This eliminates unnecessary delays and provides immediate notification of new blocks.
412-416: Good simplification of seal hash creation.Removing unnecessary hex encoding/decoding steps makes the code more efficient and easier to understand while maintaining the same cryptographic functionality.
516-516: Verify the 10x increase in update interval is optimal.The default
update_intervalhas been increased from 50,000 to 500,000. While this may reduce overhead, it also means less frequent progress updates. Please ensure this value has been tested and provides the right balance between performance and responsiveness.
520-522: Consider platform compatibility with 'fork' context.The 'fork' multiprocessing context is not available on Windows and can have issues with threaded code. While the comment claims it works on Apple Silicon, this limits the code's portability.
Consider making the context configurable or detecting the platform:
# Use 'spawn' on Windows, 'fork' elsewhere if sys.platform == 'win32': mp_context = multiprocessing.get_context('spawn') else: mp_context = multiprocessing.get_context('fork')
559-576: Good addition of solution freshness validation.The validation loop ensures that only recent solutions (within 3 blocks) are accepted, preventing the submission of stale proof-of-work solutions. This is an important safeguard.
2 participants
TODO:
run-faucetcommand on README.mdSummary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores